diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 1c6bca0..1a7e0b9 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -476,7 +476,13 @@ authCtx, err := verifier.Verify(ctx, resource.VerifyRequestFromHTTP(r, protected ``` On success, `authCtx.Subject`/`ClientID`/`Scopes`/`Claims` are what your -API handler needs to authorize the call. On failure, `err` is always a +API handler needs to authorize the call. Call +`authCtx.SetDPoPNonce(w.Header())` on the response, so a client's next +call already carries a fresh DPoP nonce when you've enabled them. A +UserInfo endpoint hosted with the authorization server builds its +response with `serverresource.UserInfoClaims(ctx, authCtx, +identityClaims)`: only the claims the client requested and the user +approved, plus `sub`. On failure, `err` is always a `*resource.Error`, and `resource.WriteError` sends it as the RFC 6750 / RFC 9449 response: the status, a `WWW-Authenticate` challenge in the scheme the request used, and the error body. A request with no diff --git a/cmd/conformance-as/resource.go b/cmd/conformance-as/resource.go index 0844999..0bc8aef 100644 --- a/cmd/conformance-as/resource.go +++ b/cmd/conformance-as/resource.go @@ -8,6 +8,7 @@ import ( fapi "github.com/idfoundry/fapigo" fapires "github.com/idfoundry/fapigo/resource" "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/serverresource" "github.com/idfoundry/fapigo/storage" ) @@ -63,50 +64,20 @@ func userinfoHandler(srv *server.Server, verifier *fapires.Verifier, userinfoURL writeResourceError(w, err) return } - hasOpenIDScope := false - for _, scope := range authCtx.Scopes { - if scope == "openid" { - hasOpenIDScope = true - break - } - } - if !hasOpenIDScope { - writeResourceErrorRaw(w, http.StatusForbidden, "insufficient_scope", "access token was not granted the openid scope") - return - } - - // requested_userinfo_claims (server.RequestedUserinfoClaimsKey) was - // embedded in this access token at issuance, carrying forward the - // authorization request's "claims" parameter — see that - // constant's doc comment for why a UserInfo call, arriving as a - // wholly separate later request, has no other way to know what - // was originally requested. No entry there means nothing was - // requested, and this endpoint must not return any identity claim - // in that case (not "return everything this binary happens to - // know" — that would leak data the client never asked for). - var requestedNames []string - if raw, ok := authCtx.Claims[server.RequestedUserinfoClaimsKey]; ok { - _ = json.Unmarshal(raw, &requestedNames) - } - claims, err := identityClaims.ResolveIdentityClaims(r.Context(), authCtx.Subject, requestedNames) + // Only the claims the authorization request's "claims" parameter + // asked for and the user approved, which the access token carries + // (server.RequestedUserinfoClaimsKey): a UserInfo call, arriving + // as a wholly separate later request, has no other way to know. + // Nothing requested means no identity claim at all, not + // everything this binary happens to know. A token without the + // openid scope gets 403 insufficient_scope. + body, err := serverresource.UserInfoClaims(r.Context(), authCtx, identityClaims) if err != nil { - writeResourceErrorRaw(w, http.StatusInternalServerError, "server_error", "failed to resolve identity claims") - return - } - subJSON, err := json.Marshal(authCtx.Subject) - if err != nil { - writeResourceErrorRaw(w, http.StatusInternalServerError, "server_error", "failed to encode subject") + writeResourceError(w, err) return } - body := make(map[string]json.RawMessage, len(claims)) - for k, v := range claims { - body[k] = v - } - body["sub"] = subJSON - if authCtx.NextDPoPNonce != "" { - w.Header().Set("DPoP-Nonce", authCtx.NextDPoPNonce) - } + authCtx.SetDPoPNonce(w.Header()) if userinfoSigning { client, err := clients.ResolveClient(r.Context(), fapi.ClientID(authCtx.ClientID)) @@ -157,9 +128,7 @@ func accountsHandler(verifier *fapires.Verifier, accountsURL *url.URL) http.Hand writeResourceError(w, err) return } - if authCtx.NextDPoPNonce != "" { - w.Header().Set("DPoP-Nonce", authCtx.NextDPoPNonce) - } + authCtx.SetDPoPNonce(w.Header()) w.Header().Set(contentTypeHeader, "application/json") _ = json.NewEncoder(w).Encode(map[string]any{"accounts": []string{}}) } diff --git a/cmd/conformance-as/smoke_test.go b/cmd/conformance-as/smoke_test.go index 56f877a..834139d 100644 --- a/cmd/conformance-as/smoke_test.go +++ b/cmd/conformance-as/smoke_test.go @@ -121,6 +121,7 @@ type smokeHarness struct { authorize string // base authorize endpoint URL, for building decision requests token string // base token endpoint URL, for tests that POST to it directly par string // base PAR endpoint URL, for tests that POST to it directly + userinfo string // UserInfo endpoint URL, for tests that call it directly // backchannelApprove is the base "/backchannel-approve" URL — // populated only when newSmokeHarnessWithOptions was built with @@ -365,6 +366,7 @@ func newSmokeHarnessWithOptions(t *testing.T, format AccessTokenFormat, dpopNonc authorize: endpoints.Authorization.String(), token: endpoints.Token.String(), par: endpoints.PushedAuthorizationRequest.String(), + userinfo: userinfoURL.String(), backchannelApprove: backchannelApprove, backchannelAuthenticate: backchannelAuthenticate, cibaApproveUI: cibaApproveUI, @@ -999,3 +1001,37 @@ func TestSmokeCIBADenied(t *testing.T) { t.Fatalf("denied.Code = %q, want %q", denied.Code, "access_denied") } } + +// TestSmokeUserInfoRefusesATokenWithoutOpenID covers the UserInfo +// endpoint's 403 insufficient_scope for an access token that wasn't +// granted the openid scope (OIDC Core §5.3). +func TestSmokeUserInfoRefusesATokenWithoutOpenID(t *testing.T) { + h := newSmokeHarness(t, AccessTokenFormatJWT) + ctx := context.Background() + scope := []string{"accounts"} + + handle := h.runToConsent(ctx, scope) + rawQuery := h.submitDecision(ctx, handle, "approve", scope) + result, err := h.client.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery, Session: h.session}) + if err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + success, ok := result.(client.CompletionSuccess) + if !ok { + t.Fatalf("CompleteAuthorization result = %T, want client.CompletionSuccess", result) + } + // Called directly: FetchUserInfo itself refuses tokens without an ID + // token to check the response's subject against. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, h.userinfo, nil) + if err != nil { + t.Fatal(err) + } + res, err := h.client.ProtectedResource(success.Tokens).Do(ctx, req) + if err != nil { + t.Fatalf("GET userinfo: %v", err) + } + _ = res.Body.Close() + if res.StatusCode != http.StatusForbidden || !strings.Contains(res.Header.Get("WWW-Authenticate"), "insufficient_scope") { + t.Errorf("GET userinfo (no openid scope) = %d %q, want 403 insufficient_scope", res.StatusCode, res.Header.Get("WWW-Authenticate")) + } +} diff --git a/examples/identity-check/identity/bank.go b/examples/identity-check/identity/bank.go index 3a2fbe8..e9ebfd0 100644 --- a/examples/identity-check/identity/bank.go +++ b/examples/identity-check/identity/bank.go @@ -6,6 +6,7 @@ import ( "crypto/hmac" "crypto/rand" "encoding/json" + "errors" "fmt" "github.com/idfoundry/fapigo/server/interactioncookie" "net/http" @@ -278,23 +279,19 @@ func (b *bank) userinfo(w http.ResponseWriter, r *http.Request) { resource.WriteError(w, err) return } - if !slices.Contains(authz.Scopes, "openid") { - resource.NewError(resource.ErrorInsufficientScope, http.StatusForbidden, "the access token wasn't granted the openid scope").WriteJSON(w) - return - } - // The access token carries which UserInfo claims were requested and - // approved (server.RequestedUserinfoClaimsKey): a UserInfo call has - // no other link back to the authorization. - var names []string - _ = json.Unmarshal(authz.Claims[server.RequestedUserinfoClaimsKey], &names) - body := map[string]json.RawMessage{} - if len(names) > 0 { - if body, err = (customerClaims{}).ResolveIdentityClaims(r.Context(), authz.Subject, names); err != nil { - resource.NewError(resource.ErrorInvalidToken, http.StatusUnauthorized, "unknown customer").WriteJSON(w) + // Only the claims the client requested and the customer approved, + // which the access token carries: a UserInfo call has no other link + // back to the authorization. + body, err := serverresource.UserInfoClaims(r.Context(), authz, customerClaims{}) + if err != nil { + var rerr *resource.Error + if errors.As(err, &rerr) { + resource.WriteError(w, rerr) return } + resource.NewError(resource.ErrorInvalidToken, http.StatusUnauthorized, "unknown customer").WriteJSON(w) + return } - body["sub"], _ = json.Marshal(authz.Subject) // A token for a client the bank no longer knows is the token's // problem, not the bank's. The lookup error names the client from the // token, so it isn't logged. @@ -308,11 +305,9 @@ func (b *bank) userinfo(w http.ResponseWriter, r *http.Request) { srvErr.WriteJSON(w) return } - if authz.NextDPoPNonce != "" { - w.Header().Set("DPoP-Nonce", authz.NextDPoPNonce) - } + authz.SetDPoPNonce(w.Header()) w.Header().Set("Content-Type", "application/jwt") - _, _ = w.Write([]byte(signed)) + _, _ = w.Write([]byte(signed)) // #nosec G705 -- a signed compact JWT (application/jwt), never rendered as HTML } // authorize starts the interaction and shows the sign-in and consent diff --git a/resource/dpop_nonce_header.go b/resource/dpop_nonce_header.go new file mode 100644 index 0000000..707609b --- /dev/null +++ b/resource/dpop_nonce_header.go @@ -0,0 +1,16 @@ +package resource + +import "net/http" + +// SetDPoPNonce sets h's DPoP-Nonce header to a.NextDPoPNonce, when Verify +// issued one, so the client's next request already carries a valid +// nonce (RFC 9449 §8). Call it on every successful response: +// +// authz, err := verifier.Verify(ctx, resource.VerifyRequestFromHTTP(r, target)) +// // ... +// authz.SetDPoPNonce(w.Header()) +func (a AuthorizationContext) SetDPoPNonce(h http.Header) { + if a.NextDPoPNonce != "" { + h.Set("DPoP-Nonce", a.NextDPoPNonce) + } +} diff --git a/resource/dpop_nonce_header_test.go b/resource/dpop_nonce_header_test.go new file mode 100644 index 0000000..bac8873 --- /dev/null +++ b/resource/dpop_nonce_header_test.go @@ -0,0 +1,21 @@ +package resource_test + +import ( + "net/http" + "testing" + + "github.com/idfoundry/fapigo/resource" +) + +func TestSetDPoPNonce(t *testing.T) { + h := http.Header{} + resource.AuthorizationContext{NextDPoPNonce: "n-2"}.SetDPoPNonce(h) + if got := h.Get("DPoP-Nonce"); got != "n-2" { + t.Errorf("DPoP-Nonce = %q, want n-2", got) + } + h = http.Header{} + resource.AuthorizationContext{}.SetDPoPNonce(h) + if _, set := h["Dpop-Nonce"]; set { + t.Error("SetDPoPNonce set a header with no nonce issued") + } +} diff --git a/resource/verify.go b/resource/verify.go index 8bc1482..ae09046 100644 --- a/resource/verify.go +++ b/resource/verify.go @@ -86,7 +86,8 @@ type AuthorizationContext struct { // NextDPoPNonce is a freshly issued DPoP nonce the caller should set // as this response's own DPoP-Nonce header, so its next call already // carries a valid one instead of needing its own challenge/retry - // round trip (RFC 9449 §8's own proactive-refresh recommendation). + // round trip (RFC 9449 §8's own proactive-refresh recommendation): + // SetDPoPNonce does that. // Always "" when Dependencies.Nonces is nil (nonce-challenge support // disabled); otherwise always populated on a successful Verify. NextDPoPNonce string diff --git a/server/identity_claims.go b/server/identity_claims.go index 2bf3814..9794605 100644 --- a/server/identity_claims.go +++ b/server/identity_claims.go @@ -43,7 +43,8 @@ type IdentityClaimsSource interface { // were actually requested: unlike the ID token (issued once, at the // same time the request is known), a UserInfo call is a wholly separate // later request carrying only the access token, with no other link back -// to what was originally asked for. +// to what was originally asked for. serverresource.UserInfoClaims reads +// it and builds the response's claims from IdentityClaimsSource. const RequestedUserinfoClaimsKey = "requested_userinfo_claims" // requestedClaimsParameter is the OIDC Core §5.5 "claims" request diff --git a/serverresource/userinfo.go b/serverresource/userinfo.go new file mode 100644 index 0000000..b8d1982 --- /dev/null +++ b/serverresource/userinfo.go @@ -0,0 +1,58 @@ +package serverresource + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "slices" + + "github.com/idfoundry/fapigo/resource" + "github.com/idfoundry/fapigo/server" +) + +// UserInfoClaims builds a UserInfo response's claims (OIDC Core §5.3.2) +// for authz, the verified access token a UserInfo endpoint hosted with +// the server was called with: +// +// - a token without the openid scope is refused, as a 403 +// insufficient_scope *resource.Error; +// - the claims are the ones the client requested and the user approved +// (server.RequestedUserinfoClaimsKey, carried in the token, since a +// UserInfo call has no other link to its authorization), resolved by +// source, which isn't asked at all when none were — never every +// claim source knows; +// - a claim source returns that wasn't requested is dropped, whatever +// source does; +// - "sub" is the token's subject. +// +// A requested-claims entry the token carries malformed is an error, not +// an empty request. An error from source is returned wrapped. Sign the +// result with server.SignUserInfoResponse when the client registered for +// signed UserInfo, or write it as JSON. +func UserInfoClaims(ctx context.Context, authz resource.AuthorizationContext, source server.IdentityClaimsSource) (map[string]json.RawMessage, error) { + if !slices.Contains(authz.Scopes, "openid") { + return nil, resource.NewError(resource.ErrorInsufficientScope, http.StatusForbidden, "the access token wasn't granted the openid scope") + } + var names []string + if raw, ok := authz.Claims[server.RequestedUserinfoClaimsKey]; ok { + if err := json.Unmarshal(raw, &names); err != nil { + return nil, fmt.Errorf("serverresource: the access token's requested UserInfo claims are malformed: %w", err) + } + } + claims := map[string]json.RawMessage{} + if len(names) > 0 { + resolved, err := source.ResolveIdentityClaims(ctx, authz.Subject, names) + if err != nil { + return nil, fmt.Errorf("serverresource: resolving UserInfo claims: %w", err) + } + for name, value := range resolved { + if slices.Contains(names, name) { + claims[name] = value + } + } + } + // A string always encodes. + claims["sub"], _ = json.Marshal(authz.Subject) + return claims, nil +} diff --git a/serverresource/userinfo_test.go b/serverresource/userinfo_test.go new file mode 100644 index 0000000..365f00c --- /dev/null +++ b/serverresource/userinfo_test.go @@ -0,0 +1,83 @@ +package serverresource_test + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "testing" + + "github.com/idfoundry/fapigo/resource" + "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/serverresource" +) + +// claimsSource returns everything it has, whatever it's asked for, so +// the tests see UserInfoClaims do the restricting. +type claimsSource struct { + asked [][]string + err error +} + +func (s *claimsSource) ResolveIdentityClaims(_ context.Context, _ string, names []string) (map[string]json.RawMessage, error) { + s.asked = append(s.asked, names) + return map[string]json.RawMessage{ + "email": json.RawMessage(`"sam@example.com"`), + "name": json.RawMessage(`"Sam Rivera"`), + "phone": json.RawMessage(`"+44 7700 900000"`), + "sub": json.RawMessage(`"someone-else"`), + }, s.err +} + +func authzFor(scopes []string, requested string) resource.AuthorizationContext { + a := resource.AuthorizationContext{Subject: "user-1", Scopes: scopes, Claims: map[string]json.RawMessage{}} + if requested != "" { + a.Claims[server.RequestedUserinfoClaimsKey] = json.RawMessage(requested) + } + return a +} + +func TestUserInfoClaimsReturnsOnlyWhatWasRequested(t *testing.T) { + source := &claimsSource{} + claims, err := serverresource.UserInfoClaims(context.Background(), authzFor([]string{"openid"}, `["email","name"]`), source) + if err != nil { + t.Fatalf("UserInfoClaims: %v", err) + } + if len(claims) != 3 || string(claims["email"]) != `"sam@example.com"` || string(claims["name"]) != `"Sam Rivera"` { + t.Errorf("claims = %s, want email, name and sub only", claims) + } + if string(claims["sub"]) != `"user-1"` { + t.Errorf("sub = %s, want the token's subject", claims["sub"]) + } + if len(source.asked) != 1 || len(source.asked[0]) != 2 { + t.Errorf("source asked for %v, want [email name]", source.asked) + } +} + +func TestUserInfoClaimsWithNothingRequestedIsSubjectOnly(t *testing.T) { + for _, requested := range []string{"", `[]`, `null`} { + source := &claimsSource{} + claims, err := serverresource.UserInfoClaims(context.Background(), authzFor([]string{"openid"}, requested), source) + if err != nil || len(claims) != 1 || string(claims["sub"]) != `"user-1"` { + t.Errorf("requested %q: claims = %s, %v; want sub alone", requested, claims, err) + } + if len(source.asked) != 0 { + t.Errorf("requested %q: the source was asked for %v, want not at all", requested, source.asked) + } + } +} + +func TestUserInfoClaimsRefuses(t *testing.T) { + _, err := serverresource.UserInfoClaims(context.Background(), authzFor([]string{"accounts"}, `["email"]`), &claimsSource{}) + var rerr *resource.Error + if !errors.As(err, &rerr) || rerr.Code() != resource.ErrorInsufficientScope || rerr.HTTPStatus() != http.StatusForbidden { + t.Errorf("UserInfoClaims(no openid scope) = %v, want a 403 insufficient_scope", err) + } + if _, err := serverresource.UserInfoClaims(context.Background(), authzFor([]string{"openid"}, `"email"`), &claimsSource{}); err == nil { + t.Error("UserInfoClaims(malformed requested claims) = nil error") + } + failing := errors.New("directory unavailable") + if _, err := serverresource.UserInfoClaims(context.Background(), authzFor([]string{"openid"}, `["email"]`), &claimsSource{err: failing}); !errors.Is(err, failing) { + t.Errorf("UserInfoClaims(source fails) = %v, want it wrapped", err) + } +}