fix(auth): load every position a user holds

Staff given a post in Smart Office and another in freight only ever loaded one of them. Two causes, both in how the IAM guard collapses the login snapshot: - `x-current-position-id` is read two ways inside one function: the employee row is matched on `position.id`, the position on `employeePositionId`. Freight sends the latter, Smart Office the former, so whichever value arrives one lookup matches nothing and falls back to `positions[0]`. FreightJwtGuard now matches both fields. - IAM keeps one employee row per organization, and EDR and EDR Freight are separate organizations, so a user holding a post in each owns two rows. Only the active row reached `collectPermissionKeys`, so the freight post's permissions disappeared whenever the other row won the active slot. `employee.positions` now unions every row, which is what the util already does for the array shape. `delegatedPositions` stays scoped to the active row on purpose: yard scope widens on it, and someone standing in on another organization's row is not this desk's stand-in. /auth/me now returns every employee row, active row first, so the position picker can offer a desk that is not on the active row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 07:23:40 +00:00 · 2026-09-01 07:03:58 +00:00
parent 3f2bcf1a1a
commit db9d6e49c7
3 changed files with 355 additions and 53 deletions
--- a/apps/edr-freight-api/src/common/freight-jwt.guard.spec.ts
+++ b/apps/edr-freight-api/src/common/freight-jwt.guard.spec.ts
@@ -0,0 +1,148 @@
+import {
+  collectAllPositions,
+  resolveActiveEmployee,
+  type SnapshotEmployee,
+} from './freight-jwt.guard';
+
+// Shapes and ids taken from the real dev session for `test_dj_gl_director`
+// (iam.sessions 800ad793-…), an employee holding two posts on one row.
+const CHIEF = {
+  id: '990189f1-e872-4b8c-9f6a-36259a0df480',
+  employeePositionId: 'd0d527f6-f344-49aa-ab8b-25a448a770b6',
+  name: { en: 'Djibouti GL Chief' },
+  isDelegate: false,
+};
+const DIRECTOR = {
+  id: '258a8d82-28c4-401f-bf88-78f58bb6bd0e',
+  employeePositionId: 'b97aa265-5de8-4ffe-95bf-01f94d38a2df',
+  name: { en: 'Djibouti GL Director' },
+  isDelegate: false,
+};
+
+const EMPLOYEE_ID = '70545ee5-c7d7-4196-af7e-a7eb7e76b21b';
+const oneRow: SnapshotEmployee[] = [
+  { id: EMPLOYEE_ID, positions: [CHIEF, DIRECTOR] },
+];
+
+describe('resolveActiveEmployee', () => {
+  it('leaves the parent guard alone when no position header is sent', () => {
+    const { owner, active } = resolveActiveEmployee(
+      oneRow,
+      undefined,
+      EMPLOYEE_ID,
+    );
+
+    expect(owner).toBe(oneRow[0]);
+    expect(active).toBeUndefined();
+  });
+
+  it("resolves freight's header value (employeePositionId)", () => {
+    const { active } = resolveActiveEmployee(
+      oneRow,
+      DIRECTOR.employeePositionId,
+      EMPLOYEE_ID,
+    );
+
+    expect(active).toBe(DIRECTOR);
+  });
+
+  // The regression this guard exists for: the stock IAM guard matches the
+  // header against employeePositionId only, so Smart Office's position.id
+  // matched nothing and every request silently ran as positions[0].
+  it("resolves Smart Office's header value (position.id)", () => {
+    const { active } = resolveActiveEmployee(oneRow, DIRECTOR.id, EMPLOYEE_ID);
+
+    expect(active).toBe(DIRECTOR);
+    expect(active).not.toBe(CHIEF);
+  });
+
+  it('falls back to the parent row when the header names nothing', () => {
+    const { owner, active } = resolveActiveEmployee(
+      oneRow,
+      'not-a-position-id',
+      EMPLOYEE_ID,
+    );
+
+    expect(owner).toBe(oneRow[0]);
+    expect(active).toBeUndefined();
+  });
+
+  describe('when the two posts sit on different employee rows', () => {
+    const smartOfficeRow: SnapshotEmployee = {
+      id: 'emp-smart-office',
+      positions: [CHIEF],
+    };
+    const freightRow: SnapshotEmployee = {
+      id: 'emp-freight',
+      positions: [DIRECTOR],
+    };
+    const twoRows = [smartOfficeRow, freightRow];
+
+    it('selects the row that owns the requested position', () => {
+      const { owner, active } = resolveActiveEmployee(
+        twoRows,
+        DIRECTOR.employeePositionId,
+        // The parent guard matches the header against position.id only, so it
+        // matched neither row and fell through to the first.
+        smartOfficeRow.id,
+      );
+
+      expect(owner).toBe(freightRow);
+      expect(active).toBe(DIRECTOR);
+    });
+
+    it('keeps the parent row when no header is sent', () => {
+      const { owner } = resolveActiveEmployee(twoRows, undefined, freightRow.id);
+
+      expect(owner).toBe(freightRow);
+    });
+
+    it('falls back to the first row when the parent row is unknown', () => {
+      const { owner } = resolveActiveEmployee(twoRows, undefined, undefined);
+
+      expect(owner).toBe(smartOfficeRow);
+    });
+  });
+});
+
+describe('collectAllPositions', () => {
+  it('unions posts held across separate employee rows', () => {
+    // The real shape: IAM keeps one employee row per organization, and "EDR"
+    // and "EDR Freight" are separate orgs, so a user holding a Smart Office
+    // post and a freight post owns one row each.
+    const smartOfficeRow: SnapshotEmployee = {
+      id: 'emp-edr',
+      organizationId: 'org-edr',
+      positions: [CHIEF],
+    };
+    const freightRow: SnapshotEmployee = {
+      id: 'emp-edr-freight',
+      organizationId: 'org-edr-freight',
+      positions: [DIRECTOR],
+    };
+
+    expect(collectAllPositions([smartOfficeRow, freightRow])).toEqual([
+      CHIEF,
+      DIRECTOR,
+    ]);
+  });
+
+  it('keeps every post when they share one row', () => {
+    expect(collectAllPositions(oneRow)).toEqual([CHIEF, DIRECTOR]);
+  });
+
+  it('de-duplicates a post repeated across rows', () => {
+    const rows: SnapshotEmployee[] = [
+      { id: 'a', positions: [CHIEF] },
+      { id: 'b', positions: [CHIEF, DIRECTOR] },
+    ];
+
+    expect(collectAllPositions(rows)).toEqual([CHIEF, DIRECTOR]);
+  });
+
+  it('tolerates rows carrying no positions', () => {
+    const rows: SnapshotEmployee[] = [{ id: 'a' }, { id: 'b', positions: [] }];
+
+    expect(collectAllPositions(rows)).toEqual([]);
+  });
+});
--- a/apps/edr-freight-api/src/common/freight-jwt.guard.ts
+++ b/apps/edr-freight-api/src/common/freight-jwt.guard.ts
@@ -3,29 +3,124 @@ 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 { CURRENT_POSITION_ID } from '@tria-plc/api-common/utils/constants/tenant.constant';
 import { DataSource } from 'typeorm';

 /** One position as the login snapshot stores it (`iam.sessions.userInfo`). */
