refactor(api): requireAuth returns resolved Principal (closes #194)

internal/api/middleware.go

@@ -43,6 +43,32 @@ 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"
+	// 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 {
+	Kind    PrincipalKind
+	UserID  string   // empty for PrincipalAdminAPIKey
+	Email   string   // empty for PrincipalAdminAPIKey; populated for session/user-api-key
+	Session *Session // non-nil only for PrincipalSession
+}
+
 // 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)
@@ -58,6 +84,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 == "" {
@@ -242,12 +343,12 @@ func logMissingCSRFToken(req *events.LambdaFunctionURLRequest, csrfToken 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

internal/api/router.go

@@ -373,7 +373,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:

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