mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-09-08 14:48:18 +00:00
fix(auth): keep secondary positions in permission checks
IAM lets an employee hold several positions, but the vendored JwtGuard collapses employee.positions[] down to a single employee.position and drops the rest. Non-delegate secondary positions vanished entirely, so staff on two posts resolved to one post's permissions and every check on the other rejected them. FreightJwtGuard re-attaches the full list from the same session snapshot the parent guard already read, so nothing extra is fetched per request beyond a cached session lookup. employee.position is left untouched, keeping audit logging and delegation unaffected. collectPermissionKeys and collectPositionTypeKeys now union across every position, and /me returns them all. Verified against a real two-position user (djibouti-gl-director + djibouti-gl-chief) on the local dev database: /me positions 1 -> 2 /me permissionKeys 17 -> 28 GET /api/interchange-documents 403 -> 200 GET /api/trains 403 -> 200 11 permissions recovered, none lost. Six single-position users return byte-identical payloads before and after.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { applyDecorators, UseGuards } from '@nestjs/common';
|
||||
import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard';
|
||||
import { FreightJwtGuard } from './freight-jwt.guard';
|
||||
|
||||
import {
|
||||
FreightPermissionGuard,
|
||||
@@ -11,7 +11,7 @@ import { FREIGHT_PERMS } from '../seed/freight-permissions.registry';
|
||||
export const BookingStaff = (permission: string | string[]) =>
|
||||
applyDecorators(
|
||||
UseGuards(
|
||||
JwtGuard,
|
||||
FreightJwtGuard,
|
||||
FreightPermissionGuard(
|
||||
Array.isArray(permission) ? permission : [permission],
|
||||
),
|
||||
@@ -26,11 +26,11 @@ export const BookingStaff = (permission: string | string[]) =>
|
||||
* BookingStaff(<view key>) or MixedAudience(); kept for routes not yet swept.
|
||||
*/
|
||||
export const StaffReference = () =>
|
||||
applyDecorators(UseGuards(JwtGuard, FreightPermissionGuard([])));
|
||||
applyDecorators(UseGuards(FreightJwtGuard, FreightPermissionGuard([])));
|
||||
|
||||
/** Portal routes: customer accounts only; ownership scoping stays in services. */
|
||||
export const PortalCustomer = () =>
|
||||
applyDecorators(UseGuards(JwtGuard, PortalCustomerGuard));
|
||||
applyDecorators(UseGuards(FreightJwtGuard, PortalCustomerGuard));
|
||||
|
||||
/**
|
||||
* Routes both audiences call (sign, shared document reads, handover): staff
|
||||
@@ -40,7 +40,7 @@ export const PortalCustomer = () =>
|
||||
export const MixedAudience = (permission: string | string[]) =>
|
||||
applyDecorators(
|
||||
UseGuards(
|
||||
JwtGuard,
|
||||
FreightJwtGuard,
|
||||
MixedAudienceGuard(
|
||||
Array.isArray(permission) ? permission : [permission],
|
||||
),
|
||||
|
||||
101
apps/edr-freight-api/src/common/freight-jwt.guard.ts
Normal file
101
apps/edr-freight-api/src/common/freight-jwt.guard.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
|
||||
import { Reflector } from '@nestjs/core';
|
||||
import { InjectDataSource } from '@nestjs/typeorm';
|
||||
import { JwtGuard as IamJwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard';
|
||||
import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type';
|
||||
import { DataSource } from 'typeorm';
|
||||
|
||||
/** One position as the login snapshot stores it (`iam.sessions.userInfo`). */
|
||||
type SnapshotPosition = { id?: string; [key: string]: unknown };
|
||||
|
||||
type SessionUserInfo = {
|
||||
employee?: { id?: string; positions?: SnapshotPosition[] }[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Like the IAM JwtGuard, but keeps the caller's SECONDARY positions.
|
||||
*
|
||||
* IAM models an employee as holding many positions, and the login snapshot in
|
||||
* `iam.sessions.userInfo` carries all of them. `JwtGuard.parseToken` then
|
||||
* collapses that to a single `employee.position` — whichever the request
|
||||
* headers select, else `positions[0]` — and drops the rest. Non-delegate
|
||||
* secondary positions vanish entirely, so staff holding two posts resolve to
|
||||
* only one post's permissions and every check on the other one rejects them.
|
||||
*
|
||||
* This re-attaches the full list as `employee.positions`. `employee.position`
|
||||
* is left exactly as the parent set it, so everything reading the single
|
||||
* position today (audit log, delegation deadline) is unaffected; only the
|
||||
* permission utils, which prefer the array, see the difference.
|
||||
*/
|
||||
@Injectable()
|
||||
export class FreightJwtGuard extends IamJwtGuard implements CanActivate {
|
||||
// ponytail: unbounded-until-TTL map, cleared wholesale when it gets big.
|
||||
// Sessions are few and the value is small; swap for an LRU if that changes.
|
||||
private static readonly CACHE_TTL_MS = 30_000;
|
||||
private static readonly CACHE_MAX_ENTRIES = 5_000;
|
||||
private readonly cache = new Map<
|
||||
string,
|
||||
{ positions: SnapshotPosition[]; expiresAt: number }
|
||||
>();
|
||||
|
||||
constructor(
|
||||
reflector: Reflector,
|
||||
@InjectDataSource() private readonly ds: DataSource,
|
||||
) {
|
||||
super(reflector, ds);
|
||||
}
|
||||
|
||||
async canActivate(context: ExecutionContext): Promise<boolean> {
|
||||
if (!(await super.canActivate(context))) return false;
|
||||
|
||||
const user = context.switchToHttp().getRequest().user as
|
||||
| TCurrentUser
|
||||
| undefined;
|
||||
const employee = user?.employee;
|
||||
if (!employee || !user?.sessionId) return true;
|
||||
|
||||
const positions = await this.positionsForSession(
|
||||
user.sessionId,
|
||||
employee.id,
|
||||
);
|
||||
// Never blank out what the parent resolved: an unreadable session or a
|
||||
// snapshot without positions must degrade to the single-position
|
||||
// behaviour, not to no positions at all.
|
||||
if (positions.length) {
|
||||
(employee as { positions?: SnapshotPosition[] }).positions = positions;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/** Every position the login snapshot holds for this employee. */
|
||||
private async positionsForSession(
|
||||
sessionId: string,
|
||||
employeeId: string | undefined,
|
||||
): Promise<SnapshotPosition[]> {
|
||||
const now = Date.now();
|
||||
const hit = this.cache.get(sessionId);
|
||||
if (hit && hit.expiresAt > now) return hit.positions;
|
||||
|
||||
let positions: SnapshotPosition[] = [];
|
||||
try {
|
||||
const rows: { userInfo: SessionUserInfo | null }[] = await this.ds.query(
|
||||
`SELECT "userInfo" FROM iam.sessions WHERE id = $1`,
|
||||
[sessionId],
|
||||
);
|
||||
const employees = rows[0]?.userInfo?.employee ?? [];
|
||||
const match =
|
||||
employees.find((e) => e?.id && e.id === employeeId) ?? employees[0];
|
||||
positions = match?.positions ?? [];
|
||||
} catch {
|
||||
return []; // iam unreachable — caller keeps the parent's single position
|
||||
}
|
||||
|
||||
if (this.cache.size >= FreightJwtGuard.CACHE_MAX_ENTRIES)
|
||||
this.cache.clear();
|
||||
this.cache.set(sessionId, {
|
||||
positions,
|
||||
expiresAt: now + FreightJwtGuard.CACHE_TTL_MS,
|
||||
});
|
||||
return positions;
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
assertCanApproveContractStep,
|
||||
canEditContractStep,
|
||||
collectPermissionKeys,
|
||||
collectPositionTypeKeys,
|
||||
hasFreightPermission,
|
||||
setPositionTypePermissionResolver,
|
||||
} from './freight-permission.util';
|
||||
@@ -121,3 +122,66 @@ describe('collectPermissionKeys — position-type grants', () => {
|
||||
expect(hasFreightPermission(direct, CLEARANCE)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* IAM lets an employee hold several positions, but the vendored `JwtGuard`
|
||||
* collapses `employee.positions[]` down to a single `employee.position` and
|
||||
* drops the rest — so staff on two posts resolved to one post's permissions
|
||||
* and every check on the other rejected them. `FreightJwtGuard` restores the
|
||||
* full list as `employee.positions`; these cover the union that depends on it.
|
||||
*/
|
||||
describe('multiple positions', () => {
|
||||
// Shaped like the real two-post employee: GL chief AND GL director.
|
||||
const twoPost = {
|
||||
employee: {
|
||||
// What the vendored guard leaves behind — one of the two, arbitrarily.
|
||||
position: {
|
||||
positionType: { key: 'djibouti-gl-chief' },
|
||||
permissions: [{ key: FREIGHT_PERMS.contracts.view }],
|
||||
},
|
||||
// What FreightJwtGuard puts back.
|
||||
positions: [
|
||||
{
|
||||
positionType: { key: 'djibouti-gl-chief' },
|
||||
permissions: [{ key: FREIGHT_PERMS.contracts.view }],
|
||||
},
|
||||
{
|
||||
positionType: { key: 'djibouti-gl-director' },
|
||||
permissions: [{ key: FREIGHT_PERMS.bookings.view }],
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
it('unions permissions across every position', () => {
|
||||
const keys = collectPermissionKeys(twoPost);
|
||||
expect(keys).toContain(FREIGHT_PERMS.contracts.view);
|
||||
expect(keys).toContain(FREIGHT_PERMS.bookings.view);
|
||||
});
|
||||
|
||||
it('grants the secondary position’s permission, not just the first', () => {
|
||||
expect(hasFreightPermission(twoPost, FREIGHT_PERMS.bookings.view)).toBe(true);
|
||||
});
|
||||
|
||||
it('answers to both position types', () => {
|
||||
expect(collectPositionTypeKeys(twoPost)).toEqual(
|
||||
expect.arrayContaining(['djibouti-gl-chief', 'djibouti-gl-director']),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not double-count the position the guard also left singular', () => {
|
||||
const keys = collectPermissionKeys(twoPost);
|
||||
expect(keys.filter((k) => k === FREIGHT_PERMS.contracts.view)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('still resolves the single position when the array is absent', () => {
|
||||
// A request that skipped FreightJwtGuard must degrade to the old behaviour,
|
||||
// not to no permissions at all.
|
||||
const onePost = {
|
||||
employee: {
|
||||
position: { permissions: [{ key: FREIGHT_PERMS.contracts.view }] },
|
||||
},
|
||||
};
|
||||
expect(hasFreightPermission(onePost, FREIGHT_PERMS.contracts.view)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,6 +17,15 @@ type MeLikeUser = {
|
||||
permissions?: PermissionLike[];
|
||||
positionType?: PositionTypeLike | null;
|
||||
};
|
||||
/**
|
||||
* Every position the employee holds, restored by `FreightJwtGuard`
|
||||
* from the login snapshot. The IAM guard only ever sets the singular
|
||||
* `position` above; without this, a second post's grants are invisible.
|
||||
*/
|
||||
positions?: {
|
||||
permissions?: PermissionLike[];
|
||||
positionType?: PositionTypeLike | null;
|
||||
}[];
|
||||
delegatedPositions?: { permissions?: PermissionLike[] }[];
|
||||
}
|
||||
| {
|
||||
@@ -98,10 +107,15 @@ export function collectPermissionKeys(user: MeLikeUser | null | undefined): stri
|
||||
return [...keys];
|
||||
}
|
||||
|
||||
for (const p of employee.position?.permissions ?? []) {
|
||||
if (p.key) keys.add(p.key);
|
||||
// `position` is whichever single post the IAM guard selected; `positions` is
|
||||
// the full set FreightJwtGuard restores. Walk both — the array is absent on
|
||||
// a session the guard could not re-read, and the two overlap harmlessly.
|
||||
for (const pos of [employee.position, ...(employee.positions ?? [])]) {
|
||||
for (const p of pos?.permissions ?? []) {
|
||||
if (p.key) keys.add(p.key);
|
||||
}
|
||||
addTypePermissions(pos?.positionType);
|
||||
}
|
||||
addTypePermissions(employee.position?.positionType);
|
||||
for (const delegated of employee.delegatedPositions ?? []) {
|
||||
for (const p of delegated.permissions ?? []) {
|
||||
if (p.key) keys.add(p.key);
|
||||
@@ -158,8 +172,10 @@ export function collectPositionTypeKeys(
|
||||
return [...keys];
|
||||
}
|
||||
|
||||
if (employee.position?.positionType?.key) {
|
||||
keys.add(employee.position.positionType.key);
|
||||
// Both shapes, same reason as collectPermissionKeys: an employee holding two
|
||||
// posts answers to both their position types.
|
||||
for (const pos of [employee.position, ...(employee.positions ?? [])]) {
|
||||
if (pos?.positionType?.key) keys.add(pos.positionType.key);
|
||||
}
|
||||
return [...keys];
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { applyDecorators, UseGuards } from '@nestjs/common';
|
||||
import { JwtGuard } from '@tria-plc/api-common/modules/auth/services/jwt.guard';
|
||||
import { FreightJwtGuard } from './freight-jwt.guard';
|
||||
|
||||
import { FreightPermissionGuard } from './freight-permission.guard';
|
||||
import {
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
|
||||
export const RuleEngineView = (slug: RuleEngineResourceSlug) =>
|
||||
applyDecorators(
|
||||
UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.view(slug)])),
|
||||
UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.view(slug)])),
|
||||
);
|
||||
|
||||
// Granular CRUD replaces the retired coarse RuleEngineManage. Each write
|
||||
@@ -18,17 +18,17 @@ export const RuleEngineView = (slug: RuleEngineResourceSlug) =>
|
||||
// update on PATCH / reorder / move-order, delete on DELETE.
|
||||
export const RuleEngineCreate = (slug: RuleEngineResourceSlug) =>
|
||||
applyDecorators(
|
||||
UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.create(slug)])),
|
||||
UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.create(slug)])),
|
||||
);
|
||||
|
||||
export const RuleEngineUpdate = (slug: RuleEngineResourceSlug) =>
|
||||
applyDecorators(
|
||||
UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.update(slug)])),
|
||||
UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.update(slug)])),
|
||||
);
|
||||
|
||||
export const RuleEngineDelete = (slug: RuleEngineResourceSlug) =>
|
||||
applyDecorators(
|
||||
UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.delete(slug)])),
|
||||
UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.delete(slug)])),
|
||||
);
|
||||
|
||||
/**
|
||||
@@ -38,5 +38,5 @@ export const RuleEngineDelete = (slug: RuleEngineResourceSlug) =>
|
||||
*/
|
||||
export const RuleEngineApprove = (slug: RuleEngineApprovableSlug) =>
|
||||
applyDecorators(
|
||||
UseGuards(JwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.approve(slug)])),
|
||||
UseGuards(FreightJwtGuard, FreightPermissionGuard([FREIGHT_PERMS.ruleEngine.approve(slug)])),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user