From 36c857092e90525932712741c466e4fed50d5ee2 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 13 Sep 2026 15:51:30 +0000 Subject: [PATCH 1/5] fix(auth): require explicit organization for sealed callers --- server/src/auth/organization-selection.ts | 24 ++ server/src/middleware/auth.ts | 22 +- .../src/routes/helpers/resolve-caller-org.ts | 45 +++- server/tests/unit/resolve-caller-org.test.ts | 42 +--- .../unit/sealed-organization-boundary.test.ts | 235 ++++++++++++++++++ 5 files changed, 308 insertions(+), 60 deletions(-) create mode 100644 server/src/auth/organization-selection.ts create mode 100644 server/tests/unit/sealed-organization-boundary.test.ts diff --git a/server/src/auth/organization-selection.ts b/server/src/auth/organization-selection.ts new file mode 100644 index 00000000000..7c8b08124d6 --- /dev/null +++ b/server/src/auth/organization-selection.ts @@ -0,0 +1,24 @@ +import type { Request } from 'express'; + +export class ConflictingOrganizationSelectionError extends Error {} + +/** Every explicit selector must agree with the authenticated provider selection. + * Request fields cannot switch an organization-bound credential to another org. + * Neither memberships nor caches pick an organization. */ +export function selectedOrganizationForAuthentication( + req: Partial>, + providerOrg?: string, +): string | null { + const selectors = [ + providerOrg, + req.headers?.['x-organization-id'], req.query?.org, req.query?.org_id, req.query?.organization_id, req.query?.organizationId, + req.body?.org_id, req.body?.organization_id, req.body?.organizationId, + req.params?.org_id, req.params?.orgId, req.params?.organizationId, + ].filter((value) => value !== undefined); + if (selectors.some((value) => typeof value !== 'string' || !value.trim())) { + throw new ConflictingOrganizationSelectionError(); + } + const ids = new Set(selectors.map((value) => (value as string).trim())); + if (ids.size > 1) throw new ConflictingOrganizationSelectionError(); + return ids.values().next().value ?? null; +} diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 88474cf2f82..9d900f1c01d 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -14,6 +14,7 @@ import { getBearerToken, getWorkOSApiKeyToken, isWorkOSApiKeyFormat } from './ap import { verifyWorkOSJWT, looksLikeJWT, isInvalidWorkOSJWTError, WorkOSJWTUnavailableError } from '../auth/workos-jwt.js'; import { storeRefreshedSession, getRefreshedSession, cleanExpiredRefreshes } from '../db/session-refresh-db.js'; import { loadAuthorizationSnapshot, AuthorizationSnapshotUnavailableError } from '../db/user-authorization-snapshot-db.js'; +import { ConflictingOrganizationSelectionError, selectedOrganizationForAuthentication } from '../auth/organization-selection.js'; import { getOrganizationAuthorizationUserId } from '../auth/organization-principal.js'; import { loadApiKeyManagementSnapshot } from '../db/api-key-management-db.js'; @@ -782,25 +783,8 @@ export function invalidateSessionsForUsers(workosUserIds: string[]): void { } class InvalidAuthorizationCredentialError extends Error {} -export class ConflictingOrganizationSelectionError extends Error {} - -/** Every explicit selector must agree with the authenticated provider selection. - * Request fields cannot switch an organization-bound credential to another org. - * Neither memberships nor caches pick an organization. */ -export function selectedOrganizationForAuthentication(req: Request, providerOrg?: string): string | null { - const selectors = [ - providerOrg, - req.headers['x-organization-id'], req.query?.org, req.query?.organization_id, req.query?.organizationId, - req.body?.organization_id, req.body?.organizationId, - req.params?.orgId, req.params?.organizationId, - ].filter((value) => value !== undefined); - if (selectors.some((value) => typeof value !== 'string' || !value.trim())) { - throw new ConflictingOrganizationSelectionError(); - } - const ids = new Set(selectors.map((value) => (value as string).trim())); - if (ids.size > 1) throw new ConflictingOrganizationSelectionError(); - return ids.values().next().value ?? null; -} +export { ConflictingOrganizationSelectionError, selectedOrganizationForAuthentication } from '../auth/organization-selection.js'; + /** Hydrate a new request object; never mutate the provider object in a cache. */ async function hydrateAuthenticatedUser( diff --git a/server/src/routes/helpers/resolve-caller-org.ts b/server/src/routes/helpers/resolve-caller-org.ts index 259458e85e8..94d050394ae 100644 --- a/server/src/routes/helpers/resolve-caller-org.ts +++ b/server/src/routes/helpers/resolve-caller-org.ts @@ -7,9 +7,9 @@ * against the WorkOS JWKS endpoint. * 2. WorkOS API key (sk_* / wos_api_key_* prefixes) — server-to-server * integrations. Validated via the existing `validateWorkOSApiKey` helper. - * 3. Cookie session — a web session unsealed in `optionalAuth`, producing - * `req.user`. With no Bearer header, the legacy read-model fallback uses - * `resolvePrimaryOrganization` and organization_memberships. + * 3. Sealed cookie session — authentication middleware supplies a fresh + * authorization snapshot. Only an explicitly selected organization + * authorized for the exact credential is returned; primary orgs are ignored. * * An explicit Bearer must supply its own verified organization. Opaque/native * session Bearers cannot use `req.user` to select an implicit primary organization. @@ -24,7 +24,9 @@ import { validateWorkOSApiKey, } from '../../middleware/auth.js'; import { isInvalidWorkOSJWTError, unavailableJWTKeyService, WorkOSJWTUnavailableError } from '../../auth/workos-jwt.js'; -import { resolvePrimaryOrganization } from '../../db/users-db.js'; +import { getAuthorizationEnforcementWorkos } from '../../auth/workos-client.js'; +import { getOrganizationAuthorizationUserId, type OrgAuthorizationPrincipal } from '../../auth/organization-principal.js'; +import { resolveUserOrgAuthorization } from '../../utils/resolve-user-org-authorization.js'; import { createLogger } from '../../logger.js'; const logger = createLogger('resolve-caller-org'); @@ -49,7 +51,9 @@ function jwksForIssuer(iss: string): { jwks: JWTVerifyGetKey; clientId: string } return { jwks, clientId }; } -export type MinimalReq = Pick & { user?: { id?: string } }; +export type MinimalReq = Pick & Partial> & { + user?: Partial; +}; export class CallerOrganizationAuthError extends Error { constructor(readonly status: 401 | 403 | 503) { @@ -128,12 +132,33 @@ async function resolveBearerOrganization(req: MinimalReq): Promise { - if (userId) { +/** Missing, denied, stale or unavailable cookie authority is public-only. + * Nullable consumers must never reinterpret it as canonical-user ownership. */ +async function resolveCookieOrganization(req: MinimalReq): Promise { + const user = req.user; + if (user?.id && user.id !== 'admin_api_key' && !user.id.startsWith('api_key_')) { + const principal: OrgAuthorizationPrincipal = { + id: user.id, + authWorkosUserId: user.authWorkosUserId, + authorizationSnapshot: user.authorizationSnapshot, + }; + const snapshot = principal.authorizationSnapshot; + if (!snapshot || snapshot.authenticatedUserId !== getOrganizationAuthorizationUserId(principal) + || snapshot.canonicalUserId !== principal.id) return null; try { - return await resolvePrimaryOrganization(userId); + // Route params may become available only after authentication middleware. + // Reuse its conflict policy before consuming the persisted selection. + const organizationId = selectedOrganizationForAuthentication(req, snapshot.selectedOrganizationId ?? undefined); + if (!organizationId) return null; + // A snapshot selection is not a grant. This resolver rechecks exact + // membership/grant and brackets WorkOS reads with fresh snapshot checks. + const authorization = await resolveUserOrgAuthorization( + getAuthorizationEnforcementWorkos(), principal, organizationId, + ); + return authorization.status === 'authorized' ? authorization.membership.organizationId : null; } catch (err) { - logger.warn({ err, userId }, 'caller org resolution failed — falling back to public-only'); + logger.warn({ err }, 'Explicit caller organization authorization unavailable or conflicting'); + return null; } } @@ -147,7 +172,7 @@ async function resolveCookieOrganization(userId: string | undefined): Promise { const authorization = req.headers.authorization; - if (getBearerToken(authorization) === null) return resolveCookieOrganization(req.user?.id); + if (getBearerToken(authorization) === null) return resolveCookieOrganization(req); // Capture the credential and selection before any await. Later request // mutation cannot substitute another key or erase an organization conflict. diff --git a/server/tests/unit/resolve-caller-org.test.ts b/server/tests/unit/resolve-caller-org.test.ts index 7856f109417..a5ac06fc157 100644 --- a/server/tests/unit/resolve-caller-org.test.ts +++ b/server/tests/unit/resolve-caller-org.test.ts @@ -7,7 +7,7 @@ * 1. WorkOS OIDC access token (RS256 JWT, `org_id` claim) — JWKS is picked * per-token from the `iss` claim, not from a server-wide env var. * 2. WorkOS API key (sk_* / wos_api_key_*) - * 3. Sealed session (middleware sets `req.user`) + * 3. Sealed session (middleware supplies a fresh authorization snapshot) * * Regression guard for the issue where OIDC JWTs silently fell through to * public-only, leaving `agents: []` for authenticated callers. @@ -367,40 +367,20 @@ describe('resolveCallerOrgId', () => { // ── Sealed-session path (existing behavior) ───────────────────── - it('falls back to users.primary_organization_id when only req.user is set', async () => { - validateWorkOSApiKeyMock.mockResolvedValueOnce(null); - // resolvePrimaryOrganization: fast-path read returns the cached column - // alongside joins_valid, so a dangling pointer can fall through. - dbQueryMock.mockResolvedValueOnce({ rows: [{ primary_organization_id: 'org_from_session', joins_valid: true }] }); - - const orgId = await resolveCallerOrgId(reqWith(undefined, { id: 'user_session' })); - - expect(orgId).toBe('org_from_session'); - // Assert the fast-path SQL — joins_valid checks both organizations and - // organization_memberships so a dangling pointer falls through. - expect(dbQueryMock.mock.calls[0][0]).toMatch(/SELECT[\s\S]*primary_organization_id[\s\S]*EXISTS[\s\S]*organizations[\s\S]*EXISTS[\s\S]*organization_memberships[\s\S]*joins_valid[\s\S]*FROM users[\s\S]*workos_user_id\s*=\s*\$1/); - expect(dbQueryMock.mock.calls[0][1]).toEqual(['user_session']); - }); + it('does not infer a primary or sole org from a bare authenticated user', async () => { + dbQueryMock.mockResolvedValue({ rows: [{ primary_organization_id: 'org_primary', joins_valid: true }] }); - it('returns null when session user has no primary org and no memberships', async () => { - validateWorkOSApiKeyMock.mockResolvedValueOnce(null); - // Fast-path: no row (column was NULL or user row missing). - dbQueryMock.mockResolvedValueOnce({ rows: [] }); - // Fallback: resolvePreferredOrganization finds no memberships. - dbQueryMock.mockResolvedValueOnce({ rows: [] }); - - const orgId = await resolveCallerOrgId(reqWith(undefined, { id: 'user_no_org' })); - - expect(orgId).toBeNull(); + expect(await resolveCallerOrgId(reqWith(undefined, { id: 'user_session' }))).toBeNull(); + expect(dbQueryMock).not.toHaveBeenCalled(); + expect(validateWorkOSApiKeyMock).not.toHaveBeenCalled(); }); - it('swallows DB errors and returns null rather than throwing', async () => { - validateWorkOSApiKeyMock.mockResolvedValueOnce(null); - dbQueryMock.mockRejectedValueOnce(new Error('connection reset')); - - const orgId = await resolveCallerOrgId(reqWith(undefined, { id: 'user_db_err' })); + it('does not substitute raw bearer authority for an authenticated user missing a snapshot', async () => { + jwtVerifyMock.mockResolvedValue({ payload: { org_id: 'org_other' } }); - expect(orgId).toBeNull(); + expect(await resolveCallerOrgId(reqWith('Bearer eyJabc.def.ghi', { id: 'user_session' }))).toBeNull(); + expect(jwtVerifyMock).not.toHaveBeenCalled(); + expect(validateWorkOSApiKeyMock).not.toHaveBeenCalled(); }); // ── Unauthenticated / malformed ──────────────────────────────── diff --git a/server/tests/unit/sealed-organization-boundary.test.ts b/server/tests/unit/sealed-organization-boundary.test.ts new file mode 100644 index 00000000000..ab79051a3f6 --- /dev/null +++ b/server/tests/unit/sealed-organization-boundary.test.ts @@ -0,0 +1,235 @@ +/** Mounted conformance route with the real snapshot and organization resolvers. */ +import express from 'express'; +import request from 'supertest'; +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { WorkOSUser } from '../../src/types.js'; + +const mocks = vi.hoisted(() => ({ + principal: undefined as WorkOSUser | undefined, + boundedQuery: vi.fn(), + memberships: vi.fn(), + validateApiKey: vi.fn(), +})); + +vi.mock('../../src/middleware/auth.js', () => ({ + requireAuth: (req: express.Request, _res: express.Response, next: express.NextFunction) => { + req.user = mocks.principal; + next(); + }, + validateWorkOSApiKey: mocks.validateApiKey, +})); +vi.mock('../../src/db/client.js', async importOriginal => ({ + ...await importOriginal(), + queryWithTimeout: mocks.boundedQuery, +})); +vi.mock('../../src/auth/workos-client.js', () => ({ + getAuthorizationEnforcementWorkos: () => ({ userManagement: { listOrganizationMemberships: mocks.memberships } }), +})); + +import { buildConformanceTokenRouter } from '../../src/conformance/token-route.js'; +import { verifyConformanceToken } from '../../src/conformance/token.js'; +import { loadAuthorizationSnapshot } from '../../src/db/user-authorization-snapshot-db.js'; +import { resolveCallerOrgId } from '../../src/routes/helpers/resolve-caller-org.js'; + +const ORG = 'org_pinnacle'; +const OTHER_ORG = 'org_streamhaus'; +const originalSecret = process.env.CONFORMANCE_JWT_SECRET; +let row: Record; + +function app() { + const instance = express(); + instance.use(express.json()); + instance.use('/api/conformance', buildConformanceTokenRouter()); + return instance; +} + +async function authenticate(authenticatedUserId = 'user_primary', canonicalUserId = 'user_linked', organizationId: string | null = ORG) { + row.authenticated_user_id = authenticatedUserId; + row.canonical_user_id = canonicalUserId; + const snapshot = await loadAuthorizationSnapshot(authenticatedUserId, organizationId); + mocks.principal = { + id: canonicalUserId, + authWorkosUserId: authenticatedUserId, + email: 'sam@pinnacle.example', + }; + Object.defineProperty(mocks.principal, 'authorizationSnapshot', { value: snapshot }); + mocks.boundedQuery.mockClear(); +} + +function post() { + return request(app()).post('/api/conformance/token').set('Authorization', 'Bearer sealed-session-fixture'); +} + +beforeEach(() => { + vi.resetAllMocks(); + process.env.CONFORMANCE_JWT_SECRET = 'sealed-organization-test-secret'; + mocks.principal = undefined; + row = { + in_recovery: false, + authenticated_user_id: 'user_primary', canonical_user_id: 'user_linked', identity_id: 'identity_linked', + authorization_epoch: '7', email: 'sam@pinnacle.example', email_verified: true, + first_name: 'Sam', last_name: 'Adeyemi', + grant_id: null, grant_organization_id: null, grant_role: null, + grant_effective_from: null, grant_effective_until: null, + }; + mocks.boundedQuery.mockImplementation(async () => ({ rows: [{ ...row }], rowCount: 1 })); + mocks.memberships.mockImplementation(async ({ userId, organizationId }) => ({ + data: [{ id: 'membership_pinnacle', userId, organizationId, status: 'active', role: { slug: 'member' } }], + })); + mocks.validateApiKey.mockResolvedValue(null); +}); + +afterAll(() => { + if (originalSecret === undefined) delete process.env.CONFORMANCE_JWT_SECRET; + else process.env.CONFORMANCE_JWT_SECRET = originalSecret; +}); + +describe('sealed-session organization boundary on POST /api/conformance/token', () => { + it.each([ + ['user_primary', 'user_linked'], + ['user_linked', 'user_primary'], + ])('uses the exact authenticated %s credential when person attribution is %s', async (authenticated, canonical) => { + await authenticate(authenticated, canonical); + const response = await post(); + expect(response.status).toBe(200); + expect(verifyConformanceToken(response.body.token).sub).toBe(ORG); + expect(mocks.memberships).toHaveBeenCalledExactlyOnceWith({ userId: authenticated, organizationId: ORG }); + expect(mocks.boundedQuery.mock.calls.every(([, parameters]) => parameters[0] === authenticated)).toBe(true); + }); + + it.each([ + ['user_primary', 'user_linked'], + ['user_linked', 'user_primary'], + ])('does not copy %s authority into authenticated %s', async (canonical, authenticated) => { + await authenticate(authenticated, canonical); + mocks.memberships.mockResolvedValue({ + data: [{ userId: canonical, organizationId: ORG, status: 'active', role: { slug: 'owner' } }], + }); + const response = await post(); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(mocks.memberships).toHaveBeenCalledExactlyOnceWith({ userId: authenticated, organizationId: ORG }); + }); + + it('requires explicit organization context even when the credential has a sole membership', async () => { + await authenticate('user_primary', 'user_linked', null); + const response = await post(); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(mocks.memberships).not.toHaveBeenCalled(); + }); + + it('accepts an explicit request selection when authentication had no selected organization', async () => { + await authenticate('user_primary', 'user_linked', null); + const response = await post().send({ organization_id: ORG }); + expect(response.status).toBe(200); + expect(verifyConformanceToken(response.body.token).sub).toBe(ORG); + }); + + it.each(['absent', 'unavailable'])('preserves an exact credential grant when WorkOS membership is %s', async state => { + Object.assign(row, { + grant_id: 'grant_exact', grant_organization_id: ORG, grant_role: 'member', + grant_effective_from: '2026-01-01T00:00:00.000000Z', + }); + await authenticate('user_linked', 'user_primary'); + if (state === 'absent') mocks.memberships.mockResolvedValue({ data: [] }); + else mocks.memberships.mockRejectedValue(new Error('provider unavailable')); + const response = await post(); + expect(response.status).toBe(200); + expect(verifyConformanceToken(response.body.token).sub).toBe(ORG); + expect(mocks.boundedQuery.mock.calls.every(([, parameters]) => parameters[0] === 'user_linked')).toBe(true); + }); + + it.each(['id', 'authWorkosUserId'] as const)('rejects a snapshot transplanted to a different principal %s', async field => { + await authenticate(); + mocks.principal![field] = 'user_other'; + const response = await post(); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(mocks.boundedQuery).not.toHaveBeenCalled(); + expect(mocks.memberships).not.toHaveBeenCalled(); + }); + + it('checks path selectors populated after authentication middleware', async () => { + await authenticate(); + const instance = express(); + // Authentication commonly runs before Express has matched a tenant route. + instance.use((req, _res, next) => { req.user = mocks.principal; next(); }); + instance.get('/organizations/:orgId/check', async (req, res) => { + const organizationId = await resolveCallerOrgId(req); + res.status(organizationId ? 200 : 403).json({ organizationId }); + }); + expect((await request(instance).get(`/organizations/${ORG}/check`)).status).toBe(200); + mocks.memberships.mockClear(); + const conflict = await request(instance).get(`/organizations/${OTHER_ORG}/check`); + expect(conflict.status).toBe(403); + expect(conflict.body.organizationId).toBeNull(); + expect(mocks.memberships).not.toHaveBeenCalled(); + }); + + it('rejects a canonical user object without authenticated snapshot provenance', async () => { + mocks.principal = { id: 'user_linked', email: 'sam@pinnacle.example' }; + const response = await post().send({ organization_id: ORG }); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(mocks.memberships).not.toHaveBeenCalled(); + expect(mocks.boundedQuery).not.toHaveBeenCalled(); + }); + + it.each(['header', 'query', 'body', 'org_id alias'])('rejects a conflicting %s selector before provider authorization', async location => { + await authenticate(); + const pending = post(); + if (location === 'header') pending.set('X-Organization-Id', OTHER_ORG); + if (location === 'query') pending.query({ organization_id: OTHER_ORG }); + if (location === 'body') pending.send({ organizationId: OTHER_ORG }); + if (location === 'org_id alias') pending.send({ org_id: OTHER_ORG }); + const response = await pending; + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(mocks.memberships).not.toHaveBeenCalled(); + }); + + it('accepts matching selectors across header, query, and body', async () => { + await authenticate(); + const response = await post().set('X-Organization-Id', ORG).query({ org: ORG }).send({ organization_id: ORG }); + expect(response.status).toBe(200); + expect(verifyConformanceToken(response.body.token).sub).toBe(ORG); + }); + + it.each(['epoch', 'identity', 'credential grant'])('rejects replay of a request snapshot after its %s changes', async changed => { + if (changed === 'credential grant') { + Object.assign(row, { grant_id: 'grant_original', grant_organization_id: ORG, grant_role: 'member', grant_effective_from: '2026-01-01T00:00:00.000000Z' }); + } + await authenticate(); + expect((await post()).status).toBe(200); + if (changed === 'epoch') row.authorization_epoch = '8'; + if (changed === 'identity') row.canonical_user_id = 'user_rebound'; + if (changed === 'credential grant') row.grant_id = 'grant_replacement'; + mocks.memberships.mockClear(); + const response = await post(); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(mocks.memberships).not.toHaveBeenCalled(); + }); + + it('rejects a concurrent epoch change while WorkOS membership is in flight', async () => { + await authenticate(); + mocks.memberships.mockImplementation(async ({ userId, organizationId }) => { + row.authorization_epoch = '8'; + return { data: [{ userId, organizationId, status: 'active', role: { slug: 'member' } }] }; + }); + const response = await post(); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + }); + + it.each(['database', 'provider'])('fails closed without issuing a token during a %s outage', async source => { + await authenticate(); + if (source === 'database') mocks.boundedQuery.mockRejectedValue(new Error('private database details')); + else mocks.memberships.mockRejectedValue(new Error('private provider details')); + const response = await post(); + expect(response.status).toBe(403); + expect(response.body.token).toBeUndefined(); + expect(response.text).not.toContain('private'); + }); +}); From eaf78047d192bcb750f5517b1e561b8685b7b7bb Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 13 Sep 2026 15:59:58 +0000 Subject: [PATCH 2/5] fix(auth): fail closed for community mirror proposal access --- server/src/routes/community-mirrors.ts | 52 ++++--- .../integration/community-mirrors.test.ts | 47 ++++--- .../unit/sealed-organization-boundary.test.ts | 131 +++++++++++++++++- 3 files changed, 181 insertions(+), 49 deletions(-) diff --git a/server/src/routes/community-mirrors.ts b/server/src/routes/community-mirrors.ts index 169cf0fbe31..4aafb6f0915 100644 --- a/server/src/routes/community-mirrors.ts +++ b/server/src/routes/community-mirrors.ts @@ -9,7 +9,7 @@ import { respondToAdminAuthorizationError } from '../auth/admin-authorization-re * * GET /api/registry/mirrors — list mirrors (public, with etags) * GET /api/registry/mirrors/:platform — read one mirror (public) - * PUT /api/registry/mirrors/:platform — publish for moderators, propose for other callers + * PUT /api/registry/mirrors/:platform — publish with admin keys, propose for other callers * GET /api/registry/mirror-proposals — review queue or caller's own proposals * POST /api/registry/mirror-proposals/:id/{approve,reject} — moderation * @@ -27,8 +27,7 @@ import type { CommunityMirror, CommunityMirrorProposal } from '../db/community-m import { PublisherDatabase } from '../db/publisher-db.js'; import type { CatalogEventsDatabase } from '../db/catalog-events-db.js'; import { getClient } from '../db/client.js'; -import { isRegistryModerator } from '../services/brand-logo-auth.js'; -import { isAuthenticatedUserAAOAdmin, type AAOAdminPrincipal } from '../addie/admin-status-lookup.js'; +import type { AAOAdminPrincipal } from '../addie/admin-status-lookup.js'; import { validateAdagentsDocument } from '../services/adagents-schema-validator.js'; import { registryReadRateLimiter, brandCreationRateLimiter } from '../middleware/rate-limit.js'; import { createLogger } from '../logger.js'; @@ -84,18 +83,13 @@ export interface CommunityMirrorRouterConfig { } /** - * Organization API keys represent an organization, not a human working-group - * member, so they intentionally submit proposals instead of passing this gate. + * Human management stays disabled until the registry management gate has a + * separately reviewed authorization-snapshot integration. + * Only the authenticated platform admin API key retains management access; + * organization API keys intentionally submit proposals instead. */ -async function canManageMirrors(user: AAOAdminPrincipal): Promise { - const userId = user.authWorkosUserId ?? user.id; - if (userId === 'admin_api_key') return true; - if (userId.startsWith('api_key_')) return false; - const [isOrganizationAdmin, isModerator] = await Promise.all([ - isAuthenticatedUserAAOAdmin(user), - isRegistryModerator(userId), - ]); - return isOrganizationAdmin || isModerator; +function canManageMirrors(user: AAOAdminPrincipal): boolean { + return (user.authWorkosUserId ?? user.id) === 'admin_api_key'; } async function resolveManager( @@ -107,9 +101,9 @@ async function resolveManager( res.status(401).json({ error: 'Authentication required' }); return null; } - if (!(await canManageMirrors(req.user!))) { + if (!canManageMirrors(req.user!)) { res.status(403).json({ - error: 'Only registry moderators or AgenticAdvertising.org administrators can manage community mirrors', + error: 'Community mirror management requires a platform admin API key', }); return null; } @@ -119,7 +113,8 @@ async function resolveManager( async function callerOrganizationId(req: Request, res: Response): Promise { try { const attached = (req as Request & { apiKey?: { organizationId?: string } }).apiKey?.organizationId; - return attached ?? await resolveCallerOrgId(req); + if (req.user?.id.startsWith('api_key_') && attached) return attached; + return await resolveCallerOrgId(req); } catch (error) { if (sendCallerOrganizationAuthError(error, res)) return undefined; throw error; @@ -212,12 +207,15 @@ export function createCommunityMirrorRouter(config: CommunityMirrorRouterConfig) if (req.query.review_queue !== undefined && !['true', 'false'].includes(String(req.query.review_queue))) { return res.status(400).json({ error: 'Invalid review_queue flag' }); } - const isManager = await canManageMirrors(req.user!); + const isManager = canManageMirrors(req.user!); if (reviewQueue && !isManager) { - return res.status(403).json({ error: 'Registry moderator access is required for the review queue' }); + return res.status(403).json({ error: 'The review queue requires a platform admin API key' }); } const organizationId = isManager ? null : await callerOrganizationId(req, res); if (organizationId === undefined) return; + if (!isManager && !organizationId) { + return res.status(404).json({ error: 'Community mirror proposal not found' }); + } const limit = req.query.limit ? parseInt(String(req.query.limit), 10) : undefined; const offset = req.query.offset ? parseInt(String(req.query.offset), 10) : undefined; try { @@ -228,7 +226,6 @@ export function createCommunityMirrorRouter(config: CommunityMirrorRouterConfig) | 'rejected' | undefined, proposedByOrganizationId: isManager ? undefined : organizationId ?? undefined, - proposedByUserId: isManager || organizationId ? undefined : userId, limit: Number.isFinite(limit) ? limit : undefined, offset: Number.isFinite(offset) ? offset : undefined, }); @@ -247,14 +244,15 @@ export function createCommunityMirrorRouter(config: CommunityMirrorRouterConfig) return res.status(400).json({ error: 'Invalid proposal identifier' }); } try { - const proposal = await mirrorDb.getProposalById(proposalId); - if (!proposal) return res.status(404).json({ error: 'Community mirror proposal not found' }); - const isManager = await canManageMirrors(req.user!); + const isManager = canManageMirrors(req.user!); const organizationId = isManager ? null : await callerOrganizationId(req, res); if (organizationId === undefined) return; - const ownsProposal = organizationId - ? proposal.proposed_by_organization_id === organizationId - : proposal.proposed_by_user_id === userId; + if (!isManager && !organizationId) { + return res.status(404).json({ error: 'Community mirror proposal not found' }); + } + const proposal = await mirrorDb.getProposalById(proposalId); + if (!proposal) return res.status(404).json({ error: 'Community mirror proposal not found' }); + const ownsProposal = organizationId !== null && proposal.proposed_by_organization_id === organizationId; if (!ownsProposal && !isManager) { return res.status(404).json({ error: 'Community mirror proposal not found' }); } @@ -464,7 +462,7 @@ export function createCommunityMirrorRouter(config: CommunityMirrorRouterConfig) const userId = req.user?.id; if (!userId) return res.status(401).json({ error: 'Authentication required' }); - const isManager = await canManageMirrors(req.user!); + const isManager = canManageMirrors(req.user!); const organizationId = isManager ? null : await callerOrganizationId(req, res); if (organizationId === undefined) return; if (!isManager && !organizationId) { diff --git a/server/tests/integration/community-mirrors.test.ts b/server/tests/integration/community-mirrors.test.ts index d7793150ff3..1bed2312f74 100644 --- a/server/tests/integration/community-mirrors.test.ts +++ b/server/tests/integration/community-mirrors.test.ts @@ -11,10 +11,10 @@ vi.hoisted(() => { process.env.WORKOS_CLIENT_ID = process.env.WORKOS_CLIENT_ID ?? 'client_test'; }); -// Mutable identity for the authenticated caller so a re-publish can come from a -// different user (used to assert created_by_* is preserved). +// Mock the identity produced by credential validation. Static admin keys retain +// manager access; organization API keys submit and read their own proposals. const authState = vi.hoisted(() => ({ - userId: 'user_test_mirrors', + userId: 'admin_api_key', email: 'mirrors@test.com', organizationId: null as string | null, })); @@ -204,7 +204,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( notifyPendingCommunityMirrorProposal.mockResolvedValue(null); notifyCommunityMirrorProposalReviewed.mockReset(); notifyCommunityMirrorProposalReviewed.mockResolvedValue(undefined); - authState.userId = 'user_test_mirrors'; + authState.userId = 'admin_api_key'; authState.email = 'mirrors@test.com'; authState.organizationId = null; await clear(); @@ -367,11 +367,13 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect(read.body.adagents_json.authorized_agents).toEqual([]); }); - it('allows an AAO admin who is not a registry moderator to publish', async () => { + it('denies human admin publishing without exact manager authorization', async () => { + authState.userId = 'user_human_admin'; isRegistryModerator.mockResolvedValue(false); isWebUserAAOAdmin.mockResolvedValue(true); const res = await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody()); - expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect((await pool.query('SELECT 1 FROM community_mirrors WHERE platform = $1', [PLATFORM])).rows).toHaveLength(0); }); it('re-publish is idempotent — updates in place, no duplicate row', async () => { @@ -471,10 +473,12 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect(projectedProperties.rows.map((row) => row.publisher_domain)).toEqual([...finalDomains].sort()); }); - it('preserves created_by_* across a re-publish by a different user', async () => { - authState.userId = 'creator-A'; + it('preserves legacy created_by_* when an admin key re-publishes', async () => { await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody({ catalog_etag: 'v1' })); - authState.userId = 'editor-B'; + await pool.query( + 'UPDATE community_mirrors SET created_by_user_id = $1 WHERE platform = $2', + ['creator-A', PLATFORM], + ); await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody({ catalog_etag: 'v2' })); const { rows } = await pool.query( @@ -569,7 +573,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( .send(publishBody({ catalog_etag: 'changed-v2', formats: [{ ...MINIMAL_FORMAT, display_name: 'Changed' }] })); expect(changed.body.proposal_id).toBe(reviewed.body.proposal_id); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); const staleApproval = await request(app) @@ -587,7 +591,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( authState.organizationId = 'org_test_mirrors'; const submitted = await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody()); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); await request(app) @@ -609,7 +613,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( authState.organizationId = 'org_test_mirrors'; const original = await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody()); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); await request(app) @@ -623,7 +627,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect(rebased.body.proposal_id).toBe(original.body.proposal_id); expect(rebased.body.proposal_digest).not.toBe(original.body.proposal_digest); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); const staleApproval = await request(app) @@ -660,7 +664,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect(ownDetail.status).toBe(200); expect(ownDetail.body.proposal).not.toHaveProperty('proposed_by_email'); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); const managerDetail = await request(app) @@ -697,7 +701,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect(row.rows[0]?.slack_thread_ts).toBe('1779110411.874'); }); - it('restricts the cross-organization review queue to moderators', async () => { + it('restricts the cross-organization review queue to a validated admin key', async () => { isRegistryModerator.mockResolvedValue(false); authState.userId = 'api_key_test_mirrors'; authState.organizationId = 'org_test_mirrors'; @@ -707,7 +711,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( .get('/api/registry/mirror-proposals?status=pending&review_queue=true'); expect(denied.status).toBe(403); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); const queue = await request(app) @@ -732,13 +736,13 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect(oversized.status).toBe(413); }); - it('lets a moderator approve and atomically publish a pending proposal', async () => { + it('lets a validated admin key approve and atomically publish a pending proposal', async () => { isRegistryModerator.mockResolvedValue(false); authState.userId = 'api_key_test_mirrors'; authState.organizationId = 'org_test_mirrors'; const submitted = await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody()); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); const approved = await request(app) @@ -761,7 +765,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( ); expect(proposal.rows[0]).toMatchObject({ status: 'approved', - reviewed_by_user_id: 'user_registry_moderator', + reviewed_by_user_id: 'admin_api_key', published_at: expect.any(Date), }); expect((await pool.query('SELECT 1 FROM community_mirrors WHERE platform = $1', [PLATFORM])).rows).toHaveLength(1); @@ -787,13 +791,13 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect((await pool.query('SELECT 1 FROM community_mirrors WHERE platform = $1', [PLATFORM])).rows).toHaveLength(0); }); - it('lets a moderator reject a pending proposal without publishing it', async () => { + it('lets a validated admin key reject a pending proposal without publishing it', async () => { isRegistryModerator.mockResolvedValue(false); authState.userId = 'api_key_test_mirrors'; authState.organizationId = 'org_test_mirrors'; const submitted = await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody()); - authState.userId = 'user_registry_moderator'; + authState.userId = 'admin_api_key'; authState.organizationId = null; isRegistryModerator.mockResolvedValue(true); const rejected = await request(app) @@ -982,6 +986,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( await request(app) .put(`/api/registry/mirrors/${PLATFORM}`) .send(publishBody({ superseded_by: 'https://meta.com/.well-known/adagents.json' })); + authState.userId = 'user_non_manager'; isRegistryModerator.mockResolvedValue(false); isWebUserAAOAdmin.mockResolvedValue(false); const del = await request(app).delete(`/api/registry/mirrors/${PLATFORM}`); diff --git a/server/tests/unit/sealed-organization-boundary.test.ts b/server/tests/unit/sealed-organization-boundary.test.ts index ab79051a3f6..99c2a88416e 100644 --- a/server/tests/unit/sealed-organization-boundary.test.ts +++ b/server/tests/unit/sealed-organization-boundary.test.ts @@ -1,4 +1,4 @@ -/** Mounted conformance route with the real snapshot and organization resolvers. */ +/** Mounted consumers with the real snapshot and organization resolvers. */ import express from 'express'; import request from 'supertest'; import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -9,6 +9,11 @@ const mocks = vi.hoisted(() => ({ boundedQuery: vi.fn(), memberships: vi.fn(), validateApiKey: vi.fn(), + listProposals: vi.fn(), + getProposal: vi.fn(), + getClient: vi.fn(), + isAdmin: vi.fn(), + isModerator: vi.fn(), })); vi.mock('../../src/middleware/auth.js', () => ({ @@ -21,18 +26,35 @@ vi.mock('../../src/middleware/auth.js', () => ({ vi.mock('../../src/db/client.js', async importOriginal => ({ ...await importOriginal(), queryWithTimeout: mocks.boundedQuery, + getClient: mocks.getClient, })); vi.mock('../../src/auth/workos-client.js', () => ({ getAuthorizationEnforcementWorkos: () => ({ userManagement: { listOrganizationMemberships: mocks.memberships } }), })); +vi.mock('../../src/db/community-mirror-db.js', () => ({ + CommunityMirrorDatabase: class { + listProposals = mocks.listProposals; + getProposalById = mocks.getProposal; + }, +})); +vi.mock('../../src/db/publisher-db.js', () => ({ PublisherDatabase: class {} })); +vi.mock('../../src/addie/admin-status-lookup.js', () => ({ isWebUserAAOAdmin: mocks.isAdmin })); +vi.mock('../../src/services/brand-logo-auth.js', () => ({ isRegistryModerator: mocks.isModerator })); +vi.mock('../../src/notifications/registry.js', () => ({})); +vi.mock('../../src/middleware/rate-limit.js', () => { + const pass = (_req: express.Request, _res: express.Response, next: express.NextFunction) => next(); + return { registryReadRateLimiter: pass, brandCreationRateLimiter: pass }; +}); import { buildConformanceTokenRouter } from '../../src/conformance/token-route.js'; +import { createCommunityMirrorRouter } from '../../src/routes/community-mirrors.js'; import { verifyConformanceToken } from '../../src/conformance/token.js'; import { loadAuthorizationSnapshot } from '../../src/db/user-authorization-snapshot-db.js'; import { resolveCallerOrgId } from '../../src/routes/helpers/resolve-caller-org.js'; const ORG = 'org_pinnacle'; const OTHER_ORG = 'org_streamhaus'; +const PROPOSAL_ID = '00000000-0000-4000-8000-000000000001'; const originalSecret = process.env.CONFORMANCE_JWT_SECRET; let row: Record; @@ -43,6 +65,19 @@ function app() { return instance; } +function mirrorApp(attachedOrganizationId?: string) { + const instance = express(); + instance.use(express.json()); + instance.use('/api/registry', createCommunityMirrorRouter({ + requireAuth: (req, _res, next) => { + req.user = mocks.principal; + if (attachedOrganizationId) Object.assign(req, { apiKey: { organizationId: attachedOrganizationId } }); + next(); + }, + })); + return instance; +} + async function authenticate(authenticatedUserId = 'user_primary', canonicalUserId = 'user_linked', organizationId: string | null = ORG) { row.authenticated_user_id = authenticatedUserId; row.canonical_user_id = canonicalUserId; @@ -77,6 +112,14 @@ beforeEach(() => { data: [{ id: 'membership_pinnacle', userId, organizationId, status: 'active', role: { slug: 'member' } }], })); mocks.validateApiKey.mockResolvedValue(null); + mocks.listProposals.mockResolvedValue({ proposals: [{ id: PROPOSAL_ID }], total: 1 }); + mocks.getProposal.mockImplementation(async () => ({ + id: PROPOSAL_ID, proposed_by_user_id: mocks.principal?.id, + proposed_by_organization_id: ORG, adagents_json: { private_proposal: true }, + })); + // A canonical admin/moderator result must never rescue denied exact authority. + mocks.isAdmin.mockResolvedValue(true); + mocks.isModerator.mockResolvedValue(true); }); afterAll(() => { @@ -84,6 +127,92 @@ afterAll(() => { else process.env.CONFORMANCE_JWT_SECRET = originalSecret; }); +describe.each([ + ['user_primary', 'user_linked'], + ['user_linked', 'user_primary'], +])('community mirror proposals with authenticated %s attributed to %s', (authenticated, canonical) => { + it.each(['no selection', 'no membership', 'canonical membership', 'conflict', 'stale epoch', 'unavailable'])( + 'denies list and detail for %s without canonical-user fallback', async denied => { + await authenticate(authenticated, canonical, denied === 'no selection' ? null : ORG); + if (denied === 'no membership') mocks.memberships.mockResolvedValue({ data: [] }); + if (denied === 'canonical membership') mocks.memberships.mockResolvedValue({ + data: [{ userId: canonical, organizationId: ORG, status: 'active', role: { slug: 'owner' } }], + }); + if (denied === 'stale epoch') row.authorization_epoch = '8'; + if (denied === 'unavailable') mocks.boundedQuery.mockRejectedValue(new Error('database unavailable')); + for (const path of ['/mirror-proposals', `/mirror-proposals/${PROPOSAL_ID}`]) { + const pending = request(mirrorApp()).get(`/api/registry${path}`); + if (denied === 'conflict') pending.set('X-Organization-Id', OTHER_ORG); + const response = await pending; + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Community mirror proposal not found' }); + } + expect(mocks.listProposals).not.toHaveBeenCalled(); + expect(mocks.getProposal).not.toHaveBeenCalled(); + expect(mocks.isAdmin).not.toHaveBeenCalled(); + expect(mocks.isModerator).not.toHaveBeenCalled(); + }, + ); + + it('scopes authorized list/detail to the selected org and denies other-org or unscoped rows', async () => { + await authenticate(authenticated, canonical); + const instance = mirrorApp(); + expect((await request(instance).get('/api/registry/mirror-proposals')).status).toBe(200); + expect(mocks.listProposals).toHaveBeenCalledWith(expect.objectContaining({ proposedByOrganizationId: ORG })); + expect(mocks.listProposals.mock.calls[0][0].proposedByUserId).toBeUndefined(); + expect((await request(instance).get(`/api/registry/mirror-proposals/${PROPOSAL_ID}`)).status).toBe(200); + for (const organizationId of [OTHER_ORG, null]) { + mocks.getProposal.mockResolvedValue({ + id: PROPOSAL_ID, proposed_by_user_id: canonical, proposed_by_organization_id: organizationId, + }); + expect((await request(instance).get(`/api/registry/mirror-proposals/${PROPOSAL_ID}`)).status).toBe(404); + } + expect(mocks.memberships.mock.calls.every(([input]) => input.userId === authenticated)).toBe(true); + }); + + it('rejects list/detail replay after revocation before reading proposals', async () => { + await authenticate(authenticated, canonical); + const instance = mirrorApp(); + for (const path of ['/mirror-proposals', `/mirror-proposals/${PROPOSAL_ID}`]) { + expect((await request(instance).get(`/api/registry${path}`)).status).toBe(200); + } + row.authorization_epoch = '8'; + mocks.listProposals.mockClear(); + mocks.getProposal.mockClear(); + for (const path of ['/mirror-proposals', `/mirror-proposals/${PROPOSAL_ID}`]) { + expect((await request(instance).get(`/api/registry${path}`)).status).toBe(404); + } + expect(mocks.listProposals).not.toHaveBeenCalled(); + expect(mocks.getProposal).not.toHaveBeenCalled(); + }); + + it.each(['no selection', 'stale epoch', 'conflict'])('does not rescue %s with an attached API-key organization', async denied => { + await authenticate(authenticated, canonical, denied === 'no selection' ? null : ORG); + if (denied === 'stale epoch') row.authorization_epoch = '8'; + for (const path of ['/mirror-proposals', `/mirror-proposals/${PROPOSAL_ID}`]) { + const pending = request(mirrorApp(ORG)).get(`/api/registry${path}`); + if (denied === 'conflict') pending.set('X-Organization-Id', OTHER_ORG); + expect((await pending).status).toBe(404); + } + expect(mocks.listProposals).not.toHaveBeenCalled(); + expect(mocks.getProposal).not.toHaveBeenCalled(); + }); + + it('denies human manager fallback for queue, approve, reject, publish and retire', async () => { + await authenticate(authenticated, canonical, null); + const instance = mirrorApp(); + expect((await request(instance).get('/api/registry/mirror-proposals?review_queue=true')).status).toBe(403); + for (const action of ['approve', 'reject']) { + expect((await request(instance).post(`/api/registry/mirror-proposals/${PROPOSAL_ID}/${action}`).send({})).status).toBe(403); + } + expect((await request(instance).put('/api/registry/mirrors/example').send({})).status).toBe(403); + expect((await request(instance).delete('/api/registry/mirrors/example')).status).toBe(403); + expect(mocks.getClient).not.toHaveBeenCalled(); + expect(mocks.isAdmin).not.toHaveBeenCalled(); + expect(mocks.isModerator).not.toHaveBeenCalled(); + }); +}); + describe('sealed-session organization boundary on POST /api/conformance/token', () => { it.each([ ['user_primary', 'user_linked'], From 30d40d8ea75b97074d082c94ec5b1b721274ab61 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 13 Sep 2026 16:10:13 +0000 Subject: [PATCH 3/5] test(auth): give registry readers explicit organization provenance --- ...y-reader-baseline-public-endpoints.test.ts | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/server/tests/integration/registry-reader-baseline-public-endpoints.test.ts b/server/tests/integration/registry-reader-baseline-public-endpoints.test.ts index a4dcfef4a85..ee9e450ef08 100644 --- a/server/tests/integration/registry-reader-baseline-public-endpoints.test.ts +++ b/server/tests/integration/registry-reader-baseline-public-endpoints.test.ts @@ -25,6 +25,7 @@ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; import request from 'supertest'; import type { Pool } from 'pg'; +import type { WorkOSUser } from '../../src/types.js'; // Set WorkOS env before vi.mock factories run — auth.ts constructs WorkOS // at module load and the factory's vi.importActual triggers that load. @@ -46,7 +47,7 @@ vi.hoisted(() => { // matrix without rebuilding the route's dependency graph. const DEFAULT_TEST_USER_ID = 'user_test_registry_baseline_endpoints'; const authState = vi.hoisted(() => ({ - optAuthUser: null as { id: string; email: string } | null, + optAuthUser: null as WorkOSUser | null, requireAuthUser: { id: 'user_test_registry_baseline_endpoints', email: 'registry-baseline@test.com', @@ -87,7 +88,7 @@ vi.mock('../../src/middleware/auth.js', async () => { }; }); -function setOptAuthUser(user: { id: string; email: string } | null) { +function setOptAuthUser(user: WorkOSUser | null) { authState.optAuthUser = user; } @@ -114,9 +115,19 @@ vi.mock('../../src/billing/stripe-client.js', () => ({ createBillingPortalSession: vi.fn().mockResolvedValue(null), })); +// Scope fixtures use real exact-credential grants and snapshots in PostgreSQL. +// No direct WorkOS membership is needed for those grants to authorize access. +vi.mock('../../src/auth/workos-client.js', async importOriginal => ({ + ...await importOriginal(), + getAuthorizationEnforcementWorkos: () => ({ + userManagement: { listOrganizationMemberships: vi.fn().mockResolvedValue({ data: [] }) }, + }), +})); + import { HTTPServer } from '../../src/http.js'; import { initializeDatabase, closeDatabase } from '../../src/db/client.js'; import { runMigrations } from '../../src/db/migrate.js'; +import { loadAuthorizationSnapshot } from '../../src/db/user-authorization-snapshot-db.js'; import { FederatedIndexDatabase } from '../../src/db/federated-index-db.js'; import { FederatedIndexService } from '../../src/federated-index.js'; import { CrawlerService } from '../../src/crawler.js'; @@ -1225,8 +1236,7 @@ describe('Registry reader baseline — public endpoints', () => { ON CONFLICT (workos_user_id) DO UPDATE SET primary_organization_id = EXCLUDED.primary_organization_id`, [userId, `${userId}@example.com`, orgId], ); - // resolvePrimaryOrganization trusts the cached column only when a - // current organization_memberships row backs it. + // Keep legacy attribution populated; it must not supply tenant authority. await pool.query( `INSERT INTO organization_memberships (workos_user_id, workos_organization_id, role, email, created_at, updated_at) @@ -1234,6 +1244,12 @@ describe('Registry reader baseline — public endpoints', () => { ON CONFLICT (workos_user_id, workos_organization_id) DO NOTHING`, [userId, orgId, `${userId}@example.com`], ); + await pool.query( + `INSERT INTO organization_credential_grants + (workos_user_id, workos_organization_id, role, granted_by_workos_user_id, reason) + VALUES ($1, $2, 'member', $1, 'Scope matrix exact-credential fixture')`, + [userId, orgId], + ); } async function clearScopeFixtures() { @@ -1304,20 +1320,30 @@ describe('Registry reader baseline — public endpoints', () => { }); type CallerKind = 'anonymous' | 'owner' | 'other_api' | 'explorer'; - function setCaller(kind: CallerKind) { + async function setCaller(kind: CallerKind, selectOrganization = true) { if (kind === 'anonymous') return setOptAuthUser(null); const id = kind === 'owner' ? SCOPE_OWNER_USER : kind === 'other_api' ? SCOPE_OTHER_API_USER : SCOPE_EXPLORER_USER; - setOptAuthUser({ id, email: `${id}@example.com` }); + const organizationId = kind === 'owner' ? SCOPE_OWNER_ORG + : kind === 'other_api' ? SCOPE_OTHER_API_ORG + : SCOPE_EXPLORER_ORG; + const snapshot = await loadAuthorizationSnapshot(id, selectOrganization ? organizationId : null); + expect(snapshot).not.toBeNull(); + const user: WorkOSUser = { id: snapshot!.canonicalUserId, authWorkosUserId: id, email: `${id}@example.com` }; + Object.defineProperty(user, 'authorizationSnapshot', { value: snapshot }); + setOptAuthUser(user); } async function fetchAgents(scope: string | null, kind: CallerKind): Promise { - setCaller(kind); + await setCaller(kind); const qs = scope === null ? '' : `&scope=${encodeURIComponent(scope)}`; - const res = await request(app).get( + const pending = request(app).get( `/api/registry/operator?domain=${encodeURIComponent(SCOPE_DOMAIN)}${qs}`, ); + const organizationId = authState.optAuthUser?.authorizationSnapshot?.selectedOrganizationId; + if (organizationId) pending.set('X-Organization-Id', organizationId); + const res = await pending; expect(res.status).toBe(200); expect(res.body.agent_visibility_summary).toEqual({ public: 1, members_only: 1 }); return (res.body.agents as Array<{ url: string }>).map(a => a.url).sort(); @@ -1330,6 +1356,16 @@ describe('Registry reader baseline — public endpoints', () => { it('scope omitted, explorer (no API tier) → public only', async () => { expect(await fetchAgents(null, 'explorer')).toEqual([PUBLIC_URL]); }); + it('does not infer owner access without an explicit organization selection', async () => { + await setCaller('owner', false); + for (const scope of ['all', 'private']) { + const res = await request(app).get( + `/api/registry/operator?domain=${encodeURIComponent(SCOPE_DOMAIN)}&scope=${scope}`, + ); + expect(res.status).toBe(200); + expect(res.body.agents.map((agent: { url: string }) => agent.url)).toEqual(scope === 'all' ? [PUBLIC_URL] : []); + } + }); it('scope omitted, API-tier non-owner → public + members_only', async () => { expect(await fetchAgents(null, 'other_api')).toEqual( [MEMBERS_URL, PUBLIC_URL].sort(), @@ -1391,7 +1427,7 @@ describe('Registry reader baseline — public endpoints', () => { // ── validation ──────────────────────────────────────────────────── it('rejects unknown scope with 400', async () => { - setCaller('owner'); + await setCaller('owner'); const res = await request(app).get( `/api/registry/operator?domain=${encodeURIComponent(SCOPE_DOMAIN)}&scope=membr`, ); From 5c5cbac299980b2189139d5cbe3bbc893564fdc7 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 14 Sep 2026 01:59:21 +0000 Subject: [PATCH 4/5] test(auth): verify explicit cookie and terminal bearer composition --- server/src/middleware/auth.ts | 1 - .../src/routes/helpers/resolve-caller-org.ts | 4 +- .../tests/unit/caller-org-auth-errors.test.ts | 8 ++-- server/tests/unit/resolve-caller-org.test.ts | 38 +++++++---------- .../unit/sealed-organization-boundary.test.ts | 42 ++++++++++++++++++- 5 files changed, 60 insertions(+), 33 deletions(-) diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 9d900f1c01d..b4562a7d606 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -785,7 +785,6 @@ export function invalidateSessionsForUsers(workosUserIds: string[]): void { class InvalidAuthorizationCredentialError extends Error {} export { ConflictingOrganizationSelectionError, selectedOrganizationForAuthentication } from '../auth/organization-selection.js'; - /** Hydrate a new request object; never mutate the provider object in a cache. */ async function hydrateAuthenticatedUser( user: WorkOSUser, organizationId: string | null, diff --git a/server/src/routes/helpers/resolve-caller-org.ts b/server/src/routes/helpers/resolve-caller-org.ts index 94d050394ae..0ec5999ae69 100644 --- a/server/src/routes/helpers/resolve-caller-org.ts +++ b/server/src/routes/helpers/resolve-caller-org.ts @@ -21,8 +21,8 @@ import { getBearerToken, isWorkOSApiKeyFormat } from '../../middleware/api-key-f import { ConflictingOrganizationSelectionError, selectedOrganizationForAuthentication, - validateWorkOSApiKey, -} from '../../middleware/auth.js'; +} from '../../auth/organization-selection.js'; +import { validateWorkOSApiKey } from '../../middleware/auth.js'; import { isInvalidWorkOSJWTError, unavailableJWTKeyService, WorkOSJWTUnavailableError } from '../../auth/workos-jwt.js'; import { getAuthorizationEnforcementWorkos } from '../../auth/workos-client.js'; import { getOrganizationAuthorizationUserId, type OrgAuthorizationPrincipal } from '../../auth/organization-principal.js'; diff --git a/server/tests/unit/caller-org-auth-errors.test.ts b/server/tests/unit/caller-org-auth-errors.test.ts index 7a90baba61f..af833e154b5 100644 --- a/server/tests/unit/caller-org-auth-errors.test.ts +++ b/server/tests/unit/caller-org-auth-errors.test.ts @@ -171,7 +171,7 @@ it('does not let spoofed authorization headers replace an invalid selected beare expect(mocks.listAllAgents).not.toHaveBeenCalled(); }); -it.each([undefined, 'Basic dXNlcjpwYXNz'])('preserves cookie-only resolution when no Bearer is selected: %s', async (authorization) => { +it.each([undefined, 'Basic dXNlcjpwYXNz'])('denies cookie authority without explicit organization provenance: %s', async (authorization) => { mocks.issueDomainClaim.mockResolvedValue({ token: 'claim_cookie', lockedToOrgId: null }); const claim = request(application) .post('/api/properties/hosted/example.test/claim') @@ -180,9 +180,9 @@ it.each([undefined, 'Basic dXNlcjpwYXNz'])('preserves cookie-only resolution whe const response = await claim.send({}); - expect(response.status).toBe(200); - expect(mocks.query).toHaveBeenCalledWith(expect.stringContaining('primary_organization_id'), ['cookie_user']); - expect(mocks.issueDomainClaim).toHaveBeenCalledWith('example.test', 'org_cookie'); + expect(response.status).toBe(403); + expect(mocks.query).not.toHaveBeenCalled(); + expect(mocks.issueDomainClaim).not.toHaveBeenCalled(); expect(mocks.jwtVerify).not.toHaveBeenCalled(); expect(mocks.validateApiKey).not.toHaveBeenCalled(); }); diff --git a/server/tests/unit/resolve-caller-org.test.ts b/server/tests/unit/resolve-caller-org.test.ts index a5ac06fc157..9f24df7726b 100644 --- a/server/tests/unit/resolve-caller-org.test.ts +++ b/server/tests/unit/resolve-caller-org.test.ts @@ -19,26 +19,8 @@ const validateWorkOSApiKeyMock = vi.fn(); const jwtVerifyMock = vi.fn(); const decodeJwtMock = vi.fn(); const dbQueryMock = vi.fn(); -const ConflictingOrganizationSelectionErrorMock = vi.hoisted(() => ( - class ConflictingOrganizationSelectionError extends Error {} -)); - -vi.mock('../../src/middleware/auth.js', () => ({ - ConflictingOrganizationSelectionError: ConflictingOrganizationSelectionErrorMock, - selectedOrganizationForAuthentication: (req: Record, providerOrg: string) => { - const headers = req.headers as Record | undefined; - const query = req.query as Record | undefined; - const body = req.body as Record | undefined; - const params = req.params as Record | undefined; - const supplied = [providerOrg, headers?.['x-organization-id'], query?.org, - query?.organization_id, query?.organizationId, body?.organization_id, - body?.organizationId, params?.orgId, params?.organizationId] - .filter((value) => value !== undefined); - if (new Set(supplied).size > 1) { - throw new ConflictingOrganizationSelectionErrorMock(); - } - return providerOrg; - }, +vi.mock('../../src/middleware/auth.js', async () => ({ + ...await import('../../src/auth/organization-selection.js'), validateWorkOSApiKey: (...args: unknown[]) => validateWorkOSApiKeyMock(...args), })); @@ -158,10 +140,13 @@ describe('resolveCallerOrgId', () => { it.each([ ['header', { headers: { 'x-organization-id': 'org_other' } }], ['query org', { query: { org: 'org_other' } }], + ['query org_id', { query: { org_id: 'org_other' } }], ['query organization_id', { query: { organization_id: 'org_other' } }], ['query organizationId', { query: { organizationId: 'org_other' } }], + ['body org_id', { body: { org_id: 'org_other' } }], ['body organization_id', { body: { organization_id: 'org_other' } }], ['body organizationId', { body: { organizationId: 'org_other' } }], + ['route org_id', { params: { org_id: 'org_other' } }], ['route orgId', { params: { orgId: 'org_other' } }], ['route organizationId', { params: { organizationId: 'org_other' } }], ] as const)('rejects an API-key provider conflict from %s without caller fallback', async (_location, selectors) => { @@ -310,6 +295,9 @@ describe('resolveCallerOrgId', () => { it.each([ ['headers', 'x-organization-id'], ['query', 'organizationId'], + ['query', 'org_id'], + ['body', 'org_id'], + ['params', 'org_id'], ['body', 'organizationId'], ['params', 'organizationId'], ] as const)('retains an original %s selector conflict across JWT verification', async (location, field) => { @@ -375,12 +363,14 @@ describe('resolveCallerOrgId', () => { expect(validateWorkOSApiKeyMock).not.toHaveBeenCalled(); }); - it('does not substitute raw bearer authority for an authenticated user missing a snapshot', async () => { - jwtVerifyMock.mockResolvedValue({ payload: { org_id: 'org_other' } }); + it('uses verified bearer authority independently of an attached cookie user without a snapshot', async () => { + decodeJwtMock.mockReturnValueOnce({ iss: ISS }); + jwtVerifyMock.mockResolvedValue({ payload: { org_id: 'org_bearer' } }); - expect(await resolveCallerOrgId(reqWith('Bearer eyJabc.def.ghi', { id: 'user_session' }))).toBeNull(); - expect(jwtVerifyMock).not.toHaveBeenCalled(); + expect(await resolveCallerOrgId(reqWith('Bearer eyJabc.def.ghi', { id: 'user_session' }))).toBe('org_bearer'); + expect(jwtVerifyMock).toHaveBeenCalledTimes(1); expect(validateWorkOSApiKeyMock).not.toHaveBeenCalled(); + expect(dbQueryMock).not.toHaveBeenCalled(); }); // ── Unauthenticated / malformed ──────────────────────────────── diff --git a/server/tests/unit/sealed-organization-boundary.test.ts b/server/tests/unit/sealed-organization-boundary.test.ts index 99c2a88416e..9a938680b20 100644 --- a/server/tests/unit/sealed-organization-boundary.test.ts +++ b/server/tests/unit/sealed-organization-boundary.test.ts @@ -92,7 +92,7 @@ async function authenticate(authenticatedUserId = 'user_primary', canonicalUserI } function post() { - return request(app()).post('/api/conformance/token').set('Authorization', 'Bearer sealed-session-fixture'); + return request(app()).post('/api/conformance/token').set('Cookie', 'wos-session=sealed-session-fixture'); } beforeEach(() => { @@ -127,6 +127,42 @@ afterAll(() => { else process.env.CONFORMANCE_JWT_SECRET = originalSecret; }); +describe.each([ + ['user_primary', 'user_linked'], + ['user_linked', 'user_primary'], +])('explicit Bearer with authenticated %s attributed to %s', (authenticated, canonical) => { + it.each([ + 'Bearer', + 'Bearer malformed opaque token', + 'Bearer sealed-session-fixture', + 'bEaReR sealed-session-fixture', + 'bEaReR\t sealed-session-fixture', + ])('rejects %s before using valid cookie authority in mounted consumers', async authorization => { + await authenticate(authenticated, canonical); + const mirror = mirrorApp(); + const responses = [ + await post().set('Authorization', authorization).set('X-Organization-Id', ORG), + await request(mirror).get('/api/registry/mirror-proposals') + .set('Cookie', 'wos-session=sealed-session-fixture') + .set('Authorization', authorization).set('X-Organization-Id', ORG), + await request(mirror).get(`/api/registry/mirror-proposals/${PROPOSAL_ID}`) + .set('Cookie', 'wos-session=sealed-session-fixture') + .set('Authorization', authorization).set('X-Organization-Id', ORG), + ]; + for (const response of responses) { + expect(response.status).toBe(401); + expect(response.body).toEqual({ error: 'invalid_bearer_token' }); + } + expect(mocks.boundedQuery).not.toHaveBeenCalled(); + expect(mocks.memberships).not.toHaveBeenCalled(); + expect(mocks.listProposals).not.toHaveBeenCalled(); + expect(mocks.getProposal).not.toHaveBeenCalled(); + expect(mocks.getClient).not.toHaveBeenCalled(); + expect(mocks.isAdmin).not.toHaveBeenCalled(); + expect(mocks.isModerator).not.toHaveBeenCalled(); + }); +}); + describe.each([ ['user_primary', 'user_linked'], ['user_linked', 'user_primary'], @@ -305,11 +341,13 @@ describe('sealed-session organization boundary on POST /api/conformance/token', expect(mocks.boundedQuery).not.toHaveBeenCalled(); }); - it.each(['header', 'query', 'body', 'org_id alias'])('rejects a conflicting %s selector before provider authorization', async location => { + it.each(['header', 'query', 'query organizationId', 'query org_id', 'body', 'org_id alias'])('rejects a conflicting %s selector before provider authorization', async location => { await authenticate(); const pending = post(); if (location === 'header') pending.set('X-Organization-Id', OTHER_ORG); if (location === 'query') pending.query({ organization_id: OTHER_ORG }); + if (location === 'query organizationId') pending.query({ organizationId: OTHER_ORG }); + if (location === 'query org_id') pending.query({ org_id: OTHER_ORG }); if (location === 'body') pending.send({ organizationId: OTHER_ORG }); if (location === 'org_id alias') pending.send({ org_id: OTHER_ORG }); const response = await pending; From 9941963a8c57381221eadeb5008ff8a52345265d Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 17 Sep 2026 18:18:26 +0000 Subject: [PATCH 5/5] test(auth): cover mirror manager principal composition --- .../tests/integration/community-mirrors.test.ts | 16 +++++++++++++++- .../unit/sealed-organization-boundary.test.ts | 3 ++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/server/tests/integration/community-mirrors.test.ts b/server/tests/integration/community-mirrors.test.ts index 1bed2312f74..f283843371d 100644 --- a/server/tests/integration/community-mirrors.test.ts +++ b/server/tests/integration/community-mirrors.test.ts @@ -15,6 +15,7 @@ vi.hoisted(() => { // manager access; organization API keys submit and read their own proposals. const authState = vi.hoisted(() => ({ userId: 'admin_api_key', + authWorkosUserId: undefined as string | undefined, email: 'mirrors@test.com', organizationId: null as string | null, })); @@ -22,7 +23,7 @@ const authState = vi.hoisted(() => ({ vi.mock('../../src/middleware/auth.js', async () => { const actual = await vi.importActual>('../../src/middleware/auth.js'); const pass = (req: { user: unknown; apiKey?: unknown }, _res: unknown, next: () => void) => { - req.user = { id: authState.userId, email: authState.email }; + req.user = { id: authState.userId, authWorkosUserId: authState.authWorkosUserId, email: authState.email }; req.apiKey = authState.organizationId ? { organizationId: authState.organizationId } : undefined; next(); }; @@ -205,6 +206,7 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( notifyCommunityMirrorProposalReviewed.mockReset(); notifyCommunityMirrorProposalReviewed.mockResolvedValue(undefined); authState.userId = 'admin_api_key'; + authState.authWorkosUserId = undefined; authState.email = 'mirrors@test.com'; authState.organizationId = null; await clear(); @@ -376,6 +378,18 @@ describe('Community-mirror lifecycle — /api/registry/mirrors + /translated', ( expect((await pool.query('SELECT 1 FROM community_mirrors WHERE platform = $1', [PLATFORM])).rows).toHaveLength(0); }); + it('does not let a canonical identity replace the exact mirror manager principal', async () => { + authState.userId = 'admin_api_key'; + authState.authWorkosUserId = 'user_exact_non_manager'; + isRegistryModerator.mockResolvedValue(true); + isWebUserAAOAdmin.mockResolvedValue(true); + + const queue = await request(app).get('/api/registry/mirror-proposals?review_queue=true'); + expect(queue.status).toBe(403); + const deletion = await request(app).delete(`/api/registry/mirrors/${PLATFORM}`); + expect(deletion.status).toBe(403); + }); + it('re-publish is idempotent — updates in place, no duplicate row', async () => { await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody({ catalog_etag: 'v1' })); await request(app).put(`/api/registry/mirrors/${PLATFORM}`).send(publishBody({ catalog_etag: 'v2' })); diff --git a/server/tests/unit/sealed-organization-boundary.test.ts b/server/tests/unit/sealed-organization-boundary.test.ts index 9a938680b20..17b02ac1c63 100644 --- a/server/tests/unit/sealed-organization-boundary.test.ts +++ b/server/tests/unit/sealed-organization-boundary.test.ts @@ -82,6 +82,7 @@ async function authenticate(authenticatedUserId = 'user_primary', canonicalUserI row.authenticated_user_id = authenticatedUserId; row.canonical_user_id = canonicalUserId; const snapshot = await loadAuthorizationSnapshot(authenticatedUserId, organizationId); + expect(snapshot).not.toBeNull(); mocks.principal = { id: canonicalUserId, authWorkosUserId: authenticatedUserId, @@ -100,7 +101,7 @@ beforeEach(() => { process.env.CONFORMANCE_JWT_SECRET = 'sealed-organization-test-secret'; mocks.principal = undefined; row = { - in_recovery: false, + in_recovery: false, terminal_marker: false, primary_count: '1', authenticated_user_id: 'user_primary', canonical_user_id: 'user_linked', identity_id: 'identity_linked', authorization_epoch: '7', email: 'sam@pinnacle.example', email_verified: true, first_name: 'Sam', last_name: 'Adeyemi',