Skip to content

internal/authutil: treat an empty scope in a token response as absent - #1308

Merged
guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
akshita317:authutil/empty-token-scope
Sep 30, 2026
Merged

guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
akshita317:authutil/empty-token-scope

Conversation

@akshita317

@akshita317 akshita317 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

ScopesFromToken returns nil when a token response has no scope, and
both callers (AuthorizationCodeHandler and ClientCredentialsHandler)
take nil to mean the requested scopes were granted, per RFC 6749
section 5.1. But a response with "scope": "" (or only whitespace) gave
strings.Fields's empty, non-nil slice, so the handler recorded that no
scope was granted. The next step-up authorization then unions that
empty set with the challenged scopes and asks only for those, dropping
the permissions granted in earlier rounds, which is what the SEP-2350
accumulation is meant to prevent.

An empty scope names no scope-token (RFC 6749 section 3.3), so it
carries no more information than an absent one. Return nil for it.

TestScopesFromToken covers absent, single, multiple, form-encoded,
non-string, empty and whitespace-only scopes; the last two fail without
the change. It also brings the package to 100% statement coverage.

Fixes #1318

@akshita317

Copy link
Copy Markdown
Contributor Author

@guglielmo-san could you take a look when you have a moment? The workflows are waiting for maintainer approval (first-time contributor). I rebuilt the branch on today's main locally: go vet, staticcheck and go test ./... are clean, and the change is limited to ScopesFromToken and its test. Thanks!

ScopesFromToken returns nil when a token response has no scope, and
both callers (AuthorizationCodeHandler and ClientCredentialsHandler)
take nil to mean the requested scopes were granted, per RFC 6749
section 5.1. But a response with "scope": "" (or only whitespace) gave
strings.Fields's empty, non-nil slice, so the handler recorded that no
scope was granted. The next step-up authorization then unions that
empty set with the challenged scopes and asks only for those, dropping
the permissions granted in earlier rounds, which is what the SEP-2350
accumulation is meant to prevent.

An empty scope names no scope-token (RFC 6749 section 3.3), so it
carries no more information than an absent one. Return nil for it.

TestScopesFromToken covers absent, single, multiple, form-encoded,
non-string, empty and whitespace-only scopes; the last two fail without
the change. It also brings the package to 100% statement coverage.

Fixes modelcontextprotocol#1318

Signed-off-by: Akshita <110122283+akshita317@users.noreply.github.com>
@akshita317
akshita317 force-pushed the authutil/empty-token-scope branch from 19bf7fb to 46fa64a Compare September 29, 2026 14:04
@guglielmo-san
guglielmo-san enabled auto-merge (squash) September 30, 2026 07:37
@guglielmo-san
guglielmo-san merged commit e2ad683 into modelcontextprotocol:main Sep 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auth: an empty scope in a token response drops earlier-granted scopes on step-up

2 participants