diff --git a/internal/api/coverage_gaps_test.go b/internal/api/coverage_gaps_test.go index 8b6bd17a9..9c3a82d5f 100644 --- a/internal/api/coverage_gaps_test.go +++ b/internal/api/coverage_gaps_test.go @@ -2,7 +2,7 @@ package api // coverage_gaps_test.go — additional tests to push internal/api coverage above 80%. // Targets: parseAccountIDs, redactEmail, mergeServiceConfig, checkRateLimit, -// checkUserAPIKey, ambientCredResult, credTypeForAccount, +// ambientCredResult, credTypeForAccount, // checkCredentialPresence, validateCSRF, requireAdmin, // getRecommendations (more cases), sendPurchaseApprovalEmail. @@ -261,37 +261,21 @@ func TestHandler_checkRateLimit_Exceeded(t *testing.T) { } // --------------------------------------------------------------------------- -// checkUserAPIKey +// user API key validation failure (via requireAuth) // --------------------------------------------------------------------------- -func TestHandler_checkUserAPIKey_ValidKey(t *testing.T) { - ctx := context.Background() - mockAuth := new(MockAuthService) - mockAuth.On("ValidateUserAPIKeyAPI", ctx, "valid-user-key"). - Return("user-id", map[string]interface{}{}, nil) - - h := &Handler{auth: mockAuth} - assert.True(t, h.checkUserAPIKey(ctx, "valid-user-key")) -} - -func TestHandler_checkUserAPIKey_InvalidKey(t *testing.T) { +func TestRequireAuth_UserAPIKey_InvalidKey(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) mockAuth.On("ValidateUserAPIKeyAPI", ctx, "bad-key"). Return(nil, nil, errors.New("invalid key")) h := &Handler{auth: mockAuth} - assert.False(t, h.checkUserAPIKey(ctx, "bad-key")) -} - -func TestHandler_checkUserAPIKey_EmptyKey(t *testing.T) { - h := &Handler{auth: nil} - assert.False(t, h.checkUserAPIKey(context.Background(), "")) -} - -func TestHandler_checkUserAPIKey_NilAuth(t *testing.T) { - h := &Handler{auth: nil} - assert.False(t, h.checkUserAPIKey(context.Background(), "some-key")) + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"X-API-Key": "bad-key"}} + _, err := h.requireAuth(ctx, req) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 401, ce.code) } // --------------------------------------------------------------------------- diff --git a/internal/api/middleware.go b/internal/api/middleware.go index f2d95cc92..b8716f6ed 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -96,25 +96,10 @@ func principalFromContext(ctx context.Context) (*Principal, bool) { return principal, ok && principal != nil } -// authenticate checks authentication via admin API key, user API key, or Bearer token. -func (h *Handler) authenticate(ctx context.Context, req *events.LambdaFunctionURLRequest) bool { - apiKey := extractAPIKey(req) - - if h.checkAdminAPIKey(apiKey) { - return true - } - - if h.checkUserAPIKey(ctx, apiKey) { - return true - } - - return h.checkBearerToken(ctx, req) -} - -// authenticatePrincipal performs the same three-path credential check as -// authenticate but returns the fully resolved Principal so callers do not -// need to repeat the lookup. Returns a non-nil Principal on success; returns -// nil and a 401 ClientError when no valid credential is present. +// authenticatePrincipal checks the admin API key, user API key and Bearer +// token credentials and returns the fully resolved Principal. Returns a +// non-nil Principal on success; returns nil and a 401 ClientError when no +// valid credential is present. func (h *Handler) authenticatePrincipal(ctx context.Context, req *events.LambdaFunctionURLRequest) (*Principal, error) { apiKey := extractAPIKey(req) @@ -216,28 +201,6 @@ func (h *Handler) checkAdminAPIKey(apiKey string) bool { return false } -func (h *Handler) checkUserAPIKey(ctx context.Context, apiKey string) bool { - if apiKey != "" && h.auth != nil { - _, _, err := h.auth.ValidateUserAPIKeyAPI(ctx, apiKey) - if err == nil { - return true - } - logging.Debugf("User API key validation failed: %v", err) - } - return false -} - -func (h *Handler) checkBearerToken(ctx context.Context, req *events.LambdaFunctionURLRequest) bool { - token := h.extractBearerToken(req) - if token != "" && h.auth != nil { - _, err := h.auth.ValidateSession(ctx, token) - if err == nil { - return true - } - } - return false -} - // extractBearerToken extracts the token from the Authorization or X-Authorization header // Note: X-Authorization is used by the frontend because CloudFront OAC signs requests // with SigV4, which overwrites the standard Authorization header. diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go index 4b7929142..af8d83d3e 100644 --- a/internal/api/middleware_test.go +++ b/internal/api/middleware_test.go @@ -135,8 +135,8 @@ func TestHandler_authenticate(t *testing.T) { Headers: tt.headers, QueryStringParameters: tt.params, } - result := handler.authenticate(ctx, req) - assert.Equal(t, tt.expected, result) + principal, err := handler.authenticatePrincipal(ctx, req) + assert.Equal(t, tt.expected, err == nil && principal != nil) }) } } @@ -217,7 +217,7 @@ func TestHandler_extractBearerToken(t *testing.T) { } } -func TestHandler_authenticate_BearerToken(t *testing.T) { +func TestHandler_authenticatePrincipal_BearerToken(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) @@ -237,7 +237,9 @@ func TestHandler_authenticate_BearerToken(t *testing.T) { "Authorization": "Bearer valid-token", }, } - assert.True(t, handler.authenticate(ctx, req)) + principal, err := handler.authenticatePrincipal(ctx, req) + require.NoError(t, err) + assert.Equal(t, PrincipalSession, principal.Kind) // Test invalid token - invalid bearer token denies access req = &events.LambdaFunctionURLRequest{ @@ -246,10 +248,11 @@ func TestHandler_authenticate_BearerToken(t *testing.T) { }, } // Should be false because token is invalid and no API key provided - assert.False(t, handler.authenticate(ctx, req)) + _, err = handler.authenticatePrincipal(ctx, req) + assert.Error(t, err) } -func TestHandler_authenticate_BearerTokenWithAPIKey(t *testing.T) { +func TestHandler_authenticatePrincipal_BearerTokenWithAPIKey(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) @@ -269,7 +272,9 @@ func TestHandler_authenticate_BearerTokenWithAPIKey(t *testing.T) { "Authorization": "Bearer valid-token", }, } - assert.True(t, handler.authenticate(ctx, req)) + principal, err := handler.authenticatePrincipal(ctx, req) + require.NoError(t, err) + assert.Equal(t, PrincipalSession, principal.Kind) // Test invalid bearer token when API key is configured req = &events.LambdaFunctionURLRequest{ @@ -278,7 +283,8 @@ func TestHandler_authenticate_BearerTokenWithAPIKey(t *testing.T) { }, } // Should be false because API key is configured and bearer token is invalid - assert.False(t, handler.authenticate(ctx, req)) + _, err = handler.authenticatePrincipal(ctx, req) + assert.Error(t, err) } // TestHandler_requiresCSRFValidation_ExactMatch asserts that paths sharing a