Skip to content

[Doc] Describe the pending authorization checks as the code establishes them - #584

Merged
koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:describe_the_pending_authorization_checks_as_the_code_establishes_them
Oct 1, 2026
Merged

koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:describe_the_pending_authorization_checks_as_the_code_establishes_them

Conversation

@koic

@koic koic commented Sep 30, 2026

Copy link
Copy Markdown
Member

Motivation and Context

The documentation of Flow#finish! and the comments beside it claimed more than the checks establish. They said that validating the RFC 9207 iss before consuming the pending authorization means a forged callback carrying a valid state cannot discard the verifier the legitimate callback needs. The check compares a present iss with the recorded issuer, a public value, and passes a callback without one unless the authorization server advertises authorization_response_iss_parameter_supported, so what it refuses is a callback from another authorization server, as in a mix-up attack; whoever holds the state and passes that check consumes the entry, with an error or an unusable code as well as with the code itself, and the legitimate callback then finds nothing. The comment on the error response likewise called the values the authorization server's own, which equality with a public issuer string does not establish. Three more statements were imprecise: the age of a pending authorization is counted from the moment run! saves it, not from the redirect; a registration is refused when its client_id or issuer changed, not on any change; and nothing said that finish! needs the redirect's query passed whole, since an iss the caller drops reads as absent.

The documentation and the comments now say what each check establishes and what remains with the sender, the storage, and the caller. No behavior changes.

How Has This Been Tested?

Documentation and comments only; the tests are unchanged.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@koic koic changed the title Describe the pending authorization checks as the code establishes them [Doc] Describe the pending authorization checks as the code establishes them Oct 1, 2026
…es them

## Motivation and Context

The documentation of `Flow#finish!` and the comments beside it claimed more than the checks establish.
They said that validating the RFC 9207 `iss` before consuming the pending authorization means a forged callback carrying
a valid `state` cannot discard the verifier the legitimate callback needs. The check compares a present `iss` with
the recorded issuer, a public value, and passes a callback without one unless the authorization server advertises
`authorization_response_iss_parameter_supported`, so what it refuses is a callback from another authorization server,
as in a mix-up attack; whoever holds the `state` and passes that check consumes the entry,
with an `error` or an unusable code as well as with the code itself, and the legitimate callback then finds nothing.
The comment on the error response likewise called the values the authorization server's own, which equality with
a public issuer string does not establish. Three more statements were imprecise: the age of a pending authorization is
counted from the moment `run!` saves it, not from the redirect; a registration is refused when its `client_id` or issuer changed,
not on any change; and nothing said that `finish!` needs the redirect's query passed whole, since an `iss` the caller
drops reads as absent.

The documentation and the comments now say what each check establishes and what remains with the sender, the storage,
and the caller. No behavior changes.

## How Has This Been Tested?

Documentation and comments only; the tests are unchanged.

## Breaking Changes

None.
@koic
koic force-pushed the describe_the_pending_authorization_checks_as_the_code_establishes_them branch from fb84b47 to 095bd81 Compare October 1, 2026 03:11
@koic
koic merged commit 73d6da3 into modelcontextprotocol:main Oct 1, 2026
11 checks passed
@koic
koic deleted the describe_the_pending_authorization_checks_as_the_code_establishes_them branch October 1, 2026 17:49
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.

1 participant