From 4af8b9d8d11e7d1910cde090b2ce93069d3472ad Mon Sep 17 00:00:00 2001 From: Cas Lubbers Date: Wed, 19 Aug 2026 17:00:56 +0200 Subject: [PATCH 1/2] feat: add namespace access validation for sealed secret routes --- package-lock.json | 37 ++++++++++++++++--- src/api/namespace-access.ts | 10 +++++ .../namespaces/{namespace}/sealedsecrets.ts | 3 ++ .../sealedsecrets/{sealedSecretName}.ts | 5 +++ src/api/v2/sealedsecrets.ts | 11 +++++- src/middleware/authz.ts | 13 ++++++- src/otomi-models.ts | 4 ++ 7 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 src/api/namespace-access.ts diff --git a/package-lock.json b/package-lock.json index 52bfbe3fe..729e2880c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -233,6 +233,7 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -2751,6 +2752,7 @@ "integrity": "sha512-kLgLShnWADDVreKC63pBrWkcvxgZzFIfO34Jhx/SWfuOIA3cD8AXT+HjyuLfoGJ7mUb58hv2kUziKzEy4INb1w==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=22.18.0" } @@ -2832,7 +2834,8 @@ "resolved": "https://registry.npmjs.org/@cspell/dict-css/-/dict-css-4.1.2.tgz", "integrity": "sha512-+ylGoKdwZ2sVOCOnU2Eq5wDZx+RaVX3HoKyNHGGsFvhSw6IidQ6tH/mAPKBDofViHJoWCPNlklE0lTr6MDG3QA==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@cspell/dict-dart": { "version": "2.3.2", @@ -2972,14 +2975,16 @@ "resolved": "https://registry.npmjs.org/@cspell/dict-html/-/dict-html-4.0.15.tgz", "integrity": "sha512-GJYnYKoD9fmo2OI0aySEGZOjThnx3upSUvV7mmqUu8oG+mGgzqm82P/f7OqsuvTaInZZwZbo+PwJQd/yHcyFIw==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@cspell/dict-html-symbol-entities": { "version": "4.0.5", "resolved": "https://registry.npmjs.org/@cspell/dict-html-symbol-entities/-/dict-html-symbol-entities-4.0.5.tgz", "integrity": "sha512-429alTD4cE0FIwpMucvSN35Ld87HCyuM8mF731KU5Rm4Je2SG6hmVx7nkBsLyrmH3sQukTcr1GaiZsiEg8svPA==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@cspell/dict-java": { "version": "5.0.12", @@ -3177,7 +3182,8 @@ "resolved": "https://registry.npmjs.org/@cspell/dict-typescript/-/dict-typescript-3.2.3.tgz", "integrity": "sha512-zXh1wYsNljQZfWWdSPYwQhpwiuW0KPW1dSd8idjMRvSD0aSvWWHoWlrMsmZeRl4qM4QCEAjua8+cjflm41cQBg==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@cspell/dict-vue": { "version": "3.0.5", @@ -4404,6 +4410,7 @@ "integrity": "sha512-ZbuY4cmXC8DkxYjfvT2DbcHWL2T6vmsMhXCDcmTB2T0y0gaezBI77ufq5ZAIdcRkYZ7NEQEDg1xFeKbxUJ5v5Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@jest/environment": "30.4.1", "@jest/expect": "30.4.1", @@ -4938,6 +4945,7 @@ "integrity": "sha512-DhGl4xMVFGVIyMwswXeyzdL4uXD5OGILGX5N8Y+f6W7LhC1Ze2poSNrkF/fedpVDHEEZ+PHFW0vL14I+mm8K3Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.3", @@ -6436,7 +6444,8 @@ "version": "7.0.15", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.15.tgz", "integrity": "sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@types/jsonfile": { "version": "6.1.4", @@ -6511,6 +6520,7 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-24.12.2.tgz", "integrity": "sha512-A1sre26ke7HDIuY/M23nd9gfB+nrmhtYyMINbjI1zHJxYteKR6qSMX56FsmjMcDb3SMcjJg5BiRRgOCC/yBD0g==", "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -6684,6 +6694,7 @@ "integrity": "sha512-X6ypGChaWYk6PBtUg2BwuTZEFFcHJAtGTVJ9/lCTOufhZ4i9fNolQNnktq+kkMCwMj7V8Svsq7+TxSDslmhE0g==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.66.0", "@typescript-eslint/types": "8.66.0", @@ -7252,6 +7263,7 @@ "integrity": "sha512-lGq+9yr1/GuAWaVYIHRjvvySG5/4VfKIvC8EWxStPdcDh/Ka7FG3twP6v4d5BkravUilhIAsG4Qj83t02LWUPQ==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -8143,6 +8155,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -9392,6 +9405,7 @@ "integrity": "sha512-hr4ihw+DBqcvrsEDioRO31Z17x71pUYoNe/4h6Z0wB72p7MU7/9gH8Q3s12NFhHPfYBBOV3qyfUxmr/Yn3shnQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "env-paths": "^2.2.1", "import-fresh": "^3.3.0", @@ -10876,6 +10890,7 @@ "integrity": "sha512-nuKKvN+oIBO0koN7Tm7dlkmnkc21mtt0QJLwAKzjLq14y6lRTdVG36MZHJ8eQHwdJMwZbQNMlPOYedMq/oVJvQ==", "dev": true, "license": "MIT", + "peer": true, "workspaces": [ "packages/*" ], @@ -11371,6 +11386,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -11457,6 +11473,7 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz", "integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==", "license": "MIT", + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", @@ -14120,6 +14137,7 @@ "integrity": "sha512-Yi1jqNC/Oq0N4hBgNH/YvBpP1P57QqundgytzYqy3yqAa7NZPNjSoi4SGbRAXDMdBzNE6xBCi5U7RgfrvMEUVQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@jest/core": "30.4.2", "@jest/types": "30.4.1", @@ -15010,6 +15028,7 @@ "resolved": "https://registry.npmjs.org/jsep/-/jsep-1.4.0.tgz", "integrity": "sha512-B7qPcEVE3NVkmSJbaYxvv4cHkVW7DQsZz13pUMrfS8z8Q/BuShN+gcTXrUlPiGqM2/t/EEaI030bpxMqY8gMlw==", "license": "MIT", + "peer": true, "engines": { "node": ">= 10.16.0" } @@ -15891,6 +15910,7 @@ "integrity": "sha512-8dD6FusOQSrpv9Z1rdNMdlSgQOIP880DHqnohobOmYLElGEqAL/JvxvuxZO16r4HtjTlfPRDC1hbvxC9dPN2nA==", "dev": true, "license": "MIT", + "peer": true, "bin": { "marked": "bin/marked.js" }, @@ -19145,6 +19165,7 @@ "dev": true, "inBundle": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -20329,6 +20350,7 @@ "integrity": "sha512-OpN0zzVdiaiAhxpuuj5efpIS4sY9j7bY6uR5mnj5yPzGkdkjNKSJeUThPb60Jw29QuAZgA4o+/iB49kFiaBX6g==", "dev": true, "license": "MIT", + "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -21466,6 +21488,7 @@ "integrity": "sha512-bxve7csK0/Txr++CkfrmV+X1r4jqiSOw2WsSad9E2S68R+ZfLBwDn8IceM8WfiOmKQIHgsQc1cNA8Dzg7U75pg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@semantic-release/commit-analyzer": "^13.0.1", "@semantic-release/error": "^4.0.0", @@ -23576,6 +23599,7 @@ "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -23786,6 +23810,7 @@ "integrity": "sha512-f0FFpIdcHgn8zcPSbf1dRevwt047YMnaiJM3u2w2RewrB+fob/zePZcrOyQoLMMO7aBIddLcQIEK5dYjkLnGrQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@cspotcode/source-map-support": "^0.8.0", "@tsconfig/node10": "^1.0.7", @@ -24049,6 +24074,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -24661,6 +24687,7 @@ "version": "7.5.7", "resolved": "https://registry.npmjs.org/ws/-/ws-7.5.7.tgz", "integrity": "sha512-KMvVuFzpKBuiIXW3E4u3mySRO2/mCHSyZDJQM5NQ9Q9KHWHWh0NHgfbRMLLrceUK5qAL4ytALJbpRMjixFZh8A==", + "peer": true, "engines": { "node": ">=8.3.0" }, diff --git a/src/api/namespace-access.ts b/src/api/namespace-access.ts new file mode 100644 index 000000000..686ccfa31 --- /dev/null +++ b/src/api/namespace-access.ts @@ -0,0 +1,10 @@ +import { HttpError } from 'src/error' +import { SessionUser } from 'src/otomi-models' + +export function assertNamespaceAccess(namespace: string, user: SessionUser): void { + if (user.isPlatformAdmin) return + const teamId = namespace.replace(/^team-/, '') + if (!namespace.startsWith('team-') || !user.teams.includes(teamId)) { + throw new HttpError(403, `User not allowed to access namespace "${namespace}"`) + } +} diff --git a/src/api/v2/namespaces/{namespace}/sealedsecrets.ts b/src/api/v2/namespaces/{namespace}/sealedsecrets.ts index d77d9f0a5..7e006dd8f 100644 --- a/src/api/v2/namespaces/{namespace}/sealedsecrets.ts +++ b/src/api/v2/namespaces/{namespace}/sealedsecrets.ts @@ -1,5 +1,6 @@ import Debug from 'debug' import { Response } from 'express' +import { assertNamespaceAccess } from 'src/api/namespace-access' import { ensureStatus } from 'src/api/response-utils' import { OpenApiRequestExt, SealedSecretManifestRequest } from 'src/otomi-models' @@ -11,6 +12,7 @@ const debug = Debug('otomi:api:v2:namespaces:sealedsecrets') */ export const getAplNamespaceSealedSecrets = async (req: OpenApiRequestExt, res: Response): Promise => { const { namespace } = req.params + assertNamespaceAccess(namespace, req.user) debug(`getAplNamespaceSealedSecrets(${namespace}, ...)`) const v = await req.otomi.getAplNamespaceSealedSecrets(namespace) res.json(v.map((secret) => ensureStatus(secret))) @@ -22,6 +24,7 @@ export const getAplNamespaceSealedSecrets = async (req: OpenApiRequestExt, res: */ export const createAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: Response): Promise => { const { namespace } = req.params + assertNamespaceAccess(namespace, req.user) debug(`createNamespaceSealedSecret(${namespace}, ...)`) const v = await req.otomi.createAplNamespaceSealedSecret(namespace, req.body as SealedSecretManifestRequest) res.json(ensureStatus(v)) diff --git a/src/api/v2/namespaces/{namespace}/sealedsecrets/{sealedSecretName}.ts b/src/api/v2/namespaces/{namespace}/sealedsecrets/{sealedSecretName}.ts index b5e476369..694023d8e 100644 --- a/src/api/v2/namespaces/{namespace}/sealedsecrets/{sealedSecretName}.ts +++ b/src/api/v2/namespaces/{namespace}/sealedsecrets/{sealedSecretName}.ts @@ -1,5 +1,6 @@ import Debug from 'debug' import { Response } from 'express' +import { assertNamespaceAccess } from 'src/api/namespace-access' import { ensureStatus } from 'src/api/response-utils' import { DeepPartial, OpenApiRequestExt, SealedSecretManifestRequest } from 'src/otomi-models' @@ -11,6 +12,7 @@ const debug = Debug('otomi:api:v2:teams:sealedsecrets') */ export const getAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: Response): Promise => { const { namespace, sealedSecretName } = req.params + assertNamespaceAccess(namespace, req.user) debug(`getSealedSecret(${sealedSecretName}) for namespace(${namespace})`) const data = await req.otomi.getAplNamespaceSealedSecret(namespace, sealedSecretName) res.json(ensureStatus(data)) @@ -22,6 +24,7 @@ export const getAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: R */ export const editAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: Response): Promise => { const { namespace, sealedSecretName } = req.params + assertNamespaceAccess(namespace, req.user) debug(`editSealedSecret(${sealedSecretName}) for namespace(${namespace})`) const data = await req.otomi.editAplNamespaceSealedSecret( namespace, @@ -37,6 +40,7 @@ export const editAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: */ export const patchAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: Response): Promise => { const { namespace, sealedSecretName } = req.params + assertNamespaceAccess(namespace, req.user) debug(`editSealedSecret(${sealedSecretName} for namespace(${namespace}), patch)`) const data = await req.otomi.editAplNamespaceSealedSecret( namespace, @@ -53,6 +57,7 @@ export const patchAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: */ export const deleteAplNamespaceSealedSecret = async (req: OpenApiRequestExt, res: Response): Promise => { const { namespace, sealedSecretName } = req.params + assertNamespaceAccess(namespace, req.user) debug(`deleteSealedSecret(${sealedSecretName}) for namespace(${namespace})`) await req.otomi.deleteAplNamespaceSealedSecret(namespace, sealedSecretName) res.status(200).end() diff --git a/src/api/v2/sealedsecrets.ts b/src/api/v2/sealedsecrets.ts index e1d714a7c..bc8c999ba 100644 --- a/src/api/v2/sealedsecrets.ts +++ b/src/api/v2/sealedsecrets.ts @@ -10,6 +10,15 @@ const debug = Debug('otomi:api:v2:sealedsecrets') */ export const getAllAplSealedSecrets = (req: OpenApiRequestExt, res: Response): void => { debug('getAllSealedSecrets') - const v = req.otomi.getAllAplSealedSecrets() + const all = req.otomi.getAllAplSealedSecrets() + // Defense in depth: even if the ACL model is misconfigured, never hand a non-platformAdmin + // caller another team's secrets from this cross-team collection endpoint. + const v = req.user.isPlatformAdmin + ? all + : all.filter((secret) => { + const teamId = + secret.metadata.namespace?.replace(/^team-/, '') ?? (secret.metadata.labels?.['apl.io/teamId'] as string) + return teamId && req.user.teams.includes(teamId) + }) res.json(v) } diff --git a/src/middleware/authz.ts b/src/middleware/authz.ts index dbec77451..6180c071d 100644 --- a/src/middleware/authz.ts +++ b/src/middleware/authz.ts @@ -17,8 +17,19 @@ const HttpMethodMapping: Record = { */ export function authorize(req: OpenApiRequestExt, authz: Authz): void { const { body, user } = req + const schema = req.openapi?.schema + + // Only trust query/body input for the authz subject when this specific operation's + // OpenAPI schema declares that field. + const queryTeamIdDeclared = schema?.parameters?.some((p) => p.name === 'teamId' && p.in === 'query') ?? false + const bodyTeamIdDeclared = !!schema?.requestBody?.content?.['application/json']?.schema?.properties?.teamId + // express-openapi-validator stores path params in req.openapi.pathParams - const teamId = req.openapi?.pathParams?.teamId ?? req.params?.teamId ?? req.query?.teamId ?? body?.teamId + const teamId = + req.openapi?.pathParams?.teamId ?? + req.params?.teamId ?? + (queryTeamIdDeclared ? req.query?.teamId : undefined) ?? + (bodyTeamIdDeclared ? body?.teamId : undefined) const action = HttpMethodMapping[req.method] // Get x-aclSchema from req.openapi.schema (set by express-openapi-validator) diff --git a/src/otomi-models.ts b/src/otomi-models.ts index 783f0d30e..20c07ffe0 100644 --- a/src/otomi-models.ts +++ b/src/otomi-models.ts @@ -185,6 +185,10 @@ export interface OpenApiRequest extends Request { security?: any[] operationId?: string 'x-aclSchema'?: string + // Resolved OpenAPI parameter definitions for this operation (path/query/etc.) + parameters?: Array<{ name: string; in: string }> + // Resolved OpenAPI requestBody schema for this operation, if any + requestBody?: { content?: { 'application/json'?: { schema?: { properties?: Record } } } } } // Path parameters parsed from the URL (e.g., {teamId} -> pathParams.teamId) pathParams?: Record From 435233055adcf293e4ccaedaa9a04bf15aca5d1c Mon Sep 17 00:00:00 2001 From: Cas Lubbers Date: Wed, 19 Aug 2026 17:23:34 +0200 Subject: [PATCH 2/2] feat: add authz tests --- src/api-v2.authz.test.ts | 177 ++++++++++++++++++++++++++++++++++++ src/api/v2/sealedsecrets.ts | 9 +- src/middleware/authz.ts | 11 ++- 3 files changed, 191 insertions(+), 6 deletions(-) diff --git a/src/api-v2.authz.test.ts b/src/api-v2.authz.test.ts index 272204a6a..8e7f5a129 100644 --- a/src/api-v2.authz.test.ts +++ b/src/api-v2.authz.test.ts @@ -79,6 +79,34 @@ const mockSealedSecretResource = withStatus({ }, }) +const mockSealedSecretResourceTeam2 = withStatus({ + kind: 'SealedSecret', + metadata: { + name: 'team2-secret', + labels: { 'apl.io/teamId': 'team2' }, + }, + spec: { + encryptedData: { key: 'value' }, + template: { + type: 'kubernetes.io/opaque', + }, + }, +}) + +const mockOperatorSealedSecretResource = withStatus({ + kind: 'SealedSecret', + metadata: { + name: 'otomi-secrets', + namespace: 'apl-secrets', + }, + spec: { + encryptedData: { adminPassword: 'value' }, + template: { + type: 'kubernetes.io/opaque', + }, + }, +}) + const mockCodeRepoResource = withStatus( createNamedTeamResource('AplTeamCodeRepo', 'my-repo', 'team1', { gitService: 'github', @@ -318,6 +346,13 @@ describe('API V2 authz tests', () => { jest.spyOn(otomiStack, 'getAllAplSealedSecrets').mockReturnValue([mockSealedSecretResource] as any) jest.spyOn(otomiStack, 'getAplSealedSecrets').mockReturnValue([mockSealedSecretResource] as any) + // Namespace-scoped sealed secrets (operator-owned + arbitrary namespace routes) + jest.spyOn(otomiStack, 'getAplNamespaceSealedSecrets').mockReturnValue([mockOperatorSealedSecretResource] as any) + jest.spyOn(otomiStack, 'getAplNamespaceSealedSecret').mockResolvedValue(mockOperatorSealedSecretResource as any) + jest.spyOn(otomiStack, 'createAplNamespaceSealedSecret').mockResolvedValue(mockOperatorSealedSecretResource as any) + jest.spyOn(otomiStack, 'editAplNamespaceSealedSecret').mockResolvedValue(mockOperatorSealedSecretResource as any) + jest.spyOn(otomiStack, 'deleteAplNamespaceSealedSecret').mockResolvedValue(undefined as any) + jest.spyOn(otomiStack, 'createAplCodeRepo').mockResolvedValue(mockCodeRepoResource as any) jest.spyOn(otomiStack, 'getAplCodeRepo').mockReturnValue(mockCodeRepoResource as any) jest.spyOn(otomiStack, 'editAplCodeRepo').mockResolvedValue(mockCodeRepoResource as any) @@ -868,6 +903,148 @@ describe('API V2 authz tests', () => { }) }) + describe('V2 Cross-Tenant Authz Bypass Regression', () => { + const secretData = { + kind: 'SealedSecret', + metadata: { name: 'test-secret' }, + spec: { encryptedData: { key: 'value' }, template: { type: 'kubernetes.io/opaque' } }, + } + + describe('GET /v2/sealedsecrets (cross-team collection)', () => { + beforeEach(() => { + jest + .spyOn(otomiStack, 'getAllAplSealedSecrets') + .mockReturnValue([mockSealedSecretResource, mockSealedSecretResourceTeam2] as any) + }) + + test('team member cannot use the cross-team collection endpoint at all (requires platformAdmin read-any; use /v2/teams/{teamId}/sealedsecrets instead)', async () => { + await agent.get('/v2/sealedsecrets').set('Authorization', `Bearer ${teamMemberToken}`).expect(403) + }) + + test('platform admin sees all teams secrets', async () => { + const res = await agent + .get('/v2/sealedsecrets') + .set('Authorization', `Bearer ${platformAdminToken}`) + .expect(200) + expect(res.body).toHaveLength(2) + }) + + test('rejects forged teamId via query string (undeclared param for this operation)', async () => { + await agent.get('/v2/sealedsecrets?teamId=team1').set('Authorization', `Bearer ${team2MemberToken}`).expect(403) + }) + + test('rejects forged teamId via JSON body on a GET (undeclared requestBody for this operation)', async () => { + await agent + .get('/v2/sealedsecrets') + .send({ teamId: 'team1' }) + .set('Authorization', `Bearer ${team2MemberToken}`) + .expect(403) + }) + }) + + describe('/v2/namespaces/{namespace}/sealedsecrets (namespace not tied to caller teams)', () => { + test('team member can access their own team namespace', async () => { + await agent + .get('/v2/namespaces/team-team1/sealedsecrets') + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(200) + }) + + test('team member cannot read another teams namespace', async () => { + await agent + .get('/v2/namespaces/team-team2/sealedsecrets') + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('team member cannot read the operator-owned apl-secrets namespace', async () => { + await agent + .get('/v2/namespaces/apl-secrets/sealedsecrets') + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('team member cannot create a sealed secret in another teams namespace', async () => { + await agent + .post('/v2/namespaces/team-team2/sealedsecrets') + .send(secretData) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('team member cannot create a sealed secret in the operator-owned apl-secrets namespace', async () => { + await agent + .post('/v2/namespaces/apl-secrets/sealedsecrets') + .send(secretData) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('team member cannot create a sealed secret in the operator-owned apl-users namespace (privilege escalation path)', async () => { + await agent + .post('/v2/namespaces/apl-users/sealedsecrets') + .send(secretData) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('team member cannot edit a sealed secret in the operator-owned apl-secrets namespace', async () => { + await agent + .put('/v2/namespaces/apl-secrets/sealedsecrets/otomi-secrets') + .send(secretData) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('team member cannot delete a sealed secret in the operator-owned apl-secrets namespace, even with a forged body', async () => { + // forged body must not bypass authz + await agent + .delete('/v2/namespaces/apl-secrets/sealedsecrets/otomi-secrets') + .send({ teamId: 'team1' }) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('platform admin can access any team namespace', async () => { + await agent + .get('/v2/namespaces/team-team2/sealedsecrets') + .set('Authorization', `Bearer ${platformAdminToken}`) + .expect(200) + }) + + // this route has no teamId param at all; query/body teamId must not override the namespace check + test('forged ?teamId query cannot unlock a foreign namespace', async () => { + await agent + .get('/v2/namespaces/team-team2/sealedsecrets?teamId=team1') + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('forged ?teamId query cannot unlock the operator-owned namespace', async () => { + await agent + .get('/v2/namespaces/apl-secrets/sealedsecrets?teamId=team1') + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('forged teamId in body cannot unlock a foreign namespace on create', async () => { + await agent + .post('/v2/namespaces/team-team2/sealedsecrets') + .send({ ...secretData, teamId: 'team1' }) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + + test('forged teamId in body cannot unlock the operator-owned namespace on create', async () => { + await agent + .post('/v2/namespaces/apl-secrets/sealedsecrets') + .send({ ...secretData, teamId: 'team1' }) + .set('Authorization', `Bearer ${teamMemberToken}`) + .expect(403) + }) + }) + }) + describe('V2 Code Repository Endpoints', () => { const repoData = createTeamResource('AplTeamCodeRepo', { gitService: 'github', diff --git a/src/api/v2/sealedsecrets.ts b/src/api/v2/sealedsecrets.ts index bc8c999ba..c3755d525 100644 --- a/src/api/v2/sealedsecrets.ts +++ b/src/api/v2/sealedsecrets.ts @@ -11,13 +11,14 @@ const debug = Debug('otomi:api:v2:sealedsecrets') export const getAllAplSealedSecrets = (req: OpenApiRequestExt, res: Response): void => { debug('getAllSealedSecrets') const all = req.otomi.getAllAplSealedSecrets() - // Defense in depth: even if the ACL model is misconfigured, never hand a non-platformAdmin - // caller another team's secrets from this cross-team collection endpoint. + // Defense in depth: filter even if ACL is misconfigured. const v = req.user.isPlatformAdmin ? all : all.filter((secret) => { - const teamId = - secret.metadata.namespace?.replace(/^team-/, '') ?? (secret.metadata.labels?.['apl.io/teamId'] as string) + const namespace = secret.metadata.namespace + const teamId = namespace?.startsWith('team-') + ? namespace.slice('team-'.length) + : (secret.metadata.labels?.['apl.io/teamId'] as string) return teamId && req.user.teams.includes(teamId) }) res.json(v) diff --git a/src/middleware/authz.ts b/src/middleware/authz.ts index 6180c071d..3ea76c74e 100644 --- a/src/middleware/authz.ts +++ b/src/middleware/authz.ts @@ -19,15 +19,22 @@ export function authorize(req: OpenApiRequestExt, authz: Authz): void { const { body, user } = req const schema = req.openapi?.schema - // Only trust query/body input for the authz subject when this specific operation's - // OpenAPI schema declares that field. + // Only trust query/body teamId if this operation's schema actually declares it. const queryTeamIdDeclared = schema?.parameters?.some((p) => p.name === 'teamId' && p.in === 'query') ?? false const bodyTeamIdDeclared = !!schema?.requestBody?.content?.['application/json']?.schema?.properties?.teamId + // Namespace routes key on `namespace` (team-{teamId} convention), not `teamId`. + const namespaceParam = req.openapi?.pathParams?.namespace ?? req.params?.namespace + const namespaceTeamId = + typeof namespaceParam === 'string' && namespaceParam.startsWith('team-') + ? namespaceParam.slice('team-'.length) + : undefined + // express-openapi-validator stores path params in req.openapi.pathParams const teamId = req.openapi?.pathParams?.teamId ?? req.params?.teamId ?? + namespaceTeamId ?? (queryTeamIdDeclared ? req.query?.teamId : undefined) ?? (bodyTeamIdDeclared ? body?.teamId : undefined) const action = HttpMethodMapping[req.method]