Add PKCE, richer user info, and Slack OAuth V2 fields#5
Conversation
- Enable PKCE by default for improved security - Add `team` to authorize_options to allow workspace pre-selection - Expose enterprise info (Enterprise Grid org-level installs) - Expose incoming webhook data (channel, url, etc.) - Expose bot_user_id for bot token identification - Expose granted scopes (scope and authed_user scope) - Add richer user info: title, phone, status, timezone, image sizes, admin/owner flags - All new fields are additive (nil when absent), no breaking changes - Tests: 17 → 33 examples, all passing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThese changes enhance the Slack OAuth strategy by expanding authorization parameters to include Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as Slack OAuth Client
participant Strategy as OmniAuth Slack Strategy
participant SlackAPI as Slack API
participant AccessToken as Access Token
User->>Client: Initiate Login with 'team' parameter
Client->>Strategy: Request Authorization
Strategy->>Strategy: Apply PKCE
Strategy->>SlackAPI: Authorize with team + user_scope + scope + PKCE
SlackAPI-->>Strategy: Return authorization code
Strategy->>SlackAPI: Exchange code for tokens
SlackAPI-->>AccessToken: Return tokens & params
Strategy->>Strategy: Extract info (user identity, profile fields)
Strategy->>Strategy: Extract extra (conditional OAuth metadata)
Strategy-->>Client: Return profile (info) + metadata (extra)
Client-->>User: Login complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/omniauth/strategies/slack.rb`:
- Line 12: The change enabling PKCE by default (the line `option :pkce, true` in
the Slack OmniAuth strategy) is a breaking change; revert this default and
instead document and gate PKCE behind an explicit opt-in (e.g., keep `option
:pkce` unset or false and allow app code to enable it), and add an end-to-end
spec exercising the full authorize + callback flow with PKCE parameters
(generate code_verifier/code_challenge, include code_challenge_method, and
assert successful callback) plus a real Slack app integration test before
flipping the default; update the strategy tests to cover both non-PKCE and PKCE
paths so enabling later is safe.
- Line 55: Replace the direct assignment of the incoming_webhook into the auth
hash so the webhook URL is redacted: when setting hash["incoming_webhook"] from
access_token.params["incoming_webhook"], copy the object but replace or remove
the "url" key (e.g. set "url" => "[REDACTED]") before assigning to hash; update
the corresponding spec for the Slack strategy to expect the redacted
incoming_webhook.url value instead of the real URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c239afbc-86a4-4f6d-bfb6-c081388fce22
📒 Files selected for processing (3)
.rubocop.ymllib/omniauth/strategies/slack.rbspec/omniauth/strategies/slack_spec.rb
| option :authorize_options, %i[user_scope scope] | ||
| option :authorize_options, %i[user_scope scope team] | ||
|
|
||
| option :pkce, true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Slack OAuth V2 support PKCE code_challenge and code_verifier for /oauth/v2/authorize and /api/oauth.v2.access?
💡 Result:
Yes, Slack OAuth V2 supports PKCE code_challenge and code_verifier for both /oauth/v2/authorize and /api/oauth.v2.access. PKCE became generally available on March 30, 2026. It must be enabled in the app settings (one-way operation marking the app as a public client). Once enabled: - For /oauth/v2/authorize: Append code_challenge, code_challenge_method=S256 (SHA-256 only), and optionally code_verifier arguments to the authorization URL. Required for custom URI schemes; optional otherwise. - For /api/oauth.v2.access: Include code_verifier (optional if client_secret provided, but used without client_secret for public clients). Do not send client_secret. Official docs confirm code_verifier as an optional parameter in oauth.v2.access, with examples and error handling for invalid code_verifier.
Citations:
- 1: https://docs.slack.dev/changelog/2026/03/30/pkce
- 2: https://docs.slack.dev/authentication/using-pkce/
- 3: https://docs.slack.dev/reference/methods/oauth.v2.access.md
- 4: https://api.slack.com/methods/oauth.v2.access
- 5: https://docs.slack.dev/reference/methods/oauth.v2.access
🌐 Web query:
How does omniauth-oauth2 implement option :pkce, true, and which gem versions support it?
💡 Result:
Omniauth-oauth2 implements the :pkce, true option by adding PKCE (Proof Key for Code Exchange) support to the OAuth2 authorization code grant flow, as per RFC 7636. When enabled via option :pkce, true in the strategy configuration, it performs the following: 1. In authorize_params (called during the authorization request phase): - Generates a random code_verifier (SecureRandom.hex(64), 128 characters, meeting the 43-128 requirement). - Computes code_challenge as Base64.urlsafe_encode64(Digest::SHA2.digest(verifier), padding: false). - Adds code_challenge and code_challenge_method: 'S256' to the authorize URL query parameters. - Stores the code_verifier in the session under "omniauth.pkce.verifier". 2. In token_params (called during the callback/token exchange phase): - Retrieves the code_verifier from the session (and deletes it). - Adds code_verifier to the token request parameters. This proves possession of the verifier to the provider, preventing authorization code interception attacks. Customizable via :pkce_options for code_challenge proc and method (defaults to S256 SHA256). The feature was added in PR #131 (merged Aug 11, 2020) and first released in v1.7.0 (Aug 11, 2020). All subsequent versions (up to latest v1.9.0, Dec 2025) support it. Versions prior to 1.7.0 do not. Source code is visible in lib/omniauth/strategies/oauth2.rb methods: pkce_authorize_params, pkce_token_params, and integration in authorize_params/token_params. Usage example: provider :my_strategy, 'client_id', 'client_secret', pkce: true
Citations:
- 1: OAuth2 - PKCE omniauth/omniauth-oauth2#131
- 2: https://github.com/omniauth/omniauth-oauth2/blob/master/lib/omniauth/strategies/oauth2.rb
- 3: https://github.com/omniauth/omniauth-oauth2/blob/8438b89b712e03337932a0d95096258c892ea0f3/lib/omniauth/strategies/oauth2.rb
- 4: https://github.com/omniauth/omniauth-oauth2
- 5: https://rubygems.org/gems/omniauth-oauth2/versions
- 6: https://rubygems.org/gems/omniauth-oauth2/versions/1.7.0
- 7: https://github.com/omniauth/omniauth-oauth2/tags
Add end-to-end PKCE testing before enabling by default.
While Slack OAuth V2 and omniauth-oauth2 both support PKCE (since March 30, 2026 and v1.7.0 respectively), enabling option :pkce, true by default is a breaking change: it requires all Slack apps to have PKCE explicitly enabled in their settings, but most existing apps do not. Without explicit opt-in, this will fail authentication for unconfigured consumers. Add an end-to-end strategy spec covering authorize + callback with PKCE params, or complete the real Slack app integration test before release to validate the flow works as expected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/omniauth/strategies/slack.rb` at line 12, The change enabling PKCE by
default (the line `option :pkce, true` in the Slack OmniAuth strategy) is a
breaking change; revert this default and instead document and gate PKCE behind
an explicit opt-in (e.g., keep `option :pkce` unset or false and allow app code
to enable it), and add an end-to-end spec exercising the full authorize +
callback flow with PKCE parameters (generate code_verifier/code_challenge,
include code_challenge_method, and assert successful callback) plus a real Slack
app integration test before flipping the default; update the strategy tests to
cover both non-PKCE and PKCE paths so enabling later is safe.
| hash["bot_info"] = bot_info if bot_access_token? | ||
| hash["authed_user"] = authed_user_info if authed_user_token? | ||
| hash["enterprise_info"] = access_token.params["enterprise"] if access_token.params["enterprise"] | ||
| hash["incoming_webhook"] = access_token.params["incoming_webhook"] if access_token.params["incoming_webhook"] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Slack incoming webhook URL secret don't share documentation
💡 Result:
Slack's official documentation explicitly warns that incoming webhook URLs contain a secret token and must be kept confidential. The key guidance is: "Keep it secret, keep it safe. Your webhook URL contains a secret. Don't share it online, including via public version control repositories. Slack actively searches out and revokes leaked secrets." This is stated directly in the primary documentation for sending messages using incoming webhooks. Sharing the URL allows anyone to post messages to the specific Slack channel it's configured for, as authentication relies solely on the URL's secrecy (no additional signatures or keys for incoming webhooks). Best practices from official sources and supporting references: - Store URLs in environment variables or secrets managers, never hardcode in code or commit to repos. - Use separate URLs for dev/staging/prod. - Rotate periodically by generating new ones. - If leaked, revoke via app settings (remove the webhook) and create a new one. Official docs note that while channel-specific (limits some risks), exposure still enables spam/phishing to that channel.
Citations:
- 1: https://docs.slack.dev/messaging/sending-messages-using-incoming-webhooks
- 2: https://api.slack.com/incoming-webhooks
- 3: https://api.slack.com/messaging/webhooks
- 4: https://stackoverflow.com/questions/63774839/how-is-a-slack-webhook-safe
- 5: https://docs.slack.dev/authentication/best-practices-for-security
- 6: https://hookdeck.com/webhooks/platforms/guide-to-slack-webhooks-features-and-best-practices
🏁 Script executed:
fd -t f -name "slack.rb" | head -20Repository: TruemarkDev/omniauth-slack
Length of output: 300
🏁 Script executed:
fd -t f -name "slack.rb"Repository: TruemarkDev/omniauth-slack
Length of output: 300
🏁 Script executed:
git ls-files | grep slack.rbRepository: TruemarkDev/omniauth-slack
Length of output: 124
🏁 Script executed:
cat -n lib/omniauth/strategies/slack.rb | head -80Repository: TruemarkDev/omniauth-slack
Length of output: 3423
🏁 Script executed:
cat -n lib/omniauth/strategies/slack.rb | tail -20Repository: TruemarkDev/omniauth-slack
Length of output: 646
🏁 Script executed:
git ls-files | grep -E "(test|spec)" | head -20Repository: TruemarkDev/omniauth-slack
Length of output: 146
🏁 Script executed:
cat -n spec/omniauth/strategies/slack_spec.rb | head -150Repository: TruemarkDev/omniauth-slack
Length of output: 5443
🏁 Script executed:
cat -n spec/omniauth/strategies/slack_spec.rb | grep -A 20 -B 5 "incoming_webhook"Repository: TruemarkDev/omniauth-slack
Length of output: 2052
Redact the webhook URL from extra metadata to prevent accidental exposure.
The incoming_webhook.url is a credential—anyone with it can post to the Slack channel. The current code exposes the entire webhook object in extra, which is part of the OmniAuth auth hash and risks being logged or stored. The url field must be redacted.
Suggested fix
- hash["incoming_webhook"] = access_token.params["incoming_webhook"] if access_token.params["incoming_webhook"]
+ if access_token.params["incoming_webhook"]
+ hash["incoming_webhook"] = access_token.params["incoming_webhook"].reject { |key, _| key == "url" }
+ endThe test at spec/omniauth/strategies/slack_spec.rb:207 will also need updating to expect the redacted object.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hash["incoming_webhook"] = access_token.params["incoming_webhook"] if access_token.params["incoming_webhook"] | |
| if access_token.params["incoming_webhook"] | |
| hash["incoming_webhook"] = access_token.params["incoming_webhook"].reject { |key, _| key == "url" } | |
| end |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/omniauth/strategies/slack.rb` at line 55, Replace the direct assignment
of the incoming_webhook into the auth hash so the webhook URL is redacted: when
setting hash["incoming_webhook"] from access_token.params["incoming_webhook"],
copy the object but replace or remove the "url" key (e.g. set "url" =>
"[REDACTED]") before assigning to hash; update the corresponding spec for the
Slack strategy to expect the redacted incoming_webhook.url value instead of the
real URL.
1 participant
Summary
teamauthorize option: Allows pre-selecting a workspace on the authorize URLenterprise_info(org id/name) inextrahashincoming_webhookdata (channel, url, etc.) inextrahashbot_user_idinextrahash for bot token identificationscopeandauthed_user_scopeinextrahashtitle,phone,status_text,status_emoji,image_192,image_512,image_original,tz,tz_offset,is_admin,is_ownertoinfohashAll new fields are additive — they return
nilwhen not present, so there are no breaking changes.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests