fix(api): enforce user API key scoped permissions in requirePermission

internal/api/handler.go

@@ -207,11 +207,15 @@ const apiKeyAdminUserID = "admin-api-key"
 
 // requirePermission validates authentication and checks if the user holds the
 // specified permission. The stateless admin API key bypasses the per-user
-// permission lookup (it is a full-access infrastructure credential). Every
-// other caller is checked against their group-derived permissions: a member of
-// the Administrators group holds {admin, *} and passes any check, while a user
-// with no groups holds no permissions and is denied (fail closed). Returns the
-// session on success so callers can read session.UserID for account filtering.
+// permission lookup (it is a full-access infrastructure credential). A user
+// API key is checked against its effective permissions: the intersection of
+// the key's scoped permissions with the owning user's group-derived
+// permissions (issue #1142 - previously user API keys 401'd here and per-key
+// scoping was never enforced). Every other caller is checked against their
+// group-derived permissions: a member of the Administrators group holds
+// {admin, *} and passes any check, while a user with no groups holds no
+// permissions and is denied (fail closed). Returns the session on success so
+// callers can read session.UserID for account filtering.
 func (h *Handler) requirePermission(ctx context.Context, req *events.LambdaFunctionURLRequest, action, resource string) (*Session, error) {
 	apiKey := extractAPIKey(req)
 	if h.checkAdminAPIKey(apiKey) {
@@ -222,6 +226,22 @@ func (h *Handler) requirePermission(ctx context.Context, req *events.LambdaFunct
 		return nil, fmt.Errorf("authentication service not configured")
 	}
 
+	// User API key: authorize against the key's effective permissions. A
+	// valid key that lacks the permission is denied (fail closed). A key
+	// that fails validation falls through to bearer-token auth, matching
+	// resolveAuthenticatedUserID, so a stale x-api-key header cannot lock
+	// out a caller that also presents a valid session.
+	if apiKey != "" {
+		userID, has, err := h.auth.HasAPIKeyPermissionAPI(ctx, apiKey, action, resource)
+		if err == nil {
+			if !has {
+				return nil, NewClientError(403, fmt.Sprintf("permission denied: requires %s on %s", action, resource))
+			}
+			return &Session{UserID: userID}, nil
+		}
+		logging.Debugf("User API key permission check failed: %v", err)
+	}
+
 	token := h.extractBearerToken(req)
 	if token == "" {
 		return nil, NewClientError(401, "no authorization token provided")

internal/api/handler_apikeys_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -649,3 +650,134 @@ func TestFormatTimePtr(t *testing.T) {
 		assert.Contains(t, result, "2024-06-15T10:30:00")
 	})
 }
+
+// TestRequirePermission_UserAPIKey is the regression test for issue #1142:
+// user API keys previously hit a hard 401 in requirePermission (only the
+// admin API key and bearer-token sessions were recognized), so the per-key
+// scoped permissions persisted at key-creation time were never enforced.
+// requirePermission now authorizes user API keys against their effective
+// permissions via HasAPIKeyPermissionAPI.
+func TestRequirePermission_UserAPIKey(t *testing.T) {
+	ctx := context.Background()
+
+	// The exact failing request shape: x-api-key header carrying a user API
+	// key, no bearer token (Authorization / X-Authorization absent).
+	newReq := func() *events.LambdaFunctionURLRequest {
+		return &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{"x-api-key": "cudly-user-key"},
+		}
+	}
+
+	t.Run("regression #1142: key with in-scope permission is authorized as its owner", func(t *testing.T) {
+		mockAuth := new(MockAuthService)
+		t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+		mockAuth.On("HasAPIKeyPermissionAPI", ctx, "cudly-user-key", "view", "recommendations").
+			Return("user-123", true, nil)
+
+		handler := &Handler{auth: mockAuth}
+		session, err := handler.requirePermission(ctx, newReq(), "view", "recommendations")
+		require.NoError(t, err)
+		require.NotNil(t, session)
+		assert.Equal(t, "user-123", session.UserID)
+	})
+
+	t.Run("regression #1142: scoped key is denied an out-of-scope permission with 403", func(t *testing.T) {
+		mockAuth := new(MockAuthService)
+		t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+		mockAuth.On("HasAPIKeyPermissionAPI", ctx, "cudly-user-key", "execute", "purchases").
+			Return("user-123", false, nil)
+
+		handler := &Handler{auth: mockAuth}
+		session, err := handler.requirePermission(ctx, newReq(), "execute", "purchases")
+		require.Error(t, err)
+		assert.Nil(t, session)
+		ce, ok := IsClientError(err)
+		require.True(t, ok)
+		assert.Equal(t, 403, ce.code)
+		assert.Contains(t, err.Error(), "permission denied")
+	})
+
+	t.Run("invalid key with no bearer token is rejected with 401", func(t *testing.T) {
+		mockAuth := new(MockAuthService)
+		t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+		mockAuth.On("HasAPIKeyPermissionAPI", ctx, "cudly-user-key", "view", "recommendations").
+			Return("", false, errors.New("invalid API key"))
+
+		handler := &Handler{auth: mockAuth}
+		session, err := handler.requirePermission(ctx, newReq(), "view", "recommendations")
+		require.Error(t, err)
+		assert.Nil(t, session)
+		ce, ok := IsClientError(err)
+		require.True(t, ok)
+		assert.Equal(t, 401, ce.code)
+	})
+
+	t.Run("invalid key falls through to a valid bearer session", func(t *testing.T) {
+		mockAuth := new(MockAuthService)
+		t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+		mockAuth.On("HasAPIKeyPermissionAPI", ctx, "stale-key", "view", "recommendations").
+			Return("", false, errors.New("invalid API key"))
+		mockAuth.On("ValidateSession", ctx, "session-token").
+			Return(&Session{UserID: "user-456"}, nil)
+		mockAuth.On("HasPermissionAPI", ctx, "user-456", "view", "recommendations").
+			Return(true, nil)
+
+		handler := &Handler{auth: mockAuth}
+		req := &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{
+				"x-api-key":     "stale-key",
+				"Authorization": "Bearer session-token",
+			},
+		}
+		session, err := handler.requirePermission(ctx, req, "view", "recommendations")
+		require.NoError(t, err)
+		require.NotNil(t, session)
+		assert.Equal(t, "user-456", session.UserID)
+	})
+
+	t.Run("admin API key still bypasses the per-key permission lookup", func(t *testing.T) {
+		mockAuth := new(MockAuthService)
+		t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+		handler := &Handler{auth: mockAuth, apiKey: "admin-infra-key"}
+		req := &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{"x-api-key": "admin-infra-key"},
+		}
+		session, err := handler.requirePermission(ctx, req, "view", "recommendations")
+		require.NoError(t, err)
+		require.NotNil(t, session)
+		assert.Equal(t, apiKeyAdminUserID, session.UserID)
+		mockAuth.AssertNotCalled(t, "HasAPIKeyPermissionAPI", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	})
+}
+
+// TestGetRecommendations_UserAPIKey exercises the real failing scenario from
+// issue #1142 end to end at the handler layer: GET /api/recommendations with
+// only an x-api-key user key returned 401 before the fix even when the key
+// was scoped to view:recommendations.
+func TestGetRecommendations_UserAPIKey(t *testing.T) {
+	ctx := context.Background()
+
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	mockAuth.On("HasAPIKeyPermissionAPI", ctx, "cudly-user-key", "view", "recommendations").
+		Return("user-123", true, nil)
+	// Account scoping resolves against the key's owning user.
+	mockAuth.On("GetAllowedAccountsAPI", ctx, "user-123").
+		Return([]string(nil), nil)
+
+	mockScheduler := new(MockScheduler)
+	t.Cleanup(func() { mockScheduler.AssertExpectations(t) })
+	mockScheduler.On("ListRecommendations", ctx, mock.Anything).
+		Return([]config.RecommendationRecord{}, nil)
+
+	handler := &Handler{auth: mockAuth, scheduler: mockScheduler}
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"x-api-key": "cudly-user-key"},
+	}
+
+	result, err := handler.getRecommendations(ctx, req, map[string]string{})
+	require.NoError(t, err)
+	require.NotNil(t, result)
+	assert.Equal(t, 0, result.Summary.TotalCount)
+}

