diff --git a/packages/spacecat-shared-http-utils/src/auth/read-only-admin-wrapper.js b/packages/spacecat-shared-http-utils/src/auth/read-only-admin-wrapper.js index 29102870c..75b687e03 100644 --- a/packages/spacecat-shared-http-utils/src/auth/read-only-admin-wrapper.js +++ b/packages/spacecat-shared-http-utils/src/auth/read-only-admin-wrapper.js @@ -15,7 +15,11 @@ import { isObject } from '@adobe/spacecat-shared-utils'; import { LaunchDarklyClient } from '@adobe/spacecat-shared-launchdarkly-client'; import { FF_READ_ONLY_ORG } from './constants.js'; -import { guardNonEmptyRouteCapabilities, resolveRouteCapability } from './route-utils.js'; +import { + extractRouteParams, + guardNonEmptyRouteCapabilities, + resolveRouteCapability, +} from './route-utils.js'; function forbidden(message) { return new Response(JSON.stringify({ message }), { @@ -24,6 +28,74 @@ function forbidden(message) { }); } +/** + * Checks whether the authenticated read-only admin user owns the resource identified + * by the path parameters or request body. Ownership is determined by whether the user's + * IMS org matches the organization that owns the referenced site or organization entity. + * + * ID resolution order: + * 1. Named path params (e.g. :siteId, :organizationId) — always preferred. + * 2. context.data (parsed request body) — used only when the matched route has no path + * params at all (e.g. POST /preflight/jobs passes siteId in the body). Requires that + * the body-parser wrapper runs before readOnlyAdminWrapper in the .with() chain. + * + * Fail-closed: returns false if dataAccess is unavailable, the entity is not found, + * no recognizable ID can be resolved, or any lookup throws. + * + * @param {Object} context - Universal context (must have context.dataAccess and context.log) + * @param {Object} authInfo - The AuthInfo instance for the current user + * @param {Object} params - Named path params extracted from the route pattern + * @returns {Promise} + */ +async function isOwnerOfResource(context, authInfo, params) { + const { dataAccess, log } = context; + if (!dataAccess) { + log.error({ tag: 'ro-admin' }, 'isOwnerOfResource: dataAccess not on context — ensure dataAccessWrapper runs before readOnlyAdminWrapper'); + return false; + } + + // Body fallback only when the route carries no path params at all (e.g. POST /preflight/jobs). + // When params has keys, the route does have path identifiers; relying on body data in that + // case could allow a caller to spoof ownership by naming params differently than :siteId. + const hasPathParams = Object.keys(params).length > 0; + const siteId = hasPathParams ? params.siteId : (params.siteId ?? context.data?.siteId); + const organizationId = hasPathParams + ? params.organizationId + : (params.organizationId ?? context.data?.organizationId); + + try { + if (siteId) { + const site = await dataAccess.Site?.findById(siteId); + if (!site) { + return false; + } + const org = await site.getOrganization(); + if (!org) { + return false; + } + return authInfo.hasOrganization(org.getImsOrgId()); + } + + if (organizationId) { + const org = await dataAccess.Organization?.findById(organizationId); + if (!org) { + return false; + } + return authInfo.hasOrganization(org.getImsOrgId()); + } + } catch (err) { + log.error({ tag: 'ro-admin', err }, 'Error checking resource ownership for RO admin'); + return false; + } + + log.warn({ + tag: 'ro-admin', + method: context.pathInfo?.method, + suffix: context.pathInfo?.suffix, + }, 'isOwnerOfResource: no siteId or organizationId found in path params or context.data'); + return false; +} + /** * Evaluates the read-only admin feature flag for the authenticated user's IMS org. * Uses {@link AuthInfo#getTenantIds} to resolve the org and @@ -35,6 +107,7 @@ function forbidden(message) { * @returns {Promise} */ async function evaluateFeatureFlag(context, authInfo) { + const { log } = context; try { const ldClient = LaunchDarklyClient.createFrom(context); if (!ldClient) { @@ -49,7 +122,8 @@ async function evaluateFeatureFlag(context, authInfo) { const imsOrgId = `${ident}@AdobeOrg`; return await ldClient.isFlagEnabledForIMSOrg(FF_READ_ONLY_ORG, imsOrgId); - } catch { + } catch (err) { + log.error({ tag: 'ro-admin', err }, 'Feature flag evaluation failed for RO admin; defaulting to deny'); return false; } } @@ -63,8 +137,9 @@ async function evaluateFeatureFlag(context, authInfo) { * * 1. Evaluates the `FT_READ_ONLY_ORG` LaunchDarkly feature flag (fail-closed). * 2. Resolves the route's action from the routeCapabilities map and blocks - * write operations (or unmapped routes) for RO admins. - * 3. Emits a structured audit log entry for allowed RO admin requests. + * write operations (or unmapped routes) for RO admins, unless they own the resource. + * 3. Emits a structured audit log entry for all allowed RO admin requests, including + * the resolved resource ID and whether it came from the path or request body. * * Non-RO-admin requests pass through untouched. * @@ -102,14 +177,38 @@ export function readOnlyAdminWrapper(fn, { routeCapabilities } = {}) { const action = capability?.split(':').pop(); if (action !== 'read' && action !== 'readAll') { - log.warn({ - tag: 'ro-admin', + // Allow the write if the RO admin owns the target resource. + let params; + try { + params = extractRouteParams(context, routeCapabilities); + } catch (err) { + log.error({ tag: 'ro-admin', err }, 'extractRouteParams failed; denying write access'); + return forbidden('Forbidden'); + } + + const isOwner = await isOwnerOfResource(context, authInfo, params); + if (!isOwner) { + log.warn({ + tag: 'ro-admin', + email: authInfo.getProfile?.()?.email, + method: context.pathInfo?.method, + suffix: context.pathInfo?.suffix, + org: authInfo.getTenantIds?.()[0], + }, 'Read-only admin blocked from route'); + return forbidden('Forbidden'); + } + + const hasPathParams = Object.keys(params).length > 0; + log.info({ + tag: 'ro-admin-write', email: authInfo.getProfile?.()?.email, method: context.pathInfo?.method, suffix: context.pathInfo?.suffix, org: authInfo.getTenantIds?.()[0], - }, 'Read-only admin blocked from route'); - return forbidden('Forbidden'); + resolvedSiteId: params.siteId ?? context.data?.siteId ?? null, + resolvedOrgId: params.organizationId ?? context.data?.organizationId ?? null, + idSource: hasPathParams ? 'path' : 'body', + }, 'RO admin write allowed on owned resource'); } } diff --git a/packages/spacecat-shared-http-utils/src/auth/route-utils.js b/packages/spacecat-shared-http-utils/src/auth/route-utils.js index 5225d7245..3b40b28b9 100644 --- a/packages/spacecat-shared-http-utils/src/auth/route-utils.js +++ b/packages/spacecat-shared-http-utils/src/auth/route-utils.js @@ -64,6 +64,44 @@ export function resolveRouteCapability(context, routeMap) { return matchedKey ? routeMap[matchedKey] : null; } +/** + * Extracts named path parameters from the route pattern that matches the current request. + * e.g. route 'PATCH /sites/:siteId', suffix '/sites/abc-123' → { siteId: 'abc-123' } + * Returns an empty object when there is no match or the match has no parameters. + * + * @param {Object} context - Universal context with pathInfo + * @param {Object} routeMap - Route pattern to value map + * @returns {Object} + */ +export function extractRouteParams(context, routeMap) { + const method = context.pathInfo?.method?.toUpperCase(); + const path = context.pathInfo?.suffix; + if (!method || !path) { + return {}; + } + + const exactKey = `${method} ${path}`; + if (routeMap[exactKey]) { + return {}; + } + + const requestSegments = path.split('/').filter(Boolean); + const matchedKey = Object.keys(routeMap) + .find((key) => matchRoute(method, requestSegments, key)); + if (!matchedKey) { + return {}; + } + + const routeSegments = matchedKey.slice(matchedKey.indexOf(' ') + 1).split('/').filter(Boolean); + const params = {}; + routeSegments.forEach((seg, i) => { + if (seg.charCodeAt(0) === 58 /* ':' */) { + params[seg.slice(1)] = requestSegments[i]; + } + }); + return params; +} + /** * Throws at wrapper creation time if routeCapabilities is an empty object. * An empty map would silently deny/block all requests, so this is a diff --git a/packages/spacecat-shared-http-utils/test/auth/read-only-admin-wrapper.test.js b/packages/spacecat-shared-http-utils/test/auth/read-only-admin-wrapper.test.js index 7bb0d17e2..5d8a05e50 100644 --- a/packages/spacecat-shared-http-utils/test/auth/read-only-admin-wrapper.test.js +++ b/packages/spacecat-shared-http-utils/test/auth/read-only-admin-wrapper.test.js @@ -137,11 +137,12 @@ describe('readOnlyAdminWrapper', () => { expect(handler.called).to.be.false; }); - it('returns 403 when createFrom throws (fail-closed)', async () => { + it('returns 403 and logs error when createFrom throws (fail-closed)', async () => { + const sdkError = new Error('SDK key missing'); const throwModule = await esmock('../../src/auth/read-only-admin-wrapper.js', { '@adobe/spacecat-shared-launchdarkly-client': { LaunchDarklyClient: { - createFrom: sinon.stub().throws(new Error('SDK key missing')), + createFrom: sinon.stub().throws(sdkError), }, }, }); @@ -152,10 +153,15 @@ describe('readOnlyAdminWrapper', () => { const body = await result.json(); expect(body.message).to.equal('Forbidden'); expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin', err: sdkError }, + 'Feature flag evaluation failed for RO admin; defaulting to deny', + )).to.be.true; }); - it('returns 403 when isFlagEnabledForIMSOrg throws (fail-closed)', async () => { - ldClient.isFlagEnabledForIMSOrg.rejects(new Error('LD unavailable')); + it('returns 403 and logs error when isFlagEnabledForIMSOrg throws (fail-closed)', async () => { + const ldError = new Error('LD unavailable'); + ldClient.isFlagEnabledForIMSOrg.rejects(ldError); const wrapped = mockedWrapper(handler, { routeCapabilities }); const result = await wrapped({}, context); @@ -163,6 +169,10 @@ describe('readOnlyAdminWrapper', () => { const body = await result.json(); expect(body.message).to.equal('Forbidden'); expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin', err: ldError }, + 'Feature flag evaluation failed for RO admin; defaulting to deny', + )).to.be.true; }); it('returns 403 when authInfo has no tenant IDs (fail-closed)', async () => { @@ -316,6 +326,361 @@ describe('readOnlyAdminWrapper', () => { }); }); + describe('write on owned resource', () => { + let mockedWrapper; + let siteStub; + let orgStub; + + beforeEach(async () => { + const ldClient = { isFlagEnabledForIMSOrg: sinon.stub().resolves(true) }; + const mockedModule = await esmock('../../src/auth/read-only-admin-wrapper.js', { + '@adobe/spacecat-shared-launchdarkly-client': { + LaunchDarklyClient: { createFrom: sinon.stub().returns(ldClient) }, + }, + }); + mockedWrapper = mockedModule.readOnlyAdminWrapper; + + orgStub = { getImsOrgId: sinon.stub().returns('org-123@AdobeOrg') }; + siteStub = { getOrganization: sinon.stub().resolves(orgStub) }; + + context.attributes.authInfo.hasOrganization = sinon.stub().returns(true); + context.attributes.authInfo.getProfile = sinon.stub().returns({ email: 'roa@example.com' }); + }); + + it('allows write on a site the RO admin owns', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result).to.deep.equal({ status: 200 }); + expect(handler.calledOnce).to.be.true; + expect(context.dataAccess.Site.findById.calledWith('abc-123')).to.be.true; + expect(context.attributes.authInfo.hasOrganization.calledWith('org-123@AdobeOrg')).to.be.true; + }); + + it('blocks write on a site the RO admin does not own', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + context.attributes.authInfo.hasOrganization = sinon.stub().returns(false); + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('blocks write when the site is not found', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/not-exist' }; + context.dataAccess = { + Site: { findById: sinon.stub().resolves(null) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('blocks write when the site has no organization', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + siteStub.getOrganization.resolves(null); + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('allows write on an organization the RO admin owns', async () => { + const orgRoutes = { + ...routeCapabilities, + 'PATCH /organizations/:organizationId': 'organization:write', + }; + context.pathInfo = { method: 'PATCH', suffix: '/organizations/org-456' }; + context.dataAccess = { + Organization: { findById: sinon.stub().resolves(orgStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities: orgRoutes }); + const result = await wrapped({}, context); + + expect(result).to.deep.equal({ status: 200 }); + expect(context.dataAccess.Organization.findById.calledWith('org-456')).to.be.true; + }); + + it('blocks write when the organization is not found', async () => { + const orgRoutes = { + ...routeCapabilities, + 'PATCH /organizations/:organizationId': 'organization:write', + }; + context.pathInfo = { method: 'PATCH', suffix: '/organizations/org-456' }; + context.dataAccess = { + Organization: { findById: sinon.stub().resolves(null) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities: orgRoutes }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('blocks write on an organization the RO admin does not own', async () => { + const orgRoutes = { + ...routeCapabilities, + 'PATCH /organizations/:organizationId': 'organization:write', + }; + context.pathInfo = { method: 'PATCH', suffix: '/organizations/org-456' }; + context.attributes.authInfo.hasOrganization = sinon.stub().returns(false); + context.dataAccess = { + Organization: { findById: sinon.stub().resolves(orgStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities: orgRoutes }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('blocks write when no siteId or organizationId in path or body and logs warn', async () => { + context.pathInfo = { method: 'POST', suffix: '/sites' }; + context.dataAccess = { Site: { findById: sinon.stub() } }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(context.dataAccess.Site.findById.called).to.be.false; + expect(logStub.warn.calledWithMatch( + { tag: 'ro-admin' }, + 'isOwnerOfResource: no siteId or organizationId found in path params or context.data', + )).to.be.true; + }); + + it('blocks write when dataAccess is not present on context and logs error (fail-closed)', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + delete context.dataAccess; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin' }, + 'isOwnerOfResource: dataAccess not on context — ensure dataAccessWrapper runs before readOnlyAdminWrapper', + )).to.be.true; + }); + + it('blocks write when site lookup throws and logs the error (fail-closed)', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + const dbError = new Error('DB error'); + context.dataAccess = { + Site: { findById: sinon.stub().rejects(dbError) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin', err: dbError }, + 'Error checking resource ownership for RO admin', + )).to.be.true; + }); + + it('allows write when siteId is in context.data (no path param)', async () => { + const dataRoutes = { + ...routeCapabilities, + 'POST /preflight/jobs': 'preflight:write', + }; + context.pathInfo = { method: 'POST', suffix: '/preflight/jobs' }; + context.data = { siteId: 'abc-123' }; + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities: dataRoutes }); + const result = await wrapped({}, context); + + expect(result).to.deep.equal({ status: 200 }); + expect(context.dataAccess.Site.findById.calledWith('abc-123')).to.be.true; + }); + + it('blocks write when context.data siteId is present but user does not own it', async () => { + const dataRoutes = { + ...routeCapabilities, + 'POST /preflight/jobs': 'preflight:write', + }; + context.pathInfo = { method: 'POST', suffix: '/preflight/jobs' }; + context.data = { siteId: 'abc-123' }; + context.attributes.authInfo.hasOrganization = sinon.stub().returns(false); + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities: dataRoutes }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('allows write when organizationId is in context.data (no path param)', async () => { + const dataRoutes = { + ...routeCapabilities, + 'POST /some/org/action': 'organization:write', + }; + context.pathInfo = { method: 'POST', suffix: '/some/org/action' }; + context.data = { organizationId: 'org-456' }; + context.dataAccess = { + Organization: { findById: sinon.stub().resolves(orgStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities: dataRoutes }); + const result = await wrapped({}, context); + + expect(result).to.deep.equal({ status: 200 }); + expect(context.dataAccess.Organization.findById.calledWith('org-456')).to.be.true; + }); + + it('path param takes precedence over context.data siteId', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/path-site-id' }; + context.data = { siteId: 'data-site-id' }; + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + await wrapped({}, context); + + expect(context.dataAccess.Site.findById.calledWith('path-site-id')).to.be.true; + expect(context.dataAccess.Site.findById.calledWith('data-site-id')).to.be.false; + }); + + it('emits ro-admin-write log and audit log when write on owned resource is allowed', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + context.dataAccess = { + Site: { findById: sinon.stub().resolves(siteStub) }, + }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + await wrapped({}, context); + + expect(logStub.info.calledWithMatch({ + tag: 'ro-admin-write', + method: 'PATCH', + suffix: '/sites/abc-123', + resolvedSiteId: 'abc-123', + idSource: 'path', + }, 'RO admin write allowed on owned resource')).to.be.true; + expect(logStub.info.calledWithMatch({ + tag: 'ro-admin-audit', method: 'PATCH', suffix: '/sites/abc-123', + }, 'RO admin accessed route')).to.be.true; + }); + + it('emits ro-admin-write log with idSource body when context.data fallback is used', async () => { + const dataRoutes = { ...routeCapabilities, 'POST /preflight/jobs': 'preflight:write' }; + context.pathInfo = { method: 'POST', suffix: '/preflight/jobs' }; + context.data = { siteId: 'abc-123' }; + context.dataAccess = { Site: { findById: sinon.stub().resolves(siteStub) } }; + const wrapped = mockedWrapper(handler, { routeCapabilities: dataRoutes }); + await wrapped({}, context); + + expect(logStub.info.calledWithMatch({ + tag: 'ro-admin-write', + resolvedSiteId: 'abc-123', + idSource: 'body', + }, 'RO admin write allowed on owned resource')).to.be.true; + }); + + it('blocks write and ignores body siteId when route has path params with a different name', async () => { + // Security: if route uses :id instead of :siteId, body siteId must NOT be used as fallback. + // Use a standalone map that has only :id (no :siteId) so the match is unambiguous. + const altRoutes = { 'PATCH /sites/:id': 'site:write' }; + context.pathInfo = { method: 'PATCH', suffix: '/sites/site-b' }; + context.data = { siteId: 'site-a' }; // attacker owns site-a, not site-b + context.dataAccess = { Site: { findById: sinon.stub().resolves(siteStub) } }; + const wrapped = mockedWrapper(handler, { routeCapabilities: altRoutes }); + const result = await wrapped({}, context); + + // body siteId must be ignored; no findById call should be made with 'site-a' + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(context.dataAccess.Site.findById.calledWith('site-a')).to.be.false; + }); + + it('blocks write when getOrganization throws and logs the error (fail-closed)', async () => { + const orgError = new Error('getOrganization failed'); + siteStub.getOrganization.rejects(orgError); + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + context.dataAccess = { Site: { findById: sinon.stub().resolves(siteStub) } }; + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin', err: orgError }, + 'Error checking resource ownership for RO admin', + )).to.be.true; + }); + + it('blocks write when Organization.findById throws and logs the error (fail-closed)', async () => { + const orgRoutes = { ...routeCapabilities, 'PATCH /organizations/:organizationId': 'organization:write' }; + const dbError = new Error('Org DB error'); + context.pathInfo = { method: 'PATCH', suffix: '/organizations/org-456' }; + context.dataAccess = { Organization: { findById: sinon.stub().rejects(dbError) } }; + const wrapped = mockedWrapper(handler, { routeCapabilities: orgRoutes }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin', err: dbError }, + 'Error checking resource ownership for RO admin', + )).to.be.true; + }); + + it('blocks write when dataAccess has no Site property (fail-closed)', async () => { + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + context.dataAccess = {}; // Site accessor absent + const wrapped = mockedWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + }); + + it('returns 403 and logs error when extractRouteParams throws (fail-closed)', async () => { + const routeError = new Error('route parse failure'); + const throwRouteModule = await esmock('../../src/auth/read-only-admin-wrapper.js', { + '@adobe/spacecat-shared-launchdarkly-client': { + LaunchDarklyClient: { + createFrom: sinon.stub().returns({ + isFlagEnabledForIMSOrg: sinon.stub().resolves(true), + }), + }, + }, + '../../src/auth/route-utils.js': { + extractRouteParams: sinon.stub().throws(routeError), + resolveRouteCapability: sinon.stub().returns('site:write'), + guardNonEmptyRouteCapabilities: sinon.stub(), + }, + }); + context.pathInfo = { method: 'PATCH', suffix: '/sites/abc-123' }; + const wrapped = throwRouteModule.readOnlyAdminWrapper(handler, { routeCapabilities }); + const result = await wrapped({}, context); + + expect(result.status).to.equal(403); + expect(handler.called).to.be.false; + expect(logStub.error.calledWithMatch( + { tag: 'ro-admin', err: routeError }, + 'extractRouteParams failed; denying write access', + )).to.be.true; + }); + }); + describe('audit logging', () => { let mockedWrapper; diff --git a/packages/spacecat-shared-http-utils/test/auth/route-utils.test.js b/packages/spacecat-shared-http-utils/test/auth/route-utils.test.js index da55fbf91..fb2d9725d 100644 --- a/packages/spacecat-shared-http-utils/test/auth/route-utils.test.js +++ b/packages/spacecat-shared-http-utils/test/auth/route-utils.test.js @@ -12,7 +12,11 @@ import { expect } from 'chai'; -import { guardNonEmptyRouteCapabilities, resolveRouteCapability } from '../../src/auth/route-utils.js'; +import { + extractRouteParams, + guardNonEmptyRouteCapabilities, + resolveRouteCapability, +} from '../../src/auth/route-utils.js'; describe('route-utils', () => { describe('resolveRouteCapability', () => { @@ -76,6 +80,57 @@ describe('route-utils', () => { }); }); + describe('extractRouteParams', () => { + const routeMap = { + 'GET /sites': 'site:read', + 'POST /sites': 'site:write', + 'GET /sites/:siteId': 'site:read', + 'PATCH /sites/:siteId': 'site:write', + 'GET /sites/:siteId/audits/:auditId': 'audit:read', + 'GET /organizations/:organizationId': 'organization:read', + }; + + it('returns empty object for an exact match (no params)', () => { + const context = { pathInfo: { method: 'GET', suffix: '/sites' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({}); + }); + + it('returns extracted params for a single-param route', () => { + const context = { pathInfo: { method: 'PATCH', suffix: '/sites/abc-123' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({ siteId: 'abc-123' }); + }); + + it('returns multiple params from a nested parameterized route', () => { + const context = { pathInfo: { method: 'GET', suffix: '/sites/abc-123/audits/def-456' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({ siteId: 'abc-123', auditId: 'def-456' }); + }); + + it('returns organizationId from an org route', () => { + const context = { pathInfo: { method: 'GET', suffix: '/organizations/org-789' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({ organizationId: 'org-789' }); + }); + + it('returns empty object for an unmatched route', () => { + const context = { pathInfo: { method: 'GET', suffix: '/unknown' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({}); + }); + + it('returns empty object when method is missing', () => { + const context = { pathInfo: { suffix: '/sites/abc-123' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({}); + }); + + it('returns empty object when suffix is missing', () => { + const context = { pathInfo: { method: 'PATCH' } }; + expect(extractRouteParams(context, routeMap)).to.deep.equal({}); + }); + + it('returns empty object when pathInfo is missing', () => { + const context = {}; + expect(extractRouteParams(context, routeMap)).to.deep.equal({}); + }); + }); + describe('guardNonEmptyRouteCapabilities', () => { it('throws when routeCapabilities is an empty object', () => { expect(() => guardNonEmptyRouteCapabilities('testWrapper', {}))