diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 524bb85..38f515b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -165,7 +165,8 @@ browser URL, and persists correlation state. `HandleAuthorizationResponse` internally validates: that the callback belongs to the user agent that began the flow — the caller passes back -the `SessionHandle` it bound to that browser (an HttpOnly cookie), and a +the `SessionHandle` it bound to that browser (an HttpOnly cookie; +`client/sessioncookie` sets and reads one), and a callback whose `state` doesn't match it is rejected before anything is consumed, closing login CSRF ([RFC 9700 §4.7][bcp]) — then correlation state, issuer, JARM signature and claims, audience, expiry, response diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 1a7e0b9..15a4985 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -259,13 +259,13 @@ browser, and `a.Interaction`. `server/interactioncookie` carries both in one encrypted cookie: ```go -cookie, err := interactioncookie.New(keys, interactioncookie.Options{}) // once; keys shared by every instance +cookie, err := interactioncookie.New(cookieKeys, interactioncookie.Options{}) // once; cookieKeys shared by every instance // GET /authorize, on server.InteractionRequired: tag, err := cookie.Set(w, a, now) // render tag in the form, as interactioncookie.FormField -// POST, the form's submission (behind your CSRF protection): -handle, interaction, err := cookie.Read(r, now, r.PostFormValue(interactioncookie.FormField)) +// POST, the form's submission (behind your CSRF protection), after r.ParseForm(): +handle, interaction, err := cookie.Read(r, now, r.PostForm.Get(interactioncookie.FormField)) // ...CompleteAuthorization with handle, then cookie.Clear(w) ``` diff --git a/README.md b/README.md index b4177d7..6110980 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,22 @@ role-level types or behaviour. `serverresource` builds a `resource` verifier matching a `server` in the same process, for an authorization server that hosts its own protected endpoints. +A few helpers cover what every deployment otherwise writes by hand, and +gets wrong in the same ways: + +- `server/interactioncookie` carries a pending authorization from + `/authorize` to the consent form's submission in one encrypted cookie, + tied to the form that was shown. +- `client/sessioncookie` binds a client's authorization to the browser + that began it (login CSRF, [RFC 9700 §4.7](https://www.rfc-editor.org/rfc/rfc9700#section-4.7)), with your own value + for it alongside. +- `client.TokenSetSealer` keeps tokens at rest, encrypted and bound to + their owner, for a client that refreshes after a restart or on + another instance. +- The `*FromHTTP` constructors (`server.PushAuthorizationRequestFromHTTP`, + `resource.VerifyRequestFromHTTP` and others) read every header and + certificate a request type needs from an `*http.Request` at once. + See [GETTING_STARTED.md](GETTING_STARTED.md) for a full walkthrough of standing up an authorization server and resource server end to end, including a runnable configuration you can start from. diff --git a/UPGRADING.md b/UPGRADING.md index 2efe0c2..96452b5 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -20,9 +20,8 @@ and `keys/ephemeral` needs only the steps not marked *production only*. ### Clients list the RAR types they may request (storage, server) **Affects:** a server using Rich Authorization Requests (`Config.RAR`), -for every client that sends `authorization_details`; and -`federation.AutomaticRegistrationConfig` for automatically registered -clients that do. +for every client that sends `authorization_details`, including clients +registered automatically through OpenID Federation. **Why:** RFC 9396 §10 defines `authorization_details_types`, the types a client may use. The server now enforces it per client, before any @@ -33,9 +32,13 @@ types. Before, any registered type reached the policy, which had to check the client itself. **What to change:** set `storage.RegisteredClientConfig.AuthorizationDetailsTypes` -to the types each client may request (and -`federation.AutomaticRegistrationConfig.AuthorizationDetailsTypes` for -automatically registered clients). A `RARPolicy` that only checked +to the types each client may request, and +`server.Config.AutomaticRegistration.AuthorizationDetailsTypes` to the +types every automatically registered client may (`New` refuses one +`Config.RAR` doesn't register). If you build a +`federation.NewAutomaticClientRepository` yourself rather than through +`server.Config`, set `federation.AutomaticRegistrationConfig.AuthorizationDetailsTypes` +there. A `RARPolicy` that only checked which types a client may use can then become `server.AllowRequestedAuthorizationDetails{}`; keep your own where it checks the details themselves. Note one difference from a policy that @@ -51,8 +54,11 @@ your `ClientRepository`. A client that's missing them is refused at runtime, at PAR, CIBA or the token endpoint, with `invalid_authorization_details`; the error's cause, for your logs, reads `authorization_details type "…" is not registered for this client`. -Grants made before the upgrade aren't checked again: a refresh keeps the -authorization details the grant already holds. +`Server.CheckClientRegistration(client)` catches a registration listing a +type `Config.RAR` doesn't register (a typo, usually): call it when you +register a client, or over every client at startup. Grants made before +the upgrade aren't checked again: a refresh keeps the authorization +details the grant already holds. ### Custom `SessionStore`s persist an opaque `Record` (client) diff --git a/client/doc.go b/client/doc.go index 305e160..4098f8e 100644 --- a/client/doc.go +++ b/client/doc.go @@ -71,7 +71,8 @@ // ARCHITECTURE.md, "Design rules"): AuthorizationSession is opaque with // no public constructor, and a SessionHandle can only be recovered from // its own String form (ParseSessionHandle) — the caller stores it with -// the user agent that began the flow, and HandleAuthorizationResponse +// the user agent that began the flow (package client/sessioncookie does +// this), and HandleAuthorizationResponse // rejects a callback that doesn't carry the matching one; // HandleAuthorizationResponse returns a closed sum type // rather than one struct with optional fields, so a caller can't assume diff --git a/client/sessioncookie/sessioncookie.go b/client/sessioncookie/sessioncookie.go index 9c9d6fa..c7de6f1 100644 --- a/client/sessioncookie/sessioncookie.go +++ b/client/sessioncookie/sessioncookie.go @@ -16,7 +16,13 @@ // with it have expired. It is HttpOnly, Secure and SameSite=Lax — the // authorization response arrives as a top-level GET from the // authorization server, which Lax lets the cookie ride along on — with -// the __Host- prefix by default. +// the __Host- prefix by default. Give it keys of its own, apart from any +// interactioncookie's or client.TokenSetSealer's. +// +// A browser keeps one cookie of a name, so a second authorization begun +// in another tab replaces the first's cookie. The first tab's callback +// then fails, as one that no longer matches the browser's session, and +// the user starts again: that is the binding working, not a fault. package sessioncookie import ( @@ -46,11 +52,13 @@ var ( // Options configures a Cookie. type Options struct { - // Name is the cookie's name: DefaultName if empty. + // Name is the cookie's name: DefaultName if empty. Keep the __Host- + // prefix: without it, a sibling subdomain can set the cookie in the + // victim's browser, with a value it got sealed for itself. Name string - // Path is the cookie's path: "/" if empty. It must be "/" for a - // __Host- name. + // Path is the cookie's path: "/" if empty. New refuses another path + // for a __Host- name, which the prefix forbids. Path string } diff --git a/examples/decoupled-checkout/checkout/api.go b/examples/decoupled-checkout/checkout/api.go index 78d2756..fbb6b15 100644 --- a/examples/decoupled-checkout/checkout/api.go +++ b/examples/decoupled-checkout/checkout/api.go @@ -115,6 +115,9 @@ func (a *api) pay(w http.ResponseWriter, r *http.Request) { resource.WriteError(w, err) return } + // The client's next call carries a fresh DPoP nonce, when the + // verifier issues them. + authz.SetDPoPNonce(w.Header()) var order paymentOrder if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&order); err != nil { resource.NewError(resource.ErrorInvalidRequest, http.StatusBadRequest, "malformed payment order").WriteJSON(w) @@ -157,6 +160,9 @@ func (a *api) readAccount(w http.ResponseWriter, r *http.Request) { resource.WriteError(w, err) return } + // The client's next call carries a fresh DPoP nonce, when the + // verifier issues them. + authz.SetDPoPNonce(w.Header()) iban, action := r.PathValue("iban"), accountEndpoints[r.PathValue("what")] if action == "" { http.NotFound(w, r) diff --git a/examples/federated-union/union/idp.go b/examples/federated-union/union/idp.go index 7ac848c..12180ef 100644 --- a/examples/federated-union/union/idp.go +++ b/examples/federated-union/union/idp.go @@ -346,17 +346,16 @@ func (p *identityProvider) authorize(w http.ResponseWriter, r *http.Request) { // decide completes the interaction with the citizen chosen and the // claims they agreed to share. func (p *identityProvider) decide(w http.ResponseWriter, r *http.Request) { - tag := r.PostFormValue(interactioncookie.FormField) + if err := r.ParseForm(); err != nil { + p.w.renderError(w, http.StatusBadRequest, "Malformed form", formUnreadable) + return + } + tag := r.PostForm.Get(interactioncookie.FormField) handle, interaction, err := p.interaction.Read(r, time.Now(), tag) if err != nil { p.w.renderError(w, http.StatusBadRequest, "Session expired", "This browser has no sign-in in progress.") return } - p.interaction.Clear(w) - if err := r.ParseForm(); err != nil { - p.w.renderError(w, http.StatusBadRequest, "Malformed form", formUnreadable) - return - } var result server.InteractionResult if r.PostForm.Get("decision") != "approve" { @@ -393,6 +392,7 @@ func (p *identityProvider) decide(w http.ResponseWriter, r *http.Request) { }) } + p.interaction.Clear(w) outcome, err := p.srv.CompleteAuthorization(r.Context(), server.CompleteAuthorizationRequest{Handle: handle, Result: result}) if err != nil { p.w.renderError(w, http.StatusInternalServerError, signInFailed, publicMessage(err, "Something went wrong. Please try again.")) diff --git a/examples/identity-check/identity/bank.go b/examples/identity-check/identity/bank.go index e9ebfd0..7e5b927 100644 --- a/examples/identity-check/identity/bank.go +++ b/examples/identity-check/identity/bank.go @@ -384,16 +384,16 @@ func (b *bank) consentPage(in server.InteractionRequest, tag, problem string) co // decide signs the customer in, the way they chose, and records which // claims they approved for release. func (b *bank) decide(w http.ResponseWriter, r *http.Request) { - tag := r.PostFormValue(interactioncookie.FormField) + if err := r.ParseForm(); err != nil { + b.w.renderError(w, bankHost, http.StatusBadRequest, "Malformed form", formUnreadable) + return + } + tag := r.PostForm.Get(interactioncookie.FormField) handle, in, err := b.interaction.Read(r, time.Now(), tag) if err != nil { b.w.renderError(w, bankHost, http.StatusBadRequest, "Session expired", "This browser has no sign-in in progress.") return } - if err := r.ParseForm(); err != nil { - b.w.renderError(w, bankHost, http.StatusBadRequest, "Malformed form", formUnreadable) - return - } result := server.Deny("the customer declined") if r.PostForm.Get("decision") == "approve" { c, ok := customerByName(r.PostForm.Get("username")) diff --git a/examples/linked-accounts/linked/api.go b/examples/linked-accounts/linked/api.go index 7fac2d1..243f354 100644 --- a/examples/linked-accounts/linked/api.go +++ b/examples/linked-accounts/linked/api.go @@ -55,6 +55,9 @@ func (a *api) accounts(w http.ResponseWriter, r *http.Request) { resource.WriteError(w, err) return } + // The client's next call carries a fresh DPoP nonce, when the + // verifier issues them. + authz.SetDPoPNonce(w.Header()) details, err := grantedAccess(authz) if err != nil { resource.NewError(resource.ErrorInvalidToken, http.StatusUnauthorized, "the token's authorization details are malformed").WriteJSON(w) diff --git a/examples/linked-accounts/linked/bank.go b/examples/linked-accounts/linked/bank.go index 9e5e84a..f7cc859 100644 --- a/examples/linked-accounts/linked/bank.go +++ b/examples/linked-accounts/linked/bank.go @@ -296,16 +296,16 @@ func (b *bank) consentPage(in server.InteractionRequest, tag, problem string) co // decide signs the customer in and records which accounts they chose // to share, under a grant ID the bank keeps on its Connected apps page. func (b *bank) decide(w http.ResponseWriter, r *http.Request) { - tag := r.PostFormValue(interactioncookie.FormField) + if err := r.ParseForm(); err != nil { + b.w.renderError(w, bankHost, http.StatusBadRequest, "Malformed form", formUnreadable) + return + } + tag := r.PostForm.Get(interactioncookie.FormField) handle, in, err := b.interaction.Read(r, b.w.clock.Now(), tag) if err != nil { b.w.renderError(w, bankHost, http.StatusBadRequest, "Session expired", "This browser has no sign-in in progress.") return } - if err := r.ParseForm(); err != nil { - b.w.renderError(w, bankHost, http.StatusBadRequest, "Malformed form", formUnreadable) - return - } result := server.Deny("the customer declined") if r.PostForm.Get("decision") == "approve" { c, ok := customerByName(r.PostForm.Get("username")) diff --git a/examples/payment-consent/payment/api.go b/examples/payment-consent/payment/api.go index b11546d..12de091 100644 --- a/examples/payment-consent/payment/api.go +++ b/examples/payment-consent/payment/api.go @@ -75,6 +75,9 @@ func (a *api) pay(w http.ResponseWriter, r *http.Request) { resource.WriteError(w, err) return } + // The client's next call carries a fresh DPoP nonce, when the + // verifier issues them. + authz.SetDPoPNonce(w.Header()) var order paymentOrder if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&order); err != nil { resource.NewError(resource.ErrorInvalidRequest, http.StatusBadRequest, "malformed payment order").WriteJSON(w) diff --git a/examples/payment-consent/payment/bank.go b/examples/payment-consent/payment/bank.go index b8ee452..541f93d 100644 --- a/examples/payment-consent/payment/bank.go +++ b/examples/payment-consent/payment/bank.go @@ -258,16 +258,16 @@ func (b *bank) consentPage(in server.InteractionRequest, tag, problem string) co // decide signs the customer in and records their decision. The payment // is granted exactly as asked: the server refuses anything else. func (b *bank) decide(w http.ResponseWriter, r *http.Request) { - tag := r.PostFormValue(interactioncookie.FormField) + if err := r.ParseForm(); err != nil { + b.w.renderError(w, bankHost, http.StatusBadRequest, "Malformed form", formUnreadable) + return + } + tag := r.PostForm.Get(interactioncookie.FormField) handle, in, err := b.interaction.Read(r, time.Now(), tag) if err != nil { b.w.renderError(w, bankHost, http.StatusBadRequest, "Session expired", "This browser has no payment approval in progress.") return } - if err := r.ParseForm(); err != nil { - b.w.renderError(w, bankHost, http.StatusBadRequest, "Malformed form", formUnreadable) - return - } result := server.Deny("the customer declined") if r.PostForm.Get("decision") == "approve" { c, ok := customerByName(r.PostForm.Get("username")) diff --git a/examples/payroll-run/payroll/api.go b/examples/payroll-run/payroll/api.go index 5ad151f..03fc86b 100644 --- a/examples/payroll-run/payroll/api.go +++ b/examples/payroll-run/payroll/api.go @@ -81,6 +81,9 @@ func (a *api) submit(w http.ResponseWriter, r *http.Request) { resource.WriteError(w, err) return } + // The client's next call carries a fresh DPoP nonce, when the + // verifier issues them. + authz.SetDPoPNonce(w.Header()) var b batch if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 64<<10)).Decode(&b); err != nil { resource.NewError(resource.ErrorInvalidRequest, http.StatusBadRequest, "malformed payroll batch").WriteJSON(w) diff --git a/internal/sealedcookie/sealedcookie.go b/internal/sealedcookie/sealedcookie.go index 7259f0b..94014f5 100644 --- a/internal/sealedcookie/sealedcookie.go +++ b/internal/sealedcookie/sealedcookie.go @@ -81,7 +81,9 @@ type envelope struct { // opened until expiresAt; it refuses a value already expired, and one // too large for a cookie (ErrTooLarge), setting nothing. func (j *Jar) Set(w http.ResponseWriter, value any, expiresAt, now time.Time) error { - if !now.Before(expiresAt) { + // Max-Age counts whole seconds, and 0 would make a session cookie: + // less than a second left is as good as expired. + if expiresAt.Sub(now) < time.Second { return fmt.Errorf("%s: already expired", j.pkg) } encoded, err := json.Marshal(value) diff --git a/server/interactioncookie/interactioncookie.go b/server/interactioncookie/interactioncookie.go index 04f139c..23430fd 100644 --- a/server/interactioncookie/interactioncookie.go +++ b/server/interactioncookie/interactioncookie.go @@ -21,6 +21,9 @@ // and Read refuses a form whose tag isn't the cookie's: that page's // interaction is gone, and the user starts again. // +// Give it keys of its own, apart from any client/sessioncookie's or +// client.TokenSetSealer's. +// // The cookie is not a CSRF defence. The consent form's submission still // needs one, such as net/http's CrossOriginProtection. package interactioncookie @@ -66,12 +69,13 @@ var ( // Options configures a Cookie. type Options struct { - // Name is the cookie's name: DefaultName if empty. A name with the - // __Host- prefix gets Path=/, as the prefix requires. + // Name is the cookie's name: DefaultName if empty. Keep the __Host- + // prefix: without it, a sibling subdomain can set the cookie in the + // victim's browser, with a value it got sealed for itself. Name string - // Path is the cookie's path: "/" if empty. It must be "/" for a - // __Host- name. + // Path is the cookie's path: "/" if empty. New refuses another path + // for a __Host- name, which the prefix forbids. Path string } diff --git a/server/interactioncookie/interactioncookie_test.go b/server/interactioncookie/interactioncookie_test.go index a53ec0f..e120e0a 100644 --- a/server/interactioncookie/interactioncookie_test.go +++ b/server/interactioncookie/interactioncookie_test.go @@ -261,7 +261,7 @@ func TestCookieExpiresWithTheInteraction(t *testing.T) { t.Errorf("Read(at ExpiresAt) = %v, want ErrNoInteraction", err) } - for name, expiresAt := range map[string]time.Time{"expired": now.Add(-time.Second), "unset": {}} { + for name, expiresAt := range map[string]time.Time{"expired": now.Add(-time.Second), "unset": {}, "under a second left": now.Add(500 * time.Millisecond)} { a.ExpiresAt = expiresAt if _, err := c.Set(httptest.NewRecorder(), a, now); err == nil { t.Errorf("Set(%s interaction) = nil error", name)