Correctly pass redirect_uri to tokens call #222
Conversation
praboud-ant
force-pushed
the
praboud/redirect-uri-fix
branch
2 times, most recently
from
3a6d5dc to
2a2392b
Compare
There was a problem hiding this comment.
We are following the OAuth 2.1 draft spec (not OAuth 2.0) for MCP, which has this to say:
In OAuth 2.1, authorization code injection is prevented by the code_challenge and code_verifier parameters, making the inclusion of the redirect_uri parameter serve no purpose in the token request. As such, it has been removed.
I don't think we care about OAuth 2.0 compatibility here, although if folks feel strongly that this is important (e.g., because we want to be compatible with existing, OAuth 2.0-only auth libraries), we can consider it.
|
I need this as well. Just submitted an issue before seeing this. #225 |
There was a problem hiding this comment.
Okay, on the basis of being compatible with Auth0 (as very widely used infrastructure), let's get this in.
Thanks for the data point @csmoakpax8!
|
@jspahrsummers Amazing thank you! |
| @@ -259,11 +260,13 @@ export async function exchangeAuthorization( | |||
| clientInformation, | |||
| authorizationCode, | |||
| codeVerifier, | |||
| redirectUri, | |||
There was a problem hiding this comment.
should this be optional? I think it's possible for the authorization request to not have ?redirect_uri=xxx defined (i.e. if the client only has a single configured redirect the server should just send them there).
There was a problem hiding this comment.
and thus by oauth 2.0, it wouldn't be required
redirect_uri
REQUIRED, if the "redirect_uri" parameter was included in the
authorization request as described in Section 4.1.1, and their
values MUST be identical.
There was a problem hiding this comment.
tbh I think it's okay as is
Motivation and Context
The OAuth client currently passes
redirect_urito/authorize, but doesn't pass it to/token. Per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3, if the client passes it to/authorize, it MUST pass the same value to/token. I ran into this when trying to test Inspector (which uses the typescript SDK client) with an OAuth server which is compliant with that part of the RFC - the server correctly rejected the requests as missing theredirect_urivalue.How Has This Been Tested?
Automated tests, and locally wiring this up to inspector & verifying that the auth flow works.
Breaking Changes
No
Types of changes
Checklist
Additional context