[DEV-3974] Enable PKCE by default and allow credential omission - #19
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Two unresolved strategy issues affect OAuth client configuration and expand_fields handling.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR updates the MLH OmniAuth strategy with default PKCE, configurable API access, optional credential persistence, narrower error handling, and version 4.2.0.
Changes:
- Adds PKCE and configurable API endpoints.
- Supports omitting persisted OAuth credentials.
- Updates tests and version metadata.
File summaries
| File | Summary |
|---|---|
spec/omni_auth/strategies/mlh_spec.rb |
Tests the updated strategy behavior. |
spec/omni_auth/mlh_spec.rb |
Updates the expected version. |
lib/omniauth/strategies/mlh.rb |
Implements the strategy changes. Critical issue (2 votes): replacing client_options can drop OAuth defaults. Moderate issue (3 votes): expand_fields: nil or false can raise an error. |
lib/omniauth-mlh/version.rb |
Sets the version to 4.2.0. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
Dependency
This is the standalone cross-fork dependency for the Forem DEV-3974 stack. GitHub native stacks cannot cross repositories.
Status
Draft only. Do not publish the gem or merge the dependent Forem layer until focused review and release checks are complete.
Security review
The complete committed delta was audited. No tracked environment file, literal credential, token, private key, credential-bearing URL, real identity fixture, or generated secret artifact was found.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.