internal/api/handler_ri_exchange_test.go

@@ -1035,6 +1035,9 @@ func (m *mockAuthForExchange) RevokeAPIKeyAPI(_ context.Context, _, _ string) er
 func (m *mockAuthForExchange) ValidateUserAPIKeyAPI(_ context.Context, _ string) (any, any, error) {
 	return nil, nil, nil
 }
+func (m *mockAuthForExchange) HasAPIKeyPermissionAPI(_ context.Context, _, _, _ string) (string, bool, error) {
+	return "admin", true, nil
+}
 func (m *mockAuthForExchange) GetAllowedAccountsAPI(_ context.Context, _ string) ([]string, error) {
 	return nil, nil
 }

internal/api/mocks_test.go

@@ -327,3 +327,8 @@ func (m *MockAuthService) ValidateUserAPIKeyAPI(ctx context.Context, apiKey stri
 	args := m.Called(ctx, apiKey)
 	return args.Get(0), args.Get(1), args.Error(2)
 }
+
+func (m *MockAuthService) HasAPIKeyPermissionAPI(ctx context.Context, apiKey, action, resource string) (string, bool, error) {
+	args := m.Called(ctx, apiKey, action, resource)
+	return args.String(0), args.Bool(1), args.Error(2)
+}

internal/api/types.go

