Repository navigation
docs: require binding the interaction handle to the browser; fix refresh-token binding claim - #409
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…esh-token binding claim InteractionHandle's docs and GETTING_STARTED never said to bind the handle to the browser that started the interaction, and the reference login UI in cmd/conformance-as carries it in a hidden form field. CompleteAuthorization accepts a handle from whoever presents it, so a login UI built that way that signs users in from an existing session cookie is open to consent CSRF: a malicious client starts its own authorization, puts its handle in a self-submitting form, and a signed-in victim's browser approves it. The handle's doc comment and GETTING_STARTED now require keeping it in an HttpOnly, Secure, SameSite cookie and CSRF-protecting the form, and the reference code is marked as not a model for this. server/doc.go also said, in two places, that a refresh token is bound to the DPoP key it was issued under and that RefreshAccessToken rejects a proof from another key. It deliberately isn't: every client is confidential and RFC 9449 §5 binds its refresh token to client authentication instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
osanderson
force-pushed
the
docs/interaction-handle-binding
branch
from
September 28, 2026 17:01
511edd2 to
30f2ca8
Compare
|
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.



Summary
Two documentation fixes from an attacker-mindset review.
Consent CSRF guidance (Medium finding)
CompleteAuthorizationaccepts anInteractionHandlefrom whoever presents it. Nothing in the docs said to bind the handle to the browser that started the interaction, and the reference login UI (cmd/conformance-as/authorize.go) carries it in a hidden form field.A real login UI built that way, which signs users in from an existing session cookie, is open to consent CSRF:
This is the authorization-server side of the login CSRF fixed for the client in #397. The library can't enforce the fix because the embedder owns the login UI, so the docs now state it explicitly:
server.InteractionHandledoc comment: the attack, and the requirement to keep the handle in an HttpOnly, Secure, SameSite cookie, read it back withParseInteractionHandle, CSRF-protect the form, and never accept it from a form field or query parameter alone.cmd/conformance-as/authorize.go: marked as not a model for a production login UI, and why.server/doc.gorefresh-token binding (doc error)It said, in two places, that a refresh token is bound to the DPoP key it was issued under and that
RefreshAccessTokenrejects a proof from another key. The code deliberately doesn't check that (server/refresh.go): every client is confidential, and RFC 9449 §5 binds a confidential client's refresh token to client authentication. One of the two sentences was introduced by #405.Testing
Docs and comments only;
go build ./...andgo vetpass.🤖 Generated with Claude Code