diff --git a/frontend/src/__tests__/api-permissions.test.ts b/frontend/src/__tests__/api-permissions.test.ts new file mode 100644 index 000000000..4de5e0a12 --- /dev/null +++ b/frontend/src/__tests__/api-permissions.test.ts @@ -0,0 +1,97 @@ +/** + * Runtime-shape-validation tests for getUserPermissions() (CR #922 F3). + * + * canAccess() iterates user.effectivePermissions with + * `for (const p of user.effectivePermissions)` and reads p.action / + * p.resource. A non-array `permissions` (or null/non-object entries, + * or non-string action/resource) would throw outside app.ts's + * fetch/merge try/catch and crash the bootstrap path. The validator + * must reject every such shape so the caller falls back to the safe + * group-membership gating instead. + */ +import { getUserPermissions } from '../api/auth'; + +beforeEach(() => { + global.fetch = jest.fn(); + localStorage.setItem('auth_token', 'tok'); +}); +afterEach(() => { + jest.restoreAllMocks(); + localStorage.clear(); +}); + +function mockFetchOk(body: unknown): void { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + json: async () => body, + }); +} + +describe('getUserPermissions()', () => { + test('returns parsed UserPermissionsResponse on a well-formed payload', async () => { + mockFetchOk({ + permissions: [ + { action: 'view', resource: 'recommendations' }, + { action: 'view', resource: 'plans' }, + ], + is_admin: false, + }); + const res = await getUserPermissions(); + expect(res.is_admin).toBe(false); + expect(res.permissions).toEqual([ + { action: 'view', resource: 'recommendations' }, + { action: 'view', resource: 'plans' }, + ]); + }); + + test('returns is_admin=true with {admin,*} entry on admin payload', async () => { + mockFetchOk({ + permissions: [{ action: 'admin', resource: '*' }], + is_admin: true, + }); + const res = await getUserPermissions(); + expect(res.is_admin).toBe(true); + expect(res.permissions).toEqual([{ action: 'admin', resource: '*' }]); + }); + + test('rejects when response is not an object (e.g. string)', async () => { + mockFetchOk('not-an-object'); + await expect(getUserPermissions()).rejects.toThrow(/was not an object/); + }); + + test('rejects when response is null', async () => { + mockFetchOk(null); + await expect(getUserPermissions()).rejects.toThrow(/was not an object/); + }); + + test('rejects when permissions is not an array (e.g. truthy non-array)', async () => { + mockFetchOk({ permissions: { 0: { action: 'view', resource: 'plans' } }, is_admin: false }); + await expect(getUserPermissions()).rejects.toThrow(/permissions is not an array/); + }); + + test('rejects when a permission entry is null', async () => { + mockFetchOk({ permissions: [null], is_admin: false }); + await expect(getUserPermissions()).rejects.toThrow(/permissions\[0\] is not an object/); + }); + + test('rejects when a permission entry is missing action', async () => { + mockFetchOk({ permissions: [{ resource: 'plans' }], is_admin: false }); + await expect(getUserPermissions()).rejects.toThrow(/permissions\[0\]\.action is not a string/); + }); + + test('rejects when a permission entry has non-string resource', async () => { + mockFetchOk({ permissions: [{ action: 'view', resource: 42 }], is_admin: false }); + await expect(getUserPermissions()).rejects.toThrow(/permissions\[0\]\.resource is not a string/); + }); + + test('rejects when is_admin is missing', async () => { + mockFetchOk({ permissions: [] }); + await expect(getUserPermissions()).rejects.toThrow(/is_admin is not a boolean/); + }); + + test('rejects when is_admin is not boolean (e.g. "true" string)', async () => { + mockFetchOk({ permissions: [], is_admin: 'true' }); + await expect(getUserPermissions()).rejects.toThrow(/is_admin is not a boolean/); + }); +}); diff --git a/frontend/src/__tests__/permissions.test.ts b/frontend/src/__tests__/permissions.test.ts index 09ebd861e..2e679df0e 100644 --- a/frontend/src/__tests__/permissions.test.ts +++ b/frontend/src/__tests__/permissions.test.ts @@ -1,16 +1,19 @@ /** * Permissions helper tests. * - * PR #912 replaced role-based gating with group-membership-based - * gating. isAdmin() now returns true when the current user is a member - * of the Administrators group (UUID 00000000-0000-5000-8000-000000000001). - * canAccess() is currently a pass-through to isAdmin() -- non-admin users - * always get false because the /me/permissions endpoint is deferred. + * Issue #917: canAccess() now consults user.effectivePermissions when + * populated (fetched from GET /api/auth/me/permissions on bootstrap). + * When effectivePermissions is absent (loading race) it falls back to + * the group-membership admin check: admin passes, others block. + * + * isAdmin() returns true when the current user is a member of the + * Administrators group (UUID 00000000-0000-5000-8000-000000000001). * * getRolePermissions() is kept for the effective-permissions display in * the admin Users page and still returns the same sets as before. */ import { canAccess, getRolePermissions, isAdmin, ADMINISTRATORS_GROUP_ID } from '../permissions'; +import type { PermissionEntry } from '../api/types'; jest.mock('../state', () => ({ getCurrentUser: jest.fn(), @@ -22,9 +25,9 @@ const ADMIN_GID = ADMINISTRATORS_GROUP_ID; const STD_GID = '00000000-0000-5000-8000-000000000005'; const RO_GID = '00000000-0000-5000-8000-000000000006'; -const mockUserWithGroups = (groups: string[]) => { +const mockUserWithGroups = (groups: string[], effectivePermissions?: PermissionEntry[]) => { (state.getCurrentUser as jest.Mock).mockReturnValue( - { id: 'u1', email: 'u@example.com', groups }, + { id: 'u1', email: 'u@example.com', groups, effectivePermissions }, ); }; @@ -122,8 +125,8 @@ describe('permissions', () => { }); }); - describe('canAccess', () => { - test('Administrators group member passes all checks', () => { + describe('canAccess - fallback (effectivePermissions absent)', () => { + test('Administrators group member passes all checks via group-membership fallback', () => { mockUserWithGroups([ADMIN_GID]); expect(canAccess('admin', '*')).toBe(true); expect(canAccess('view', 'users')).toBe(true); @@ -132,14 +135,15 @@ describe('permissions', () => { expect(canAccess('view', 'accounts')).toBe(true); }); - test('Standard Users group member fails all checks (no /me/permissions endpoint yet)', () => { + test('Standard Users group member blocked during loading (effectivePermissions undefined)', () => { + // Before /me/permissions returns, non-admins are blocked (fails closed). mockUserWithGroups([STD_GID]); expect(canAccess('view', 'recommendations')).toBe(false); expect(canAccess('view', 'plans')).toBe(false); expect(canAccess('admin', '*')).toBe(false); }); - test('Read-Only Users group member fails all checks', () => { + test('Read-Only Users group member blocked during loading', () => { mockUserWithGroups([RO_GID]); expect(canAccess('view', 'recommendations')).toBe(false); expect(canAccess('admin', '*')).toBe(false); @@ -158,4 +162,71 @@ describe('permissions', () => { expect(canAccess('create', 'plans')).toBe(false); }); }); + + describe('canAccess - effective permissions (post /me/permissions fetch)', () => { + test('Standard Users group member gets explicit grants from effectivePermissions', () => { + const perms: PermissionEntry[] = [ + { action: 'view', resource: 'recommendations' }, + { action: 'view', resource: 'plans' }, + { action: 'view', resource: 'purchases' }, + { action: 'view', resource: 'history' }, + { action: 'create', resource: 'plans' }, + { action: 'cancel-own', resource: 'purchases' }, + ]; + mockUserWithGroups([STD_GID], perms); + expect(canAccess('view', 'recommendations')).toBe(true); + expect(canAccess('view', 'plans')).toBe(true); + expect(canAccess('view', 'purchases')).toBe(true); + expect(canAccess('view', 'history')).toBe(true); + expect(canAccess('create', 'plans')).toBe(true); + expect(canAccess('cancel-own', 'purchases')).toBe(true); + }); + + test('Standard Users group member is denied for permissions not in effective set', () => { + const perms: PermissionEntry[] = [ + { action: 'view', resource: 'recommendations' }, + ]; + mockUserWithGroups([STD_GID], perms); + expect(canAccess('delete', 'plans')).toBe(false); + expect(canAccess('execute', 'purchases')).toBe(false); + expect(canAccess('admin', '*')).toBe(false); + expect(canAccess('view', 'users')).toBe(false); + }); + + test('admin wildcard in effectivePermissions grants everything', () => { + const perms: PermissionEntry[] = [ + { action: 'admin', resource: '*' }, + ]; + mockUserWithGroups([ADMIN_GID], perms); + expect(canAccess('admin', '*')).toBe(true); + expect(canAccess('delete', 'plans')).toBe(true); + expect(canAccess('view', 'users')).toBe(true); + expect(canAccess('execute', 'purchases')).toBe(true); + }); + + test('empty effectivePermissions array denies everything', () => { + mockUserWithGroups([STD_GID], []); + expect(canAccess('view', 'recommendations')).toBe(false); + expect(canAccess('admin', '*')).toBe(false); + }); + + test('multi-group user gets the union: all granted actions from both groups', () => { + const perms: PermissionEntry[] = [ + { action: 'view', resource: 'recommendations' }, + { action: 'view', resource: 'plans' }, + { action: 'create', resource: 'plans' }, + ]; + mockUserWithGroups([STD_GID, RO_GID], perms); + expect(canAccess('view', 'recommendations')).toBe(true); + expect(canAccess('view', 'plans')).toBe(true); + expect(canAccess('create', 'plans')).toBe(true); + expect(canAccess('delete', 'plans')).toBe(false); + }); + + test('null user (logged out) fails regardless of any permissions', () => { + mockNoUser(); + expect(canAccess('view', 'recommendations')).toBe(false); + expect(canAccess('admin', '*')).toBe(false); + }); + }); }); diff --git a/frontend/src/api/auth.ts b/frontend/src/api/auth.ts index a8b5c50e2..2c7d3b786 100644 --- a/frontend/src/api/auth.ts +++ b/frontend/src/api/auth.ts @@ -6,6 +6,8 @@ import { apiRequest, getAuthHeaders, setAuthToken, setCsrfToken, clearAuth, addC import type { LoginResponse, User, + PermissionEntry, + UserPermissionsResponse, PublicInfo, MFASetupResponse, MFARecoveryCodesResponse, @@ -110,6 +112,50 @@ export async function getCurrentUser(): Promise { return apiRequest('/auth/me'); } +/** + * Fetch the effective permission set for the current user from + * GET /api/auth/me/permissions (issue #917). Returns the full + * UserPermissionsResponse including is_admin. + * + * Called on login/bootstrap and the result is merged onto the + * current-user state so canAccess() can consult it. + * + * Runtime-validates the response shape (same pattern as setupMFA / + * getResetTokenStatus) so a malicious or misconfigured server can't + * inject a non-array `permissions` value that would later crash + * canAccess()'s `for (const p of user.effectivePermissions)` loop. On + * any shape mismatch this throws so the caller (app.ts) falls back to + * the safe group-membership gating instead of rendering with broken + * permission state. + */ +export async function getUserPermissions(): Promise { + const data = await apiRequest('/auth/me/permissions'); + if (data === null || typeof data !== 'object') { + throw new Error(`getUserPermissions response was not an object: ${JSON.stringify(data)}`); + } + const { permissions, is_admin } = data as { permissions?: unknown; is_admin?: unknown }; + if (!Array.isArray(permissions)) { + throw new Error(`getUserPermissions response.permissions is not an array: ${JSON.stringify(permissions)}`); + } + const validated: PermissionEntry[] = permissions.map((entry, i) => { + if (entry === null || typeof entry !== 'object') { + throw new Error(`getUserPermissions response.permissions[${i}] is not an object: ${JSON.stringify(entry)}`); + } + const { action, resource } = entry as { action?: unknown; resource?: unknown }; + if (typeof action !== 'string') { + throw new Error(`getUserPermissions response.permissions[${i}].action is not a string: ${JSON.stringify(action)}`); + } + if (typeof resource !== 'string') { + throw new Error(`getUserPermissions response.permissions[${i}].resource is not a string: ${JSON.stringify(resource)}`); + } + return { action, resource }; + }); + if (typeof is_admin !== 'boolean') { + throw new Error(`getUserPermissions response.is_admin is not a boolean: ${JSON.stringify(is_admin)}`); + } + return { permissions: validated, is_admin }; +} + /** * Request password reset */ diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 7feb085bf..4aa14e779 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -9,6 +9,8 @@ export type { PaymentOption, RampSchedule, User, + PermissionEntry, + UserPermissionsResponse, LoginResponse, DashboardSummary, UpcomingPurchase, @@ -79,6 +81,7 @@ export { login, logout, getCurrentUser, + getUserPermissions, requestPasswordReset, resetPassword, getResetTokenStatus, diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 2b26c3d79..8e11c05fa 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -8,6 +8,25 @@ export type PaymentOption = 'no-upfront' | 'partial-upfront' | 'all-upfront'; export type RampSchedule = 'immediate' | 'weekly-25pct' | 'monthly-10pct' | 'custom'; // User types + +/** + * PermissionEntry matches the JSON shape returned by GET /api/auth/me/permissions. + * Both action and resource are string to stay forward-compatible with new + * constants added to internal/auth/types.go without a frontend change. + */ +export interface PermissionEntry { + action: string; + resource: string; +} + +/** + * UserPermissionsResponse is the shape of GET /api/auth/me/permissions. + */ +export interface UserPermissionsResponse { + permissions: PermissionEntry[]; + is_admin: boolean; +} + export interface User { id: string; email: string; @@ -21,6 +40,11 @@ export interface User { // responses may not include it. The profile/MFA section in // auth.ts treats `mfa_enabled === true` (strict) as "enabled". mfa_enabled?: boolean; + // Effective permissions fetched from GET /api/auth/me/permissions + // on login/bootstrap (issue #917). Undefined until the fetch + // completes; canAccess() falls back to group-membership-only + // gating (admins pass, others blocked) while this is loading. + effectivePermissions?: PermissionEntry[]; } export interface LoginResponse { diff --git a/frontend/src/app.ts b/frontend/src/app.ts index 124b7e0dc..0ade07243 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -59,6 +59,21 @@ export async function init(): Promise { try { const user = await api.getCurrentUser(); state.setCurrentUser(user); + + // Fetch the effective permission set from /api/auth/me/permissions + // (issue #917) and merge it onto the user state so canAccess() can + // consult the real group-derived set instead of blocking non-admins. + // Best-effort: a failure here must not prevent the app from loading. + try { + const permsResp = await api.getUserPermissions(); + const updatedUser = state.getCurrentUser(); + if (updatedUser) { + state.setCurrentUser({ ...updatedUser, effectivePermissions: permsResp.permissions }); + } + } catch (permErr) { + console.warn('Failed to fetch effective permissions, using fallback gating:', permErr); + } + initRouter(); // Deep-link check BEFORE tab routing: the path /purchases/{approve, // cancel}/:id?token=… isn't a tab — it's a one-shot action landing diff --git a/frontend/src/permissions.ts b/frontend/src/permissions.ts index d742a5794..c7157e481 100644 --- a/frontend/src/permissions.ts +++ b/frontend/src/permissions.ts @@ -1,21 +1,17 @@ /** * Permission helper for CUDly frontend. * - * PR #912 removed the `role` column from users and sessions. - * Authorization is now purely group-membership based. The server - * derives every permission from the union of the groups a user - * belongs to via HasPermissionAPI; the frontend mirrors that by - * checking group membership for the UI-gating predicates below. + * Issue #917: authorization is group-membership based. The server + * exposes the effective permission set (union of all user groups) via + * GET /api/auth/me/permissions. The frontend fetches this on + * login/bootstrap and caches it on the current-user state so + * canAccess() can consult the real set rather than blocking all + * non-admins. * * Admin status = member of the Administrators group * (UUID 00000000-0000-5000-8000-000000000001). That group carries * the `admin:*` capability on the backend, which grants every action - * on every resource. The three built-in groups seeded by migration - * 000057 are: - * - * Administrators 00000000-0000-5000-8000-000000000001 (admin:*) - * Standard Users 00000000-0000-5000-8000-000000000005 - * Read-Only Users 00000000-0000-5000-8000-000000000006 + * on every resource. * * The closed-union Action and Resource types below are hand-written * and mirror the Action* / Resource* constants in @@ -28,7 +24,6 @@ * The ADMIN_PERMS / USER_PERMS / READONLY_PERMS sets from * permissions.generated.ts remain exported for the effective- * permissions display in the admin Users page expand panel. - * They are not used for session gating anymore. */ import * as state from './state'; @@ -88,25 +83,37 @@ export function isAdmin(): boolean { } /** - * Returns true when the current session's group membership grants the - * specified permission. + * Returns true when the current session's effective permissions grant + * the specified action on the specified resource. + * + * When effectivePermissions is populated (fetched from + * GET /api/auth/me/permissions on login/bootstrap) the set is + * consulted directly: admin:* grants everything; otherwise an exact + * action:resource match is required. * - * Administrators-group members pass every check (admin:* covers every - * action/resource pair). For all other groups a full /me/permissions - * round-trip is needed to resolve fine-grained permissions; that - * endpoint is not yet available, so non-admins return false here and - * the backend remains the authoritative gate. + * While effectivePermissions is not yet loaded (e.g. during the first + * render before the async fetch completes) the function falls back to + * the group-membership admin check so Administrators-group members + * aren't locked out during bootstrap. Non-admins see buttons hidden + * briefly -- acceptable because the full set loads immediately after + * login. * - * UX-only gate. A wrong-positive surfaces as a 403 on click; a - * wrong-negative just hides a button. + * UX-only gate. The backend still enforces on every request. */ export function canAccess(action: Action, resource: Resource): boolean { - // Suppress unused-variable warning -- action/resource are kept in - // the signature for forward-compatibility with the /me/permissions - // endpoint that will replace this stub. - void action; void resource; const user = state.getCurrentUser(); if (!user) return false; + + // Use the server-provided effective permission set when available. + if (user.effectivePermissions) { + for (const p of user.effectivePermissions) { + if (p.action === 'admin' && p.resource === '*') return true; + if (p.action === action && (p.resource === resource || p.resource === '*')) return true; + } + return false; + } + + // Fallback while permissions are still loading: admins pass, others block. return isAdmin(); } diff --git a/internal/api/handler_auth.go b/internal/api/handler_auth.go index 0d3f25819..5d0509578 100644 --- a/internal/api/handler_auth.go +++ b/internal/api/handler_auth.go @@ -103,6 +103,105 @@ func (h *Handler) getCurrentUser(ctx context.Context, req *events.LambdaFunction }, nil } +// getCurrentUserPermissions handles GET /api/auth/me/permissions. +// It returns the effective permission set for the authenticated user, +// derived from the union of all their group permissions. The same path +// the backend uses for enforcement (GetUserPermissionsAPI -> GetUserPermissions). +// +// Auth: the route is AuthUser, which admits admin API key, user API +// key, OR a bearer-token session (see router.go AuthLevel doc). The +// handler resolves the authenticated user from any of those three +// credentials rather than re-requiring a bearer session — otherwise a +// caller that authenticated upstream with an API key would still get a +// 401 here. +func (h *Handler) getCurrentUserPermissions(ctx context.Context, req *events.LambdaFunctionURLRequest) (*UserPermissionsResponse, error) { + if h.auth == nil { + return nil, fmt.Errorf("authentication service not configured") + } + + userID, err := h.resolveAuthenticatedUserID(ctx, req) + if err != nil { + return nil, err + } + + // The stateless admin API key has no backing user row, so the + // per-user permission lookup would fail. It is a full-access + // infrastructure credential — surface it as {admin, *} + is_admin + // (matches the requireAdmin / requirePermission short-circuit). + if userID == apiKeyAdminUserID { + return &UserPermissionsResponse{ + Permissions: []PermissionEntry{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}, + IsAdmin: true, + }, nil + } + + raw, err := h.auth.GetUserPermissionsAPI(ctx, userID) + if err != nil { + return nil, err + } + + // GetUserPermissionsAPI returns []auth.APIPermission via any to avoid + // an import cycle between the api and auth packages. Fail loudly on + // an unexpected payload shape: a silent fall-through to an empty + // slice would render as "user lost all access" in the frontend and + // mask the real server bug. + apiPerms, ok := raw.([]auth.APIPermission) + if !ok { + logging.Errorf("getCurrentUserPermissions: GetUserPermissionsAPI returned %T, want []auth.APIPermission", raw) + return nil, fmt.Errorf("GetUserPermissionsAPI returned unexpected payload type %T", raw) + } + entries := make([]PermissionEntry, len(apiPerms)) + isAdmin := false + for i, p := range apiPerms { + entries[i] = PermissionEntry{Action: p.Action, Resource: p.Resource} + if p.Action == auth.ActionAdmin && p.Resource == auth.ResourceAll { + isAdmin = true + } + } + + return &UserPermissionsResponse{ + Permissions: entries, + IsAdmin: isAdmin, + }, nil +} + +// resolveAuthenticatedUserID resolves the calling user's ID from any of +// the three auth modes admitted by AuthUser routes (admin API key, user +// API key, bearer-token session). Returns a 401 ClientError if no valid +// credential is present. The stateless admin API key returns the +// apiKeyAdminUserID sentinel — callers that need a real user row must +// special-case it (see getCurrentUserPermissions for the {admin, *} +// short-circuit). +func (h *Handler) resolveAuthenticatedUserID(ctx context.Context, req *events.LambdaFunctionURLRequest) (string, error) { + // Admin API key first (stateless, no per-user lookup). + apiKey := extractAPIKey(req) + if h.checkAdminAPIKey(apiKey) { + return apiKeyAdminUserID, nil + } + + // User API key: resolves to the owning user row. + if apiKey != "" { + _, user, err := h.auth.ValidateUserAPIKeyAPI(ctx, apiKey) + if err == nil { + if u, ok := user.(*auth.User); ok && u != nil { + return u.ID, nil + } + } + // fall through to bearer-session if the API key didn't validate. + } + + // Bearer-token session. + token := h.extractBearerToken(req) + if token == "" { + return "", NewClientError(401, "no authorization token provided") + } + session, err := h.auth.ValidateSession(ctx, token) + if err != nil || session == nil { + return "", NewClientError(401, "invalid session") + } + return session.UserID, nil +} + func (h *Handler) checkAdminExists(ctx context.Context, req *events.LambdaFunctionURLRequest) (*AdminExistsResponse, error) { if h.auth == nil { return nil, fmt.Errorf("authentication service not configured") diff --git a/internal/api/handler_auth_test.go b/internal/api/handler_auth_test.go index e43b70cc3..29a17fd39 100644 --- a/internal/api/handler_auth_test.go +++ b/internal/api/handler_auth_test.go @@ -6,6 +6,7 @@ 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" @@ -1189,3 +1190,182 @@ func TestHandler_mfaRegenerateRecoveryCodes_HappyPath(t *testing.T) { // returns the same value the real service would. func ErrMFARequired_test() error { return mfaRequiredSentinel } func ErrInvalidMFACode_test() error { return mfaInvalidSentinel } + +// Tests for GET /api/auth/me/permissions (issue #917). + +func TestHandler_getCurrentUserPermissions_NoAuthService(t *testing.T) { + ctx := context.Background() + handler := &Handler{auth: nil} + req := &events.LambdaFunctionURLRequest{} + _, err := handler.getCurrentUserPermissions(ctx, req) + require.Error(t, err) + assert.Contains(t, err.Error(), "authentication service not configured") +} + +func TestHandler_getCurrentUserPermissions_NoToken(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{}} + _, err := handler.getCurrentUserPermissions(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 401, ce.code) +} + +func TestHandler_getCurrentUserPermissions_InvalidSession(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + mockAuth.On("ValidateSession", ctx, "bad-token").Return((*Session)(nil), errors.New("expired")) + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer bad-token"}} + _, err := handler.getCurrentUserPermissions(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 401, ce.code) +} + +// TestHandler_getCurrentUserPermissions_RegularUser asserts that a non-admin user +// with two groups gets the union of their groups' permissions and is_admin == false. +func TestHandler_getCurrentUserPermissions_RegularUser(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + session := &Session{UserID: "user-1"} + mockAuth.On("ValidateSession", ctx, "tok").Return(session, nil) + + // Two groups: one grants view:recommendations, the other view:plans. + perms := []auth.APIPermission{ + {Action: "view", Resource: "recommendations"}, + {Action: "view", Resource: "plans"}, + } + mockAuth.On("GetUserPermissionsAPI", ctx, "user-1").Return(perms, nil) + + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer tok"}} + result, err := handler.getCurrentUserPermissions(ctx, req) + require.NoError(t, err) + assert.False(t, result.IsAdmin) + require.Len(t, result.Permissions, 2) + assert.Equal(t, PermissionEntry{Action: "view", Resource: "recommendations"}, result.Permissions[0]) + assert.Equal(t, PermissionEntry{Action: "view", Resource: "plans"}, result.Permissions[1]) +} + +// TestHandler_getCurrentUserPermissions_Admin asserts that an Administrators-group +// member gets the {admin, *} wildcard and is_admin == true. +func TestHandler_getCurrentUserPermissions_Admin(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + session := &Session{UserID: "admin-1"} + mockAuth.On("ValidateSession", ctx, "admin-tok").Return(session, nil) + + perms := []auth.APIPermission{ + {Action: auth.ActionAdmin, Resource: auth.ResourceAll}, + } + mockAuth.On("GetUserPermissionsAPI", ctx, "admin-1").Return(perms, nil) + + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-tok"}} + result, err := handler.getCurrentUserPermissions(ctx, req) + require.NoError(t, err) + assert.True(t, result.IsAdmin) + require.Len(t, result.Permissions, 1) + assert.Equal(t, PermissionEntry{Action: "admin", Resource: "*"}, result.Permissions[0]) +} + +// TestHandler_getCurrentUserPermissions_Unauth ensures unauthenticated requests get 401. +func TestHandler_getCurrentUserPermissions_Unauth(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + mockAuth.On("ValidateSession", ctx, mock.Anything).Return((*Session)(nil), errors.New("invalid")) + + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer bogus"}} + _, err := handler.getCurrentUserPermissions(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 401, ce.code) +} + +// TestHandler_getCurrentUserPermissions_UnexpectedPayload guards CR #922 F1: +// when GetUserPermissionsAPI returns an unexpected payload shape, the handler +// must fail loudly (server error) rather than silently degrade to an empty +// permission set (which would render as "user lost access" in the frontend). +func TestHandler_getCurrentUserPermissions_UnexpectedPayload(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + session := &Session{UserID: "user-1"} + mockAuth.On("ValidateSession", ctx, "tok").Return(session, nil) + // Adapter returns the wrong concrete type (e.g. a misconfigured + // implementation). The handler must surface this as a server error, + // not silently return an empty PermissionEntry slice. + mockAuth.On("GetUserPermissionsAPI", ctx, "user-1").Return("not-a-permissions-slice", nil) + + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer tok"}} + _, err := handler.getCurrentUserPermissions(ctx, req) + require.Error(t, err) + // Not a 4xx ClientError — it's a server bug, must surface as 500. + _, isClient := IsClientError(err) + assert.False(t, isClient, "unexpected payload should NOT be a ClientError (would mask the server bug as a 4xx)") + assert.Contains(t, err.Error(), "GetUserPermissionsAPI returned unexpected payload type") +} + +// TestHandler_getCurrentUserPermissions_AdminAPIKey guards CR #922 F2: +// the AuthUser route admits the stateless admin API key as well as bearer +// sessions, so the handler must honour an X-API-Key-authenticated request +// instead of forcing a bearer session a second time and returning 401. +func TestHandler_getCurrentUserPermissions_AdminAPIKey(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + handler := &Handler{auth: mockAuth, apiKey: "admin-secret"} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"X-API-Key": "admin-secret"}} + result, err := handler.getCurrentUserPermissions(ctx, req) + require.NoError(t, err) + // The stateless admin API key has no backing user row, so the handler + // short-circuits to {admin, *} + is_admin=true rather than calling + // GetUserPermissionsAPI (which would fail to find an admin-api-key row). + assert.True(t, result.IsAdmin) + require.Len(t, result.Permissions, 1) + assert.Equal(t, PermissionEntry{Action: auth.ActionAdmin, Resource: auth.ResourceAll}, result.Permissions[0]) +} + +// TestHandler_getCurrentUserPermissions_UserAPIKey guards CR #922 F2: +// a user API key resolves to the owning user, and the handler must look +// up that user's effective permissions instead of returning 401 because +// the request has no bearer token. +func TestHandler_getCurrentUserPermissions_UserAPIKey(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + user := &auth.User{ID: "user-42"} + mockAuth.On("ValidateUserAPIKeyAPI", ctx, "user-key").Return((any)(nil), any(user), nil) + + perms := []auth.APIPermission{ + {Action: "view", Resource: "recommendations"}, + } + mockAuth.On("GetUserPermissionsAPI", ctx, "user-42").Return(perms, nil) + + handler := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"X-API-Key": "user-key"}} + result, err := handler.getCurrentUserPermissions(ctx, req) + require.NoError(t, err) + assert.False(t, result.IsAdmin) + require.Len(t, result.Permissions, 1) + assert.Equal(t, PermissionEntry{Action: "view", Resource: "recommendations"}, result.Permissions[0]) +} diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index a7fe60244..014c9625a 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -851,6 +851,9 @@ func (m *mockAuthForExchange) ListGroupsAPI(_ context.Context) (any, error) func (m *mockAuthForExchange) HasPermissionAPI(_ context.Context, _, _, _ string) (bool, error) { return true, nil } +func (m *mockAuthForExchange) GetUserPermissionsAPI(_ context.Context, _ string) (any, error) { + return nil, nil +} func (m *mockAuthForExchange) CreateAPIKeyAPI(_ context.Context, _ string, _ any) (any, error) { return nil, nil } diff --git a/internal/api/mocks_test.go b/internal/api/mocks_test.go index 18223b334..858618a43 100644 --- a/internal/api/mocks_test.go +++ b/internal/api/mocks_test.go @@ -860,6 +860,11 @@ func (m *MockAuthService) HasPermissionAPI(ctx context.Context, userID, action, return args.Bool(0), args.Error(1) } +func (m *MockAuthService) GetUserPermissionsAPI(ctx context.Context, userID string) (any, error) { + args := m.Called(ctx, userID) + return args.Get(0), args.Error(1) +} + // grantAdmin makes every HasPermissionAPI check succeed, modelling an // Administrators-group member. Authorization is group-membership-only after // issue #907, so admin-gated handlers resolve "is admin" / specific permissions diff --git a/internal/api/router.go b/internal/api/router.go index 75e38059d..e5a3e700e 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -203,6 +203,10 @@ func (r *Router) registerRoutes() { {ExactPath: "/api/auth/login", Method: "POST", Handler: r.loginHandler, Auth: AuthPublic}, {ExactPath: "/api/auth/logout", Method: "POST", Handler: r.logoutHandler, Auth: AuthUser}, {ExactPath: "/api/auth/me", Method: "GET", Handler: r.getCurrentUserHandler, Auth: AuthUser}, + // Effective permission set for the authenticated user (issue #917). + // Must be listed before /api/auth/me so an exact-path match fires + // rather than a future prefix match. Auth level mirrors /api/auth/me. + {ExactPath: "/api/auth/me/permissions", Method: "GET", Handler: r.getCurrentUserPermissionsHandler, Auth: AuthUser}, {ExactPath: "/api/auth/check-admin", Method: "GET", Handler: r.checkAdminExistsHandler, Auth: AuthPublic}, {ExactPath: "/api/auth/setup-admin", Method: "POST", Handler: r.setupAdminHandler, Auth: AuthPublic}, {ExactPath: "/api/auth/forgot-password", Method: "POST", Handler: r.forgotPasswordHandler, Auth: AuthPublic}, @@ -577,6 +581,10 @@ func (r *Router) getCurrentUserHandler(ctx context.Context, req *events.LambdaFu return r.h.getCurrentUser(ctx, req) } +func (r *Router) getCurrentUserPermissionsHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.getCurrentUserPermissions(ctx, req) +} + func (r *Router) checkAdminExistsHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { return r.h.checkAdminExists(ctx, req) } diff --git a/internal/api/types.go b/internal/api/types.go index 867b4eecd..2e93dd06c 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -185,6 +185,10 @@ type AuthServiceInterface interface { ListGroupsAPI(ctx context.Context) (any, error) // Permission checking HasPermissionAPI(ctx context.Context, userID, action, resource string) (bool, error) + // GetUserPermissionsAPI returns the effective permission set for a user + // (union of all group permissions). Used by GET /api/auth/me/permissions. + // Returns []auth.APIPermission converted to []PermissionEntry by the handler. + GetUserPermissionsAPI(ctx context.Context, userID string) (any, error) // Account access - returns the union of allowed_accounts from all user groups (empty = all access) GetAllowedAccountsAPI(ctx context.Context, userID string) ([]string, error) // API Key management @@ -395,6 +399,22 @@ type AdminExistsResponse struct { AdminExists bool `json:"admin_exists"` } +// PermissionEntry is a single {action, resource} pair in the permissions +// response. Constraints are omitted from the wire shape for now; the +// frontend uses the pair for UX gating only. +type PermissionEntry struct { + Action string `json:"action"` + Resource string `json:"resource"` +} + +// UserPermissionsResponse is the response shape for GET /api/auth/me/permissions. +// Permissions is the effective set derived from the union of the user's groups. +// IsAdmin mirrors whether the effective set contains the {admin, *} wildcard. +type UserPermissionsResponse struct { + Permissions []PermissionEntry `json:"permissions"` + IsAdmin bool `json:"is_admin"` +} + // MFA enrollment + lifecycle DTOs (issue #497). Passwords carried by // these requests are base64-encoded by the frontend (same convention // as login / change-password / reset-password); the handler decodes diff --git a/internal/auth/service_api.go b/internal/auth/service_api.go index 24a94b01b..de98144b3 100644 --- a/internal/auth/service_api.go +++ b/internal/auth/service_api.go @@ -359,6 +359,22 @@ func (s *Service) HasPermissionAPI(ctx context.Context, userID, action, resource return s.HasPermission(ctx, userID, action, resource, nil) } +// GetUserPermissionsAPI returns the effective permission set for a user via +// the API. Calls GetUserPermissions (the same union path the server enforces +// with) and converts each Permission to an APIPermission for the wire format. +// The handler asserts the return value to []APIPermission. +func (s *Service) GetUserPermissionsAPI(ctx context.Context, userID string) (any, error) { + perms, err := s.GetUserPermissions(ctx, userID) + if err != nil { + return nil, err + } + result := make([]APIPermission, len(perms)) + for i, p := range perms { + result[i] = permissionToAPIPermission(p) + } + return result, nil +} + // MFASetupAPI starts an MFA enrollment via the API. Returns the // freshly-generated secret + provisioning URI (the otpauth:// URI // the frontend renders as a QR code). Wraps MFASetup; thin shim diff --git a/internal/server/app.go b/internal/server/app.go index 90a8ab254..7546f9f10 100644 --- a/internal/server/app.go +++ b/internal/server/app.go @@ -1013,6 +1013,10 @@ func (a *authServiceAdapter) HasPermissionAPI(ctx context.Context, userID, actio return a.service.HasPermissionAPI(ctx, userID, action, resource) } +func (a *authServiceAdapter) GetUserPermissionsAPI(ctx context.Context, userID string) (any, error) { + return a.service.GetUserPermissionsAPI(ctx, userID) +} + // Account access func (a *authServiceAdapter) GetAllowedAccountsAPI(ctx context.Context, userID string) ([]string, error) { authCtx, err := a.service.BuildAuthContext(ctx, userID)