@@ -211,6 +211,15 @@ type AuthServiceInterface interface {
 	DeleteAPIKeyAPI(ctx context.Context, userID, keyID string) error
 	RevokeAPIKeyAPI(ctx context.Context, userID, keyID string) error
 	ValidateUserAPIKeyAPI(ctx context.Context, apiKey string) (any, any, error)
+	// HasAPIKeyPermissionAPI validates a user API key and checks the
+	// requested action/resource against the key's effective permissions
+	// (the intersection of the key's scoped permissions with the owning
+	// user's group-derived permissions). Returns the owning user's ID and
+	// whether the permission is held; a non-nil error means the key did
+	// not validate or the lookup failed, and callers must deny (fail
+	// closed). Wired into requirePermission so per-key scoping is
+	// enforced at request time (issue #1142).
+	HasAPIKeyPermissionAPI(ctx context.Context, apiKey, action, resource string) (string, bool, error)
 }
 
 // Auth request/response types (to avoid import cycle with auth package).

internal/auth/service_apikeys_api.go

@@ -113,3 +113,31 @@ func (s *Service) RevokeAPIKeyAPI(ctx context.Context, userID, keyID string) err
 func (s *Service) ValidateUserAPIKeyAPI(ctx context.Context, apiKey string) (*UserAPIKey, *User, error) {
 	return s.ValidateUserAPIKey(ctx, apiKey)
 }
+
+// HasAPIKeyPermissionAPI validates a user API key and checks the requested
+// action/resource against the key's effective permissions: the intersection
+// of the key's scoped permissions with the owning user's group-derived
+// permissions (ComputeEffectivePermissions). A key created without explicit
+// permissions inherits the owner's full permission set. Returns the owning
+// user's ID and whether the permission is held.
+//
+// Fail closed: any validation failure (unknown, revoked, or expired key,
+// inactive owner) or permission-lookup error returns a non-nil error and
+// callers must deny access.
+func (s *Service) HasAPIKeyPermissionAPI(ctx context.Context, apiKey, action, resource string) (userID string, allowed bool, err error) {
+	key, user, err := s.ValidateUserAPIKey(ctx, apiKey)
+	if err != nil {
+		return "", false, err
+	}
+
+	perms, err := s.ComputeEffectivePermissions(ctx, key, user)
+	if err != nil {
+		return "", false, fmt.Errorf("computing effective API key permissions: %w", err)
+	}
+
+	// AuthContext.HasPermission applies the same matching semantics as
+	// session-based checks, including the admin:* wildcard with its
+	// money-spending carve-outs (issue #923).
+	effective := &AuthContext{User: user, Permissions: perms}
+	return user.ID, effective.HasPermission(action, resource), nil
+}

internal/auth/service_apikeys_test.go

@@ -797,6 +797,105 @@ func TestService_ComputeEffectivePermissions(t *testing.T) {
 	})
 }
 
