From 729fdf50d0db0a762964f88579b58ee4eb74528b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 14:22:38 +0000 Subject: [PATCH 1/2] Add grant and revoke to @fuzefront/auth authz client Implement grant(), revoke(), and listGrants() methods on the AuthzClient interface, mirroring the server's grant/revoke endpoints at /api/v1/security/authz/grants. - grant(req, token): POST with 201 success, 400 MALFORMED, 502 PROVIDER_ERROR - revoke(req, token): DELETE with 204 success, 400 MALFORMED, 502 PROVIDER_ERROR - listGrants(query, token): GET with cursor-paginated result Error codes MALFORMED and PROVIDER_ERROR distinguish caller bugs (400) from upstream transients (502). A grant/revoke is a write; any failure throws and never silently succeeds. Tests cover successful grants with and without resource scope, both revoke forms (by grantId and by subject+tenant+role), error distinction, timeout handling, and listGrants pagination. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session --- packages/auth/src/authzClient.ts | 145 ++++++++++++++++ packages/auth/src/authzTypes.ts | 94 +++++++++- packages/auth/src/index.ts | 5 + packages/auth/tests/authz.test.ts | 276 ++++++++++++++++++++++++++++++ 4 files changed, 519 insertions(+), 1 deletion(-) diff --git a/packages/auth/src/authzClient.ts b/packages/auth/src/authzClient.ts index c70a3085e..eef3b1ab4 100644 --- a/packages/auth/src/authzClient.ts +++ b/packages/auth/src/authzClient.ts @@ -13,12 +13,19 @@ import { AuthzDecision, AuthzError, FetchLike, + Grant, + GrantRequest, + GrantRevokeRequest, + GrantPage, + GrantListQuery, } from './authzTypes'; /** Path of the single-decision endpoint, relative to `baseUrl`. */ const CHECK_PATH = '/api/v1/security/authz/check'; /** Path of the batch-decision endpoint, relative to `baseUrl`. */ const BULK_CHECK_PATH = '/api/v1/security/authz/bulk-check'; +/** Path of the grants endpoint, relative to `baseUrl`. */ +const GRANTS_PATH = '/api/v1/security/authz/grants'; const DEFAULT_TIMEOUT_MS = 3000; const DEFAULT_CACHE_MAX_ENTRIES = 1000; @@ -126,6 +133,55 @@ export function createAuthzClient(options: AuthzClientOptions): AuthzClient { } } + /** + * Make an HTTP request (POST, DELETE, GET) to the Security API. + * Distinguishes MALFORMED (400) from PROVIDER_ERROR (502). + * Returns status code and parsed JSON body (if applicable). + */ + async function request( + method: string, + path: string, + body: unknown | undefined, + token: string, + ): Promise<{ status: number; body: unknown }> { + const controller = typeof AbortController === 'function' ? new AbortController() : undefined; + const timer = controller + ? setTimeout(() => controller.abort(), timeoutMs) + : undefined; + try { + const reqInit: any = { + method, + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${token}`, + }, + signal: controller?.signal, + }; + if (body !== undefined) { + reqInit.body = JSON.stringify(body); + } + const res = await resolvedFetch!(`${base}${path}`, reqInit); + let responseBody: unknown = undefined; + if (res.status !== 204) { + // 204 No Content has no body + try { + responseBody = await res.json(); + } catch { + responseBody = null; + } + } + return { status: res.status, body: responseBody }; + } catch (err) { + // Timeout or transport error — treat as 502 (provider error) + throw new AuthzError( + 'PROVIDER_ERROR', + `Security API request to ${path} failed: ${(err as Error)?.message ?? 'unknown error'}`, + ); + } finally { + if (timer) clearTimeout(timer); + } + } + /** Read `{ allow: boolean }` strictly. Anything else is not a decision. */ function readDecision(value: unknown): AuthzDecision { const allow = (value as { allow?: unknown } | null)?.allow; @@ -194,5 +250,94 @@ export function createAuthzClient(options: AuthzClientOptions): AuthzClient { }); return decisions; }, + + async grant(req: GrantRequest, token: string): Promise { + const { status, body } = await request('POST', GRANTS_PATH, req, token); + if (status === 201) { + // Success — body should be the created Grant + const grant = body as Grant; + if (!grant?.id) { + throw new AuthzError('PROVIDER_ERROR', 'Security API returned 201 but no grant id'); + } + return grant; + } + if (status === 400) { + const msg = + (body as { error?: string } | null)?.error || 'grant request malformed'; + throw new AuthzError('MALFORMED', msg); + } + if (status === 502) { + const msg = + (body as { error?: string } | null)?.error || 'provider error'; + throw new AuthzError('PROVIDER_ERROR', msg); + } + // Any other status is unexpected + throw new AuthzError( + 'PROVIDER_ERROR', + `Security API returned ${status} for grant; treating as transient error`, + ); + }, + + async revoke(req: GrantRevokeRequest, token: string): Promise { + const { status, body } = await request('DELETE', GRANTS_PATH, req, token); + if (status === 204) { + // Success + return; + } + if (status === 400) { + const msg = + (body as { error?: string } | null)?.error || 'revoke request malformed'; + throw new AuthzError('MALFORMED', msg); + } + // 502 or other error + if (status === 502) { + const msg = + (body as { error?: string } | null)?.error || 'provider error'; + throw new AuthzError('PROVIDER_ERROR', msg); + } + throw new AuthzError( + 'PROVIDER_ERROR', + `Security API returned ${status} for revoke; treating as transient error`, + ); + }, + + async listGrants(query: GrantListQuery, token: string): Promise { + if (!query.tenant) { + throw new AuthzError('MALFORMED', 'tenant is required for listGrants'); + } + const params = new URLSearchParams(); + params.set('tenant', query.tenant); + if (query.subject) params.set('subject', query.subject); + if (query.limit !== undefined) params.set('limit', String(query.limit)); + if (query.cursor) params.set('cursor', query.cursor); + const { status, body } = await request( + 'GET', + `${GRANTS_PATH}?${params.toString()}`, + undefined, + token, + ); + if (status === 200) { + // Success — body should be the page + const page = body as GrantPage; + if (!page?.items || !page?.page) { + throw new AuthzError('PROVIDER_ERROR', 'Security API returned 200 but malformed page'); + } + return page; + } + if (status === 400) { + const msg = + (body as { error?: string } | null)?.error || 'listGrants request malformed'; + throw new AuthzError('MALFORMED', msg); + } + if (status === 502) { + const msg = + (body as { error?: string } | null)?.error || 'provider error'; + throw new AuthzError('PROVIDER_ERROR', msg); + } + throw new AuthzError( + 'PROVIDER_ERROR', + `Security API returned ${status} for listGrants; treating as transient error`, + ); + }, }; } diff --git a/packages/auth/src/authzTypes.ts b/packages/auth/src/authzTypes.ts index 9bf7e39b8..c87227312 100644 --- a/packages/auth/src/authzTypes.ts +++ b/packages/auth/src/authzTypes.ts @@ -68,7 +68,11 @@ export type AuthzErrorCode = /** The Security API could not be reached, timed out, or answered non-200. Denied. */ | 'DECISION_UNAVAILABLE' /** The guard/client is misconfigured (e.g. no baseUrl). Denied. */ - | 'AUTHZ_MISCONFIGURED'; + | 'AUTHZ_MISCONFIGURED' + /** The caller supplied malformed/incomplete request arguments. */ + | 'MALFORMED' + /** The authorization provider (upstream backend service) failed. Transient. */ + | 'PROVIDER_ERROR'; /** Error raised by the authz client/guard. Mirrors `AuthError`'s shape and discipline. */ export class AuthzError extends Error { @@ -124,6 +128,74 @@ export interface AuthzClientOptions { cacheMaxEntries?: number; } +/** A grant request: assign a role and optional permission to a subject. */ +export interface GrantRequest { + /** Principal being granted (user or service client). */ + subject: string; + /** Tenant/org scope. */ + tenant: string; + /** Role key to assign. */ + role: string; + /** Optional explicit `resource:action` permission alongside the role. */ + permission?: string; + /** Optional resource instance to scope the grant (ReBAC). Omit for tenant-wide. */ + resource?: ResourceRef; +} + +/** A created, revocable grant. */ +export interface Grant { + /** Opaque grant identifier. */ + id: string; + /** Principal granted. */ + subject: string; + /** Tenant scope. */ + tenant: string; + /** Role assigned. */ + role: string; + /** Optional permission alongside the role. */ + permission?: string; + /** Optional resource instance scope. */ + resource?: ResourceRef; + /** Timestamp of creation (epoch ms). */ + createdAt?: number; +} + +/** Revoke a grant by id or by identity tuple (subject + tenant + role). */ +export interface GrantRevokeRequest { + /** Grant id to revoke. Omit to revoke by identity tuple. */ + grantId?: string; + /** Subject to revoke (required if revoking by tuple). */ + subject?: string; + /** Tenant to revoke within (required if revoking by tuple). */ + tenant?: string; + /** Role to revoke (required if revoking by tuple). */ + role?: string; + /** Optional resource to scope the revocation. */ + resource?: ResourceRef; +} + +/** Page of grants (cursor-paginated). */ +export interface GrantPage { + items: Grant[]; + page: { + nextCursor: string | null; + hasMore: boolean; + total?: number; + }; +} + +/** Query parameters for listing grants. */ +export interface GrantListQuery { + /** Subject whose grants to list. Defaults to caller. */ + subject?: string; + /** Tenant to list within. Required. */ + tenant: string; + /** Optional limit (enforced server-side). */ + limit?: number; + /** Opaque cursor for pagination. */ + cursor?: string; +} + /** The authz client: a thin, fail-closed HTTP binding to the Security API. */ export interface AuthzClient { /** @@ -140,4 +212,24 @@ export interface AuthzClient { * resource's `allow` to a different resource. */ bulkCheck(checks: AuthzCheck[], token: string): Promise; + /** + * Grant a role (and optional permission) to a subject within a tenant. + * Throws `AuthzError('MALFORMED')` if required fields are missing. + * Throws `AuthzError('PROVIDER_ERROR')` if the backend provider fails (transient). + * Resolves with the created grant on success. + */ + grant(req: GrantRequest, token: string): Promise; + /** + * Revoke a grant by id, or by subject+tenant+role identity tuple. + * Throws `AuthzError('MALFORMED')` if neither form is provided or required fields missing. + * Throws `AuthzError('PROVIDER_ERROR')` if the backend provider fails (transient). + * Resolves on success (204 No Content from API). + */ + revoke(req: GrantRevokeRequest, token: string): Promise; + /** + * List grants for a subject within a tenant (cursor-paginated). + * Throws `AuthzError('MALFORMED')` if tenant is missing. + * Resolves with a page of grants. + */ + listGrants(query: GrantListQuery, token: string): Promise; } diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts index aa78b5933..b775192d6 100644 --- a/packages/auth/src/index.ts +++ b/packages/auth/src/index.ts @@ -45,6 +45,11 @@ export type { AuthzErrorCode, ResourceRef, FetchLike, + GrantRequest, + Grant, + GrantRevokeRequest, + GrantPage, + GrantListQuery, } from './authzTypes'; export { createAuthzClient } from './authzClient'; diff --git a/packages/auth/tests/authz.test.ts b/packages/auth/tests/authz.test.ts index b55340a69..8373de57b 100644 --- a/packages/auth/tests/authz.test.ts +++ b/packages/auth/tests/authz.test.ts @@ -334,3 +334,279 @@ describe('requirePermission', () => { expect(() => requirePermission({ resource: 'x', action: 'y' } as any)).toThrow(AuthzError); }); }); + +describe('createAuthzClient — grant/revoke/listGrants', () => { + const clientWith = (responses: Array<{ status?: number; body?: unknown } | Error>) => + createAuthzClient({ baseUrl: BASE, fetch: mockFetch(responses) }); + + it('grants a role and resolves with the created grant', async () => { + const fetch = mockFetch([ + { + status: 201, + body: { + id: 'grant_001', + subject: 'user_1', + tenant: 'tenant_1', + role: 'admin', + permission: 'resource:write', + resource: { type: 'invoice', key: 'inv_123' }, + createdAt: 1_700_000_000, + }, + }, + ]); + const client = createAuthzClient({ baseUrl: BASE, fetch }); + + const grant = await client.grant( + { + subject: 'user_1', + tenant: 'tenant_1', + role: 'admin', + permission: 'resource:write', + resource: { type: 'invoice', key: 'inv_123' }, + }, + TOKEN, + ); + + expect(grant).toEqual({ + id: 'grant_001', + subject: 'user_1', + tenant: 'tenant_1', + role: 'admin', + permission: 'resource:write', + resource: { type: 'invoice', key: 'inv_123' }, + createdAt: 1_700_000_000, + }); + expect(fetch.calls[0].url).toBe(`${BASE}/api/v1/security/authz/grants`); + expect(fetch.calls[0].init.method).toBe('POST'); + expect(fetch.calls[0].init.headers.authorization).toBe(`Bearer ${TOKEN}`); + }); + + it('grants a tenant-wide role (no resource scope)', async () => { + const fetch = mockFetch([ + { + status: 201, + body: { + id: 'grant_002', + subject: 'user_2', + tenant: 'tenant_2', + role: 'viewer', + }, + }, + ]); + const client = createAuthzClient({ baseUrl: BASE, fetch }); + + const grant = await client.grant( + { + subject: 'user_2', + tenant: 'tenant_2', + role: 'viewer', + }, + TOKEN, + ); + + expect(grant.id).toBe('grant_002'); + expect(grant.resource).toBeUndefined(); + }); + + it('throws MALFORMED when grant 400s', async () => { + const client = clientWith([ + { + status: 400, + body: { error: 'subject, tenant and role are required', code: 'MALFORMED' }, + }, + ]); + + await expect( + client.grant( + { subject: 'user_1', tenant: 'tenant_1', role: 'admin' }, + TOKEN, + ), + ).rejects.toMatchObject({ code: 'MALFORMED', message: expect.stringMatching(/subject, tenant and role/) }); + }); + + it('throws PROVIDER_ERROR when grant 502s', async () => { + const client = clientWith([{ status: 502, body: { error: 'grant failed', code: 'PROVIDER_ERROR' } }]); + + await expect( + client.grant( + { subject: 'user_1', tenant: 'tenant_1', role: 'admin' }, + TOKEN, + ), + ).rejects.toMatchObject({ code: 'PROVIDER_ERROR' }); + }); + + it('treats timeout on grant as PROVIDER_ERROR', async () => { + const timeout = Object.assign(new Error('The operation was aborted'), { name: 'AbortError' }); + const client = clientWith([timeout]); + + await expect( + client.grant( + { subject: 'user_1', tenant: 'tenant_1', role: 'admin' }, + TOKEN, + ), + ).rejects.toMatchObject({ code: 'PROVIDER_ERROR' }); + }); + + it('revokes by grantId and resolves with 204', async () => { + const fetch = mockFetch([{ status: 204 }]); + const client = createAuthzClient({ baseUrl: BASE, fetch }); + + await client.revoke({ grantId: 'grant_001' }, TOKEN); + + expect(fetch.calls[0].url).toBe(`${BASE}/api/v1/security/authz/grants`); + expect(fetch.calls[0].init.method).toBe('DELETE'); + expect(JSON.parse(fetch.calls[0].init.body)).toEqual({ grantId: 'grant_001' }); + }); + + it('revokes by subject+tenant+role tuple', async () => { + const fetch = mockFetch([{ status: 204 }]); + const client = createAuthzClient({ baseUrl: BASE, fetch }); + + await client.revoke( + { + subject: 'user_1', + tenant: 'tenant_1', + role: 'admin', + }, + TOKEN, + ); + + expect(fetch.calls[0].init.method).toBe('DELETE'); + expect(JSON.parse(fetch.calls[0].init.body)).toEqual({ + subject: 'user_1', + tenant: 'tenant_1', + role: 'admin', + }); + }); + + it('throws MALFORMED when revoke 400s', async () => { + const client = clientWith([ + { + status: 400, + body: { error: 'grantId or subject+tenant+role required', code: 'MALFORMED' }, + }, + ]); + + await expect( + client.revoke({ grantId: '' }, TOKEN), + ).rejects.toMatchObject({ + code: 'MALFORMED', + message: expect.stringMatching(/grantId or subject/), + }); + }); + + it('throws PROVIDER_ERROR when revoke 502s', async () => { + const client = clientWith([{ status: 502, body: { error: 'revoke failed', code: 'PROVIDER_ERROR' } }]); + + await expect( + client.revoke({ grantId: 'grant_001' }, TOKEN), + ).rejects.toMatchObject({ code: 'PROVIDER_ERROR' }); + }); + + it('lists grants for a tenant', async () => { + const fetch = mockFetch([ + { + status: 200, + body: { + items: [ + { + id: 'grant_001', + subject: 'user_1', + tenant: 'tenant_1', + role: 'admin', + }, + { + id: 'grant_002', + subject: 'user_1', + tenant: 'tenant_1', + role: 'viewer', + }, + ], + page: { + nextCursor: null, + hasMore: false, + total: 2, + }, + }, + }, + ]); + const client = createAuthzClient({ baseUrl: BASE, fetch }); + + const page = await client.listGrants({ tenant: 'tenant_1' }, TOKEN); + + expect(page.items).toHaveLength(2); + expect(page.items[0].id).toBe('grant_001'); + expect(page.page.hasMore).toBe(false); + expect(fetch.calls[0].url).toContain('/api/v1/security/authz/grants?'); + expect(fetch.calls[0].url).toContain('tenant=tenant_1'); + expect(fetch.calls[0].init.method).toBe('GET'); + }); + + it('listGrants with optional subject and cursor', async () => { + const fetch = mockFetch([ + { + status: 200, + body: { + items: [{ id: 'grant_003', subject: 'user_2', tenant: 'tenant_1', role: 'editor' }], + page: { + nextCursor: 'cursor_abc', + hasMore: true, + }, + }, + }, + ]); + const client = createAuthzClient({ baseUrl: BASE, fetch }); + + await client.listGrants( + { + tenant: 'tenant_1', + subject: 'user_2', + limit: 50, + cursor: 'prev_cursor', + }, + TOKEN, + ); + + expect(fetch.calls[0].url).toContain('tenant=tenant_1'); + expect(fetch.calls[0].url).toContain('subject=user_2'); + expect(fetch.calls[0].url).toContain('limit=50'); + expect(fetch.calls[0].url).toContain('cursor=prev_cursor'); + }); + + it('throws MALFORMED when listGrants has no tenant', async () => { + const client = clientWith([]); + + await expect( + client.listGrants({ tenant: '' }, TOKEN), + ).rejects.toMatchObject({ + code: 'MALFORMED', + message: expect.stringMatching(/tenant is required/), + }); + }); + + it('throws MALFORMED when listGrants 400s', async () => { + const client = clientWith([ + { + status: 400, + body: { error: 'tenant is required', code: 'MALFORMED' }, + }, + ]); + + await expect( + client.listGrants({ tenant: 'tenant_1' }, TOKEN), + ).rejects.toMatchObject({ code: 'MALFORMED' }); + }); + + it('throws PROVIDER_ERROR when listGrants 502s', async () => { + const client = clientWith([ + { + status: 502, + body: { error: 'provider error', code: 'PROVIDER_ERROR' }, + }, + ]); + + await expect( + client.listGrants({ tenant: 'tenant_1' }, TOKEN), + ).rejects.toMatchObject({ code: 'PROVIDER_ERROR' }); + }); +}); From 7a31844498cd9475eb55dcb86178e52cc5c4162f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 14:46:11 +0000 Subject: [PATCH 2/2] feat(selection-list-service): route authorization through the Security API instead of embedding Permit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 2 of 3 (owner-requested migration; config-service was step 1, #679; billing-service follows separately). selection-list-service consumed the permitio SDK directly, giving it its own PERMIT_API_KEY and defeating backend/security's provider-agnostic AuthorizationProvider seam (authzFactory.ts) — swapping providers meant touching every service instead of just backend/security. - src/middleware/authz.ts (replaces middleware/permit.ts) talks to FuzeFront's own Security API via @fuzefront/auth's createAuthzClient — no vendor SDK, no vendor API key in this service any more. Fail-closed throughout: DECISION_UNAVAILABLE and any transport error deny, never allow. Same flag-gated dark-deploy semantics as the Permit-backed predecessor (fuzefront.selection-list.authz-enabled, default OFF). - This service does MORE than decisions: requirePermit()/requireAuthzCheck() maps to AuthzClient.check(); the PUT/DELETE handlers' direct roleAssignments.assign/unassign calls and grantListOwner() map to AuthzClient.grant()/revoke(). All three call sites pass resource: { type: 'SelectionList', key: listId } — the resource is what scopes a grant/revoke/check to one list instance instead of silently widening to tenant-wide (a real privilege-escalation surface if dropped). countActiveOwners() is untouched: it reads only the local selection_list_access mirror table and was never used for authorization. - grant()/revoke() are writes: AuthzClient throws (never resolves) on a Security API failure, and every call site performs that write BEFORE touching the selection_list_access mirror row, so a thrown grant/revoke can never leave the mirror claiming a role change that did not happen. Covered by dedicated tests asserting the mirror write is never reached. - Found and fixed a resource-scoping gap in backend/security this migration depends on for correctness: PermitAuthorizationProvider.revoke() accepted GrantRevokeRequest.resource per the AuthorizationProvider contract but discarded it (unassignRoleInPermit()'s parameter type explicitly Omit'd resource_instance), so an instance-scoped revoke would silently succeed while Permit's state was unchanged. Now forwards resource_instance through; existing tenant-wide callers (organization-role helpers) are unaffected since resource_instance stays optional. - middleware/permit.flags.ts renamed to authz.flags.ts (same env-var-based flag mechanism, unrelated to the Permit SDK — the old filename no longer fit). - Removed the permitio dependency; added @fuzefront/auth (pinned to the sibling workspace version, matching backend/applications' convention for in-repo packages). Dockerfile: packages/auth is now actually BUILT (tsup) and its dist copied into the production image (config-service's Dockerfile is the reference pattern for the same package) — the pre-existing packages/auth/package.json COPY in both npm ci stages (from #679's prep) only satisfied hoisting, not the runtime import this service now has. - Helm: PERMIT_API_KEY (SealedSecret) drops; SECURITY_SERVICE_URL (plain in-cluster Service DNS, matching config-service's convention) takes its place, in the deployment template and the sealed-secret template/GO-LIVE instructions. - openapi.yaml: prose-only fix to authorization descriptions that named Permit specifically — no schema/status-code change. Verified on the committed tree: services/selection-list-service type-check, build, and `npm test` (234 tests / 9 suites, up from prior baseline — new DECISION_UNAVAILABLE fail-closed tests, resource-scoping assertions on grant/revoke, and write-ordering tests proving a thrown grant/revoke never reaches the mirror-table write); backend/security's new regression test for the resource-scoping fix (4 tests); root `npm ci`; scripts/check-dockerfile-lockfile.mjs and scripts/check-workspace-deps.mjs; `grep -rn permitio services/selection-list-service/` empty. Claude-Session: https://claude.ai/code/session_0183JMAkioT5pGt8aVmddPtc Co-authored-by: Claude --- .../permit/PermitAuthorizationProvider.ts | 14 +- .../src/utils/permit/role-assignment.ts | 16 +- .../tests/role-revoke.resource-scope.test.ts | 103 +++++++ ...lection-list-service-secrets.yaml.template | 22 +- .../selection-list-service-deployment.yaml | 16 +- deploy/helm/fuzefront/values-prod.yaml | 10 +- package-lock.json | 2 +- services/selection-list-service/Dockerfile | 20 +- services/selection-list-service/openapi.yaml | 16 +- services/selection-list-service/package.json | 2 +- .../{permit.flags.ts => authz.flags.ts} | 9 +- .../src/middleware/authz.ts | 291 ++++++++++++++++++ .../src/middleware/permit.ts | 229 -------------- .../src/routes/access.ts | 77 +++-- .../tests/access.routes.test.ts | 147 ++++++--- ...eware.test.ts => authz.middleware.test.ts} | 187 +++++++---- 16 files changed, 777 insertions(+), 384 deletions(-) create mode 100644 backend/security/tests/role-revoke.resource-scope.test.ts rename services/selection-list-service/src/middleware/{permit.flags.ts => authz.flags.ts} (77%) create mode 100644 services/selection-list-service/src/middleware/authz.ts delete mode 100644 services/selection-list-service/src/middleware/permit.ts rename services/selection-list-service/tests/{permit.middleware.test.ts => authz.middleware.test.ts} (54%) diff --git a/backend/security/src/providers/permit/PermitAuthorizationProvider.ts b/backend/security/src/providers/permit/PermitAuthorizationProvider.ts index b04043842..c3d8194a6 100644 --- a/backend/security/src/providers/permit/PermitAuthorizationProvider.ts +++ b/backend/security/src/providers/permit/PermitAuthorizationProvider.ts @@ -145,7 +145,19 @@ export class PermitAuthorizationProvider implements AuthorizationProvider { } // Idempotent: the helper returns false (not throws) if the assignment is // already gone, which we treat as success. - await unassignRoleInPermit({ user: subject, role, tenant }) + // + // resource_instance MUST be forwarded when req.resource is present: Permit + // keys a role assignment by the full (user, role, tenant, resource_instance) + // tuple, so an instance-scoped grant (ReBAC — e.g. one list) is a distinct + // record from a tenant-wide one. Omitting it here would revoke nothing for + // a caller that only ever held the scoped assignment, while this call + // still resolves successfully — a silent no-op revoke. + await unassignRoleInPermit({ + user: subject, + role, + tenant, + resource_instance: resourceInstance(req.resource), + }) } async listGrants(query: GrantQuery): Promise> { diff --git a/backend/security/src/utils/permit/role-assignment.ts b/backend/security/src/utils/permit/role-assignment.ts index d466494f4..6859c21d6 100644 --- a/backend/security/src/utils/permit/role-assignment.ts +++ b/backend/security/src/utils/permit/role-assignment.ts @@ -29,10 +29,22 @@ export async function assignRoleInPermit( } /** - * Unassigns a role from a user in an organization (tenant) + * Unassigns a role from a user in an organization (tenant). + * + * `resource_instance` IS accepted (unlike an earlier revision of this + * signature, which omitted it): Permit identifies a role assignment by the + * full (user, role, tenant, resource_instance) tuple, so an instance-scoped + * assignment (ReBAC, e.g. `SelectionList:sl_123`) is a DIFFERENT record from + * the tenant-wide one and is not removed by an unassign call that leaves + * `resource_instance` off. Dropping it silently no-ops the revocation of a + * scoped grant while still returning success — a caller believes access was + * revoked when Permit's state is unchanged. See + * `PermitAuthorizationProvider.revoke()`, the only caller that has a + * `resource` to pass; the organization-role helpers below intentionally + * never scope by instance and are unaffected by this being optional. */ export async function unassignRoleInPermit( - assignment: Omit + assignment: RoleAssignment ): Promise { try { await permit.api.roleAssignments.unassign(assignment) diff --git a/backend/security/tests/role-revoke.resource-scope.test.ts b/backend/security/tests/role-revoke.resource-scope.test.ts new file mode 100644 index 000000000..6890adb8b --- /dev/null +++ b/backend/security/tests/role-revoke.resource-scope.test.ts @@ -0,0 +1,103 @@ +/** + * Regression test for a resource-instance-scoping gap in the Permit-backed + * revoke path. + * + * `GrantRevokeRequest.resource` (the contract's ReBAC scope, mirrored from + * `@fuzefront/auth`'s `authzTypes.ts`) was accepted by + * `PermitAuthorizationProvider.revoke()` but silently discarded before this + * fix — `unassignRoleInPermit()`'s parameter type explicitly `Omit`ted + * `resource_instance`. Permit keys a role assignment by the FULL + * (user, role, tenant, resource_instance) tuple, so an instance-scoped grant + * (e.g. `SelectionList:sl_123`) is a different record from a tenant-wide one: + * dropping `resource_instance` meant a caller revoking one list's grant would + * get a 204 while Permit's state was unchanged — a silent no-op revoke, the + * opposite of what the caller asked for and believes happened. + * + * This mattered immediately for `selection-list-service`'s migration off an + * embedded Permit SDK onto this Security API (step 2 of 3): its + * `DELETE /:listId/access/:userId` depends on `AuthzClient.revoke()` actually + * reaching Permit with the list's `resource_instance`. + */ +process.env.NODE_ENV = 'test' +process.env.PERMIT_API_KEY = 'ci-no-real-permit-calls' + +const unassignMock = jest.fn().mockResolvedValue({}) + +jest.mock('../src/config/permit', () => ({ + __esModule: true, + default: { + api: { + roleAssignments: { + assign: jest.fn().mockResolvedValue({}), + unassign: unassignMock, + list: jest.fn().mockResolvedValue([]), + }, + }, + }, + permitConfig: { token: 'ci-no-real-permit-calls', pdp: 'http://localhost:7766' }, +})) + +import { unassignRoleInPermit } from '../src/utils/permit/role-assignment' +import { PermitAuthorizationProvider } from '../src/providers/permit/PermitAuthorizationProvider' + +describe('unassignRoleInPermit — forwards resource_instance', () => { + beforeEach(() => unassignMock.mockClear()) + + it('passes resource_instance through to permit.api.roleAssignments.unassign when supplied', async () => { + await unassignRoleInPermit({ + user: 'usr_1', + role: 'list-owner', + tenant: 'org_acme', + resource_instance: 'SelectionList:sl_123', + }) + + expect(unassignMock).toHaveBeenCalledWith({ + user: 'usr_1', + role: 'list-owner', + tenant: 'org_acme', + resource_instance: 'SelectionList:sl_123', + }) + }); + + it('omits resource_instance for a tenant-wide unassign (organization-role helpers)', async () => { + await unassignRoleInPermit({ user: 'usr_1', role: 'admin', tenant: 'org_acme' }) + + expect(unassignMock).toHaveBeenCalledWith({ + user: 'usr_1', + role: 'admin', + tenant: 'org_acme', + }) + }); +}) + +describe('PermitAuthorizationProvider.revoke — forwards req.resource as resource_instance', () => { + beforeEach(() => unassignMock.mockClear()) + const provider = new PermitAuthorizationProvider() + + it('scopes the revoke to the resource instance when req.resource is present', async () => { + await provider.revoke({ + subject: 'usr_1', + tenant: 'org_acme', + role: 'list-owner', + resource: { type: 'SelectionList', key: 'sl_123' }, + }) + + expect(unassignMock).toHaveBeenCalledWith({ + user: 'usr_1', + role: 'list-owner', + tenant: 'org_acme', + resource_instance: 'SelectionList:sl_123', + }) + }); + + it('leaves resource_instance undefined (tenant-wide) when req.resource is absent', async () => { + await provider.revoke({ subject: 'usr_1', tenant: 'org_acme', role: 'admin' }) + + expect(unassignMock).toHaveBeenCalledWith({ + user: 'usr_1', + role: 'admin', + tenant: 'org_acme', + resource_instance: undefined, + }) + }); +}) diff --git a/deploy/contabo/sealed/selection-list-service-secrets.yaml.template b/deploy/contabo/sealed/selection-list-service-secrets.yaml.template index af8e00d0f..f64daaf4e 100644 --- a/deploy/contabo/sealed/selection-list-service-secrets.yaml.template +++ b/deploy/contabo/sealed/selection-list-service-secrets.yaml.template @@ -4,14 +4,13 @@ # THIS FILE IS A TEMPLATE. IT CONTAINS NO SECRET AND IS NOT APPLIED. # ============================================================================ # -# Seal the three service credentials with kubeseal before enabling the service. +# Seal the two service credentials with kubeseal before enabling the service. # Each key must be sealed against the cluster's sealing certificate: # # kubectl create secret generic selection-list-secrets \ # -n fuzefront \ # --from-literal=DATABASE_URL='postgresql://:@/' \ # --from-literal=JWT_SECRET='' \ -# --from-literal=PERMIT_API_KEY='' \ # --dry-run=client -o yaml \ # | kubeseal \ # --controller-name sealed-secrets \ @@ -33,14 +32,20 @@ # JWT_SECRET) so the service can verify tokens the backend mints. # Alternative: merge this key into fuzefront-secrets with # `--merge-into` and mount from that Secret instead. -# PERMIT_API_KEY — Permit.io API key scoped to the FuzeFront environment. -# Same key as in fuzefront-secrets PERMIT_API_KEY; same -# alternative applies. +# +# NOTE: selection-list-service's authorization is routed through FuzeFront's +# own Security API (SECURITY_SERVICE_URL, a plain in-cluster Service DNS +# value — see templates/selection-list-service-deployment.yaml) instead of an +# embedded Permit.io SDK (step 2 of a 3-step migration; config-service was +# step 1 — see its own sealed-secret template for that precedent). There is +# no PERMIT_API_KEY/permitPdpUrl here any more: selection-list-service +# carries no vendor SDK or vendor API key at all — the Security API is the +# only thing it talks to, and Permit (if used) is entirely behind that API. # # --------------------------------------------------------------------------- # GO-LIVE sequence (deploy window) # --------------------------------------------------------------------------- -# 1. Seal all three keys (run command above, commit the output). +# 1. Seal both keys (run command above, commit the output). # 2. Apply the SealedSecret: # kubectl apply -f deploy/contabo/sealed/selection-list-service-secrets.yaml # 3. Flip `selectionListService.enabled: true` in values-prod.yaml via GitOps. @@ -50,8 +55,8 @@ # --------------------------------------------------------------------------- # Rotation # --------------------------------------------------------------------------- -# Re-seal all three keys in one change; a partial re-seal with `--merge-into` -# can be used for individual key rotation without touching the others. +# Re-seal both keys in one change; a partial re-seal with `--merge-into` +# can be used for individual key rotation without touching the other. # --- # Replace the entire block below with the kubeseal output. @@ -65,7 +70,6 @@ spec: # Replace with kubeseal output. Do NOT commit plaintext. DATABASE_URL: JWT_SECRET: - PERMIT_API_KEY: template: metadata: name: selection-list-secrets diff --git a/deploy/helm/fuzefront/templates/selection-list-service-deployment.yaml b/deploy/helm/fuzefront/templates/selection-list-service-deployment.yaml index debe09bbe..851eff9e4 100644 --- a/deploy/helm/fuzefront/templates/selection-list-service-deployment.yaml +++ b/deploy/helm/fuzefront/templates/selection-list-service-deployment.yaml @@ -45,11 +45,17 @@ spec: secretKeyRef: name: selection-list-secrets key: JWT_SECRET - - name: PERMIT_API_KEY - valueFrom: - secretKeyRef: - name: selection-list-secrets - key: PERMIT_API_KEY + - name: SECURITY_SERVICE_URL + # fuzefront-security is the in-cluster Service name for security-service + # (port 3002). See templates/security.yaml and + # services/provisioning-service.yaml / config-service-deployment.yaml, + # which use the same convention. selection-list-service's + # authorization is now routed through this Service + # (src/middleware/authz.ts) instead of an embedded Permit.io SDK -- + # PERMIT_API_KEY/permitPdpUrl are gone; this is not a secret, it is + # just the internal DNS name of another Service in the same + # namespace. + value: "http://fuzefront-security:{{ .Values.securityService.port }}" readinessProbe: httpGet: path: /health diff --git a/deploy/helm/fuzefront/values-prod.yaml b/deploy/helm/fuzefront/values-prod.yaml index 5beed8b44..083bb9031 100644 --- a/deploy/helm/fuzefront/values-prod.yaml +++ b/deploy/helm/fuzefront/values-prod.yaml @@ -618,8 +618,14 @@ notificationService: # Selection-list-service (S14 / FFRNT-200). Ships dark (enabled: false) until # the SealedSecret `selection-list-secrets` is provisioned in a deploy window. -# Feature flag: fuzefront.selection-lists.service (S15). GO-LIVE steps: -# 1. Seal DATABASE_URL, JWT_SECRET, PERMIT_API_KEY into selection-list-secrets +# Feature flag: fuzefront.selection-lists.service (S15). Authorization is +# routed through FuzeFront's own Security API (SECURITY_SERVICE_URL — plain +# in-cluster Service DNS, set in templates/selection-list-service-deployment.yaml, +# no secret needed) rather than an embedded Permit.io SDK, so there is no +# PERMIT_API_KEY/permitPdpUrl to seal here (step 2 of the 3-step migration — +# config-service was step 1; see the configService comment below for that +# precedent). GO-LIVE steps: +# 1. Seal DATABASE_URL, JWT_SECRET into selection-list-secrets # (see deploy/contabo/sealed/selection-list-service-secrets.yaml.template). # 2. Flip `enabled` to true here via GitOps in a deploy window. # The tag: line MUST stay immediately after repository: — release.yml's GitOps diff --git a/package-lock.json b/package-lock.json index 9d07b028f..3bb1c1f58 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35224,11 +35224,11 @@ "services/selection-list-service": { "version": "1.0.0", "dependencies": { + "@fuzefront/auth": "0.2.0", "express": "4.19.2", "js-yaml": "^4.3.1", "jsonwebtoken": "^9.0.2", "knex": "^3.1.0", - "permitio": "^2.7.6", "pg": "^8.11.5", "swagger-ui-express": "^5.0.0", "zod": "3.22.4" diff --git a/services/selection-list-service/Dockerfile b/services/selection-list-service/Dockerfile index 6b48b4a9f..2a791bdb0 100644 --- a/services/selection-list-service/Dockerfile +++ b/services/selection-list-service/Dockerfile @@ -89,9 +89,18 @@ COPY services/selection-list-service/package.json ./services/selection-list-serv RUN npm ci --workspace=services/selection-list-service --ignore-scripts # Copy source +COPY packages/auth/ ./packages/auth/ COPY services/selection-list-service/ ./services/selection-list-service/ -# Build +# Build @fuzefront/auth first (src/middleware/authz.ts -- authorization +# routed through FuzeFront's Security API instead of an embedded Permit.io +# SDK -- imports it at module load; createAuthzClient() is constructed at +# authz.ts's module scope), then selection-list-service. packages/auth IS a +# root workspace member, but the --workspace=services/selection-list-service +# filter above only installs SELECTION-LIST-SERVICE's own devDependencies, +# not packages/auth's (tsup) -- mirrors services/config-service/Dockerfile's +# identical treatment of the same package. +RUN cd packages/auth && npm install --ignore-scripts && npm run build RUN cd services/selection-list-service && npm run build # ---------- production ---------- @@ -104,6 +113,15 @@ RUN apk add --no-cache dumb-init && \ COPY --from=base /app/node_modules ./node_modules COPY --from=base /app/services/selection-list-service/node_modules ./services/selection-list-service/node_modules COPY --from=build /app/services/selection-list-service/dist ./services/selection-list-service/dist +# @fuzefront/auth is a workspace dependency, so npm LINKS it -- +# node_modules/@fuzefront/auth is a symlink to /app/packages/auth. The base +# stage above ships packages/auth/package.json (workspace manifest list) so +# that link resolves instead of dangling; here we populate its target with +# the built output (mirrors services/config-service/Dockerfile's identical +# treatment of the same package -- the "Cannot find module '@fuzefront/auth'" +# crashloop it documents hitting first for @izzywdev/fuzefront-identity). +COPY --from=build /app/packages/auth/package.json ./packages/auth/package.json +COPY --from=build /app/packages/auth/dist ./packages/auth/dist # Copy openapi.yaml so the docs route can serve swagger-ui-express at runtime COPY --from=build /app/services/selection-list-service/openapi.yaml ./services/selection-list-service/openapi.yaml diff --git a/services/selection-list-service/openapi.yaml b/services/selection-list-service/openapi.yaml index ca48c7984..3f41d1f10 100644 --- a/services/selection-list-service/openapi.yaml +++ b/services/selection-list-service/openapi.yaml @@ -110,10 +110,12 @@ info: ### Authorization - Resource `SelectionList` in Permit, with actions `read`, `add_value`, - `update_value`, `remove_value`, `translate`, `update`, `delete`, - `manage_access`. Grants are ReBAC resource-instance roles on the list - instance, mirroring `Organization.roles['org-admin']` in + Resource `SelectionList` in FuzeFront's Security API (backend/security's + `/api/v1/security/authz/*`, called via `@fuzefront/auth`'s `AuthzClient` + — this service embeds no policy-vendor SDK), with actions `read`, + `add_value`, `update_value`, `remove_value`, `translate`, `update`, + `delete`, `manage_access`. Grants are ReBAC resource-instance roles on the + list instance, mirroring `Organization.roles['org-admin']` in `backend/src/permit/schema.ts`. | role | read | add_value | update_value | remove_value | translate | update | delete | manage_access | @@ -126,9 +128,9 @@ info: Each operation declares the action it requires in `x-permit-action`. **An id is never a capability** — knowing a list id grants nothing; every - route re-checks the caller against Permit, and a read the caller is not - entitled to returns `404`, not `403`, so the API is not an existence oracle - across orgs. + route re-checks the caller against the Security API, and a read the caller + is not entitled to returns `404`, not `403`, so the API is not an + existence oracle across orgs. contact: name: FuzeFront Platform Team url: https://github.com/izzywdev/FuzeFront diff --git a/services/selection-list-service/package.json b/services/selection-list-service/package.json index e89d03def..e0daf8629 100644 --- a/services/selection-list-service/package.json +++ b/services/selection-list-service/package.json @@ -16,11 +16,11 @@ "npm": ">=10.0.0" }, "dependencies": { + "@fuzefront/auth": "0.2.0", "express": "4.19.2", "js-yaml": "^4.3.1", "jsonwebtoken": "^9.0.2", "knex": "^3.1.0", - "permitio": "^2.7.6", "pg": "^8.11.5", "swagger-ui-express": "^5.0.0", "zod": "3.22.4" diff --git a/services/selection-list-service/src/middleware/permit.flags.ts b/services/selection-list-service/src/middleware/authz.flags.ts similarity index 77% rename from services/selection-list-service/src/middleware/permit.flags.ts rename to services/selection-list-service/src/middleware/authz.flags.ts index e8f0df38d..5e16b8752 100644 --- a/services/selection-list-service/src/middleware/permit.flags.ts +++ b/services/selection-list-service/src/middleware/authz.flags.ts @@ -1,4 +1,8 @@ -// permit.flags.ts — lightweight env-var-based feature flag helper for selection-list-service. +// authz.flags.ts — lightweight env-var-based feature flag helper for selection-list-service. +// (Renamed from permit.flags.ts alongside middleware/permit.ts -> middleware/authz.ts: +// the flag itself was never Permit-specific — it gates this service's authz call +// site generally, first against Permit directly and now against FuzeFront's +// Security API — but the old filename read that way and no longer should.) // // Rather than pulling a full OpenFeature SDK (which requires network I/O), this // service resolves flags from environment variables for simple on/off release gating. @@ -12,7 +16,8 @@ // Administration: feature-flags-engineer (Unleash config); this file just reads env. export const FLAGS = { - /** Release flag — gates all Permit.io authz checks on list-access endpoints. + /** Release flag — gates all authz checks (routed through the Security API) + * on list-access endpoints. * Default: false (OFF). Enable by setting env var to 'true'. * Kill-switch: set to 'false' to revert to pass-through mode with warning logs. */ AUTHZ_ENABLED: 'fuzefront.selection-list.authz-enabled', diff --git a/services/selection-list-service/src/middleware/authz.ts b/services/selection-list-service/src/middleware/authz.ts new file mode 100644 index 000000000..86277cce6 --- /dev/null +++ b/services/selection-list-service/src/middleware/authz.ts @@ -0,0 +1,291 @@ +// authz.ts — authorization for selection-list-service, routed through +// FuzeFront's Security API (backend/security's `/api/v1/security/authz/*`) +// instead of an embedded Permit.io SDK. Replaces middleware/permit.ts. +// +// Step 2 of 3 in an owner-requested migration off the embedded Permit SDK, +// onto backend/security's provider-agnostic `AuthorizationProvider` seam +// (`authzFactory.ts`) via the shared `@fuzefront/auth` client. config-service +// was step 1 (#679); billing-service follows separately (step 3). Permit is +// now purely an implementation detail of that seam — this service knows +// nothing about it. It talks to exactly one thing: FuzeFront's own Security +// API, via `createAuthzClient`. No vendor SDK, no vendor API key here. +// +// Design decisions (preserved from the Permit-backed predecessor): +// +// 1. Fail CLOSED: any error talking to the Security API -> 403. Never fail +// open. `AuthzClient.check()` never throws for a policy denial (that's +// `{ allow: false }`, a normal decision) — only for DECISION_UNAVAILABLE +// (transport error, timeout, non-200, malformed response), which this +// module treats as a deny, never an uncaught rejection that could +// somehow resolve to "allowed". +// +// 2. CI / unit-test no-op mode: when NODE_ENV=test, a recursive no-op proxy +// stands in for the real client — no real HTTP call, no live Security +// API needed for unit tests. Tests that need real behaviour (an actual +// denial, a thrown DECISION_UNAVAILABLE, asserting the exact request +// body) inject a mock via `_setAuthzClientForTesting()`. +// +// 3. Feature flag gate: requireAuthzCheck() checks the authz-enabled flag +// first. If OFF, the middleware passes through with a warning log (dark +// deploy / kill-switch mode). If ON, it does a real Security API check. +// Unchanged by this migration — same flag, same env var, same +// dark-deploy semantics; only what happens when the flag is ON changed. +// +// 4. The selection_list_access table is a READ-MODEL MIRROR — it is never +// consulted for authorization. It is updated by grantListOwner() and +// src/routes/access.ts's PUT/DELETE handlers for display purposes and +// for the last-owner guard. countActiveOwners() (routes/access.ts) reads +// ONLY this mirror and is untouched by this migration. +// +// 5. `resource` is what makes a grant/revoke/check INSTANCE-scoped (ReBAC). +// Every call site that has a listId passes +// `resource: { type: 'SelectionList', key: listId }` through to the +// `AuthzClient` — omitting it silently widens a list-scoped grant/check +// to tenant-wide, a real privilege-escalation surface. See +// `grantListOwner()` and `src/routes/access.ts`'s PUT/DELETE handlers. +// +// 6. grant()/revoke() are WRITES. A transport failure/timeout THROWS +// (`AuthzError`) rather than resolving — it is never swallowed into a +// false "succeeded". Call sites in `routes/access.ts` let that throw +// propagate to their route-level try/catch (-> 500), and — critically — +// always perform the Security API write BEFORE touching the +// `selection_list_access` mirror row, so a thrown grant/revoke never +// leaves the mirror claiming a role change that did not actually happen +// in the authorization backend. + +import { Request, Response, NextFunction } from 'express'; +import { AuthzClient, createAuthzClient } from '@fuzefront/auth'; +import { db } from '../db'; +import { getBooleanFlag, FLAGS, FlagContext } from './authz.flags'; + +// --------------------------------------------------------------------------- +// Configuration +// --------------------------------------------------------------------------- + +/** + * In-cluster Service DNS for backend/security + * (`deploy/helm/fuzefront/templates/security.yaml`), matching the + * `SECURITY_SERVICE_URL` convention config-service and provisioning-service + * already use for the same Service. `createAuthzClient` appends + * `/api/v1/security/authz/{check,bulk-check,grants}` itself. + */ +const SECURITY_SERVICE_URL = process.env.SECURITY_SERVICE_URL ?? 'http://fuzefront-security:3002'; + +/** `NODE_ENV=test` with no explicit client wired -> allow-all, no network. */ +export const isNoOpMode: boolean = process.env.NODE_ENV === 'test'; + +// --------------------------------------------------------------------------- +// No-op proxy (CI / test safety net) +// --------------------------------------------------------------------------- + +/** + * A fully-typed allow-all `AuthzClient` used only when `NODE_ENV=test` and no + * explicit mock has been wired via `_setAuthzClientForTesting()`. Every + * method resolves successfully — no real HTTP call, ever, from a unit test. + */ +function makeNoOpProxy(): AuthzClient { + return { + check: async () => ({ allow: true }), + bulkCheck: async (checks) => checks.map(() => ({ allow: true })), + grant: async (req) => ({ + id: `${req.tenant}:${req.subject}:${req.role}`, + subject: req.subject, + tenant: req.tenant, + role: req.role, + permission: req.permission, + resource: req.resource, + }), + revoke: async () => undefined, + listGrants: async () => ({ items: [], page: { nextCursor: null, hasMore: false } }), + }; +} + +// --------------------------------------------------------------------------- +// AuthzClient singleton with test seam +// --------------------------------------------------------------------------- + +let _authzClient: AuthzClient = isNoOpMode + ? makeNoOpProxy() + : createAuthzClient({ baseUrl: SECURITY_SERVICE_URL }); + +/** Returns the active authz client (real or no-op). */ +export function getAuthzClient(): AuthzClient { + return _authzClient; +} + +/** + * Test seam: swap the authz client for a mock. + * Call with `makeNoOpProxy()` or a jest mock object. + * Tests should restore the original client in afterEach. + */ +export function _setAuthzClientForTesting(client: AuthzClient): void { + _authzClient = client; +} + +/** Re-export so tests can create a fresh no-op without importing the factory. */ +export { makeNoOpProxy }; + +// --------------------------------------------------------------------------- +// bearer — re-read the raw token so a decision is always asked for the +// CALLER's real credential, never a service-wide one. +// --------------------------------------------------------------------------- + +export function bearer(req: Request): string | null { + const header = req.headers['authorization']; + if (!header || Array.isArray(header)) return null; + const [scheme, token] = header.split(' '); + return scheme?.toLowerCase() === 'bearer' && token ? token : null; +} + +// --------------------------------------------------------------------------- +// requireAuthzCheck — Express middleware factory (replaces requirePermit) +// --------------------------------------------------------------------------- + +/** + * Returns an Express middleware that enforces an authorization decision via + * FuzeFront's Security API. + * + * @param resource Security API resource type (e.g. 'SelectionList'). + * @param action Security API action (e.g. 'read', 'admin'). + * + * The listId is extracted from req.params.listId. If absent, the check is + * performed without a resource-instance key (tenant-level check only). + * + * Flag OFF → pass-through with a warning log (dark deploy / kill-switch). + * Flag ON → perform a real check, fail closed on any error. + */ +export function requireAuthzCheck(resource: string, action: string) { + return async function authzMiddleware( + req: Request, + res: Response, + next: NextFunction, + ): Promise { + const userId = req.userId; + const orgId = req.orgId; + + if (!userId || !orgId) { + res.status(401).json({ code: 'UNAUTHENTICATED', message: 'Missing identity claims.' }); + return; + } + + const flagCtx: FlagContext = { userId, orgId, appId: req.appId }; + const authzEnabled = await getBooleanFlag(FLAGS.AUTHZ_ENABLED, false, flagCtx); + + if (!authzEnabled) { + console.warn( + '[authz] authz-enabled flag is OFF — passing through without a Security API check.', + { userId, orgId, resource, action }, + ); + next(); + return; + } + + const token = bearer(req); + if (!token) { + res.status(401).json({ code: 'UNAUTHENTICATED', message: 'Missing bearer token.' }); + return; + } + + const listId = req.params['listId']; + try { + const decision = await getAuthzClient().check( + { + subject: userId, + tenant: orgId, + resource: listId ? { type: resource, key: listId } : { type: resource }, + action, + }, + token, + ); + if (!decision.allow) { + res.status(403).json({ code: 'FORBIDDEN', message: 'Permission denied.' }); + return; + } + next(); + } catch (err) { + // Fail CLOSED: any Security API error (including a thrown + // AuthzError('DECISION_UNAVAILABLE') for a timeout/non-200) must never + // grant access. + console.error('[authz] Security API check threw — failing closed.', { err, userId, orgId, resource, action }); + res.status(403).json({ code: 'FORBIDDEN', message: 'Authorization service unavailable.' }); + } + }; +} + +// --------------------------------------------------------------------------- +// grantListOwner — grant list-owner via the Security API + upsert mirror row +// --------------------------------------------------------------------------- + +/** + * Grants the list-owner role to userId on listId within orgId. + * Performs two writes in order: + * 1. Security API grant (source of truth for authz) — instance-scoped via + * `resource: { type: 'SelectionList', key: listId }`, so this reaches + * Permit as `resource_instance: 'SelectionList:${listId}'`, exactly the + * scope the embedded-SDK predecessor used. + * 2. Upsert into selection_list_access (read-model mirror for display / the + * last-owner guard). + * + * Throws on a Security API error (grant() never swallows a write failure); + * the mirror row is only reached — and only written — once the grant + * succeeded, so a thrown grant can never leave a mirror row claiming success. + * + * @param token The ACTING caller's bearer token — the Security API decides + * (and records) the grant for the real principal, never a + * service-wide credential. + */ +export async function grantListOwner( + userId: string, + orgId: string, + listId: string, + grantedBy: string, + token: string, +): Promise { + await getAuthzClient().grant( + { + subject: userId, + tenant: orgId, + role: 'list-owner', + resource: { type: 'SelectionList', key: listId }, + }, + token, + ); + + // Upsert the mirror row. Only reached if the grant above succeeded. + await db('selection_list_access') + .insert({ + list_id: listId, + user_id: userId, + role: 'list-owner', + granted_by: grantedBy, + org_id: orgId, + granted_at: db.fn.now(), + updated_at: db.fn.now(), + revoked_at: null, + }) + .onConflict(['list_id', 'user_id']) + .merge(['role', 'granted_by', 'org_id', 'updated_at', 'revoked_at']); +} + +// --------------------------------------------------------------------------- +// countActiveOwners — last-owner guard helper (UNCHANGED by this migration) +// --------------------------------------------------------------------------- + +/** + * Returns the number of active (non-revoked) list-owner assignments for listId + * in the read-model mirror. + * + * Used ONLY for the last-owner guard (409 check before demotion / revocation). + * NOT used for authorization. Reads the local mirror table exclusively — it + * has nothing to do with Permit or the Security API, and this migration does + * not touch it. + */ +export async function countActiveOwners(listId: string): Promise { + const result = await db('selection_list_access') + .where({ list_id: listId, role: 'list-owner' }) + .whereNull('revoked_at') + .count<{ count: string }>('user_id as count') + .first(); + + return result ? parseInt(result.count, 10) : 0; +} diff --git a/services/selection-list-service/src/middleware/permit.ts b/services/selection-list-service/src/middleware/permit.ts deleted file mode 100644 index afb73dc54..000000000 --- a/services/selection-list-service/src/middleware/permit.ts +++ /dev/null @@ -1,229 +0,0 @@ -// permit.ts — Permit.io client and middleware for selection-list-service. -// -// Design decisions: -// -// 1. Fail CLOSED: any error from Permit.io → 403. Never fail open. -// -// 2. CI / unit-test no-op mode: when PERMIT_API_KEY is a known CI dummy key -// OR (NODE_ENV=test AND the key does not start with 'permit_key_'), we -// substitute a Recursive Proxy that returns a truthy value for every -// .check() call and a no-op for every .api.*() call. Tests that need real -// Permit behaviour inject a mock via _setPermitClientForTesting(). -// -// 3. Feature flag gate: requirePermit() checks the authz-enabled flag first. -// If the flag is OFF, the middleware passes through with a warning log (dark -// deploy / kill-switch mode). If ON, it does a real Permit check. -// -// 4. The selection_list_access table is a READ-MODEL MIRROR — it is never -// consulted for authorization. It is updated by grantListOwner() and -// revokeListAccess() for display purposes and for the last-owner guard. - -import { Request, Response, NextFunction } from 'express'; -import { Permit } from 'permitio'; -import { db } from '../db'; -import { getBooleanFlag, FLAGS, FlagContext } from './permit.flags'; - -// --------------------------------------------------------------------------- -// Configuration -// --------------------------------------------------------------------------- - -const PERMIT_API_KEY = process.env.PERMIT_API_KEY ?? ''; -const PERMIT_PDP_URL = process.env.PERMIT_PDP_URL ?? 'https://cloudpdp.api.permit.io'; - -/** Keys that signal "no real Permit account available" — CI / offline mode. */ -const CI_DUMMY_KEYS = new Set([ - 'ci-no-real-permit-calls', - 'ci-noop', - 'ci-offline-pdp-key', -]); - -/** - * True when we should substitute a no-op proxy for the real Permit client. - * Conditions: - * - The API key is a known CI dummy, OR - * - We're in test mode and the key does not look like a real permit key. - */ -export const isNoOpMode: boolean = - CI_DUMMY_KEYS.has(PERMIT_API_KEY) || - (process.env.NODE_ENV === 'test' && !PERMIT_API_KEY.startsWith('permit_key_')); - -// --------------------------------------------------------------------------- -// No-op proxy (CI / test safety net) -// --------------------------------------------------------------------------- - -/** - * Returns a recursive Proxy that: - * - For .check() → returns true (allow). - * - For any other method call → returns a Promise that resolves to undefined. - * - For any property access → returns another Proxy (so chaining works). - */ -function makeNoOpProxy(): any { - const handler: ProxyHandler = { - get(_target, prop: string) { - if (prop === 'check') { - return () => Promise.resolve(true); - } - // Any other property access (e.g. .api.roleAssignments.assign()) returns - // a new Proxy so chaining does not throw. - return new Proxy( - function noOp() { - return Promise.resolve(undefined); - }, - handler, - ); - }, - apply(_target, _thisArg, _args) { - return Promise.resolve(undefined); - }, - }; - return new Proxy({}, handler); -} - -// --------------------------------------------------------------------------- -// Permit client singleton with test seam -// --------------------------------------------------------------------------- - -let _permitClient: any = isNoOpMode - ? makeNoOpProxy() - : new Permit({ token: PERMIT_API_KEY, pdp: PERMIT_PDP_URL }); - -/** Returns the active Permit client (real or no-op). */ -export function getPermitClient(): any { - return _permitClient; -} - -/** - * Test seam: swap the Permit client for a mock. - * Call with `makeNoOpProxy()` or a jest mock object. - * Tests should restore the original client in afterEach. - */ -export function _setPermitClientForTesting(client: any): void { - _permitClient = client; -} - -/** Re-export so tests can create a fresh no-op without importing the factory. */ -export { makeNoOpProxy }; - -// --------------------------------------------------------------------------- -// requirePermit — Express middleware factory -// --------------------------------------------------------------------------- - -/** - * Returns an Express middleware that enforces a Permit.io check. - * - * @param resource Permit resource type (e.g. 'SelectionList'). - * @param action Permit action (e.g. 'read', 'write', 'admin'). - * - * The listId is extracted from req.params.listId. If absent, the check is - * performed without a resource-instance key (tenant-level check only). - * - * Flag OFF → pass-through with a warning log (dark deploy / kill-switch). - * Flag ON → perform real check, fail closed on any error. - */ -export function requirePermit(resource: string, action: string) { - return async function permitMiddleware( - req: Request, - res: Response, - next: NextFunction, - ): Promise { - const userId = req.userId; - const orgId = req.orgId; - - if (!userId || !orgId) { - res.status(401).json({ code: 'UNAUTHENTICATED', message: 'Missing identity claims.' }); - return; - } - - const flagCtx: FlagContext = { userId, orgId, appId: req.appId }; - const authzEnabled = await getBooleanFlag(FLAGS.AUTHZ_ENABLED, false, flagCtx); - - if (!authzEnabled) { - console.warn( - '[permit] authz-enabled flag is OFF — passing through without Permit check.', - { userId, orgId, resource, action }, - ); - next(); - return; - } - - const listId = req.params['listId']; - try { - const resourceInstance = listId - ? { type: resource, tenant: orgId, key: listId } - : { type: resource, tenant: orgId }; - - const allowed: boolean = await _permitClient.check(userId, action, resourceInstance); - if (!allowed) { - res.status(403).json({ code: 'FORBIDDEN', message: 'Permission denied.' }); - return; - } - next(); - } catch (err) { - // Fail CLOSED: Permit.io errors must never grant access. - console.error('[permit] Permit.io check threw — failing closed.', { err, userId, orgId, resource, action }); - res.status(403).json({ code: 'FORBIDDEN', message: 'Authorization service unavailable.' }); - } - }; -} - -// --------------------------------------------------------------------------- -// grantListOwner — assign list-owner role in Permit + upsert mirror row -// --------------------------------------------------------------------------- - -/** - * Grants the list-owner role to userId on listId within orgId. - * Performs two writes atomically from the caller's perspective: - * 1. Permit.io role assignment (source of truth for authz). - * 2. Upsert into selection_list_access (read-model mirror for display / guard). - * - * Throws on Permit.io error; the caller is responsible for propagating. - */ -export async function grantListOwner( - userId: string, - orgId: string, - listId: string, - grantedBy: string, -): Promise { - await _permitClient.api.roleAssignments.assign({ - user: userId, - role: 'list-owner', - tenant: orgId, - resource_instance: `SelectionList:${listId}`, - }); - - // Upsert the mirror row. - await db('selection_list_access') - .insert({ - list_id: listId, - user_id: userId, - role: 'list-owner', - granted_by: grantedBy, - org_id: orgId, - granted_at: db.fn.now(), - updated_at: db.fn.now(), - revoked_at: null, - }) - .onConflict(['list_id', 'user_id']) - .merge(['role', 'granted_by', 'org_id', 'updated_at', 'revoked_at']); -} - -// --------------------------------------------------------------------------- -// countActiveOwners — last-owner guard helper -// --------------------------------------------------------------------------- - -/** - * Returns the number of active (non-revoked) list-owner assignments for listId - * in the read-model mirror. - * - * Used ONLY for the last-owner guard (409 check before demotion / revocation). - * NOT used for authorization. - */ -export async function countActiveOwners(listId: string): Promise { - const result = await db('selection_list_access') - .where({ list_id: listId, role: 'list-owner' }) - .whereNull('revoked_at') - .count<{ count: string }>('user_id as count') - .first(); - - return result ? parseInt(result.count, 10) : 0; -} diff --git a/services/selection-list-service/src/routes/access.ts b/services/selection-list-service/src/routes/access.ts index 706c8833b..072079a5f 100644 --- a/services/selection-list-service/src/routes/access.ts +++ b/services/selection-list-service/src/routes/access.ts @@ -11,8 +11,10 @@ // - PUT requires 'admin' on SelectionList (list-owner only) // - DELETE requires 'admin' on SelectionList (list-owner only) // -// The selection_list_access table is a READ-MODEL MIRROR of Permit.io state. -// It is NEVER consulted for authorization decisions — only for: +// The selection_list_access table is a READ-MODEL MIRROR of the authorization +// backend's state (FuzeFront's Security API, via @fuzefront/auth's +// AuthzClient — see middleware/authz.ts). It is NEVER consulted for +// authorization decisions — only for: // a) returning the grant roster on GET // b) the last-owner guard (count of non-revoked owners before demotion/revoke) // @@ -23,7 +25,7 @@ import { Router, Request, Response } from 'express'; import { db } from '../db'; -import { requirePermit, countActiveOwners, getPermitClient } from '../middleware/permit'; +import { requireAuthzCheck, countActiveOwners, getAuthzClient, bearer } from '../middleware/authz'; import { authMiddleware } from '../middleware/auth'; const router = Router(); @@ -62,7 +64,7 @@ function decodeCursor(cursor: string): string { router.get( '/:listId/access', authMiddleware, - requirePermit('SelectionList', 'read'), + requireAuthzCheck('SelectionList', 'read'), async (req: Request, res: Response): Promise => { const { listId } = req.params; @@ -123,7 +125,7 @@ router.get( router.put( '/:listId/access/:userId', authMiddleware, - requirePermit('SelectionList', 'admin'), + requireAuthzCheck('SelectionList', 'admin'), async (req: Request, res: Response): Promise => { const { listId, userId } = req.params; const orgId = req.orgId!; @@ -139,6 +141,12 @@ router.put( return; } + const token = bearer(req); + if (!token) { + res.status(401).json({ code: 'UNAUTHENTICATED', message: 'Missing bearer token.' }); + return; + } + try { // Last-owner guard: if target currently has list-owner and we're changing // them to a non-owner role, ensure there is at least one other owner. @@ -161,16 +169,23 @@ router.put( } } - // Assign role in Permit.io (source of truth for authz). - const permitClient = getPermitClient(); - await permitClient.api.roleAssignments.assign({ - user: userId, - role, - tenant: orgId, - resource_instance: `SelectionList:${listId}`, - }); + // Assign role via the Security API (source of truth for authz). + // resource is REQUIRED here: it is what scopes this grant to this one + // list (resource_instance 'SelectionList:${listId}' on the wire) rather + // than tenant-wide. This is a WRITE — grant() throws (never resolves) + // on a Security API failure, so a 502/timeout is caught below and + // surfaced as 500 WITHOUT ever reaching the mirror upsert. + await getAuthzClient().grant( + { + subject: userId, + tenant: orgId, + role, + resource: { type: 'SelectionList', key: listId }, + }, + token, + ); - // Upsert the mirror row. + // Upsert the mirror row. Only reached if the grant above succeeded. await db('selection_list_access') .insert({ list_id: listId, @@ -206,11 +221,17 @@ router.put( router.delete( '/:listId/access/:userId', authMiddleware, - requirePermit('SelectionList', 'admin'), + requireAuthzCheck('SelectionList', 'admin'), async (req: Request, res: Response): Promise => { const { listId, userId } = req.params; const orgId = req.orgId!; + const token = bearer(req); + if (!token) { + res.status(401).json({ code: 'UNAUTHENTICATED', message: 'Missing bearer token.' }); + return; + } + try { // Fetch current grant for last-owner guard and idempotency. const existing = await db('selection_list_access') @@ -237,16 +258,24 @@ router.delete( } } - // Unassign in Permit.io. - const permitClient = getPermitClient(); - await permitClient.api.roleAssignments.unassign({ - user: userId, - role: existing['role'], - tenant: orgId, - resource_instance: `SelectionList:${listId}`, - }); + // Revoke via the Security API. resource is REQUIRED here for the same + // reason as the PUT handler's grant() call: it scopes the revocation + // to this list's instance rather than the tenant-wide role. This is a + // WRITE — revoke() throws (never resolves) on a Security API failure, + // caught below and surfaced as 500 WITHOUT ever reaching the mirror's + // soft-delete, so a failed revoke never leaves the mirror claiming + // access was removed when it was not. + await getAuthzClient().revoke( + { + subject: userId, + tenant: orgId, + role: existing['role'], + resource: { type: 'SelectionList', key: listId }, + }, + token, + ); - // Soft-delete the mirror row. + // Soft-delete the mirror row. Only reached if the revoke above succeeded. await db('selection_list_access') .where({ list_id: listId, user_id: userId }) .update({ revoked_at: db.fn.now(), updated_at: db.fn.now() }); diff --git a/services/selection-list-service/tests/access.routes.test.ts b/services/selection-list-service/tests/access.routes.test.ts index cca0f3323..eb544d2c1 100644 --- a/services/selection-list-service/tests/access.routes.test.ts +++ b/services/selection-list-service/tests/access.routes.test.ts @@ -5,8 +5,13 @@ // B) PUT /:listId/access/:userId — grant/update role // C) DELETE /:listId/access/:userId — revoke access // -// DB is mocked via jest.mock('../src/db'); Permit client is injected via -// _setPermitClientForTesting(); feature flag is controlled via env var. +// DB is mocked via jest.mock('../src/db'); the Security API's AuthzClient +// (@fuzefront/auth, via middleware/authz.ts) is injected via +// _setAuthzClientForTesting(); feature flag is controlled via env var. +// +// Routed through FuzeFront's Security API instead of an embedded Permit.io +// SDK — grant()/revoke() replace the old direct +// permitClient.api.roleAssignments.{assign,unassign}() calls. // ─── Mock DB before any imports ─────────────────────────────────────────────── jest.mock('../src/db', () => { @@ -33,8 +38,9 @@ jest.mock('../src/db', () => { import express from 'express'; import request from 'supertest'; import jwt from 'jsonwebtoken'; +import { AuthzClient, AuthzError } from '@fuzefront/auth'; import accessRouter from '../src/routes/access'; -import { _setPermitClientForTesting, makeNoOpProxy } from '../src/middleware/permit'; +import { _setAuthzClientForTesting, makeNoOpProxy } from '../src/middleware/authz'; import { db } from '../src/db'; // ─── Constants ──────────────────────────────────────────────────────────────── @@ -82,14 +88,22 @@ function restoreDbMock(): void { mockDb.update = jest.fn(() => Promise.resolve(1)); } -// Default Permit mock: allow everything. -function makeAllowPermit(): any { - return { check: jest.fn().mockResolvedValue(true) }; +/** A fully-typed allow-all AuthzClient, mirroring makeNoOpProxy but with + * jest.fn() spies so individual tests can assert on the grant/revoke calls. */ +function makeAllowAuthzClient(overrides: Partial = {}): AuthzClient { + return { + check: jest.fn().mockResolvedValue({ allow: true }), + bulkCheck: jest.fn().mockResolvedValue([]), + grant: jest.fn().mockResolvedValue({ id: 'g1', subject: USER_ID, tenant: ORG_ID, role: 'list-viewer' }), + revoke: jest.fn().mockResolvedValue(undefined), + listGrants: jest.fn().mockResolvedValue({ items: [], page: { nextCursor: null, hasMore: false } }), + ...overrides, + }; } afterEach(() => { delete process.env['FUZEFRONT_SELECTION_LIST_AUTHZ_ENABLED']; - _setPermitClientForTesting(makeNoOpProxy()); + _setAuthzClientForTesting(makeNoOpProxy()); jest.clearAllMocks(); restoreDbMock(); }); @@ -236,13 +250,10 @@ describe('PUT /:listId/access/:userId', () => { expect(res.status).toBe(400); }); - it('grants list-viewer role successfully (flag OFF)', async () => { + it('grants list-viewer role successfully (flag OFF) and scopes the grant to this list instance', async () => { restoreDbMock(); - const mockPermit = { - check: jest.fn().mockResolvedValue(true), - api: { roleAssignments: { assign: jest.fn().mockResolvedValue(undefined) } }, - }; - _setPermitClientForTesting(mockPermit); + const grantMock = jest.fn().mockResolvedValue({ id: 'g1', subject: USER_ID, tenant: ORG_ID, role: 'list-viewer' }); + _setAuthzClientForTesting(makeAllowAuthzClient({ grant: grantMock })); const app = makeApp(); const res = await request(app) @@ -256,6 +267,17 @@ describe('PUT /:listId/access/:userId', () => { listId: LIST_ID, role: 'list-viewer', }); + // resource MUST reach the wire — its absence would silently turn this + // list-scoped grant into a tenant-wide one (real privilege escalation). + expect(grantMock).toHaveBeenCalledWith( + { + subject: USER_ID, + tenant: ORG_ID, + role: 'list-viewer', + resource: { type: 'SelectionList', key: LIST_ID }, + }, + expect.any(String), + ); }); it('returns 409 when demoting last owner (flag OFF)', async () => { @@ -280,11 +302,8 @@ describe('PUT /:listId/access/:userId', () => { it('allows demoting an owner when another owner exists (flag OFF)', async () => { restoreDbMock(); - const assignMock = jest.fn().mockResolvedValue(undefined); - _setPermitClientForTesting({ - check: jest.fn().mockResolvedValue(true), - api: { roleAssignments: { assign: assignMock } }, - }); + const grantMock = jest.fn().mockResolvedValue({ id: 'g1', subject: USER_ID, tenant: ORG_ID, role: 'list-editor' }); + _setAuthzClientForTesting(makeAllowAuthzClient({ grant: grantMock })); // existing row is list-owner, but 2 owners exist. mockDb.first = jest.fn() @@ -300,17 +319,27 @@ describe('PUT /:listId/access/:userId', () => { expect(res.status).toBe(200); }); + it('returns 500 and does NOT upsert the mirror row when AuthzClient.grant() throws (write-ordering fail-closed guarantee)', async () => { + restoreDbMock(); + const grantMock = jest.fn().mockRejectedValue(new AuthzError('PROVIDER_ERROR', 'Security API returned 502')); + _setAuthzClientForTesting(makeAllowAuthzClient({ grant: grantMock })); + + const app = makeApp(); + const res = await request(app) + .put(`/lists/${LIST_ID}/access/${USER_ID}`) + .set('Authorization', `Bearer ${makeToken()}`) + .send({ role: 'list-viewer' }); + + expect(res.status).toBe(500); + expect(mockDb.insert).not.toHaveBeenCalled(); + }); + const VALID_ROLES = ['list-owner', 'list-editor', 'list-contributor', 'list-translator', 'list-viewer']; VALID_ROLES.forEach((role) => { it(`accepts valid role: ${role}`, async () => { jest.clearAllMocks(); restoreDbMock(); - _setPermitClientForTesting(makeNoOpProxy()); - const mockPermit = { - check: jest.fn().mockResolvedValue(true), - api: { roleAssignments: { assign: jest.fn().mockResolvedValue(undefined) } }, - }; - _setPermitClientForTesting(mockPermit); + _setAuthzClientForTesting(makeAllowAuthzClient()); const app = makeApp(); const res = await request(app) @@ -359,13 +388,10 @@ describe('DELETE /:listId/access/:userId', () => { expect(res.body.code).toBe('LAST_OWNER'); }); - it('revokes access and returns 204 when not last owner', async () => { + it('revokes access and returns 204 when not last owner, scoping the revoke to this list instance', async () => { restoreDbMock(); - const unassignMock = jest.fn().mockResolvedValue(undefined); - _setPermitClientForTesting({ - check: jest.fn().mockResolvedValue(true), - api: { roleAssignments: { unassign: unassignMock } }, - }); + const revokeMock = jest.fn().mockResolvedValue(undefined); + _setAuthzClientForTesting(makeAllowAuthzClient({ revoke: revokeMock })); mockDb.first = jest.fn() .mockResolvedValueOnce({ role: 'list-owner' }) // existing grant @@ -377,16 +403,23 @@ describe('DELETE /:listId/access/:userId', () => { .set('Authorization', `Bearer ${makeToken()}`); expect(res.status).toBe(204); - expect(unassignMock).toHaveBeenCalled(); + // resource MUST reach the wire on revoke too — see the same rationale as + // the PUT/grant test above. + expect(revokeMock).toHaveBeenCalledWith( + { + subject: USER_ID, + tenant: ORG_ID, + role: 'list-owner', + resource: { type: 'SelectionList', key: LIST_ID }, + }, + expect.any(String), + ); }); it('revokes a non-owner role successfully', async () => { restoreDbMock(); - const unassignMock = jest.fn().mockResolvedValue(undefined); - _setPermitClientForTesting({ - check: jest.fn().mockResolvedValue(true), - api: { roleAssignments: { unassign: unassignMock } }, - }); + const revokeMock = jest.fn().mockResolvedValue(undefined); + _setAuthzClientForTesting(makeAllowAuthzClient({ revoke: revokeMock })); mockDb.first = jest.fn().mockResolvedValueOnce({ role: 'list-viewer' }); // non-owner @@ -396,14 +429,34 @@ describe('DELETE /:listId/access/:userId', () => { .set('Authorization', `Bearer ${makeToken()}`); expect(res.status).toBe(204); - expect(unassignMock).toHaveBeenCalled(); + expect(revokeMock).toHaveBeenCalled(); }); - it('returns 403 (fail closed) when Permit.io check throws during authz ON', async () => { + it('returns 500 and does NOT soft-delete the mirror row when AuthzClient.revoke() throws (write-ordering fail-closed guarantee)', async () => { + restoreDbMock(); + const revokeMock = jest.fn().mockRejectedValue(new AuthzError('PROVIDER_ERROR', 'Security API returned 502')); + _setAuthzClientForTesting(makeAllowAuthzClient({ revoke: revokeMock })); + + mockDb.first = jest.fn().mockResolvedValueOnce({ role: 'list-viewer' }); // non-owner, no last-owner guard involved + + const app = makeApp(); + const res = await request(app) + .delete(`/lists/${LIST_ID}/access/${USER_ID}`) + .set('Authorization', `Bearer ${makeToken()}`); + + expect(res.status).toBe(500); + expect(mockDb.update).not.toHaveBeenCalled(); + }); + + it('returns 403 (fail closed) when the Security API check throws AuthzError(DECISION_UNAVAILABLE) during authz ON', async () => { process.env['FUZEFRONT_SELECTION_LIST_AUTHZ_ENABLED'] = 'true'; - _setPermitClientForTesting({ - check: jest.fn().mockRejectedValue(new Error('Permit.io timeout')), - }); + _setAuthzClientForTesting({ + check: jest.fn().mockRejectedValue(new AuthzError('DECISION_UNAVAILABLE', 'Security API request failed: timeout; denying.')), + bulkCheck: jest.fn(), + grant: jest.fn(), + revoke: jest.fn(), + listGrants: jest.fn(), + } as unknown as AuthzClient); const app = makeApp(); const res = await request(app) @@ -414,11 +467,15 @@ describe('DELETE /:listId/access/:userId', () => { expect(res.body.code).toBe('FORBIDDEN'); }); - it('returns 403 when Permit.check denies delete and flag is ON', async () => { + it('returns 403 when the Security API denies delete and flag is ON', async () => { process.env['FUZEFRONT_SELECTION_LIST_AUTHZ_ENABLED'] = 'true'; - _setPermitClientForTesting({ - check: jest.fn().mockResolvedValue(false), - }); + _setAuthzClientForTesting({ + check: jest.fn().mockResolvedValue({ allow: false }), + bulkCheck: jest.fn(), + grant: jest.fn(), + revoke: jest.fn(), + listGrants: jest.fn(), + } as unknown as AuthzClient); const app = makeApp(); const res = await request(app) diff --git a/services/selection-list-service/tests/permit.middleware.test.ts b/services/selection-list-service/tests/authz.middleware.test.ts similarity index 54% rename from services/selection-list-service/tests/permit.middleware.test.ts rename to services/selection-list-service/tests/authz.middleware.test.ts index fecbb48f9..12072e6d0 100644 --- a/services/selection-list-service/tests/permit.middleware.test.ts +++ b/services/selection-list-service/tests/authz.middleware.test.ts @@ -1,10 +1,16 @@ -// permit.middleware.test.ts — unit tests for S7 Permit.io middleware. +// authz.middleware.test.ts — unit tests for S7 authz middleware, now routed +// through FuzeFront's Security API (@fuzefront/auth's AuthzClient) instead of +// an embedded Permit.io SDK. // // Tests both flag OFF and flag ON paths. -// Uses _setPermitClientForTesting() to inject mocks — never hits a real Permit API. +// Uses _setAuthzClientForTesting() to inject mocks — never hits a real +// Security API / network. // -// Flag OFF path (4 tests): pass-through with no Permit call. -// Flag ON path (6 tests): real check, fail-closed on error, grantListOwner, countActiveOwners. +// Flag OFF path (4 tests): pass-through with no Security API call. +// Flag ON path (7 tests): real check, fail-closed on error (including the +// explicit DECISION_UNAVAILABLE case), grantListOwner +// (including its own throw-must-not-write-mirror path), +// countActiveOwners. // ─── Mock DB before any imports ─────────────────────────────────────────────── jest.mock('../src/db', () => { @@ -31,18 +37,19 @@ jest.mock('../src/db', () => { import express, { Request, Response } from 'express'; import request from 'supertest'; import jwt from 'jsonwebtoken'; +import { AuthzClient, AuthzError } from '@fuzefront/auth'; import { - requirePermit, - _setPermitClientForTesting, + requireAuthzCheck, + _setAuthzClientForTesting, makeNoOpProxy, grantListOwner, countActiveOwners, -} from '../src/middleware/permit'; +} from '../src/middleware/authz'; import { db } from '../src/db'; // ─── Helpers ────────────────────────────────────────────────────────────────── -const JWT_SECRET = 'test-secret-s7-permit'; +const JWT_SECRET = 'test-secret-s7-authz'; process.env.JWT_SECRET = JWT_SECRET; function makeToken(overrides: Record = {}): string { @@ -73,7 +80,7 @@ function buildApp(resource: string, action: string): express.Application { app.get( '/lists/:listId', - requirePermit(resource, action), + requireAuthzCheck(resource, action), (_req: Request, res: Response) => res.status(200).json({ ok: true }), ); return app; @@ -82,18 +89,18 @@ function buildApp(resource: string, action: string): express.Application { // ─── Restore no-op between tests ────────────────────────────────────────────── afterEach(() => { delete process.env['FUZEFRONT_SELECTION_LIST_AUTHZ_ENABLED']; - _setPermitClientForTesting(makeNoOpProxy()); + _setAuthzClientForTesting(makeNoOpProxy()); }); // ─── Flag OFF tests ─────────────────────────────────────────────────────────── -describe('requirePermit — flag OFF (default)', () => { +describe('requireAuthzCheck — flag OFF (default)', () => { beforeEach(() => { delete process.env['FUZEFRONT_SELECTION_LIST_AUTHZ_ENABLED']; }); - it('passes through without calling Permit when flag is OFF', async () => { - const mockPermit = { check: jest.fn() }; - _setPermitClientForTesting(mockPermit); + it('passes through without calling the Security API when flag is OFF', async () => { + const check = jest.fn(); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); const app = buildApp('SelectionList', 'read'); const res = await request(app) @@ -101,7 +108,7 @@ describe('requirePermit — flag OFF (default)', () => { .set('Authorization', `Bearer ${makeToken()}`); expect(res.status).toBe(200); - expect(mockPermit.check).not.toHaveBeenCalled(); + expect(check).not.toHaveBeenCalled(); }); it('returns 401 when no token present (flag OFF)', async () => { @@ -110,9 +117,9 @@ describe('requirePermit — flag OFF (default)', () => { expect(res.status).toBe(401); }); - it('passes through regardless of what Permit would return when flag is OFF', async () => { - const mockPermit = { check: jest.fn().mockResolvedValue(false) }; // would deny - _setPermitClientForTesting(mockPermit); + it('passes through regardless of what the Security API would return when flag is OFF', async () => { + const check = jest.fn().mockResolvedValue({ allow: false }); // would deny + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); const app = buildApp('SelectionList', 'admin'); const res = await request(app) @@ -120,7 +127,7 @@ describe('requirePermit — flag OFF (default)', () => { .set('Authorization', `Bearer ${makeToken()}`); expect(res.status).toBe(200); - expect(mockPermit.check).not.toHaveBeenCalled(); + expect(check).not.toHaveBeenCalled(); }); it('returns 401 when userId is missing from token (flag OFF)', async () => { @@ -135,14 +142,14 @@ describe('requirePermit — flag OFF (default)', () => { }); // ─── Flag ON tests ──────────────────────────────────────────────────────────── -describe('requirePermit — flag ON', () => { +describe('requireAuthzCheck — flag ON', () => { beforeEach(() => { process.env['FUZEFRONT_SELECTION_LIST_AUTHZ_ENABLED'] = 'true'; }); - it('allows access when Permit.check returns true', async () => { - const mockPermit = { check: jest.fn().mockResolvedValue(true) }; - _setPermitClientForTesting(mockPermit); + it('allows access when the Security API returns { allow: true }', async () => { + const check = jest.fn().mockResolvedValue({ allow: true }); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); const app = buildApp('SelectionList', 'read'); const res = await request(app) @@ -150,16 +157,20 @@ describe('requirePermit — flag ON', () => { .set('Authorization', `Bearer ${makeToken()}`); expect(res.status).toBe(200); - expect(mockPermit.check).toHaveBeenCalledWith( - 'usr_tester01', - 'read', - { type: 'SelectionList', tenant: 'org_acme', key: 'sl_abc123' }, + expect(check).toHaveBeenCalledWith( + { + subject: 'usr_tester01', + tenant: 'org_acme', + resource: { type: 'SelectionList', key: 'sl_abc123' }, + action: 'read', + }, + expect.any(String), ); }); - it('returns 403 when Permit.check returns false', async () => { - const mockPermit = { check: jest.fn().mockResolvedValue(false) }; - _setPermitClientForTesting(mockPermit); + it('returns 403 when the Security API returns { allow: false }', async () => { + const check = jest.fn().mockResolvedValue({ allow: false }); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); const app = buildApp('SelectionList', 'admin'); const res = await request(app) @@ -170,9 +181,9 @@ describe('requirePermit — flag ON', () => { expect(res.body.code).toBe('FORBIDDEN'); }); - it('returns 403 (fail closed) when Permit.check throws', async () => { - const mockPermit = { check: jest.fn().mockRejectedValue(new Error('Permit network error')) }; - _setPermitClientForTesting(mockPermit); + it('returns 403 (fail closed) when the Security API check throws a generic error', async () => { + const check = jest.fn().mockRejectedValue(new Error('Security API network error')); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); const app = buildApp('SelectionList', 'read'); const res = await request(app) @@ -183,19 +194,47 @@ describe('requirePermit — flag ON', () => { expect(res.body.code).toBe('FORBIDDEN'); }); - it('calls Permit with correct resource instance key from route param', async () => { - const mockPermit = { check: jest.fn().mockResolvedValue(true) }; - _setPermitClientForTesting(mockPermit); + it('returns 403 (fail closed) when the Security API throws AuthzError(DECISION_UNAVAILABLE) — the timeout/unreachable case', async () => { + const check = jest + .fn() + .mockRejectedValue(new AuthzError('DECISION_UNAVAILABLE', 'Security API request failed: timeout; denying.')); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); + const app = buildApp('SelectionList', 'read'); + + const res = await request(app) + .get('/lists/sl_abc123') + .set('Authorization', `Bearer ${makeToken()}`); + + expect(res.status).toBe(403); + expect(res.body.code).toBe('FORBIDDEN'); + }); + + it('calls the Security API with correct resource instance key from route param', async () => { + const check = jest.fn().mockResolvedValue({ allow: true }); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); const app = buildApp('SelectionList', 'write'); await request(app) .get('/lists/sl_unique999') .set('Authorization', `Bearer ${makeToken()}`); - const [userId, action, resourceInstance] = mockPermit.check.mock.calls[0]; - expect(userId).toBe('usr_tester01'); - expect(action).toBe('write'); - expect(resourceInstance).toMatchObject({ key: 'sl_unique999', tenant: 'org_acme' }); + const [checkArg, tokenArg] = check.mock.calls[0]; + expect(checkArg.subject).toBe('usr_tester01'); + expect(checkArg.action).toBe('write'); + expect(checkArg).toMatchObject({ resource: { type: 'SelectionList', key: 'sl_unique999' }, tenant: 'org_acme' }); + expect(typeof tokenArg).toBe('string'); + }); + + it('forwards the CALLER bearer token to the Security API, not a service-wide credential', async () => { + const check = jest.fn().mockResolvedValue({ allow: true }); + _setAuthzClientForTesting({ check, bulkCheck: jest.fn() } as unknown as AuthzClient); + const app = buildApp('SelectionList', 'read'); + const token = makeToken({ userId: 'usr_specific_caller' }); + + await request(app).get('/lists/sl_abc123').set('Authorization', `Bearer ${token}`); + + const [, tokenArg] = check.mock.calls[0]; + expect(tokenArg).toBe(token); }); }); @@ -213,22 +252,33 @@ describe('grantListOwner', () => { mockDb.fn = { now: () => new Date().toISOString() }; }); - it('calls Permit.api.roleAssignments.assign and then upserts the mirror row', async () => { - const assignMock = jest.fn().mockResolvedValue(undefined); - const mockPermit = { - api: { roleAssignments: { assign: assignMock } }, - }; - _setPermitClientForTesting(mockPermit); - - await grantListOwner('usr_newowner', 'org_acme', 'sl_mylist', 'usr_admin'); - - expect(assignMock).toHaveBeenCalledWith( - expect.objectContaining({ - user: 'usr_newowner', - role: 'list-owner', + it('calls AuthzClient.grant() with an INSTANCE-scoped resource, then upserts the mirror row', async () => { + const grantMock = jest.fn().mockResolvedValue({ + id: 'org_acme:usr_newowner:list-owner', + subject: 'usr_newowner', + tenant: 'org_acme', + role: 'list-owner', + }); + _setAuthzClientForTesting({ + check: jest.fn(), + bulkCheck: jest.fn(), + grant: grantMock, + revoke: jest.fn(), + listGrants: jest.fn(), + } as unknown as AuthzClient); + + await grantListOwner('usr_newowner', 'org_acme', 'sl_mylist', 'usr_admin', 'caller-token'); + + // The resource MUST reach the wire — omitting it silently widens a + // list-scoped grant to tenant-wide (the exact bug this test guards). + expect(grantMock).toHaveBeenCalledWith( + { + subject: 'usr_newowner', tenant: 'org_acme', - resource_instance: 'SelectionList:sl_mylist', - }), + role: 'list-owner', + resource: { type: 'SelectionList', key: 'sl_mylist' }, + }, + 'caller-token', ); expect(mockDb.insert).toHaveBeenCalledWith( expect.objectContaining({ @@ -240,6 +290,33 @@ describe('grantListOwner', () => { revoked_at: null, }), ); + + // Ordering guarantee: the Security API write happened BEFORE the mirror + // upsert, asserted via invocationCallOrder so a future reordering + // regression fails loudly rather than silently by coincidence. + const grantOrder = grantMock.mock.invocationCallOrder[0]; + const insertOrder = mockDb.insert.mock.invocationCallOrder[0]; + expect(grantOrder).toBeLessThan(insertOrder); + }); + + it('does NOT write the mirror row when AuthzClient.grant() throws — the write-ordering fail-closed guarantee', async () => { + const grantMock = jest.fn().mockRejectedValue(new AuthzError('PROVIDER_ERROR', 'Security API returned 502')); + _setAuthzClientForTesting({ + check: jest.fn(), + bulkCheck: jest.fn(), + grant: grantMock, + revoke: jest.fn(), + listGrants: jest.fn(), + } as unknown as AuthzClient); + + await expect( + grantListOwner('usr_newowner', 'org_acme', 'sl_mylist', 'usr_admin', 'caller-token'), + ).rejects.toThrow(); + + // The mirror upsert must never be reached — a caller retrying/observing + // this failure must not find a mirror row claiming a grant that never + // actually happened in the authorization backend. + expect(mockDb.insert).not.toHaveBeenCalled(); }); });