diff --git a/internal/api/middleware.go b/internal/api/middleware.go index ee28a9d3a..bb56a5c05 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -50,6 +50,34 @@ func (h *Handler) isPublicEndpoint(path string) bool { return false } +// PrincipalKind identifies which credential type was used to authenticate. +type PrincipalKind string + +const ( + // PrincipalAdminAPIKey is set when the request authenticated with the + // shared admin API key (X-API-Key header matching h.apiKey). + PrincipalAdminAPIKey PrincipalKind = "admin-api-key" + // PrincipalUserAPIKey is set when the request authenticated with a + // per-user API key issued via /api/api-keys. + PrincipalUserAPIKey PrincipalKind = "user-api-key" // #nosec G101 -- credential-type label, not a credential value + // PrincipalSession is set when the request authenticated with a + // bearer-token session (X-Authorization / Authorization header). + PrincipalSession PrincipalKind = "session" +) + +// Principal carries the resolved caller identity returned by +// authenticatePrincipal and requireAuth. Handlers that need the caller's +// identity read it from here rather than re-resolving it through a second +// ValidateSession / ValidateUserAPIKeyAPI call. +type Principal struct { + // Field order groups the pointer-bearing Session ahead of the trailing + // string so the struct satisfies govet fieldalignment. + Kind PrincipalKind + Session *Session // non-nil only for PrincipalSession + UserID string // empty for PrincipalAdminAPIKey + Email string // empty for PrincipalAdminAPIKey; populated for session/user-api-key +} + // 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) @@ -65,6 +93,81 @@ func (h *Handler) authenticate(ctx context.Context, req *events.LambdaFunctionUR 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. +func (h *Handler) authenticatePrincipal(ctx context.Context, req *events.LambdaFunctionURLRequest) (*Principal, error) { + apiKey := extractAPIKey(req) + + if h.checkAdminAPIKey(apiKey) { + return &Principal{Kind: PrincipalAdminAPIKey}, nil + } + + if h.auth == nil { + return nil, NewClientError(401, "authentication required") + } + + if p := h.principalFromUserAPIKey(ctx, apiKey); p != nil { + return p, nil + } + + if p := h.principalFromBearerToken(ctx, req); p != nil { + return p, nil + } + + return nil, NewClientError(401, "authentication required") +} + +// principalFromUserAPIKey resolves a Principal from a user API key. +// Returns nil when the key is empty, validation fails, or the user record +// cannot be retrieved. +func (h *Handler) principalFromUserAPIKey(ctx context.Context, apiKey string) *Principal { + if apiKey == "" { + return nil + } + _, userRaw, err := h.auth.ValidateUserAPIKeyAPI(ctx, apiKey) + if err != nil { + logging.Debugf("User API key validation failed: %v", err) + return nil + } + if userRaw == nil { + return nil + } + // ValidateUserAPIKeyAPI returns *auth.User as any via the AuthServiceInterface. + // Assert to the concrete type; if validation succeeded but the concrete type is + // unexpected we deny rather than returning a partially-populated Principal: fail closed. + user, ok := userRaw.(*auth.User) + if !ok { + logging.Debugf("User API key: userRaw has unexpected type (%T); denying", userRaw) + return nil + } + return &Principal{ + Kind: PrincipalUserAPIKey, + UserID: user.ID, + Email: user.Email, + } +} + +// principalFromBearerToken resolves a Principal from a session bearer token. +// Returns nil when no token is present or the session is invalid. +func (h *Handler) principalFromBearerToken(ctx context.Context, req *events.LambdaFunctionURLRequest) *Principal { + token := h.extractBearerToken(req) + if token == "" { + return nil + } + session, err := h.auth.ValidateSession(ctx, token) + if err != nil || session == nil { + return nil + } + return &Principal{ + Kind: PrincipalSession, + UserID: session.UserID, + Email: session.Email, + Session: session, + } +} + func extractAPIKey(req *events.LambdaFunctionURLRequest) string { apiKey := req.Headers["x-api-key"] if apiKey == "" { @@ -289,12 +392,12 @@ func redactQueryParam(u, param string) string { // validateSecurity → authenticate already runs before dispatch, but if a // future refactor reorders middleware or a new route bypasses // validateSecurity, this check still rejects unauthenticated requests at -// the router level. Returns nil on success, a 401 ClientError otherwise. -func (h *Handler) requireAuth(ctx context.Context, req *events.LambdaFunctionURLRequest) error { - if h.authenticate(ctx, req) { - return nil - } - return NewClientError(401, "authentication required") +// the router level. Returns the resolved Principal on success, a 401 +// ClientError otherwise. Callers should use the returned Principal rather +// than re-resolving the caller's identity through a second ValidateSession +// or ValidateUserAPIKeyAPI call. +func (h *Handler) requireAuth(ctx context.Context, req *events.LambdaFunctionURLRequest) (*Principal, error) { + return h.authenticatePrincipal(ctx, req) } // requireAdmin gates the coarse admin-only routes (AuthAdmin). "Admin" is now diff --git a/internal/api/router.go b/internal/api/router.go index 81e5c5ffc..7e152cd45 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -385,7 +385,7 @@ func (r *Router) Route(ctx context.Context, method, path string, req *events.Lam return nil, err } case AuthUser: - if err := r.h.requireAuth(ctx, req); err != nil { + if _, err := r.h.requireAuth(ctx, req); err != nil { return nil, err } case AuthPublic: diff --git a/internal/api/router_authuser_test.go b/internal/api/router_authuser_test.go index 39368d5e6..f1d36a01f 100644 --- a/internal/api/router_authuser_test.go +++ b/internal/api/router_authuser_test.go @@ -5,8 +5,10 @@ import ( "errors" "testing" + "github.com/LeanerCloud/CUDly/internal/auth" "github.com/aws/aws-lambda-go/events" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -100,18 +102,21 @@ func TestRouterAuthPublic_NoCredentials_Accepts(t *testing.T) { require.NoError(t, err) } -// TestRequireAuth_AdminAPIKey verifies the new requireAuth helper accepts -// the admin API key. +// TestRequireAuth_AdminAPIKey verifies that requireAuth accepts the admin +// API key and returns a Principal of kind PrincipalAdminAPIKey. func TestRequireAuth_AdminAPIKey(t *testing.T) { h := &Handler{apiKey: "admin-secret"} req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{"X-API-Key": "admin-secret"}, } - require.NoError(t, h.requireAuth(context.Background(), req)) + p, err := h.requireAuth(context.Background(), req) + require.NoError(t, err) + require.NotNil(t, p) + assert.Equal(t, PrincipalAdminAPIKey, p.Kind) } // TestRequireAuth_UserSession verifies requireAuth accepts a valid -// non-admin user session. +// non-admin user session and returns a populated Principal. func TestRequireAuth_UserSession(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) @@ -121,7 +126,12 @@ func TestRequireAuth_UserSession(t *testing.T) { req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{"Authorization": "Bearer user-token"}, } - require.NoError(t, h.requireAuth(ctx, req)) + p, err := h.requireAuth(ctx, req) + require.NoError(t, err) + require.NotNil(t, p) + assert.Equal(t, PrincipalSession, p.Kind) + assert.Equal(t, "uid", p.UserID) + assert.Equal(t, userSession, p.Session) } // TestRequireAuth_NoCredential_Rejects verifies requireAuth returns a 401 @@ -130,9 +140,79 @@ func TestRequireAuth_NoCredential_Rejects(t *testing.T) { mockAuth := new(MockAuthService) h := &Handler{auth: mockAuth} req := &events.LambdaFunctionURLRequest{Headers: map[string]string{}} - err := h.requireAuth(context.Background(), req) + _, err := h.requireAuth(context.Background(), req) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) assert.Equal(t, 401, ce.code) } + +// TestRequireAuth_UserAPIKey verifies that a valid user API key yields a +// Principal with Kind == PrincipalUserAPIKey and populated UserID/Email. +// The mock returns the real *auth.User concrete type — the same type that +// auth.Service.ValidateUserAPIKeyAPI returns in production — so this test +// exercises the same type assertion that principalFromUserAPIKey performs. +func TestRequireAuth_UserAPIKey(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + userRec := &auth.User{ID: "uid-123", Email: "alice@example.com"} + mockAuth.On("ValidateUserAPIKeyAPI", ctx, "valid-user-key"). + Return(nil, userRec, nil) + + h := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "valid-user-key"}, + } + p, err := h.requireAuth(ctx, req) + require.NoError(t, err) + require.NotNil(t, p) + assert.Equal(t, PrincipalUserAPIKey, p.Kind) + assert.Equal(t, "uid-123", p.UserID) + assert.Equal(t, "alice@example.com", p.Email) +} + +// TestRequireAuth_UserAPIKey_BadRecord verifies that principalFromUserAPIKey +// fails closed when ValidateUserAPIKeyAPI succeeds but returns a userRaw +// value that is not *auth.User (unexpected concrete type). Post-fix it +// must return nil and the overall requireAuth must return a 401 ClientError. +func TestRequireAuth_UserAPIKey_BadRecord(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + // Return a value that is NOT *auth.User — the concrete type principalFromUserAPIKey asserts. + type unexpectedType struct{ Name string } + mockAuth.On("ValidateUserAPIKeyAPI", ctx, "bad-record-key"). + Return(nil, &unexpectedType{Name: "oops"}, nil) + // Bearer and session path must also return nothing. + mockAuth.On("ValidateSession", ctx, mock.Anything). + Return(nil, errors.New("no session")).Maybe() + + h := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "bad-record-key"}, + } + _, err := h.requireAuth(ctx, req) + require.Error(t, err, "expected denial when user record has unexpected type") + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 401, ce.code) +} + +// TestRequireAuth_NilAuth_Rejects verifies that a Handler constructed with +// auth == nil returns a 401 ClientError for a non-admin credential, confirming +// the h.auth == nil branch fails closed. +func TestRequireAuth_NilAuth_Rejects(t *testing.T) { + // No admin key configured, auth is nil. + h := &Handler{auth: nil} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "some-key"}, + } + _, err := h.requireAuth(context.Background(), req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 401, ce.code) +}