diff --git a/client/production_urls_test.go b/client/production_urls_test.go index 96778990..ff747eed 100644 --- a/client/production_urls_test.go +++ b/client/production_urls_test.go @@ -86,38 +86,48 @@ func TestProtectedResourceDoRejectsLoopbackHTTPUnderProduction(t *testing.T) { {client.AssuranceDevelopment, false}, } { t.Run(fmt.Sprint(tc.assurance), func(t *testing.T) { - cfg := validConfig(t) - cfg.Assurance = tc.assurance - deps := validDependencies(t) - if tc.assurance == client.AssuranceProduction { - deps = productionDeps(t) - } - var calls int - deps.HTTP = countingHTTP{calls: &calls} - c, err := client.New(cfg, deps) - if err != nil { - t.Fatalf("New: %v", err) - } - rc := c.ProtectedResource(client.TokenSet{AccessToken: fapi.NewSecret("test-access-token")}) - req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://127.0.0.1:9999/accounts", nil) - if err != nil { - t.Fatalf("NewRequestWithContext: %v", err) - } - _, err = rc.Do(context.Background(), req) + calls, err := doLoopbackResourceRequest(t, tc.assurance) refused := err != nil && strings.Contains(err.Error(), "protected resource URL was parsed with fapi.AllowLoopbackHTTP") if refused != tc.refused { t.Fatalf("Do(loopback http) error = %v, want refused = %v", err, tc.refused) } - if tc.refused && calls != 0 { - t.Fatalf("Do sent %d request(s), want none", calls) + wantCalls := 1 + if tc.refused { + wantCalls = 0 } - if !tc.refused && calls != 1 { - t.Fatalf("Do sent %d request(s), want 1", calls) + if calls != wantCalls { + t.Fatalf("Do sent %d request(s), want %d", calls, wantCalls) } }) } } +// doLoopbackResourceRequest builds a client at assurance and sends a +// protected resource request to a loopback http URL through it, +// returning how many requests reached the HTTP client and Do's error. +func doLoopbackResourceRequest(t *testing.T, assurance client.AssuranceLevel) (int, error) { + t.Helper() + cfg := validConfig(t) + cfg.Assurance = assurance + deps := validDependencies(t) + if assurance == client.AssuranceProduction { + deps = productionDeps(t) + } + var calls int + deps.HTTP = countingHTTP{calls: &calls} + c, err := client.New(cfg, deps) + if err != nil { + t.Fatalf("New: %v", err) + } + rc := c.ProtectedResource(client.TokenSet{AccessToken: fapi.NewSecret("test-access-token")}) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://127.0.0.1:9999/accounts", nil) + if err != nil { + t.Fatalf("NewRequestWithContext: %v", err) + } + _, err = rc.Do(context.Background(), req) + return calls, err +} + // TestNewValidatesRedirectURI: Config.RedirectURI must be one an // authorization server following FAPI 2.0 would accept — https, a // native app's loopback http or a private-use scheme — checked at New; diff --git a/conformance/scripts/run-all.sh b/conformance/scripts/run-all.sh index 7d3497d6..4f90bb1d 100755 --- a/conformance/scripts/run-all.sh +++ b/conformance/scripts/run-all.sh @@ -545,7 +545,7 @@ run_federation_plan() { # bearer token (docker-compose.yml passes this variable). Random per # run unless the caller set one, since its port is published on # every interface. - if [ -z "${FEDERATION_TRUST_ANCHOR_ADMIN_TOKEN:-}" ]; then + if [[ -z "${FEDERATION_TRUST_ANCHOR_ADMIN_TOKEN:-}" ]]; then FEDERATION_TRUST_ANCHOR_ADMIN_TOKEN="$(python3 -c 'import secrets; print(secrets.token_hex(32))')" fi export FEDERATION_TRUST_ANCHOR_ADMIN_TOKEN diff --git a/internal/clientassertion/typ_test.go b/internal/clientassertion/typ_test.go index 1e6320a2..06ae2126 100644 --- a/internal/clientassertion/typ_test.go +++ b/internal/clientassertion/typ_test.go @@ -28,7 +28,7 @@ func TestParseRefusesOtherExplicitTypes(t *testing.T) { header = fmt.Sprintf(`{"alg":"ES256","typ":%q}`, typ) } _, err := Parse(enc([]byte(header)) + "." + enc([]byte(payload)) + "." + enc(make([]byte, 64))) - if got := errors.Is(err, jose.ErrOtherExplicitType); got != refused { + if errors.Is(err, jose.ErrOtherExplicitType) != refused { t.Fatalf("Parse(typ %q) error = %v, want refused = %v", typ, err, refused) } if !refused && err != nil { diff --git a/internal/jarm/typ_test.go b/internal/jarm/typ_test.go index 9e1c1e43..9eb11e10 100644 --- a/internal/jarm/typ_test.go +++ b/internal/jarm/typ_test.go @@ -24,7 +24,7 @@ func TestParseRefusesOtherExplicitTypes(t *testing.T) { header = fmt.Sprintf(`{"alg":"ES256","typ":%q}`, typ) } _, err := Parse(enc([]byte(header)) + "." + enc([]byte(payload)) + "." + enc(make([]byte, 64))) - if got := errors.Is(err, jose.ErrOtherExplicitType); got != refused { + if errors.Is(err, jose.ErrOtherExplicitType) != refused { t.Fatalf("Parse(typ %q) error = %v, want refused = %v", typ, err, refused) } if !refused && err != nil { diff --git a/internal/jose/typ_test.go b/internal/jose/typ_test.go index fbced6d2..80cee458 100644 --- a/internal/jose/typ_test.go +++ b/internal/jose/typ_test.go @@ -50,7 +50,7 @@ func TestRefuseOtherExplicitType(t *testing.T) { {"at+jwt", []string{"client-authentication+jwt"}, true}, } { err := RefuseOtherExplicitType(tc.typ, tc.own...) - if refused := errors.Is(err, ErrOtherExplicitType); refused != tc.refused { + if errors.Is(err, ErrOtherExplicitType) != tc.refused { t.Errorf("RefuseOtherExplicitType(%q, %v) = %v, want refused = %v", tc.typ, tc.own, err, tc.refused) } } diff --git a/internal/nofollow/nofollow_test.go b/internal/nofollow/nofollow_test.go index 32e27ab5..d051f54d 100644 --- a/internal/nofollow/nofollow_test.go +++ b/internal/nofollow/nofollow_test.go @@ -73,7 +73,7 @@ func TestClientNil(t *testing.T) { t.Fatalf("Client(nil) = %v, want nil", got) } var typedNil *http.Client - if got := nofollow.Client(typedNil); got != nofollow.Doer(typedNil) { + if nofollow.Client(typedNil) != nofollow.Doer(typedNil) { t.Fatal("Client(typed nil) should return it unchanged") } } diff --git a/internal/token/typ_test.go b/internal/token/typ_test.go index abe4620f..12950374 100644 --- a/internal/token/typ_test.go +++ b/internal/token/typ_test.go @@ -37,7 +37,7 @@ func TestParseIDTokenRefusesOtherExplicitTypes(t *testing.T) { } { t.Run(typ, func(t *testing.T) { _, err := ParseIDToken(unsignedCompact(typHeader(typ), payload)) - if got := errors.Is(err, jose.ErrOtherExplicitType); got != refused { + if errors.Is(err, jose.ErrOtherExplicitType) != refused { t.Fatalf("ParseIDToken(typ %q) error = %v, want refused = %v", typ, err, refused) } if !refused && err != nil { diff --git a/resource/bundled_resolver_validation_test.go b/resource/bundled_resolver_validation_test.go index 801c0f9e..b4bb60de 100644 --- a/resource/bundled_resolver_validation_test.go +++ b/resource/bundled_resolver_validation_test.go @@ -28,12 +28,7 @@ func TestNewVerifierRefusesIncompleteBundledResolvers(t *testing.T) { mutate(&j) for form, resolver := range map[string]resource.AccessTokenResolver{"value": j, "pointer": &j} { t.Run("JWTAccessTokens "+form+" "+want, func(t *testing.T) { - deps := validDependencies(t) - deps.AccessTokens = resolver - _, err := resource.NewVerifier(validConfig(t), deps) - if err == nil || !strings.Contains(err.Error(), "access_tokens (JWTAccessTokens): "+want) { - t.Fatalf("NewVerifier = %v, want %q", err, want) - } + expectResolverRefused(t, resolver, "access_tokens (JWTAccessTokens): "+want) }) } } @@ -43,12 +38,7 @@ func TestNewVerifierRefusesIncompleteBundledResolvers(t *testing.T) { "pointer": &resource.OpaqueAccessTokens{}, } { t.Run("OpaqueAccessTokens "+form+" without a store", func(t *testing.T) { - deps := validDependencies(t) - deps.AccessTokens = resolver - _, err := resource.NewVerifier(validConfig(t), deps) - if err == nil || !strings.Contains(err.Error(), "access_tokens (OpaqueAccessTokens): store is required") { - t.Fatalf("NewVerifier = %v, want the store required", err) - } + expectResolverRefused(t, resolver, "access_tokens (OpaqueAccessTokens): store is required") }) } @@ -57,11 +47,7 @@ func TestNewVerifierRefusesIncompleteBundledResolvers(t *testing.T) { "nil *OpaqueAccessTokens": (*resource.OpaqueAccessTokens)(nil), } { t.Run(name, func(t *testing.T) { - deps := validDependencies(t) - deps.AccessTokens = resolver - if _, err := resource.NewVerifier(validConfig(t), deps); err == nil || !strings.Contains(err.Error(), "is a "+name) { - t.Fatalf("NewVerifier = %v, want %s refused", err, name) - } + expectResolverRefused(t, resolver, "is a "+name) }) } @@ -80,3 +66,14 @@ func TestNewVerifierRefusesIncompleteBundledResolvers(t *testing.T) { } }) } + +// expectResolverRefused checks that NewVerifier refuses resolver as its +// access token resolver with an error containing want. +func expectResolverRefused(t *testing.T, resolver resource.AccessTokenResolver, want string) { + t.Helper() + deps := validDependencies(t) + deps.AccessTokens = resolver + if _, err := resource.NewVerifier(validConfig(t), deps); err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("NewVerifier = %v, want an error containing %q", err, want) + } +} diff --git a/server/begin_authorization_audit_test.go b/server/begin_authorization_audit_test.go index 383eddce..e1240aff 100644 --- a/server/begin_authorization_audit_test.go +++ b/server/begin_authorization_audit_test.go @@ -16,28 +16,6 @@ func TestBeginAuthorizationAuditsUnverifiedClientIDAsNone(t *testing.T) { const forged = "nobody\r\n2026-10-09T00:00:00Z AUDIT begin_authorization client=admin outcome=success" ctx := context.Background() - push := func(t *testing.T, h harness) string { - t.Helper() - pushed, err := h.server.PushAuthorizationRequest(ctx, server.PushAuthorizationRequest{ - HTTP: server.FormRequest{Parameters: plainFormParameters(t, h.clientAssertion(t), nil)}, - }) - if err != nil { - t.Fatalf("PushAuthorizationRequest: %v", err) - } - return pushed.RequestURI.String() - } - lastBegin := func(t *testing.T, h harness) server.AuditEvent { - t.Helper() - events := h.audit.all() - for i := len(events) - 1; i >= 0; i-- { - if events[i].Type == server.AuditEventBeginAuthorization { - return events[i] - } - } - t.Fatal("no begin_authorization audit event") - return server.AuditEvent{} - } - for name, tc := range map[string]struct { requestURI func(t *testing.T, h harness) string clientID string @@ -46,7 +24,7 @@ func TestBeginAuthorizationAuditsUnverifiedClientIDAsNone(t *testing.T) { "unknown request_uri": {func(*testing.T, harness) string { return "urn:ietf:params:oauth:request_uri:unknown" }, forged}, - "client_id doesn't match the pushed request": {push, forged}, + "client_id doesn't match the pushed request": {pushForAudit, forged}, } { t.Run(name, func(t *testing.T) { h := newHarness(t, server.ProfileFAPISecurity, true) @@ -57,7 +35,7 @@ func TestBeginAuthorizationAuditsUnverifiedClientIDAsNone(t *testing.T) { if _, ok := action.(server.LocalErrorResponse); !ok { t.Fatalf("action = %T, want LocalErrorResponse", action) } - if ev := lastBegin(t, h); ev.Outcome != server.AuditOutcomeFailure || ev.ClientID != "" { + if ev := lastBeginAuthorizationAudit(t, h); ev.Outcome != server.AuditOutcomeFailure || ev.ClientID != "" { t.Fatalf("audit event = %+v, want a failure with no client", ev) } }) @@ -65,11 +43,38 @@ func TestBeginAuthorizationAuditsUnverifiedClientIDAsNone(t *testing.T) { t.Run("verified client is audited", func(t *testing.T) { h := newHarness(t, server.ProfileFAPISecurity, true) - if _, err := h.server.BeginAuthorization(ctx, server.BeginAuthorizationRequest{RequestURI: push(t, h), ClientID: testClientID}); err != nil { + if _, err := h.server.BeginAuthorization(ctx, server.BeginAuthorizationRequest{RequestURI: pushForAudit(t, h), ClientID: testClientID}); err != nil { t.Fatalf("BeginAuthorization: %v", err) } - if ev := lastBegin(t, h); ev.Outcome != server.AuditOutcomeSuccess || ev.ClientID != testClientID { + if ev := lastBeginAuthorizationAudit(t, h); ev.Outcome != server.AuditOutcomeSuccess || ev.ClientID != testClientID { t.Fatalf("audit event = %+v, want a success for %s", ev, testClientID) } }) } + +// pushForAudit pushes a plain-parameter request for h's client and +// returns its request_uri. +func pushForAudit(t *testing.T, h harness) string { + t.Helper() + pushed, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: plainFormParameters(t, h.clientAssertion(t), nil)}, + }) + if err != nil { + t.Fatalf("PushAuthorizationRequest: %v", err) + } + return pushed.RequestURI.String() +} + +// lastBeginAuthorizationAudit returns h's most recent +// begin_authorization audit event. +func lastBeginAuthorizationAudit(t *testing.T, h harness) server.AuditEvent { + t.Helper() + events := h.audit.all() + for i := len(events) - 1; i >= 0; i-- { + if events[i].Type == server.AuditEventBeginAuthorization { + return events[i] + } + } + t.Fatal("no begin_authorization audit event") + return server.AuditEvent{} +} diff --git a/server/claims_parameter.go b/server/claims_parameter.go index 32bd05f5..914611e2 100644 --- a/server/claims_parameter.go +++ b/server/claims_parameter.go @@ -33,14 +33,24 @@ func validateClaimsParameter(params map[string]json.RawMessage) error { if !ok || isJSONNull(value) { continue } - var requests map[string]json.RawMessage - if trimmed := bytes.TrimSpace(value); len(trimmed) == 0 || trimmed[0] != '{' || json.Unmarshal(trimmed, &requests) != nil { - return fmt.Errorf("claims.%s must be a JSON object", member) + if err := validateClaimsLocationValue(member, value); err != nil { + return err } - for name, request := range requests { - if trimmed := bytes.TrimSpace(request); !isJSONNull(trimmed) && (len(trimmed) == 0 || trimmed[0] != '{') { - return fmt.Errorf("claims.%s.%s must be null or a JSON object", member, name) - } + } + return nil +} + +// validateClaimsLocationValue checks one present, non-null location +// member of the "claims" parameter ("id_token" or "userinfo"): it must +// be a JSON object mapping each claim name to null or an object. +func validateClaimsLocationValue(member string, value json.RawMessage) error { + var requests map[string]json.RawMessage + if trimmed := bytes.TrimSpace(value); len(trimmed) == 0 || trimmed[0] != '{' || json.Unmarshal(trimmed, &requests) != nil { + return fmt.Errorf("claims.%s must be a JSON object", member) + } + for name, request := range requests { + if trimmed := bytes.TrimSpace(request); !isJSONNull(trimmed) && (len(trimmed) == 0 || trimmed[0] != '{') { + return fmt.Errorf("claims.%s.%s must be null or a JSON object", member, name) } } return nil diff --git a/server/essential_acr.go b/server/essential_acr.go index 3d23d226..3d4ec114 100644 --- a/server/essential_acr.go +++ b/server/essential_acr.go @@ -24,6 +24,28 @@ import ( // parseRequestedClaimNames). A "claims" value that is neither is left // to the claims parameter's own handling. func essentialACRValues(params map[string]json.RawMessage) ([]string, error) { + entry, err := idTokenACREntry(params) + if entry == nil || err != nil { + return nil, err + } + essential, err := acrEntryEssential(entry) + if err != nil { + return nil, err + } + values, err := acrEntryValues(entry) + if err != nil { + return nil, err + } + if !essential || len(values) == 0 { + return nil, nil + } + return dedupeStrings(values), nil +} + +// idTokenACREntry returns the "claims" parameter's id_token "acr" entry +// with its member names checked: nil, with no error, when there is no +// such entry or it's null. +func idTokenACREntry(params map[string]json.RawMessage) (map[string]json.RawMessage, error) { idToken, ok := claimsIDTokenMembers(params["claims"]) if !ok { return nil, nil @@ -39,13 +61,24 @@ func essentialACRValues(params map[string]json.RawMessage) ([]string, error) { if err := checkMemberCase(entry, `claims: the id_token "acr" entry's`, "essential", "value", "values"); err != nil { return nil, err } + return entry, nil +} +// acrEntryEssential reads the "acr" entry's "essential" member, false +// when absent. +func acrEntryEssential(entry map[string]json.RawMessage) (bool, error) { essential := false if v, ok := entry["essential"]; ok { if err := json.Unmarshal(v, &essential); err != nil { - return nil, errors.New(`claims: the id_token "acr" entry's "essential" must be a boolean`) + return false, errors.New(`claims: the id_token "acr" entry's "essential" must be a boolean`) } } + return essential, nil +} + +// acrEntryValues reads the "acr" entry's "value" and then "values" +// members, in that order. +func acrEntryValues(entry map[string]json.RawMessage) ([]string, error) { var values []string if v, ok := entry["value"]; ok { var value string @@ -61,16 +94,18 @@ func essentialACRValues(params map[string]json.RawMessage) ([]string, error) { } values = append(values, list...) } - if !essential || len(values) == 0 { - return nil, nil - } + return values, nil +} + +// dedupeStrings returns values without repeats, in first-seen order. +func dedupeStrings(values []string) []string { out := make([]string, 0, len(values)) for _, v := range values { if !slices.Contains(out, v) { out = append(out, v) } } - return out, nil + return out } // claimsIDTokenMembers returns the "claims" parameter's id_token diff --git a/server/essential_acr_test.go b/server/essential_acr_test.go index 9919fcca..baa1052d 100644 --- a/server/essential_acr_test.go +++ b/server/essential_acr_test.go @@ -65,15 +65,7 @@ func forgetEssentialACR(t *testing.T, h harness) { // a voluntary request, acr_values, and a record written before the // requirement was kept aren't enforced. func TestCompleteAuthorizationEnforcesEssentialACR(t *testing.T) { - for _, tc := range []struct { - name string - claims string - acrValues string - acr string - legacy bool - wantError string - wantValues []string - }{ + for _, tc := range []essentialACRCase{ {name: "values, met", claims: acrClaims(`{"essential":true,"values":["urn:gold","urn:silver"]}`), acr: "urn:silver", wantValues: []string{"urn:gold", "urn:silver"}}, {name: "values, not met", claims: acrClaims(`{"essential":true,"values":["urn:gold"]}`), acr: "urn:silver", wantError: "login_required", wantValues: []string{"urn:gold"}}, {name: "single value, met", claims: acrClaims(`{"essential":true,"value":"urn:gold"}`), acr: "urn:gold", wantValues: []string{"urn:gold"}}, @@ -85,42 +77,58 @@ func TestCompleteAuthorizationEnforcesEssentialACR(t *testing.T) { {name: "acr_values only", acrValues: "urn:gold", acr: "urn:bronze"}, {name: "record without the requirement", claims: acrClaims(`{"essential":true,"value":"urn:gold"}`), acr: "urn:bronze", legacy: true, wantValues: []string{"urn:gold"}}, } { - t.Run(tc.name, func(t *testing.T) { - h := newHarness(t, server.ProfileFAPISecurity, true) - extra := map[string]string{} - if tc.claims != "" { - extra["claims"] = tc.claims - } - if tc.acrValues != "" { - extra["acr_values"] = tc.acrValues - } - required := interactionFor(t, h, extra) - if !slices.Equal(required.Interaction.EssentialACRValues, tc.wantValues) { - t.Errorf("EssentialACRValues = %q, want %q", required.Interaction.EssentialACRValues, tc.wantValues) - } - if tc.legacy { - forgetEssentialACR(t, h) - } - completed, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{Handle: required.Handle, Result: authorizedWithACR(t, h, tc.acr)}) - if err != nil { - t.Fatalf("CompleteAuthorization: %v", err) - } - redirect, ok := completed.(server.AuthorizationRedirect) - if !ok { - t.Fatalf("result = %T, want a redirect", completed) - } - dest := redirect.Destination().URL() - q := dest.Query() - if q.Get("error") != tc.wantError { - t.Errorf("error = %q (%s), want %q", q.Get("error"), q.Get("error_description"), tc.wantError) - } - if (q.Get("code") != "") != (tc.wantError == "") { - t.Errorf("code present = %v, want %v", q.Get("code") != "", tc.wantError == "") - } - if tc.wantError != "" && len(h.grants.all()) != 0 { - t.Error("a code was stored for a refused completion") - } - }) + t.Run(tc.name, func(t *testing.T) { runEssentialACRCase(t, tc) }) + } +} + +// essentialACRCase is one TestCompleteAuthorizationEnforcesEssentialACR +// case. +type essentialACRCase struct { + name string + claims string + acrValues string + acr string + legacy bool + wantError string + wantValues []string +} + +// runEssentialACRCase pushes tc's request, checks the interaction's +// EssentialACRValues, completes it with tc.acr and checks the redirect. +func runEssentialACRCase(t *testing.T, tc essentialACRCase) { + h := newHarness(t, server.ProfileFAPISecurity, true) + extra := map[string]string{} + if tc.claims != "" { + extra["claims"] = tc.claims + } + if tc.acrValues != "" { + extra["acr_values"] = tc.acrValues + } + required := interactionFor(t, h, extra) + if !slices.Equal(required.Interaction.EssentialACRValues, tc.wantValues) { + t.Errorf("EssentialACRValues = %q, want %q", required.Interaction.EssentialACRValues, tc.wantValues) + } + if tc.legacy { + forgetEssentialACR(t, h) + } + completed, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{Handle: required.Handle, Result: authorizedWithACR(t, h, tc.acr)}) + if err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + redirect, ok := completed.(server.AuthorizationRedirect) + if !ok { + t.Fatalf("result = %T, want a redirect", completed) + } + dest := redirect.Destination().URL() + q := dest.Query() + if q.Get("error") != tc.wantError { + t.Errorf("error = %q (%s), want %q", q.Get("error"), q.Get("error_description"), tc.wantError) + } + if (q.Get("code") != "") != (tc.wantError == "") { + t.Errorf("code present = %v, want %v", q.Get("code") != "", tc.wantError == "") + } + if tc.wantError != "" && len(h.grants.all()) != 0 { + t.Error("a code was stored for a refused completion") } } diff --git a/server/interaction_result.go b/server/interaction_result.go index e8672791..b8b47e4e 100644 --- a/server/interaction_result.go +++ b/server/interaction_result.go @@ -110,7 +110,10 @@ type interactionNeededResult struct { } // Discriminator for InteractionResult — deliberately empty. -func (interactionNeededResult) interactionResult() {} +func (interactionNeededResult) interactionResult() { + // Marks interactionNeededResult as an InteractionResult; it has no + // behaviour of its own. +} // InteractionNeeded records that the interaction couldn't be concluded // without showing the end user something it wasn't allowed to show: diff --git a/server/metadata.go b/server/metadata.go index af14e19d..d36e31ad 100644 --- a/server/metadata.go +++ b/server/metadata.go @@ -268,24 +268,7 @@ func (s *Server) Metadata(_ context.Context) Metadata { // UserInfo, so advertising them would describe capabilities it // doesn't have. if !s.cfg.OAuthOnly { - md.SubjectTypesSupported = []string{"public"} - md.IDTokenSigningAlgValuesSupported = []string{s.cfg.Algorithms.IDToken.String()} - md.ClaimsParameterSupported = s.deps.IdentityClaims != nil - if len(s.cfg.Algorithms.IDTokenEncryptionKeyManagement) > 0 { - md.IDTokenEncryptionAlgValuesSupported = s.cfg.Algorithms.IDTokenEncryptionKeyManagement.Strings() - } - if len(s.cfg.Algorithms.IDTokenEncryptionContentEncryption) > 0 { - md.IDTokenEncryptionEncValuesSupported = s.cfg.Algorithms.IDTokenEncryptionContentEncryption.Strings() - } - if s.cfg.Algorithms.UserInfo != 0 { - md.UserinfoSigningAlgValuesSupported = []string{s.cfg.Algorithms.UserInfo.String()} - } - if len(s.cfg.Algorithms.UserInfoEncryptionKeyManagement) > 0 { - md.UserinfoEncryptionAlgValuesSupported = s.cfg.Algorithms.UserInfoEncryptionKeyManagement.Strings() - } - if len(s.cfg.Algorithms.UserInfoEncryptionContentEncryption) > 0 { - md.UserinfoEncryptionEncValuesSupported = s.cfg.Algorithms.UserInfoEncryptionContentEncryption.Strings() - } + s.addOIDCMetadata(&md) } if len(s.cfg.AutomaticRegistration.TrustAnchors) > 0 { @@ -363,6 +346,30 @@ func (s *Server) Metadata(_ context.Context) Metadata { return md } +// addOIDCMetadata sets the OpenID Connect Discovery 1.0 fields: the +// ones that only mean something with ID tokens or UserInfo, which an +// OAuthOnly server leaves out (see Metadata). +func (s *Server) addOIDCMetadata(md *Metadata) { + md.SubjectTypesSupported = []string{"public"} + md.IDTokenSigningAlgValuesSupported = []string{s.cfg.Algorithms.IDToken.String()} + md.ClaimsParameterSupported = s.deps.IdentityClaims != nil + if len(s.cfg.Algorithms.IDTokenEncryptionKeyManagement) > 0 { + md.IDTokenEncryptionAlgValuesSupported = s.cfg.Algorithms.IDTokenEncryptionKeyManagement.Strings() + } + if len(s.cfg.Algorithms.IDTokenEncryptionContentEncryption) > 0 { + md.IDTokenEncryptionEncValuesSupported = s.cfg.Algorithms.IDTokenEncryptionContentEncryption.Strings() + } + if s.cfg.Algorithms.UserInfo != 0 { + md.UserinfoSigningAlgValuesSupported = []string{s.cfg.Algorithms.UserInfo.String()} + } + if len(s.cfg.Algorithms.UserInfoEncryptionKeyManagement) > 0 { + md.UserinfoEncryptionAlgValuesSupported = s.cfg.Algorithms.UserInfoEncryptionKeyManagement.Strings() + } + if len(s.cfg.Algorithms.UserInfoEncryptionContentEncryption) > 0 { + md.UserinfoEncryptionEncValuesSupported = s.cfg.Algorithms.UserInfoEncryptionContentEncryption.Strings() + } +} + // urlOrNil returns nil for u's zero value, and a pointer to u otherwise // — the constructor every optional *fapi.URL metadata field goes // through, so its own "omitted unless set" contract actually holds (see diff --git a/server/prompt_test.go b/server/prompt_test.go index 14589b4e..7ae8fae2 100644 --- a/server/prompt_test.go +++ b/server/prompt_test.go @@ -85,31 +85,7 @@ func TestCompleteAuthorizationInteractionNeeded(t *testing.T) { server.NeedAccountSelection: "account_selection_required", server.NeedInteraction: "interaction_required", } { - t.Run(code, func(t *testing.T) { - h := newHarness(t, server.ProfileFAPISecurity, true) - handle := beginInteraction(t, h) - result, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{ - Handle: handle, Result: server.InteractionNeeded(need, "no session"), - }) - if err != nil { - t.Fatalf("CompleteAuthorization: %v", err) - } - redirect, ok := result.(server.AuthorizationRedirect) - if !ok { - t.Fatalf("result = %T, want server.AuthorizationRedirect", result) - } - dest := redirect.Destination().URL() - query := dest.Query() - if got := query.Get("error"); got != code { - t.Fatalf("error = %q, want %q", got, code) - } - if got := query.Get("error_description"); got != "no session" { - t.Errorf("error_description = %q, want the reason", got) - } - if query.Has("code") || len(h.grants.all()) != 0 { - t.Fatalf("a code was issued for %s", code) - } - }) + t.Run(code, func(t *testing.T) { runInteractionNeededCase(t, need, code) }) } t.Run("not an InteractionNeed", func(t *testing.T) { @@ -128,6 +104,35 @@ func TestCompleteAuthorizationInteractionNeeded(t *testing.T) { }) } +// runInteractionNeededCase completes an interaction with +// InteractionNeeded(need) and checks the client is sent code, with the +// reason and no authorization code. +func runInteractionNeededCase(t *testing.T, need server.InteractionNeed, code string) { + h := newHarness(t, server.ProfileFAPISecurity, true) + handle := beginInteraction(t, h) + result, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{ + Handle: handle, Result: server.InteractionNeeded(need, "no session"), + }) + if err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + redirect, ok := result.(server.AuthorizationRedirect) + if !ok { + t.Fatalf("result = %T, want server.AuthorizationRedirect", result) + } + dest := redirect.Destination().URL() + query := dest.Query() + if got := query.Get("error"); got != code { + t.Fatalf("error = %q, want %q", got, code) + } + if got := query.Get("error_description"); got != "no session" { + t.Errorf("error_description = %q, want the reason", got) + } + if query.Has("code") || len(h.grants.all()) != 0 { + t.Fatalf("a code was issued for %s", code) + } +} + // forgetPushedAt rewrites every pending interaction's stored request // without pushed_at, as a record written before it existed is. func forgetPushedAt(t *testing.T, h harness) { @@ -161,14 +166,7 @@ func forgetPushedAt(t *testing.T, h harness) { // client with login_required rather than a code; other prompts and a // record written before the push time was kept aren't checked. func TestCompleteAuthorizationEnforcesPromptLogin(t *testing.T) { - for _, tc := range []struct { - name string - prompt string - authAgo time.Duration - legacy bool - zeroAuth bool - wantError string - }{ + for _, tc := range []promptLoginCase{ {name: "authenticated for the request", prompt: "login", authAgo: 0}, {name: "authenticated within the skew", prompt: "login", authAgo: 4 * time.Second}, {name: "existing session", prompt: "login", authAgo: time.Minute, wantError: "login_required"}, @@ -180,41 +178,56 @@ func TestCompleteAuthorizationEnforcesPromptLogin(t *testing.T) { {name: "prompt=none", prompt: "none", authAgo: time.Hour}, {name: "no prompt", authAgo: time.Hour}, } { - t.Run(tc.name, func(t *testing.T) { - h := newHarness(t, server.ProfileFAPISecurity, true) - extra := map[string]string{} - if tc.prompt != "" { - extra["prompt"] = tc.prompt - } - handle := interactionFor(t, h, extra).Handle - if tc.legacy { - forgetPushedAt(t, h) - } - result := authorizedAt(t, h.now.Add(-tc.authAgo)) - if tc.zeroAuth { - subjectID, _ := server.NewSubjectID("user-1") - subject, _ := server.NewAuthenticatedSubject(subjectID) - result = server.Authorize(subject, server.AuthenticationContext{}, server.GrantedAuthorization{Scope: []string{"openid", "accounts"}}) - } - completed, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{Handle: handle, Result: result}) - if err != nil { - t.Fatalf("CompleteAuthorization: %v", err) - } - redirect, ok := completed.(server.AuthorizationRedirect) - if !ok { - t.Fatalf("result = %T, want a redirect", completed) - } - dest := redirect.Destination().URL() - q := dest.Query() - if q.Get("error") != tc.wantError { - t.Errorf("error = %q (%s), want %q", q.Get("error"), q.Get("error_description"), tc.wantError) - } - if (q.Get("code") != "") != (tc.wantError == "") { - t.Errorf("code present = %v, want %v", q.Get("code") != "", tc.wantError == "") - } - if tc.wantError != "" && len(h.grants.all()) != 0 { - t.Error("a code was stored for a refused completion") - } - }) + t.Run(tc.name, func(t *testing.T) { runPromptLoginCase(t, tc) }) + } +} + +// promptLoginCase is one TestCompleteAuthorizationEnforcesPromptLogin +// case. +type promptLoginCase struct { + name string + prompt string + authAgo time.Duration + legacy bool + zeroAuth bool + wantError string +} + +// runPromptLoginCase pushes tc's request, completes it with an +// authentication tc.authAgo before now and checks the redirect. +func runPromptLoginCase(t *testing.T, tc promptLoginCase) { + h := newHarness(t, server.ProfileFAPISecurity, true) + extra := map[string]string{} + if tc.prompt != "" { + extra["prompt"] = tc.prompt + } + handle := interactionFor(t, h, extra).Handle + if tc.legacy { + forgetPushedAt(t, h) + } + result := authorizedAt(t, h.now.Add(-tc.authAgo)) + if tc.zeroAuth { + subjectID, _ := server.NewSubjectID("user-1") + subject, _ := server.NewAuthenticatedSubject(subjectID) + result = server.Authorize(subject, server.AuthenticationContext{}, server.GrantedAuthorization{Scope: []string{"openid", "accounts"}}) + } + completed, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{Handle: handle, Result: result}) + if err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + redirect, ok := completed.(server.AuthorizationRedirect) + if !ok { + t.Fatalf("result = %T, want a redirect", completed) + } + dest := redirect.Destination().URL() + q := dest.Query() + if q.Get("error") != tc.wantError { + t.Errorf("error = %q (%s), want %q", q.Get("error"), q.Get("error_description"), tc.wantError) + } + if (q.Get("code") != "") != (tc.wantError == "") { + t.Errorf("code present = %v, want %v", q.Get("code") != "", tc.wantError == "") + } + if tc.wantError != "" && len(h.grants.all()) != 0 { + t.Error("a code was stored for a refused completion") } } diff --git a/server/refresh_registration_test.go b/server/refresh_registration_test.go index 2fb9ebc2..0e88af54 100644 --- a/server/refresh_registration_test.go +++ b/server/refresh_registration_test.go @@ -134,7 +134,7 @@ func TestCIBAExchangeRechecksClient(t *testing.T) { }}, DPoPProofs: []string{createDPoPProof(t, generateKey(t), h.now)}, }) - if code := serverErrorCode(t, err); code != tc.want { + if serverErrorCode(t, err) != tc.want { t.Fatalf("ExchangeBackchannelAuthentication after re-registration: %v, want %s", err, tc.want) } }) @@ -222,7 +222,7 @@ func TestCIBAExchangeRechecksServerRARRegistry(t *testing.T) { }}, DPoPProofs: []string{createDPoPProof(t, generateKey(t), h.now)}, }) - if code := serverErrorCode(t, err); code != server.ErrorInvalidGrant { + if serverErrorCode(t, err) != server.ErrorInvalidGrant { t.Fatalf("ExchangeBackchannelAuthentication after the registry changed: %v, want invalid_grant", err) } }) diff --git a/serverresource/sign_userinfo_test.go b/serverresource/sign_userinfo_test.go index b9e88592..7088d2c5 100644 --- a/serverresource/sign_userinfo_test.go +++ b/serverresource/sign_userinfo_test.go @@ -128,9 +128,7 @@ func signedPayload(t *testing.T, compact string) map[string]any { // client the token was issued to, about the token's subject, and // refused for any other subject or for a client no longer registered. func TestSignUserInfoResponseIsForTheTokensClient(t *testing.T) { - srv, clients := userInfoServer(t) - authz := resource.AuthorizationContext{Subject: "user-1", ClientID: "rp-2", Scopes: []string{"openid"}} - claims := map[string]json.RawMessage{"sub": json.RawMessage(`"user-1"`), "email": json.RawMessage(`"sam@example.com"`)} + srv, clients, authz, claims := signUserInfoFixture(t) signed, err := serverresource.SignUserInfoResponse(context.Background(), srv, clients, authz, claims) if err != nil { @@ -161,6 +159,16 @@ func TestSignUserInfoResponseIsForTheTokensClient(t *testing.T) { } }) +} + +// TestSignUserInfoResponseReportsLookupAndSigningFailures covers how +// SignUserInfoResponse reports a client it can't resolve, an +// unavailable client repository, and a failure to sign: an unknown +// client is 401 invalid_token, an outage 500 server_error, both keeping +// the repository's error, and a signing failure the server's own error. +func TestSignUserInfoResponseReportsLookupAndSigningFailures(t *testing.T) { + srv, clients, authz, claims := signUserInfoFixture(t) + t.Run("unknown client", func(t *testing.T) { unknown := authz unknown.ClientID = "rp-gone" @@ -204,3 +212,14 @@ func TestSignUserInfoResponseIsForTheTokensClient(t *testing.T) { } }) } + +// signUserInfoFixture returns a server, its client repository, the +// authorization context of an openid access token for user-1 issued to +// rp-2, and UserInfo claims about user-1. +func signUserInfoFixture(t *testing.T) (*server.Server, storage.ClientRepository, resource.AuthorizationContext, map[string]json.RawMessage) { + t.Helper() + srv, clients := userInfoServer(t) + authz := resource.AuthorizationContext{Subject: "user-1", ClientID: "rp-2", Scopes: []string{"openid"}} + claims := map[string]json.RawMessage{"sub": json.RawMessage(`"user-1"`), "email": json.RawMessage(`"sam@example.com"`)} + return srv, clients, authz, claims +}