Skip to content

refactor(api): remove unused authenticate and checkUserAPIKey helpers #599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
Oct 6, 2026
Merged

refactor(api): remove unused authenticate and checkUserAPIKey helpers #599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 8 additions & 24 deletions internal/api/coverage_gaps_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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)
}

// ---------------------------------------------------------------------------
Expand Down
45 changes: 4 additions & 41 deletions internal/api/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand Down
22 changes: 14 additions & 8 deletions internal/api/middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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)

Expand All @@ -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{
Expand All @@ -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)

Expand All @@ -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{
Expand All @@ -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
Expand Down
Loading