From a4ac0b2a6b416797d871ff453b4f34a7d7f5c286 Mon Sep 17 00:00:00 2001 From: zouantchaw <44246692+zouantchaw@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:48:43 +0100 Subject: [PATCH] fix(authz): tighten policy scope enforcement --- backend/command-api/src/middleware/auth.js | 38 +++++- .../src/services/command-service.js | 68 ++++++++++ backend/command-api/src/services/policy.js | 128 +++++++++++++++++- backend/command-api/test/app.test.js | 34 +++++ backend/command-api/test/policy.test.js | 86 ++++++++++++ backend/core-api/src/middleware/auth.js | 36 ++++- backend/core-api/src/services/policy.js | 49 ++++++- backend/core-api/test/policy.test.js | 33 +++++ backend/query-api/src/middleware/auth.js | 38 +++++- backend/query-api/src/routes/query.js | 26 +++- backend/query-api/src/services/policy.js | 121 ++++++++++++++++- .../query-api/src/services/query-service.js | 1 + backend/query-api/test/app.test.js | 29 ++++ backend/query-api/test/policy.test.js | 86 ++++++++++++ 14 files changed, 743 insertions(+), 30 deletions(-) create mode 100644 backend/command-api/test/policy.test.js create mode 100644 backend/core-api/test/policy.test.js create mode 100644 backend/query-api/test/policy.test.js diff --git a/backend/command-api/src/middleware/auth.js b/backend/command-api/src/middleware/auth.js index 9c62c86d..d6fa04f8 100644 --- a/backend/command-api/src/middleware/auth.js +++ b/backend/command-api/src/middleware/auth.js @@ -9,6 +9,30 @@ function getBearerToken(header) { return token; } +function buildBypassActor() { + let policyContext = { + user: { userId: 'test-user' }, + tenant: { tenantId: '*' }, + business: { businessId: '*' }, + staff: { staffId: '*', workforceId: '*' }, + }; + + if (process.env.AUTH_BYPASS_CONTEXT) { + try { + policyContext = JSON.parse(process.env.AUTH_BYPASS_CONTEXT); + } catch (_error) { + policyContext = { + user: { userId: 'test-user' }, + tenant: { tenantId: '*' }, + business: { businessId: '*' }, + staff: { staffId: '*', workforceId: '*' }, + }; + } + } + + return { uid: 'test-user', email: 'test@krow.local', role: 'TEST', policyContext }; +} + export async function requireAuth(req, _res, next) { try { const token = getBearerToken(req.get('Authorization')); @@ -17,7 +41,7 @@ export async function requireAuth(req, _res, next) { } if (process.env.AUTH_BYPASS === 'true') { - req.actor = { uid: 'test-user', email: 'test@krow.local', role: 'TEST' }; + req.actor = buildBypassActor(); return next(); } @@ -36,10 +60,14 @@ export async function requireAuth(req, _res, next) { } export function requirePolicy(action, resource) { - return (req, _res, next) => { - if (!can(action, resource, req.actor)) { - return next(new AppError('FORBIDDEN', 'Not allowed to perform this action', 403)); + return async (req, _res, next) => { + try { + if (!(await can(action, resource, req.actor, req))) { + return next(new AppError('FORBIDDEN', 'Not allowed to perform this action', 403)); + } + return next(); + } catch (error) { + return next(error); } - return next(); }; } diff --git a/backend/command-api/src/services/command-service.js b/backend/command-api/src/services/command-service.js index d8fb721e..ce516acb 100644 --- a/backend/command-api/src/services/command-service.js +++ b/backend/command-api/src/services/command-service.js @@ -4,6 +4,10 @@ import { recordGeofenceIncident } from './attendance-monitoring.js'; import { recordAttendanceSecurityProof } from './attendance-security.js'; import { evaluateClockInAttempt } from './clock-in-policy.js'; import { enqueueHubManagerAlert } from './notification-outbox.js'; +import { + requireClientContext as requireActorClientContext, + requireStaffContext as requireActorStaffContext, +} from './actor-context.js'; function toIsoOrNull(value) { return value ? new Date(value).toISOString() : null; @@ -68,6 +72,33 @@ async function ensureStaffNotBlockedByBusiness(client, { tenantId, businessId, s } } +function assertTenantScope(context, tenantId) { + if (context.tenant.tenantId !== tenantId) { + throw new AppError('FORBIDDEN', 'Resource is outside actor tenant scope', 403, { + tenantId, + actorTenantId: context.tenant.tenantId, + }); + } +} + +function assertBusinessScope(context, businessId) { + if (context.business && context.business.businessId !== businessId) { + throw new AppError('FORBIDDEN', 'Resource is outside actor business scope', 403, { + businessId, + actorBusinessId: context.business.businessId, + }); + } +} + +function assertStaffScope(context, staffId) { + if (context.staff.staffId !== staffId) { + throw new AppError('FORBIDDEN', 'Resource is outside actor staff scope', 403, { + staffId, + actorStaffId: context.staff.staffId, + }); + } +} + async function insertDomainEvent(client, { tenantId, aggregateType, @@ -451,6 +482,9 @@ function buildOrderUpdateStatement(payload) { export async function createOrder(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); + assertBusinessScope(actorContext, payload.businessId); await requireBusiness(client, payload.tenantId, payload.businessId); if (payload.vendorId) { await requireVendor(client, payload.tenantId, payload.vendorId); @@ -620,8 +654,10 @@ export async function createOrder(actor, payload) { export async function acceptShift(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorStaffContext(actor.uid); const shiftRole = await requireShiftRole(client, payload.shiftRoleId); + assertTenantScope(actorContext, shiftRole.tenant_id); if (payload.shiftId && shiftRole.shift_id !== payload.shiftId) { throw new AppError('VALIDATION_ERROR', 'shiftId does not match shiftRoleId', 400, { shiftId: payload.shiftId, @@ -629,6 +665,13 @@ export async function acceptShift(actor, payload) { }); } + if (!actorContext.staff.workforceId || actorContext.staff.workforceId !== payload.workforceId) { + throw new AppError('FORBIDDEN', 'Staff can only accept shifts for their own workforce record', 403, { + workforceId: payload.workforceId, + actorWorkforceId: actorContext.staff.workforceId || null, + }); + } + if (shiftRole.assigned_count >= shiftRole.workers_needed) { const existingFilledAssignment = await findAssignmentForShiftRoleWorkforce( client, @@ -736,7 +779,10 @@ export async function acceptShift(actor, payload) { export async function updateOrder(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); const existingOrder = await requireOrder(client, payload.tenantId, payload.orderId); + assertBusinessScope(actorContext, existingOrder.business_id); if (Object.prototype.hasOwnProperty.call(payload, 'vendorId') && payload.vendorId) { await requireVendor(client, payload.tenantId, payload.vendorId); @@ -787,7 +833,10 @@ export async function updateOrder(actor, payload) { export async function cancelOrder(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); const order = await requireOrder(client, payload.tenantId, payload.orderId); + assertBusinessScope(actorContext, order.business_id); if (order.status === 'CANCELLED') { return { @@ -910,7 +959,10 @@ export async function cancelOrder(actor, payload) { export async function changeShiftStatus(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); const shift = await requireShift(client, payload.tenantId, payload.shiftId); + assertBusinessScope(actorContext, shift.business_id); if (payload.status === 'COMPLETED') { const openSession = await client.query( @@ -999,7 +1051,10 @@ export async function changeShiftStatus(actor, payload) { export async function assignStaffToShift(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); const shift = await requireShift(client, payload.tenantId, payload.shiftId); + assertBusinessScope(actorContext, shift.business_id); const shiftRole = await requireShiftRole(client, payload.shiftRoleId); if (shiftRole.shift_id !== shift.id) { @@ -1120,7 +1175,10 @@ export async function assignStaffToShift(actor, payload) { async function createAttendanceEvent(actor, payload, eventType) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorStaffContext(actor.uid); const assignment = await requireAssignment(client, payload.assignmentId); + assertTenantScope(actorContext, assignment.tenant_id); + assertStaffScope(actorContext, assignment.staff_id); const capturedAt = toIsoOrNull(payload.capturedAt) || new Date().toISOString(); let securityProof = null; @@ -1553,6 +1611,9 @@ export async function clockOut(actor, payload) { export async function addFavoriteStaff(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); + assertBusinessScope(actorContext, payload.businessId); await requireBusiness(client, payload.tenantId, payload.businessId); const staffResult = await client.query( @@ -1605,6 +1666,9 @@ export async function addFavoriteStaff(actor, payload) { export async function removeFavoriteStaff(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); + assertBusinessScope(actorContext, payload.businessId); const deleted = await client.query( ` DELETE FROM staff_favorites @@ -1640,7 +1704,11 @@ export async function removeFavoriteStaff(actor, payload) { export async function createStaffReview(actor, payload) { return withTransaction(async (client) => { await ensureActorUser(client, actor); + const actorContext = await requireActorClientContext(actor.uid); + assertTenantScope(actorContext, payload.tenantId); + assertBusinessScope(actorContext, payload.businessId); const assignment = await requireAssignment(client, payload.assignmentId); + assertBusinessScope(actorContext, assignment.business_id); if (assignment.business_id !== payload.businessId || assignment.staff_id !== payload.staffId) { throw new AppError('VALIDATION_ERROR', 'Assignment does not match business/staff review target', 400, { assignmentId: payload.assignmentId, diff --git a/backend/command-api/src/services/policy.js b/backend/command-api/src/services/policy.js index 44e7e371..ec31b98a 100644 --- a/backend/command-api/src/services/policy.js +++ b/backend/command-api/src/services/policy.js @@ -1,5 +1,125 @@ -export function can(action, resource, actor) { - void action; - void resource; - return Boolean(actor?.uid); +import { loadActorContext } from './actor-context.js'; + +const TENANT_ADMIN_ROLES = new Set(['OWNER', 'ADMIN']); + +function normalize(value) { + return `${value || ''}`.trim(); +} + +function requestField(req, field) { + return normalize( + req?.params?.[field] + ?? req?.body?.[field] + ?? req?.query?.[field] + ); +} + +function isTenantAdmin(context) { + return TENANT_ADMIN_ROLES.has(normalize(context?.tenant?.role).toUpperCase()); +} + +function hasTenantScope(context) { + return Boolean(context?.user && context?.tenant); +} + +function hasClientScope(context) { + return hasTenantScope(context) && Boolean(context?.business || isTenantAdmin(context)); +} + +function hasStaffScope(context) { + return hasTenantScope(context) && Boolean(context?.staff); +} + +function requiredScopeFor(action) { + if (action === 'notifications.device.write') { + return 'tenant'; + } + + if ( + action === 'orders.create' + || action === 'orders.update' + || action === 'orders.cancel' + || action === 'shifts.change-status' + || action === 'shifts.assign-staff' + || action === 'business.favorite-staff' + || action === 'business.unfavorite-staff' + || action === 'assignments.review-staff' + || action.startsWith('client.') + || action.startsWith('billing.') + || action.startsWith('coverage.') + || action.startsWith('hubs.') + || action.startsWith('vendors.') + || action.startsWith('reports.') + ) { + return 'client'; + } + + if ( + action === 'shifts.accept' + || action === 'attendance.clock-in' + || action === 'attendance.clock-out' + || action === 'attendance.location-stream.write' + || action.startsWith('staff.') + || action.startsWith('payments.') + ) { + return 'staff'; + } + + return 'deny'; +} + +async function resolveActorContext(actor) { + if (!actor?.uid) { + return null; + } + if (actor.policyContext) { + return actor.policyContext; + } + const context = await loadActorContext(actor.uid); + actor.policyContext = context; + return context; +} + +function requestScopeMatches(req, context, requiredScope) { + const tenantId = requestField(req, 'tenantId'); + if (tenantId && context?.tenant?.tenantId !== '*' && context?.tenant?.tenantId !== tenantId) { + return false; + } + + const businessId = requestField(req, 'businessId'); + if ( + requiredScope === 'client' + && businessId + && context?.business?.businessId + && context.business.businessId !== '*' + && context.business.businessId !== businessId + ) { + return false; + } + + return true; +} + +export async function can(action, resource, actor, req) { + void resource; + const context = await resolveActorContext(actor); + const requiredScope = requiredScopeFor(action); + + if (requiredScope === 'deny' || !context?.user) { + return false; + } + + if (requiredScope === 'tenant') { + return hasTenantScope(context) && requestScopeMatches(req, context, requiredScope); + } + + if (requiredScope === 'client') { + return hasClientScope(context) && requestScopeMatches(req, context, requiredScope); + } + + if (requiredScope === 'staff') { + return hasStaffScope(context) && requestScopeMatches(req, context, requiredScope); + } + + return false; } diff --git a/backend/command-api/test/app.test.js b/backend/command-api/test/app.test.js index dc9c6f7e..e9362286 100644 --- a/backend/command-api/test/app.test.js +++ b/backend/command-api/test/app.test.js @@ -40,6 +40,7 @@ function validOrderCreatePayload() { beforeEach(() => { process.env.IDEMPOTENCY_STORE = 'memory'; + delete process.env.AUTH_BYPASS_CONTEXT; delete process.env.IDEMPOTENCY_DATABASE_URL; delete process.env.DATABASE_URL; __resetIdempotencyStoreForTests(); @@ -126,3 +127,36 @@ test('command route is idempotent by key and only executes handler once', async assert.equal(first.body.idempotencyKey, 'abc-123'); assert.equal(second.body.idempotencyKey, 'abc-123'); }); + +test('client command routes deny mismatched business scope before handler execution', async () => { + process.env.AUTH_BYPASS_CONTEXT = JSON.stringify({ + user: { userId: 'test-user' }, + tenant: { tenantId, role: 'MANAGER' }, + business: { businessId: '99999999-9999-4999-8999-999999999999' }, + }); + + const app = createApp({ + commandHandlers: { + createOrder: async () => assert.fail('createOrder should not be called'), + acceptShift: async () => assert.fail('acceptShift should not be called'), + clockIn: async () => assert.fail('clockIn should not be called'), + clockOut: async () => assert.fail('clockOut should not be called'), + addFavoriteStaff: async () => assert.fail('addFavoriteStaff should not be called'), + removeFavoriteStaff: async () => assert.fail('removeFavoriteStaff should not be called'), + createStaffReview: async () => assert.fail('createStaffReview should not be called'), + updateOrder: async () => assert.fail('updateOrder should not be called'), + cancelOrder: async () => assert.fail('cancelOrder should not be called'), + changeShiftStatus: async () => assert.fail('changeShiftStatus should not be called'), + assignStaffToShift: async () => assert.fail('assignStaffToShift should not be called'), + }, + }); + + const res = await request(app) + .post('/commands/orders/create') + .set('Authorization', 'Bearer test-token') + .set('Idempotency-Key', 'scope-mismatch') + .send(validOrderCreatePayload()); + + assert.equal(res.status, 403); + assert.equal(res.body.code, 'FORBIDDEN'); +}); diff --git a/backend/command-api/test/policy.test.js b/backend/command-api/test/policy.test.js new file mode 100644 index 00000000..d5e30b65 --- /dev/null +++ b/backend/command-api/test/policy.test.js @@ -0,0 +1,86 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { can } from '../src/services/policy.js'; + +test('client actions require business scope and matching business id', async () => { + const allowed = await can( + 'orders.create', + 'order', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1', role: 'MANAGER' }, + business: { businessId: 'business-1' }, + }, + }, + { body: { tenantId: 'tenant-1', businessId: 'business-1' } } + ); + + const denied = await can( + 'orders.create', + 'order', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1', role: 'MANAGER' }, + business: { businessId: 'business-1' }, + }, + }, + { body: { tenantId: 'tenant-1', businessId: 'business-2' } } + ); + + assert.equal(allowed, true); + assert.equal(denied, false); +}); + +test('staff actions require staff scope', async () => { + const allowed = await can( + 'shifts.accept', + 'shift', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + staff: { staffId: 'staff-1', workforceId: 'workforce-1' }, + }, + }, + { body: { tenantId: 'tenant-1' } } + ); + + const denied = await can( + 'shifts.accept', + 'shift', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + business: { businessId: 'business-1' }, + }, + }, + { body: { tenantId: 'tenant-1' } } + ); + + assert.equal(allowed, true); + assert.equal(denied, false); +}); + +test('notifications.device.write allows tenant-scoped actor', async () => { + const allowed = await can( + 'notifications.device.write', + 'device', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + }, + }, + { body: { tenantId: 'tenant-1' } } + ); + + assert.equal(allowed, true); +}); diff --git a/backend/core-api/src/middleware/auth.js b/backend/core-api/src/middleware/auth.js index 9c62c86d..d2aa36ff 100644 --- a/backend/core-api/src/middleware/auth.js +++ b/backend/core-api/src/middleware/auth.js @@ -9,6 +9,28 @@ function getBearerToken(header) { return token; } +function buildBypassActor() { + let policyContext = { + user: { userId: 'test-user' }, + tenant: { tenantId: '*' }, + staff: { staffId: '*', workforceId: '*' }, + }; + + if (process.env.AUTH_BYPASS_CONTEXT) { + try { + policyContext = JSON.parse(process.env.AUTH_BYPASS_CONTEXT); + } catch (_error) { + policyContext = { + user: { userId: 'test-user' }, + tenant: { tenantId: '*' }, + staff: { staffId: '*', workforceId: '*' }, + }; + } + } + + return { uid: 'test-user', email: 'test@krow.local', role: 'TEST', policyContext }; +} + export async function requireAuth(req, _res, next) { try { const token = getBearerToken(req.get('Authorization')); @@ -17,7 +39,7 @@ export async function requireAuth(req, _res, next) { } if (process.env.AUTH_BYPASS === 'true') { - req.actor = { uid: 'test-user', email: 'test@krow.local', role: 'TEST' }; + req.actor = buildBypassActor(); return next(); } @@ -36,10 +58,14 @@ export async function requireAuth(req, _res, next) { } export function requirePolicy(action, resource) { - return (req, _res, next) => { - if (!can(action, resource, req.actor)) { - return next(new AppError('FORBIDDEN', 'Not allowed to perform this action', 403)); + return async (req, _res, next) => { + try { + if (!(await can(action, resource, req.actor, req))) { + return next(new AppError('FORBIDDEN', 'Not allowed to perform this action', 403)); + } + return next(); + } catch (error) { + return next(error); } - return next(); }; } diff --git a/backend/core-api/src/services/policy.js b/backend/core-api/src/services/policy.js index 44e7e371..12fa6e2f 100644 --- a/backend/core-api/src/services/policy.js +++ b/backend/core-api/src/services/policy.js @@ -1,5 +1,46 @@ -export function can(action, resource, actor) { - void action; - void resource; - return Boolean(actor?.uid); +import { loadActorContext } from './actor-context.js'; + +function normalize(value) { + return `${value || ''}`.trim(); +} + +function requestField(req, field) { + return normalize( + req?.params?.[field] + ?? req?.body?.[field] + ?? req?.query?.[field] + ); +} + +async function resolveActorContext(actor) { + if (!actor?.uid) { + return null; + } + if (actor.policyContext) { + return actor.policyContext; + } + const context = await loadActorContext(actor.uid); + actor.policyContext = context; + return context; +} + +export async function can(action, resource, actor, req) { + void resource; + if (!action.startsWith('core.')) { + return false; + } + + const context = await resolveActorContext(actor); + if (!context?.user || !context?.tenant) { + return false; + } + + const tenantId = requestField(req, 'tenantId'); + if (!tenantId) { + return true; + } + if (context.tenant.tenantId === '*') { + return true; + } + return context.tenant.tenantId === tenantId; } diff --git a/backend/core-api/test/policy.test.js b/backend/core-api/test/policy.test.js new file mode 100644 index 00000000..d4beef5d --- /dev/null +++ b/backend/core-api/test/policy.test.js @@ -0,0 +1,33 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { can } from '../src/services/policy.js'; + +test('core actions require tenant scope', async () => { + const allowed = await can( + 'core.verification.read', + 'verification', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + }, + }, + {} + ); + + const denied = await can( + 'core.verification.read', + 'verification', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + }, + }, + {} + ); + + assert.equal(allowed, true); + assert.equal(denied, false); +}); diff --git a/backend/query-api/src/middleware/auth.js b/backend/query-api/src/middleware/auth.js index 9c62c86d..d6fa04f8 100644 --- a/backend/query-api/src/middleware/auth.js +++ b/backend/query-api/src/middleware/auth.js @@ -9,6 +9,30 @@ function getBearerToken(header) { return token; } +function buildBypassActor() { + let policyContext = { + user: { userId: 'test-user' }, + tenant: { tenantId: '*' }, + business: { businessId: '*' }, + staff: { staffId: '*', workforceId: '*' }, + }; + + if (process.env.AUTH_BYPASS_CONTEXT) { + try { + policyContext = JSON.parse(process.env.AUTH_BYPASS_CONTEXT); + } catch (_error) { + policyContext = { + user: { userId: 'test-user' }, + tenant: { tenantId: '*' }, + business: { businessId: '*' }, + staff: { staffId: '*', workforceId: '*' }, + }; + } + } + + return { uid: 'test-user', email: 'test@krow.local', role: 'TEST', policyContext }; +} + export async function requireAuth(req, _res, next) { try { const token = getBearerToken(req.get('Authorization')); @@ -17,7 +41,7 @@ export async function requireAuth(req, _res, next) { } if (process.env.AUTH_BYPASS === 'true') { - req.actor = { uid: 'test-user', email: 'test@krow.local', role: 'TEST' }; + req.actor = buildBypassActor(); return next(); } @@ -36,10 +60,14 @@ export async function requireAuth(req, _res, next) { } export function requirePolicy(action, resource) { - return (req, _res, next) => { - if (!can(action, resource, req.actor)) { - return next(new AppError('FORBIDDEN', 'Not allowed to perform this action', 403)); + return async (req, _res, next) => { + try { + if (!(await can(action, resource, req.actor, req))) { + return next(new AppError('FORBIDDEN', 'Not allowed to perform this action', 403)); + } + return next(); + } catch (error) { + return next(error); } - return next(); }; } diff --git a/backend/query-api/src/routes/query.js b/backend/query-api/src/routes/query.js index 5fe33090..46109012 100644 --- a/backend/query-api/src/routes/query.js +++ b/backend/query-api/src/routes/query.js @@ -27,6 +27,11 @@ function requireUuid(value, field) { export function createQueryRouter(queryService = defaultQueryService) { const router = Router(); + function actorBusinessId(actor) { + const businessId = actor?.policyContext?.business?.businessId; + return businessId && businessId !== '*' ? businessId : null; + } + router.get( '/tenants/:tenantId/orders', requireAuth, @@ -34,9 +39,10 @@ export function createQueryRouter(queryService = defaultQueryService) { async (req, res, next) => { try { const tenantId = requireUuid(req.params.tenantId, 'tenantId'); + const scopedBusinessId = actorBusinessId(req.actor); const orders = await queryService.listOrders({ tenantId, - businessId: req.query.businessId, + businessId: scopedBusinessId || req.query.businessId, status: req.query.status, limit: req.query.limit, offset: req.query.offset, @@ -57,10 +63,16 @@ export function createQueryRouter(queryService = defaultQueryService) { requirePolicy('orders.read', 'order'), async (req, res, next) => { try { + const scopedBusinessId = actorBusinessId(req.actor); const order = await queryService.getOrderDetail({ tenantId: requireUuid(req.params.tenantId, 'tenantId'), orderId: requireUuid(req.params.orderId, 'orderId'), }); + if (scopedBusinessId && order.businessId !== scopedBusinessId) { + throw new AppError('FORBIDDEN', 'Order is outside actor business scope', 403, { + orderId: req.params.orderId, + }); + } return res.status(200).json({ ...order, requestId: req.requestId, @@ -77,9 +89,10 @@ export function createQueryRouter(queryService = defaultQueryService) { requirePolicy('business.favorite-staff.read', 'staff'), async (req, res, next) => { try { + const scopedBusinessId = actorBusinessId(req.actor); const items = await queryService.listFavoriteStaff({ tenantId: requireUuid(req.params.tenantId, 'tenantId'), - businessId: requireUuid(req.params.businessId, 'businessId'), + businessId: requireUuid(scopedBusinessId || req.params.businessId, 'businessId'), limit: req.query.limit, offset: req.query.offset, }); @@ -120,12 +133,19 @@ export function createQueryRouter(queryService = defaultQueryService) { requirePolicy('attendance.read', 'attendance'), async (req, res, next) => { try { + const scopedBusinessId = actorBusinessId(req.actor); const attendance = await queryService.getAssignmentAttendance({ tenantId: requireUuid(req.params.tenantId, 'tenantId'), assignmentId: requireUuid(req.params.assignmentId, 'assignmentId'), }); + if (scopedBusinessId && attendance.businessId !== scopedBusinessId) { + throw new AppError('FORBIDDEN', 'Assignment attendance is outside actor business scope', 403, { + assignmentId: req.params.assignmentId, + }); + } + const { businessId: _businessId, ...publicAttendance } = attendance; return res.status(200).json({ - ...attendance, + ...publicAttendance, requestId: req.requestId, }); } catch (error) { diff --git a/backend/query-api/src/services/policy.js b/backend/query-api/src/services/policy.js index 44e7e371..22e3d9c9 100644 --- a/backend/query-api/src/services/policy.js +++ b/backend/query-api/src/services/policy.js @@ -1,5 +1,118 @@ -export function can(action, resource, actor) { - void action; - void resource; - return Boolean(actor?.uid); +import { loadActorContext } from './actor-context.js'; + +const TENANT_ADMIN_ROLES = new Set(['OWNER', 'ADMIN']); + +function normalize(value) { + return `${value || ''}`.trim(); +} + +function requestField(req, field) { + return normalize( + req?.params?.[field] + ?? req?.body?.[field] + ?? req?.query?.[field] + ); +} + +function isTenantAdmin(context) { + return TENANT_ADMIN_ROLES.has(normalize(context?.tenant?.role).toUpperCase()); +} + +function hasTenantScope(context) { + return Boolean(context?.user && context?.tenant); +} + +function hasClientScope(context) { + return hasTenantScope(context) && Boolean(context?.business || isTenantAdmin(context)); +} + +function hasStaffScope(context) { + return hasTenantScope(context) && Boolean(context?.staff); +} + +function requiredScopeFor(action) { + if (action === 'attendance.read') { + return 'tenant'; + } + + if ( + action === 'orders.read' + || action === 'orders.reorder.read' + || action === 'business.favorite-staff.read' + || action === 'staff.reviews.read' + || action.startsWith('client.') + || action.startsWith('billing.') + || action.startsWith('coverage.') + || action.startsWith('hubs.') + || action.startsWith('vendors.') + || action.startsWith('reports.') + ) { + return 'client'; + } + + if ( + action === 'shifts.read' + || action.startsWith('staff.') + || action.startsWith('payments.') + ) { + return 'staff'; + } + + return 'deny'; +} + +async function resolveActorContext(actor) { + if (!actor?.uid) { + return null; + } + if (actor.policyContext) { + return actor.policyContext; + } + const context = await loadActorContext(actor.uid); + actor.policyContext = context; + return context; +} + +function requestScopeMatches(req, context, requiredScope) { + const tenantId = requestField(req, 'tenantId'); + if (tenantId && context?.tenant?.tenantId !== '*' && context?.tenant?.tenantId !== tenantId) { + return false; + } + + const businessId = requestField(req, 'businessId'); + if ( + requiredScope === 'client' + && businessId + && context?.business?.businessId + && context.business.businessId !== '*' + && context.business.businessId !== businessId + ) { + return false; + } + + return true; +} + +export async function can(action, resource, actor, req) { + void resource; + const context = await resolveActorContext(actor); + const requiredScope = requiredScopeFor(action); + + if (requiredScope === 'deny' || !context?.user) { + return false; + } + + if (requiredScope === 'tenant') { + return hasTenantScope(context) && requestScopeMatches(req, context, requiredScope); + } + + if (requiredScope === 'client') { + return hasClientScope(context) && requestScopeMatches(req, context, requiredScope); + } + + if (requiredScope === 'staff') { + return hasStaffScope(context) && requestScopeMatches(req, context, requiredScope); + } + + return false; } diff --git a/backend/query-api/src/services/query-service.js b/backend/query-api/src/services/query-service.js index 02a5e795..54db3274 100644 --- a/backend/query-api/src/services/query-service.js +++ b/backend/query-api/src/services/query-service.js @@ -233,6 +233,7 @@ export async function getAssignmentAttendance({ tenantId, assignmentId }) { SELECT a.id AS "assignmentId", a.status, + a.business_id AS "businessId", a.shift_id AS "shiftId", a.staff_id AS "staffId", s.title AS "shiftTitle", diff --git a/backend/query-api/test/app.test.js b/backend/query-api/test/app.test.js index 26f751e2..35444559 100644 --- a/backend/query-api/test/app.test.js +++ b/backend/query-api/test/app.test.js @@ -37,6 +37,10 @@ test('GET /readyz reports database not configured when no database env is presen assert.equal(res.body.status, 'DATABASE_NOT_CONFIGURED'); }); +test.afterEach(() => { + delete process.env.AUTH_BYPASS_CONTEXT; +}); + test('createApp fails fast in protected env when auth bypass is enabled', async () => { process.env.APP_ENV = 'staging'; process.env.AUTH_BYPASS = 'true'; @@ -134,3 +138,28 @@ test('GET /query/tenants/:tenantId/businesses/:businessId/favorite-staff validat assert.equal(res.status, 200); assert.equal(res.body.items[0].staffId, staffId); }); + +test('GET /query/tenants/:tenantId/orders denies mismatched tenant scope before handler execution', async () => { + process.env.AUTH_BYPASS_CONTEXT = JSON.stringify({ + user: { userId: 'test-user' }, + tenant: { tenantId: '99999999-9999-4999-8999-999999999999', role: 'MANAGER' }, + business: { businessId }, + }); + + const app = createApp({ + queryService: { + listOrders: async () => assert.fail('listOrders should not be called'), + getOrderDetail: async () => assert.fail('getOrderDetail should not be called'), + listFavoriteStaff: async () => assert.fail('listFavoriteStaff should not be called'), + getStaffReviewSummary: async () => assert.fail('getStaffReviewSummary should not be called'), + getAssignmentAttendance: async () => assert.fail('getAssignmentAttendance should not be called'), + }, + }); + + const res = await request(app) + .get(`/query/tenants/${tenantId}/orders`) + .set('Authorization', 'Bearer test-token'); + + assert.equal(res.status, 403); + assert.equal(res.body.code, 'FORBIDDEN'); +}); diff --git a/backend/query-api/test/policy.test.js b/backend/query-api/test/policy.test.js new file mode 100644 index 00000000..0ac9c357 --- /dev/null +++ b/backend/query-api/test/policy.test.js @@ -0,0 +1,86 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { can } from '../src/services/policy.js'; + +test('orders.read requires client scope and matching tenant/business scope', async () => { + const allowed = await can( + 'orders.read', + 'order', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1', role: 'MANAGER' }, + business: { businessId: 'business-1' }, + }, + }, + { params: { tenantId: 'tenant-1' }, query: { businessId: 'business-1' } } + ); + + const denied = await can( + 'orders.read', + 'order', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1', role: 'MANAGER' }, + business: { businessId: 'business-1' }, + }, + }, + { params: { tenantId: 'tenant-2' }, query: { businessId: 'business-1' } } + ); + + assert.equal(allowed, true); + assert.equal(denied, false); +}); + +test('shifts.read requires staff scope', async () => { + const allowed = await can( + 'shifts.read', + 'shift', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + staff: { staffId: 'staff-1' }, + }, + }, + { params: {} } + ); + + const denied = await can( + 'shifts.read', + 'shift', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + business: { businessId: 'business-1' }, + }, + }, + { params: {} } + ); + + assert.equal(allowed, true); + assert.equal(denied, false); +}); + +test('attendance.read allows tenant-scoped actor', async () => { + const allowed = await can( + 'attendance.read', + 'attendance', + { + uid: 'user-1', + policyContext: { + user: { userId: 'user-1' }, + tenant: { tenantId: 'tenant-1' }, + }, + }, + { params: { tenantId: 'tenant-1' } } + ); + + assert.equal(allowed, true); +});