+// TestService_HasAPIKeyPermissionAPI is the regression test for issue #1142:
+// per-key scoped permissions were persisted and UI-exposed but never enforced
+// at request time (ComputeEffectivePermissions had no request-path caller).
+// HasAPIKeyPermissionAPI is the request-path enforcement point wired into
+// requirePermission.
+func TestService_HasAPIKeyPermissionAPI(t *testing.T) {
+	ctx := context.Background()
+
+	rawKey := "test-api-key-123456"
+	hash := sha256.Sum256([]byte(rawKey))
+	keyHash := base64.RawURLEncoding.EncodeToString(hash[:])
+
+	userGrpID := "00000000-0000-5000-8000-000000000005"
+	userGrp := func() *Group {
+		return &Group{ID: userGrpID, Permissions: []Permission{
+			{Action: ActionView, Resource: ResourceRecommendations},
+			{Action: ActionCreate, Resource: ResourcePlans},
+		}}
+	}
+
+	// setup returns a service whose store resolves rawKey to a key scoped to
+	// keyPerms, owned by an active user whose group grants view:recommendations
+	// and create:plans. This is the exact data shape of a scoped key created
+	// through POST /api/auth/api-keys.
+	setup := func(t *testing.T, keyPerms []Permission) *Service {
+		t.Helper()
+		mockStore := new(MockStore)
+		service := &Service{store: mockStore}
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+		user := &User{ID: "user-123", Email: "test@example.com", Active: true, GroupIDs: []string{userGrpID}}
+		keyRecord := &UserAPIKey{
+			ID:          "key-1",
+			UserID:      "user-123",
+			KeyHash:     keyHash,
+			IsActive:    true,
+			Permissions: keyPerms,
+		}
+
+		mockStore.On("GetAPIKeyByHash", ctx, keyHash).Return(keyRecord, nil)
+		mockStore.On("GetUserByID", ctx, "user-123").Return(user, nil)
+		mockStore.On("UpdateAPIKeyLastUsed", mock.Anything, "key-1").Return(nil).Maybe()
+		mockStore.On("GetGroup", ctx, userGrpID).Return(userGrp(), nil)
+		return service
+	}
+
+	t.Run("scoped key grants its in-scope permission", func(t *testing.T) {
+		service := setup(t, []Permission{{Action: ActionView, Resource: ResourceRecommendations}})
+
+		userID, has, err := service.HasAPIKeyPermissionAPI(ctx, rawKey, ActionView, ResourceRecommendations)
+		require.NoError(t, err)
+		assert.Equal(t, "user-123", userID)
+		assert.True(t, has)
+	})
+
+	t.Run("regression #1142: scoped key cannot exceed its scope even when the user holds the permission", func(t *testing.T) {
+		// The key is scoped to view:recommendations only; the owning user's
+		// group also grants create:plans. The key must NOT inherit it.
+		service := setup(t, []Permission{{Action: ActionView, Resource: ResourceRecommendations}})
+
+		userID, has, err := service.HasAPIKeyPermissionAPI(ctx, rawKey, ActionCreate, ResourcePlans)
+		require.NoError(t, err)
+		assert.Equal(t, "user-123", userID)
+		assert.False(t, has, "scoped key must not grant permissions outside its scope")
+	})
+
+	t.Run("key permission the user lacks is denied (intersection)", func(t *testing.T) {
+		// The key claims admin:users but the user's group never granted it.
+		service := setup(t, []Permission{{Action: ActionAdmin, Resource: ResourceUsers}})
+
+		userID, has, err := service.HasAPIKeyPermissionAPI(ctx, rawKey, ActionAdmin, ResourceUsers)
+		require.NoError(t, err)
+		assert.Equal(t, "user-123", userID)
+		assert.False(t, has, "key must not grant permissions the owning user does not hold")
+	})
+
+	t.Run("unscoped key inherits the owner's group permissions", func(t *testing.T) {
+		service := setup(t, nil)
+
+		userID, has, err := service.HasAPIKeyPermissionAPI(ctx, rawKey, ActionCreate, ResourcePlans)
+		require.NoError(t, err)
+		assert.Equal(t, "user-123", userID)
+		assert.True(t, has)
+	})
+
+	t.Run("invalid key fails closed with an error", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := &Service{store: mockStore}
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+		mockStore.On("GetAPIKeyByHash", ctx, keyHash).Return(nil, nil)
+
+		userID, has, err := service.HasAPIKeyPermissionAPI(ctx, rawKey, ActionView, ResourceRecommendations)
+		assert.Error(t, err)
+		assert.Empty(t, userID)
+		assert.False(t, has)
+	})
+}
+
 func TestService_validateAPIKeyPermissions(t *testing.T) {
 	ctx := context.Background()
 

internal/server/app.go

@@ -1167,3 +1167,8 @@ func (a *authServiceAdapter) RevokeAPIKeyAPI(ctx context.Context, userID, keyID
 func (a *authServiceAdapter) ValidateUserAPIKeyAPI(ctx context.Context, apiKey string) (any, any, error) { //nolint:gocritic // unnamedResult: return names would conflict with body locals
 	return a.service.ValidateUserAPIKeyAPI(ctx, apiKey)
 }
+
+func (a *authServiceAdapter) HasAPIKeyPermissionAPI(ctx context.Context, apiKey, action, resource string) (userID string, allowed bool, err error) {
+	userID, allowed, err = a.service.HasAPIKeyPermissionAPI(ctx, apiKey, action, resource)
+	return
+}