Update OAuth consent UI to handle OAuth 2.0 Auth Code + PKCE#13819
Update OAuth consent UI to handle OAuth 2.0 Auth Code + PKCE#13819rickyrombo wants to merge 1 commit intomjp-oauth-standalonefrom
Conversation
|
There was a problem hiding this comment.
Pull request overview
Adds support to the OAuth consent/login UI for OAuth 2.0 Authorization Code + PKCE by parsing PKCE params, validating them, exchanging the existing signed JWT for an authorization code, and redirecting/postMessaging the code back to the client.
Changes:
- Add
exchangeForAuthorizationCode()helper to POST JWT + PKCE params to/v1/oauth/authorize. - Parse
response_type,code_challenge,code_challenge_method, and acceptclient_idas an alias forapi_key. - Add PKCE-specific query param validation + user-facing error messages.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/web/src/pages/oauth-login-page/utils.ts | Adds the JWT→auth-code exchange helper calling the backend OAuth authorize endpoint. |
| packages/web/src/pages/oauth-login-page/hooks.ts | Parses PKCE params, validates them, and adds an auth-code redirect/postMessage path when response_type=code. |
| packages/web/src/pages/oauth-login-page/messages.ts | Adds new PKCE validation error strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (scope === 'read') { | ||
| // Read scope-specific validations: | ||
| if (!appName && !apiKey) { | ||
| error = messages.missingAppNameError | ||
| } | ||
| } else if (scope === 'write') { | ||
| // Write scope-specific validations: | ||
| if (!apiKey) { | ||
| error = messages.missingApiKeyError | ||
| } else if (!isValidApiKey(apiKey)) { | ||
| error = messages.invalidApiKeyError | ||
| } | ||
| // PKCE-specific validations when response_type=code | ||
| if (!error && responseType === 'code') { | ||
| if (!codeChallenge || typeof codeChallenge !== 'string') { | ||
| error = messages.missingCodeChallengeError | ||
| } else if (codeChallengeMethod !== 'S256') { | ||
| error = messages.invalidCodeChallengeMethodError | ||
| } | ||
| } |
There was a problem hiding this comment.
PKCE query param validation is currently only applied inside the scope === 'write' branch. If response_type=code is used with scope=read (which is currently allowed when app_name is present) the UI will proceed without requiring client_id/apiKey, code_challenge, or a valid code_challenge_method, and later authorize() will attempt the code exchange with apiKey undefined. Consider moving the response_type=code validations outside the scope-specific branches (or explicitly rejecting response_type=code unless a client_id is present) so the required PKCE params are enforced consistently.
| account, | ||
| userEmail, | ||
| apiKey: apiKey as string, | ||
| redirectUri: (redirectUri as string) ?? 'postMessage', |
There was a problem hiding this comment.
redirectUri is defaulted to 'postMessage' here, but the rest of this module treats the special value as lowercase 'postmessage' (see parsing and redirect validation). This mismatch can cause the backend authorize call to receive an unexpected redirect_uri value if redirect_uri is ever missing/undefined. Prefer using 'postmessage' consistently (or better: derive the value from parsedRedirectUri and avoid a fallback that can bypass earlier validation).
| redirectUri: (redirectUri as string) ?? 'postMessage', | |
| redirectUri: (redirectUri as string) ?? 'postmessage', |
| const res = await fetch(`${env.API_URL}/v1/oauth/authorize`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ | ||
| token: jwt, | ||
| client_id: apiKey, | ||
| redirect_uri: redirectUri, | ||
| code_challenge: codeChallenge, | ||
| code_challenge_method: codeChallengeMethod, | ||
| scope | ||
| }) | ||
| }) | ||
| if (!res.ok) { | ||
| onError() | ||
| return null | ||
| } | ||
| const { code } = await res.json() | ||
| return code |
There was a problem hiding this comment.
fetch() can throw (network error, CORS failure), which will currently reject the promise and skip the !res.ok handling and onError() callback. Wrap the request + JSON parsing in a try/catch and call onError() on exceptions to keep error handling consistent with the rest of this file.
| const res = await fetch(`${env.API_URL}/v1/oauth/authorize`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ | |
| token: jwt, | |
| client_id: apiKey, | |
| redirect_uri: redirectUri, | |
| code_challenge: codeChallenge, | |
| code_challenge_method: codeChallengeMethod, | |
| scope | |
| }) | |
| }) | |
| if (!res.ok) { | |
| onError() | |
| return null | |
| } | |
| const { code } = await res.json() | |
| return code | |
| try { | |
| const res = await fetch(`${env.API_URL}/v1/oauth/authorize`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ | |
| token: jwt, | |
| client_id: apiKey, | |
| redirect_uri: redirectUri, | |
| code_challenge: codeChallenge, | |
| code_challenge_method: codeChallengeMethod, | |
| scope | |
| }) | |
| }) | |
| if (!res.ok) { | |
| onError() | |
| return null | |
| } | |
| const { code } = await res.json() | |
| return code | |
| } catch { | |
| onError() | |
| return null | |
| } |
No description provided.