fix(oauth): support Basic Auth token exchange for Notion - #711
Merged
Merged
Conversation
Notion's token endpoint requires HTTP Basic Auth and a JSON body, but oauth_exchange only sent form-urlencoded client_secret. Provider 401s were also passed through and misread as session expiry in /connect/oauth. Add tokenExchangeStyle (form | basic-json) with Notion host auto-detect, map provider failures to 502 with error details, and show the real provider error in the connect UI. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughOAuth token exchange now supports ChangesOAuth token exchange
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client as connect-oauth
participant Handler as account-secrets
participant Helpers as oauth-token-exchange
participant Provider as OAuth provider
Client->>Handler: oauth_exchange with tokenExchangeStyle
Handler->>Helpers: resolve and build token request
Helpers->>Provider: form or basic-json token request
Provider-->>Handler: tokens or provider error
Handler-->>Client: tokens or standardized failure
Client->>Client: redirect only for session-expired classification
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
kody-bot
marked this pull request as ready for review
July 10, 2026 18:32
Contributor
|
🔎 Preview deployed: https://kody-pr-711.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/worker/client/routes/connect-oauth.tsx (1)
300-321: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
isConnectOauthConfigdoesn't validate the now-requiredtokenExchangeStyle.
ConnectOauthConfig.tokenExchangeStyleis required (Line 69), but this guard doesn't check it, so a config persisted by an older build (before this field existed) still passes and is used withtokenExchangeStyle: undefined. It's currently self-recovering —JSON.stringifydrops the undefined key and the server re-infers the style fromtokenUrl— but validating it here keeps the type contract honest.♻️ Optional guard addition
typeof record.flow === 'string' && + (record.tokenExchangeStyle === 'form' || + record.tokenExchangeStyle === 'basic-json') && typeof record.scopeSeparator === 'string' &&🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/routes/connect-oauth.tsx` around lines 300 - 321, Add validation for the required ConnectOauthConfig.tokenExchangeStyle field in isConnectOauthConfig, ensuring it is a string (or otherwise matches the declared TokenExchangeStyle type) before accepting the record as valid. Place the check alongside the existing scalar property validations so persisted legacy configs without this field are rejected.packages/worker/src/mcp/capabilities/integrations/integration-shared.ts (1)
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive this from
packages/worker/src/app/oauth-token-exchange.ts'stokenExchangeStyles. Keeping one source for the literals avoids the schema and resolver drifting apart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/integrations/integration-shared.ts` around lines 8 - 9, Derive tokenExchangeStyleValues from the tokenExchangeStyles definition in oauth-token-exchange.ts instead of duplicating the literal values, and update its type/export usage as needed so the schema and resolver share a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/client/routes/connect-oauth.tsx`:
- Around line 300-321: Add validation for the required
ConnectOauthConfig.tokenExchangeStyle field in isConnectOauthConfig, ensuring it
is a string (or otherwise matches the declared TokenExchangeStyle type) before
accepting the record as valid. Place the check alongside the existing scalar
property validations so persisted legacy configs without this field are
rejected.
In `@packages/worker/src/mcp/capabilities/integrations/integration-shared.ts`:
- Around line 8-9: Derive tokenExchangeStyleValues from the tokenExchangeStyles
definition in oauth-token-exchange.ts instead of duplicating the literal values,
and update its type/export usage as needed so the schema and resolver share a
single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a09bdb59-031f-4fab-853b-b0d7739de8ca
📒 Files selected for processing (8)
packages/worker/client/routes/connect-oauth.node.test.tspackages/worker/client/routes/connect-oauth.tsxpackages/worker/src/app/handlers/account-secrets.node.test.tspackages/worker/src/app/handlers/account-secrets.tspackages/worker/src/app/oauth-token-exchange.node.test.tspackages/worker/src/app/oauth-token-exchange.tspackages/worker/src/mcp/capabilities/integrations/integration-save.tspackages/worker/src/mcp/capabilities/integrations/integration-shared.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Notion OAuth via
/connect/oauthfailed after the user approved access: Kody exchanged the code with a form-urlencodedclient_secretbody (no Basic Auth), Notion returned HTTP 401, and the connect UI treated that 401 as a Kody session expiry ("Session expired.") instead of showing the provider error. Tokens were never saved.Fix
tokenExchangeStyle:form(default, existing confidential body-secret flows) andbasic-json(HTTP Basic + JSON body).basic-jsonforapi.notion.com; allow explicit override via request body / integration metadata.error/error_description/providerStatusinstead of passthrough 401.Unauthorized.) from provider failures and surfaceserror_description.Test plan
npm run typecheckSystem recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@0e95e1a8· Head:620a996dClassification: extends — outbound OAuth token exchange and integration config gain a
tokenExchangeStylecontract; connect UI error handling for/account/secrets.jsonoauth_exchange changes.Primitives touched
app-ui/connect/oauthexchange + failure messagingsecretsoauth_exchangeBasic Auth/JSON styles; provider errors → 502valuestokenExchangeStyleon integration configSystem map
Connect OAuth token exchange now chooses form vs Notion-style Basic+JSON before calling the provider, and provider auth failures no longer look like Kody session expiry.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Change flow
Before / after
client_secreterror_descriptionInvariants
userId.Summary by CodeRabbit