refactor: clear 19 SonarCloud code smells

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;

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

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 {

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 {

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)
 		}
 	}

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")
 	}
 }

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 {

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)
+	}
+}

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,19 +35,46 @@ 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)
 			}
 		})
 	}
 
 	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{}
+}

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

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