feat(client): add getAuthorizationCode() to OAuthClientProvider for headless OAuth flows#1662
Conversation
🦋 Changeset detectedLatest commit: eac311a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
…eadless OAuth flows - Add optional `getAuthorizationCode()` method to `OAuthClientProvider` interface - Update `withOAuth` middleware to automatically complete the authorization code exchange when the provider implements `getAuthorizationCode()` after a REDIRECT - Handle 403 responses the same as 401 in `withOAuth` (upscoping) - Update conformance `ConformanceOAuthProvider` to implement `getAuthorizationCode()` - Update conformance `withOAuthRetry` to use the new method name and remove TODO Closes modelcontextprotocol#1370
rechedev9
force-pushed
the
fix/oauth-retry-middleware-1370
branch
from
6b9cb53 to
57c7c85
Compare
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No bugs found, but this PR modifies OAuth authentication middleware (handling 403 as 401, adding getAuthorizationCode to the public OAuthClientProvider interface) — worth a human look given the security sensitivity and behavioral change.
Extended reasoning...
Overview
This PR adds an optional getAuthorizationCode() method to the OAuthClientProvider interface and updates withOAuth middleware to use it for automatic authorization code exchange. It also changes withOAuth to treat HTTP 403 responses the same as 401 (triggering re-authentication), and renames getAuthCode to getAuthorizationCode in the conformance test helpers.
Security risks
The changes are in OAuth authentication middleware — a security-sensitive area. The 403-as-401 change broadens the set of responses that trigger re-authentication flows, which could have subtle implications: a legitimate 403 (e.g., the user genuinely lacks permission) would now trigger a full re-auth cycle instead of being returned to the caller. This deserves human consideration for whether the behavior is appropriate in all cases.
Level of scrutiny
This warrants careful human review. It modifies a public TypeScript interface (OAuthClientProvider), changes middleware behavior for all consumers of withOAuth, and touches auth-critical code paths. The implementation looks clean and tests cover the new behaviors well, but the design decisions (especially 403 handling) should be validated by a maintainer.
Other factors
The PR includes 4 well-structured new tests. The changeset is correctly marked as a minor version bump. The conformance helper changes are consistent with the SDK changes. No existing tests appear to be broken. The author field shows as Unknown, and no human reviewers have weighed in yet.
2 participants
Summary
Resolves #1370.
The conformance test helpers contained a TODO noting that the OAuth retry logic (handling REDIRECT results and completing the code exchange inline) should be moved into the SDK itself. This PR does exactly that.
getAuthorizationCode?(): string | Promise<string>method to theOAuthClientProviderinterfacewithOAuthmiddleware to automatically complete the authorization code exchange when the provider implementsgetAuthorizationCode()after an auth REDIRECT — no manual intervention required403responses the same as401inwithOAuth(a 403 can indicate the server requires a broader scope / upscoping)ConformanceOAuthProvider.getAuthCode()→getAuthorizationCode()to implement the updated interfacewithOAuthRetryconformance helper to use the new method name and remove the TODO commentBreaking Changes
getAuthorizationCode()is an optional method onOAuthClientProvider. Existing providers without it continue to work — on REDIRECT,withOAuthfalls back to throwingUnauthorizedErroras before.Test plan
should retry request after successful auth on 403 responseshould throw UnauthorizedError on persistent 403 after re-authshould complete auth code flow when provider implements getAuthorizationCodeshould throw UnauthorizedError when auth returns REDIRECT and provider has no getAuthorizationCodepnpm typecheck:allpasses