auth: advertise RFC 6750 error code in WWW-Authenticate challenge - #1135
Open
PratikDhanave wants to merge 1 commit into
Open
auth: advertise RFC 6750 error code in WWW-Authenticate challenge#1135PratikDhanave wants to merge 1 commit into
PratikDhanave wants to merge 1 commit into
Conversation
RequireBearerToken never added an error= auth-param to the WWW-Authenticate header, so on insufficient scope it returned 403 with resource_metadata/scope but no error="insufficient_scope". The SDK's own client gates step-up re-authorization on exactly that value (AuthorizationCodeHandler.Authorize), so errorFromChallenges returned "" and step-up never fired — a client hitting an SDK server that needs more scopes just kept failing with 403 instead of upgrading scopes. Thread the RFC 6750 error code out of verify and add the error= param: insufficient_scope on the 403 scope check, invalid_token on the token-invalid 401 paths, invalid_request on the 400. A missing token (no credentials presented) and 5xx failures advertise no code. The change is additive to the header, non-breaking. Fixes modelcontextprotocol#1134
| } | ||
| if errors.Is(err, ErrOAuth) { | ||
| return nil, err.Error(), http.StatusBadRequest | ||
| return nil, err.Error(), http.StatusBadRequest, "invalid_request" |
Contributor
There was a problem hiding this comment.
this will not trigger the authorization flow as the code in @authorization_code is just checking for http.StatuForbidden case.
I am not sure tho if we should extend the case for the flow re-trigger
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
2 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
RequireBearerTokennever added anerror=auth-param to theWWW-Authenticateheader it emits. On insufficient scope it returned403withresource_metadata/scopebut noerror="insufficient_scope".The SDK's own client gates step-up re-authorization on exactly that value (
AuthorizationCodeHandler.Authorize):errorFromChallengesreadsParams["error"]; since the server never set it, it returned"", the condition was true, and step-up never fired. A client hitting an SDK-based server that needs additional scopes just kept failing with403instead of upgrading scopes. It's also an RFC 6750 §3.1 deviation.Fixes #1134
Change
Thread the RFC 6750 error code out of
verifyand add theerror=auth-param to the challenge:error=insufficient_scopeinvalid_tokenErrOAuthinvalid_requestThe change only adds a param to the header — it is non-breaking.
Testing
TestVerifyto assert the error code returned for each failure path.TestRequireBearerTokenAdvertisesInsufficientScope, which drives the middleware end-to-end and parses the emitted header with the sameoauthex.ParseWWWAuthenticatethe client uses, assertingerror="insufficient_scope"is visible.TestRequireBearerToken's expectedWWW-Authenticateheaders to include the new code.error param = "", want "insufficient_scope") and passes with it.go vet ./auth/and the fullgo test ./auth/suite pass.