diff --git a/internal/federation/federation_test.go b/internal/federation/federation_test.go index ccbd836c..ecf3588d 100644 --- a/internal/federation/federation_test.go +++ b/internal/federation/federation_test.go @@ -968,7 +968,7 @@ func TestParseRefusesMisplacedClaims(t *testing.T) { t.Fatal(err) } _, err = Parse(token) - if gotErr := errors.Is(err, ErrMalformedClaims); gotErr != tc.wantErr { + if errors.Is(err, ErrMalformedClaims) != tc.wantErr { t.Errorf("%s: Parse = %v, want ErrMalformedClaims %v", name, err, tc.wantErr) } } diff --git a/internal/numericdate/numericdate_fuzz_test.go b/internal/numericdate/numericdate_fuzz_test.go index 4a425e03..3ab9c486 100644 --- a/internal/numericdate/numericdate_fuzz_test.go +++ b/internal/numericdate/numericdate_fuzz_test.go @@ -28,24 +28,32 @@ func FuzzNumericDateParse(f *testing.F) { } return } - if err != nil { - return + if err == nil { + checkAccepted(t, raw, got) } - var v any - if json.Unmarshal(raw, &v) != nil { - t.Fatalf("Parse accepted %q, which isn't JSON", raw) + }) +} + +// checkAccepted checks a value Parse accepted that isn't an int64: it's +// null, as 0, or a JSON number, floored to within a second of its +// float64 value. JSON has no NaN, so the comparison with 1<<53 is +// always defined. +func checkAccepted(t *testing.T, raw []byte, got int64) { + t.Helper() + var v any + if json.Unmarshal(raw, &v) != nil { + t.Fatalf("Parse accepted %q, which isn't JSON", raw) + } + switch n := v.(type) { + case nil: + if got != 0 { + t.Fatalf("Parse(null) = %d, want 0", got) } - switch n := v.(type) { - case nil: - if got != 0 { - t.Fatalf("Parse(null) = %d, want 0", got) - } - case float64: - if d := n - float64(got); d < 0 || d >= 1 && !(math.Abs(n) >= 1<<53) { - t.Fatalf("Parse(%q) = %d, not the floor of %v", raw, got, n) - } - default: - t.Fatalf("Parse accepted %q, a JSON %T", raw, v) + case float64: + if d := n - float64(got); d < 0 || d >= 1 && math.Abs(n) < 1<<53 { + t.Fatalf("Parse(%q) = %d, not the floor of %v", raw, got, n) } - }) + default: + t.Fatalf("Parse accepted %q, a JSON %T", raw, v) + } } diff --git a/server/complete_backchannel_authentication.go b/server/complete_backchannel_authentication.go index b97fb0a6..1458be82 100644 --- a/server/complete_backchannel_authentication.go +++ b/server/complete_backchannel_authentication.go @@ -10,6 +10,10 @@ import ( "github.com/idfoundry/fapigo/storage" ) +// invalidBackchannelHandleMessage is the one description given for a +// handle that can't be completed, whatever the reason. +const invalidBackchannelHandleMessage = "backchannel authentication handle is invalid, expired, or already decided" + // backchannelHandleHash is the SHA-256 digest of handle's raw wire // value — matching storage.NewBackchannelAuthentication.HandleHash's // digest-only discipline. @@ -57,13 +61,13 @@ func (s *Server) CompleteBackchannelAuthentication(ctx context.Context, req Comp // decision of any kind is recorded once the request has expired. pending, lookupErr := s.deps.Backchannel.LookupBackchannelAuthentication(ctx, handleHash) if lookupErr != nil { - wrapped := newError(ErrorInvalidRequest, 400, "backchannel authentication handle is invalid, expired, or already decided", lookupErr) + wrapped := newError(ErrorInvalidRequest, 400, invalidBackchannelHandleMessage, lookupErr) s.audit(ctx, AuditEventCompleteBackchannelAuthentication, "", AuditOutcomeFailure, string(wrapped.Code())) return wrapped } request, decodeErr := s.decodeOwnRequestRecord(pending.Request) if decodeErr != nil { - wrapped := recordFailure(decodeErr, newError(ErrorInvalidRequest, 400, "backchannel authentication handle is invalid, expired, or already decided", nil), "backchannel authentication request") + wrapped := recordFailure(decodeErr, newError(ErrorInvalidRequest, 400, invalidBackchannelHandleMessage, nil), "backchannel authentication request") s.audit(ctx, AuditEventCompleteBackchannelAuthentication, pending.ClientID, AuditOutcomeFailure, string(wrapped.Code())) return wrapped } @@ -92,7 +96,7 @@ func (s *Server) CompleteBackchannelAuthentication(ctx context.Context, req Comp decided, err := s.deps.Backchannel.DecideBackchannelAuthentication(ctx, decision) if err != nil { - wrapped := newError(ErrorInvalidRequest, 400, "backchannel authentication handle is invalid, expired, or already decided", err) + wrapped := newError(ErrorInvalidRequest, 400, invalidBackchannelHandleMessage, err) s.audit(ctx, AuditEventCompleteBackchannelAuthentication, "", AuditOutcomeFailure, string(wrapped.Code())) return wrapped } diff --git a/server/prompt_test.go b/server/prompt_test.go index c2f31f62..0784147b 100644 --- a/server/prompt_test.go +++ b/server/prompt_test.go @@ -84,60 +84,71 @@ func TestPushAuthorizationRequestRejectsPromptNoneWithAnotherValue(t *testing.T) // endpoint parse an arbitrarily long list. Repeats of one value within // the byte bound are still accepted. func TestPushAuthorizationRequestBoundsPrompt(t *testing.T) { - distinct := func(n int) string { - values := make([]string, n) - for i := range values { - values[i] = fmt.Sprintf("v%d", i) + for _, requestObject := range []bool{false, true} { + t.Run(fmt.Sprintf("request object %v", requestObject), func(t *testing.T) { + checkPromptBound(t, newHarness(t, server.ProfileFAPISecurity, true), requestObject) + }) + } +} + +// checkPromptBound pushes prompts over and within the bound, in a plain +// form or a request object. +func checkPromptBound(t *testing.T, h harness, requestObject bool) { + t.Helper() + for name, prompt := range map[string]string{ + "17 distinct values": distinctPromptValues(17), + "257 bytes": strings.Repeat("a", 257), + "20000 values": distinctPromptValues(20000), + } { + // A request object that long is already refused by its own size + // bound, as invalid_request_object. + want := server.ErrorInvalidRequest + if requestObject && len(prompt) > 4096 { + want = server.ErrorInvalidRequestObject + } + if code := serverErrorCode(t, pushPrompt(t, h, prompt, requestObject)); code != want { + t.Errorf("%s: error code %q, want %q", name, code, want) } - return strings.Join(values, " ") } - push := func(t *testing.T, h harness, prompt string, requestObject bool) error { - t.Helper() - if !requestObject { - _, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ - HTTP: server.FormRequest{Parameters: plainFormParameters(t, h.clientAssertion(t), map[string]string{"prompt": prompt})}, - }) - return err + for name, prompt := range map[string]string{ + "16 distinct values": distinctPromptValues(16), + "repeats within 256 bytes": strings.TrimSpace(strings.Repeat("login ", 42)), + } { + if err := pushPrompt(t, h, prompt, requestObject); err != nil { + t.Errorf("%s: %v, want accepted", name, err) } - params := standardAuthParams(t) - params["prompt"] = jsonRaw(t, prompt) + } +} + +// distinctPromptValues returns a prompt of n distinct values. +func distinctPromptValues(n int) string { + values := make([]string, n) + for i := range values { + values[i] = fmt.Sprintf("v%d", i) + } + return strings.Join(values, " ") +} + +// pushPrompt pushes a request carrying prompt, in a plain form or a +// request object. +func pushPrompt(t *testing.T, h harness, prompt string, requestObject bool) error { + t.Helper() + if !requestObject { _, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ - HTTP: server.FormRequest{Parameters: []server.FormParameter{ - formParam("client_assertion", h.clientAssertion(t)), - formParam("client_assertion_type", clientassertion.AssertionType), - formParam("request", h.requestObject(t, params)), - }}, + HTTP: server.FormRequest{Parameters: plainFormParameters(t, h.clientAssertion(t), map[string]string{"prompt": prompt})}, }) return err } - for _, requestObject := range []bool{false, true} { - t.Run(fmt.Sprintf("request object %v", requestObject), func(t *testing.T) { - h := newHarness(t, server.ProfileFAPISecurity, true) - for name, prompt := range map[string]string{ - "17 distinct values": distinct(17), - "257 bytes": strings.Repeat("a", 257), - "20000 values": distinct(20000), - } { - // A request object that long is already refused by its - // own size bound, as invalid_request_object. - want := server.ErrorInvalidRequest - if requestObject && len(prompt) > 4096 { - want = server.ErrorInvalidRequestObject - } - if code := serverErrorCode(t, push(t, h, prompt, requestObject)); code != want { - t.Errorf("%s: error code %q, want %q", name, code, want) - } - } - for name, prompt := range map[string]string{ - "16 distinct values": distinct(16), - "repeats within 256 bytes": strings.TrimSpace(strings.Repeat("login ", 42)), - } { - if err := push(t, h, prompt, requestObject); err != nil { - t.Errorf("%s: %v, want accepted", name, err) - } - } - }) - } + params := standardAuthParams(t) + params["prompt"] = jsonRaw(t, prompt) + _, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: []server.FormParameter{ + formParam("client_assertion", h.clientAssertion(t)), + formParam("client_assertion_type", clientassertion.AssertionType), + formParam("request", h.requestObject(t, params)), + }}, + }) + return err } // TestCompleteAuthorizationInteractionNeeded covers answering a diff --git a/server/response_type_test.go b/server/response_type_test.go index 56eb7b89..c2beb88a 100644 --- a/server/response_type_test.go +++ b/server/response_type_test.go @@ -27,18 +27,7 @@ func TestPushAuthorizationRequestRefusesResponseTypesOtherThanCode(t *testing.T) } { t.Run(name, func(t *testing.T) { h := newHarness(t, server.ProfileFAPISecurity, true) - var params []server.FormParameter - for _, p := range plainFormParameters(t, h.clientAssertion(t), nil) { - if p.Name != "response_type" { - params = append(params, p) - } - } - if responseType != missing { - params = append(params, formParam("response_type", responseType)) - } - _, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ - HTTP: server.FormRequest{Parameters: params}, - }) + err := pushResponseType(t, h, responseType, responseType == missing) want := server.ErrorUnsupportedResponseType if responseType == "" || responseType == missing { want = server.ErrorInvalidRequest @@ -49,3 +38,22 @@ func TestPushAuthorizationRequestRefusesResponseTypesOtherThanCode(t *testing.T) }) } } + +// pushResponseType pushes a plain request whose response_type is +// responseType, or that has none if omit. +func pushResponseType(t *testing.T, h harness, responseType string, omit bool) error { + t.Helper() + var params []server.FormParameter + for _, p := range plainFormParameters(t, h.clientAssertion(t), nil) { + if p.Name != "response_type" { + params = append(params, p) + } + } + if !omit { + params = append(params, formParam("response_type", responseType)) + } + _, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: params}, + }) + return err +}