[Doc] Describe the pending authorization checks as the code establishes them - #584
Merged
Conversation
…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
force-pushed
the
describe_the_pending_authorization_checks_as_the_code_establishes_them
branch
from
October 1, 2026 03:11
fb84b47 to
095bd81
Compare
koic
deleted the
describe_the_pending_authorization_checks_as_the_code_establishes_them
branch
October 1, 2026 17:49
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
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.
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 9207issbefore consuming the pending authorization means a forged callback carrying a validstatecannot discard the verifier the legitimate callback needs. The check compares a presentisswith the recorded issuer, a public value, and passes a callback without one unless the authorization server advertisesauthorization_response_iss_parameter_supported, so what it refuses is a callback from another authorization server, as in a mix-up attack; whoever holds thestateand passes that check consumes the entry, with anerroror 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 momentrun!saves it, not from the redirect; a registration is refused when itsclient_idor issuer changed, not on any change; and nothing said thatfinish!needs the redirect's query passed whole, since anissthe 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
Checklist