-type SnapshotPosition = { id?: string; [key: string]: unknown };
+export type SnapshotPosition = {
+  id?: string;
+  employeePositionId?: string;
+  isDelegate?: boolean;
+  [key: string]: unknown;
+};

-type SessionUserInfo = {
-  employee?: { id?: string; positions?: SnapshotPosition[] }[];
+/** One employee row as the snapshot stores it. A user may hold several. */
+export type SnapshotEmployee = {
+  id?: string;
+  positions?: SnapshotPosition[];
+  [key: string]: unknown;
+};
+
+type SessionUserInfo = { employee?: SnapshotEmployee[] };
+
+/**
+ * `x-current-position-id` is sent with two different meanings by two different
+ * frontends, and the IAM guard reads it both ways in the same function: it
+ * picks the EMPLOYEE row by `position.id` but the POSITION by
+ * `employeePositionId`. Freight sends `employeePositionId`, Smart Office sends
+ * `position.id` — so whichever value arrives, one of the two lookups silently
+ * matches nothing and falls back to the first entry.
+ *
+ * Matching both fields is what makes the header mean one thing again.
+ */
+const identifies = (position: SnapshotPosition, id: string): boolean =>
+  position?.id === id || position?.employeePositionId === id;
+
+/**
+ * Every post the user holds, across every employee row, first occurrence kept.
+ *
+ * IAM keeps one employee row per ORGANIZATION, and "EDR" and "EDR Freight" are
+ * separate organizations — so a user given a freight post and a Smart Office
+ * post owns two rows, one post on each. Only one row can be the active one, and
+ * a permission check that reads only that row cannot see the other post at all.
+ */
+export const collectAllPositions = (
+  employees: SnapshotEmployee[],
+): SnapshotPosition[] => {
+  const seen = new Set<string>();
+  const all: SnapshotPosition[] = [];
+
+  for (const employee of employees) {
+    for (const position of employee.positions ?? []) {
+      const key = position.employeePositionId ?? position.id;
+      if (key) {
+        if (seen.has(key)) continue;
+        seen.add(key);
+      }
+      all.push(position);
+    }
+  }
+
+  return all;
 };

 /**
- * Like the IAM JwtGuard, but keeps the caller's SECONDARY positions.
+ * Which employee row the caller is acting as, and which of its positions the
+ * request selected. Pure so it can be tested without a session or a token.
 *
- * 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.
+ * `owner` is the row holding the requested position; failing that the row the
+ * parent guard already picked; failing that the first. `active` is undefined
+ * when no header was sent or it names nothing — the caller then leaves the
+ * parent's choice of `employee.position` alone.
+ */
+export const resolveActiveEmployee = (
+  employees: SnapshotEmployee[],
+  requestedId: string | undefined,
+  parentEmployeeId: string | undefined,
+): { owner: SnapshotEmployee | undefined; active: SnapshotPosition | undefined } => {
+  const owner =
+    (requestedId &&
+      employees.find((candidate) =>
+        (candidate.positions ?? []).some((position) =>
+          identifies(position, requestedId),
+        ),
+      )) ||
+    employees.find(
+      (candidate) => candidate.id && candidate.id === parentEmployeeId,
+    ) ||
+    employees[0];
+
+  const active = requestedId
+    ? (owner?.positions ?? []).find((position) =>
+        identifies(position, requestedId),
+      )
+    : undefined;
+
+  return { owner, active };
+};
+
+/**
+ * Like the IAM JwtGuard, but resolves the caller's position honestly.
 *
- * 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.
+ * IAM models an employee as holding many positions — and a user as possibly
+ * holding several employee rows — and the login snapshot in
+ * `iam.sessions.userInfo` carries all of them. `JwtGuard.parseToken` collapses
+ * that to a single `employee.position` and drops the rest, so staff holding two
+ * posts resolve to one post's permissions and every check on the other one
+ * rejects them.
+ *
+ * This guard re-reads the snapshot and fixes three things the parent gets wrong:
+ *
+ *  1. re-attaches the full position list as `employee.positions`, which is what
+ *     the permission utils union over;
+ *  2. selects the employee row that actually owns the requested position, so a
+ *     post held on a second employee row is reachable at all;
+ *  3. sets `employee.position` to the requested position when the parent's
+ *     one-sided id match missed it, keeping `auditUser` in step.
+ *
+ * Every correction is skipped unless the snapshot positively resolves it, so an
+ * unreadable session degrades to the parent's single-position behaviour rather
+ * than to no position at all.
 */
@Injectable()
 export class FreightJwtGuard extends IamJwtGuard implements CanActivate {
@@ -35,7 +130,7 @@ export class FreightJwtGuard extends IamJwtGuard implements CanActivate {
  private static readonly CACHE_MAX_ENTRIES = 5_000;
  private readonly cache = new Map<
    string,
-    { positions: SnapshotPosition[]; expiresAt: number }
+    { employees: SnapshotEmployee[]; expiresAt: number }
  >();

  constructor(
@@ -48,44 +143,78 @@ export class FreightJwtGuard extends IamJwtGuard implements CanActivate {
  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;
+    const request = context.switchToHttp().getRequest();
+    const user = request.user as TCurrentUser | undefined;
+    const employee = user?.employee as SnapshotEmployee | undefined;
    if (!employee || !user?.sessionId) return true;

-    const positions = await this.positionsForSession(
-      user.sessionId,
+    const employees = await this.employeesForSession(user.sessionId);
+    if (!employees.length) return true;
+
+    const requestedId = request.headers?.[CURRENT_POSITION_ID] as
+      | string
+      | undefined;
+
+    const { owner, active } = resolveActiveEmployee(
+      employees,
+      requestedId,
      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;
+
+    const ownerPositions = owner?.positions ?? [];
+    // Never blank out what the parent resolved: a snapshot without positions
+    // must degrade to the single-position behaviour, not to no positions.
+    if (!ownerPositions.length) return true;
+
+    // Carries the owning row's id / unitId / organizationId too, which unit
+    // scoping downstream reads — a swapped row must be swapped whole.
+    Object.assign(employee, owner);
+
+    // `collectPermissionKeys` / `collectPositionTypeKeys` union over this, and
+    // a user's posts can span several employee rows (one per organization), so
+    // it carries every row's — otherwise a freight post is invisible whenever
+    // another organization's row wins the active slot.
+    employee.positions = collectAllPositions(employees);
+
+    // Delegation stays scoped to the active desk: yard scope widens on
+    // `delegatedPositions`, and someone standing in on another organization's
+    // row is not this desk's stand-in.
+    employee.delegatedPositions = ownerPositions.filter(
+      (position) => position.isDelegate,
+    );
+
+    // The full set, for `/auth/me` — the position picker has to be able to
+    // offer a desk on a row that is not the active one.
+    (user as { employeeRows?: SnapshotEmployee[] }).employeeRows = employees;
+
+    if (active) {
+      employee.position = active;
+      // The parent already built `auditUser` from the position it guessed.
+      if (request.auditUser) {
+        request.auditUser.employeeId = employee.id;
+        request.auditUser.positionId = active.id;
+        request.auditUser.employeePositionId = active.employeePositionId;
+      }
    }
+
    return true;
  }

-  /** Every position the login snapshot holds for this employee. */
-  private async positionsForSession(
+  /** Every employee row the login snapshot holds for this session. */
+  private async employeesForSession(
    sessionId: string,
-    employeeId: string | undefined,
-  ): Promise<SnapshotPosition[]> {
+  ): Promise<SnapshotEmployee[]> {
    const now = Date.now();
    const hit = this.cache.get(sessionId);
-    if (hit && hit.expiresAt > now) return hit.positions;
+    if (hit && hit.expiresAt > now) return hit.employees;

-    let positions: SnapshotPosition[] = [];
+    let employees: SnapshotEmployee[] = [];
    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 ?? [];
+      employees = rows[0]?.userInfo?.employee ?? [];
    } catch {
      return []; // iam unreachable — caller keeps the parent's single position
    }
@@ -93,9 +222,9 @@ export class FreightJwtGuard extends IamJwtGuard implements CanActivate {
    if (this.cache.size >= FreightJwtGuard.CACHE_MAX_ENTRIES)
      this.cache.clear();
    this.cache.set(sessionId, {
-      positions,
+      employees,
      expiresAt: now + FreightJwtGuard.CACHE_TTL_MS,
    });
-    return positions;
+    return employees;
  }
 }
--- a/apps/edr-freight-api/src/modules/auth/freight-me.service.ts
+++ b/apps/edr-freight-api/src/modules/auth/freight-me.service.ts
@@ -3,6 +3,7 @@ import { InjectDataSource } from '@nestjs/typeorm';
 import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type';
 import { DataSource } from 'typeorm';

+import type { SnapshotEmployee } from '../../common/freight-jwt.guard';
 import {
  collectPermissionKeys,
  isSuperAdmin,
@@ -82,8 +83,26 @@ export class FreightMeService {
        ? [employeeRecord.position]
        : [];

-    const enrichedPositions = await Promise.all(
-      rawPositions.map(async (position) => {
+    // IAM keeps one employee row per organization, so a user holding a freight
+    // post and a Smart Office post owns two rows. The backoffice reads
+    // `employee` as an array and the position picker lists what it finds there
+    // — returning only the active row hides the other desk and makes it
+    // unselectable. `FreightJwtGuard` leaves the full set here.
+    const employeeRows = (user as { employeeRows?: SnapshotEmployee[] })
+      .employeeRows;
+
+    // Active row first: the backoffice reads `employee[0]` for
+    // unitId/organizationId, so the desk the caller is acting as must lead.
+    const rows: SnapshotEmployee[] = employeeRows?.length
+      ? [
+          ...employeeRows.filter((row) => row.id === employeeRecord?.id),
+          ...employeeRows.filter((row) => row.id !== employeeRecord?.id),
+        ]
+      : employeeRecord
+        ? [{ ...employeeRecord, positions: rawPositions } as SnapshotEmployee]
+        : [];
+
+    const enrichPosition = async (position: TokenPosition) => {
        const [positionType, positionTypePermissionKeys] = await Promise.all([
          this.lookupPositionType(position.id),
          this.lookupPositionTypePermissions(position.id),
@@ -114,20 +133,24 @@ export class FreightMeService {
            positionType,
          },
        };
-      }),
+    };
+
+    const enrichedRows = await Promise.all(
+      rows.map(async (row) => ({
+        row,
+        positions: await Promise.all(
+          ((row.positions ?? []) as TokenPosition[]).map(enrichPosition),
+        ),
+      })),
    );

-    const employee = employeeRecord
-      ? [
-          {
-            id: employeeRecord.id,
-            organizationId: employeeRecord.organizationId,
-            unitId: employeeRecord.unitId,
-            name: employeeRecord.name,
-            positions: enrichedPositions.map((p) => p.position),
-          },
-        ]
-      : [];
+    const employee = enrichedRows.map(({ row, positions }) => ({
+      id: row.id as string,
+      organizationId: row.organizationId as string,
+      unitId: row.unitId as string,
+      name: row.name,
+      positions: positions.map((p) => p.position),
+    }));

    // `collectPermissionKeys` reads the raw token (position-level only), so
    // union the type-level grants in — the backoffice prefers this flat list
@@ -135,7 +158,9 @@ export class FreightMeService {
    const permissionKeys = [
      ...new Set([
        ...collectPermissionKeys(user),
-        ...enrichedPositions.flatMap((p) => p.positionTypePermissionKeys),
+        ...enrichedRows.flatMap(({ positions }) =>
+          positions.flatMap((p) => p.positionTypePermissionKeys),
+        ),
      ]),
    ];