Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions GETTING_STARTED.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,24 @@ case server.AuthorizationLocalError:
}
```

**Bind the interaction handle to the browser.** `CompleteAuthorization`
accepts a handle from whoever presents it, so a login UI that carries it
in a form field and 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 on its own page, and a
signed-in victim's browser approves it, sending that client a code for
the victim's account. Set the handle (`a.Handle.String()`) in an
HttpOnly, Secure, SameSite cookie when the browser reaches `/authorize`,
read it back with `server.ParseInteractionHandle` when the form is
submitted, and protect that form with your usual CSRF defence. See
`server.InteractionHandle`'s doc comment.

`cmd/conformance-as/authorize.go` is a complete, working version of
exactly this — read it for the full picture, including how it bridges
an `InteractionHandle` across the GET (render the form) and POST
(handle the submission) halves of a real HTTP flow. Its own login form
exactly this — read it for the full picture of the GET (render the
form) and POST (handle the submission) halves of a real HTTP flow. Its
handle bridge is not one to copy: it carries the handle in a hidden form
field, which is only tolerable because it authenticates nobody — use
the cookie binding above instead. Its own login form
(`consent_template.go`) is a deliberate non-example: a free-text "type
any username" field with no real authentication behind it, because this
binary exists to drive OIDF conformance testing, not to demonstrate
Expand Down
7 changes: 7 additions & 0 deletions cmd/conformance-as/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ const consentPendingTTL = 5 * time.Minute
// genuine HTML consent form — not an auto-approve shortcut — so a real
// browser (including an automated conformance-suite browser driver) has
// something to load and submit.
//
// Not a model for a production login UI: it carries the interaction
// handle in a hidden form field, with no cookie binding it to the
// browser and no CSRF protection on the POST, which a real deployment
// must have — see server.InteractionHandle's doc comment for the
// consent CSRF this would otherwise allow. It gets away with it only
// because it authenticates nobody (the form's username is free text).
type consentHandler struct {
srv *server.Server
clients storage.ClientRepository
Expand Down
15 changes: 10 additions & 5 deletions server/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@
// rotated — FAPI 2.0 Security Profile Final §5.3.2.1 says an
// authorization server "shall not use refresh token rotation except
// in extraordinary circumstances" — so it stays valid for repeated
// use until it expires or is revoked, and is bound to its client
// and (under DPoP) to the key it was issued under.
// use until it expires or is revoked, and is bound to the client it
// was issued to — only that client, authenticating itself, can
// redeem it.
// - AuthorizationAction (from BeginAuthorization) and AuthorizationResult
// (from CompleteAuthorization) are closed sum types, not structs with
// optional fields, so a caller can never mistake a local error for a
Expand Down Expand Up @@ -75,9 +76,13 @@
// ExchangeAuthorizationCode and RefreshAccessToken each require a
// valid DPoP proof bound to the token endpoint and reject a replayed
// proof jti the same way they reject a replayed client assertion or
// request object, and a refresh token is bound to the DPoP key it
// was issued under: RefreshAccessToken rejects a proof from any
// other key, even one belonging to the same client. Bearer
// request object. RefreshAccessToken does not require the proof to
// come from the key the refresh token was first issued under: every
// client here is confidential and authenticates at the token
// endpoint, and RFC 9449 §5 binds a confidential client's refresh
// token to that client authentication rather than to a DPoP key.
// Each new access token is bound to the key of the proof presented
// with it. Bearer
// (non-sender-constrained) tokens are not supported.
// - An ID token is issued alongside the access token exactly when the
// (possibly refresh-narrowed) granted scope includes "openid";
Expand Down
16 changes: 16 additions & 0 deletions server/interaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,22 @@ const interactionHandleSize = 32
// confused with, or substituted for, a request_uri or an authorization
// code: the login UI receives only this handle, never a storage
// primary key or the underlying PAR reference.
//
// Bind it to the browser that started the interaction. The handle
// identifies a pending authorization, not who may complete it, and
// CompleteAuthorization accepts it from whoever presents it. If the
// login UI round-trips it through a form field and authenticates the
// user from an existing session cookie, a malicious client can start
// its own authorization, place its handle in a form on its own page
// that submits itself, and have a victim's already-signed-in browser
// approve that authorization, so the client receives a code for the
// victim's account (consent CSRF, the authorization-server side of RFC
// 9700 §4.7's login CSRF). Keep the handle in an HttpOnly, Secure,
// SameSite cookie set when the browser reaches the authorization
// endpoint, read it back from that cookie (ParseInteractionHandle) when
// the login or consent form is submitted, and protect that form with
// the application's usual CSRF defence. Never accept the handle from
// a form field or query parameter alone.
type InteractionHandle struct {
value string
}
Expand Down
Loading