diff --git a/apps/edr-freight-api/src/common/freight-jwt.guard.spec.ts b/apps/edr-freight-api/src/common/freight-jwt.guard.spec.ts new file mode 100644 index 000000000..817e967f8 --- /dev/null +++ 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([]); + }); +}); diff --git a/apps/edr-freight-api/src/common/freight-jwt.guard.ts b/apps/edr-freight-api/src/common/freight-jwt.guard.ts index 68b842217..9dbc36b73 100644 --- 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(); + 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 { 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 { + ): Promise { 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; } } diff --git a/apps/edr-freight-api/src/modules/audit/audit.interceptor.ts b/apps/edr-freight-api/src/modules/audit/audit.interceptor.ts index cf12fd4d3..8b5c82e9c 100644 --- a/apps/edr-freight-api/src/modules/audit/audit.interceptor.ts +++ b/apps/edr-freight-api/src/modules/audit/audit.interceptor.ts @@ -4,24 +4,17 @@ import { HttpException, Injectable, NestInterceptor, -} from '@nestjs/common'; -import { Observable, tap } from 'rxjs'; -import type { Request, Response } from 'express'; +} from "@nestjs/common"; +import { Observable, tap } from "rxjs"; +import type { Request, Response } from "express"; -import { AuditService } from './audit.service'; -import { - auditEndpointMatcher, - type MatchedAuditEndpoint, -} from './audit-endpoint-matcher'; -import { - isAuditableActor, - resolveAuditActor, - type AuditActorSource, -} from './audit-actor'; -import { redactUrlQuery, sanitizeRequestPayload } from './audit.sanitizer'; +import { AuditService } from "./audit.service"; +import { auditEndpointMatcher, type MatchedAuditEndpoint } from "./audit-endpoint-matcher"; +import { isAuditableActor, resolveAuditActor, type AuditActorSource } from "./audit-actor"; +import { redactUrlQuery, sanitizeRequestPayload } from "./audit.sanitizer"; /** Methods that can change state. Everything else is never audited. */ -const AUDITED_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); +const AUDITED_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]); /** `error_message` ceiling — stack traces do not belong in this column. */ const MAX_ERROR_LENGTH = 2_000; @@ -52,7 +45,7 @@ export class AuditInterceptor implements NestInterceptor { intercept(context: ExecutionContext, next: CallHandler): Observable { // Non-HTTP contexts (the RabbitMQ microservice transport) have no request. - if (context.getType() !== 'http') return next.handle(); + if (context.getType() !== "http") return next.handle(); const httpContext = context.switchToHttp(); const request = httpContext.getRequest(); @@ -72,10 +65,7 @@ export class AuditInterceptor implements NestInterceptor { const startedAt = Date.now(); // The body is captured up front: handlers are free to mutate the DTO they // are given, so reading it after the fact can record post-mutation values. - const requestPayload = sanitizeRequestPayload( - request.body, - request.files ?? request.file, - ); + const requestPayload = sanitizeRequestPayload(request.body, request.files ?? request.file); return next.handle().pipe( tap({ @@ -137,7 +127,6 @@ export class AuditInterceptor implements NestInterceptor { resourceId: matched.resourceId, request: requestPayload, ipAddress: resolveIp(request), - userAgent: request.headers['user-agent'] ?? null, requestId: resolveRequestId(request), durationMs: Date.now() - startedAt, }); @@ -154,10 +143,10 @@ function resolveErrorMessage(error: unknown): string | null { if (error instanceof HttpException) { const response = error.getResponse(); const message = - typeof response === 'string' + typeof response === "string" ? response : ((response as { message?: unknown })?.message ?? error.message); - const text = Array.isArray(message) ? message.join('; ') : String(message); + const text = Array.isArray(message) ? message.join("; ") : String(message); return text.slice(0, MAX_ERROR_LENGTH); } @@ -171,19 +160,19 @@ function resolveErrorMessage(error: unknown): string | null { * entry (the original client) taken. */ function resolveIp(request: Request): string | null { - const forwarded = request.headers['x-forwarded-for']; + const forwarded = request.headers["x-forwarded-for"]; const raw = Array.isArray(forwarded) ? forwarded[0] : forwarded; - const candidate = raw?.split(',')[0]?.trim() || request.ip; + const candidate = raw?.split(",")[0]?.trim() || request.ip; if (!candidate) return null; // Normalize IPv4-mapped IPv6 (`::ffff:10.0.0.1`), which the `inet` column // accepts but which reads badly and breaks grouping by address. - return candidate.startsWith('::ffff:') ? candidate.slice(7) : candidate; + return candidate.startsWith("::ffff:") ? candidate.slice(7) : candidate; } /** Correlation id from the proxy/tracing layer, when present. */ function resolveRequestId(request: RequestWithUser): string | null { - const header = request.headers['x-request-id'] ?? request.headers['x-correlation-id']; + const header = request.headers["x-request-id"] ?? request.headers["x-correlation-id"]; const value = Array.isArray(header) ? header[0] : header; return (value ?? request.id ?? null)?.toString().slice(0, 64) ?? null; } diff --git a/apps/edr-freight-api/src/modules/auth/freight-me.service.ts b/apps/edr-freight-api/src/modules/auth/freight-me.service.ts index b897c5fef..0195bba47 100644 --- 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), + ), ]), ]; diff --git a/apps/edr-freight-api/src/modules/chat/chat-bridge.service.spec.ts b/apps/edr-freight-api/src/modules/chat/chat-bridge.service.spec.ts new file mode 100644 index 000000000..05dc84b29 --- /dev/null +++ b/apps/edr-freight-api/src/modules/chat/chat-bridge.service.spec.ts @@ -0,0 +1,78 @@ +import 'reflect-metadata'; + +import { NotificationType, type NotifyInput } from '@edr/types'; + +import type { ChatConfig } from '../../config/chat.config'; +import { ChatBridgeService } from './chat-bridge.service'; +import type { MatrixClient } from './matrix.client'; + +const config: ChatConfig = { + enabled: true, + baseUrl: 'https://matrix.test', + publicBaseUrl: 'https://matrix.test', + webUrl: 'https://chat.test', + serverName: 'matrix.test', + jwtSecret: 'secret', + adminToken: 'syt_whatever', +}; + +function harness(overrides: Partial = {}) { + const matrix = { + ensureRoom: jest.fn(async (alias: string) => `!${alias}:matrix.test`), + sendMessage: jest.fn( + async (_roomId: string, _body: string, _html?: string) => undefined, + ), + }; + const service = new ChatBridgeService( + { ...config, ...overrides }, + matrix as unknown as MatrixClient, + ); + return { service, matrix }; +} + +const notification = (type: NotificationType): NotifyInput => + ({ type, title: 'Booking BK-1', body: 'needs review' }) as unknown as NotifyInput; + +describe('ChatBridgeService', () => { + it('posts every notification type into #freight-alerts', async () => { + // This used to route REQUEST_SUBMITTED and CLEARANCE_REVIEW to a hardcoded + // `dept-operation` alias, but the reconcile derives dept aliases from the + // IAM position key (`edr_freight_app/opn` shaped), so nothing it created + // ever matched. The bridge made its own empty room and posted there, where + // no employee was a member. + const { service, matrix } = harness(); + + for (const type of [ + NotificationType.REQUEST_SUBMITTED, + NotificationType.CLEARANCE_REVIEW, + NotificationType.GENERIC, + ]) { + await service.bridge(notification(type)); + } + + expect(new Set(matrix.ensureRoom.mock.calls.map(([alias]) => alias))).toEqual( + new Set(['freight-alerts']), + ); + expect(matrix.sendMessage).toHaveBeenCalledTimes(3); + }); + + it('does nothing at all when chat is switched off', async () => { + const { service, matrix } = harness({ enabled: false }); + + await service.bridge(notification(NotificationType.GENERIC)); + + expect(matrix.ensureRoom).not.toHaveBeenCalled(); + expect(matrix.sendMessage).not.toHaveBeenCalled(); + }); + + it('never lets a chat failure escape into the notification that triggered it', async () => { + // Same contract as NotificationInboxService.notify(): bridging is + // best-effort and must not roll back the caller's transaction. + const { service, matrix } = harness(); + matrix.ensureRoom.mockRejectedValueOnce(new Error('Matrix POST ... -> 429')); + + await expect( + service.bridge(notification(NotificationType.GENERIC)), + ).resolves.toBeUndefined(); + }); +}); diff --git a/apps/edr-freight-api/src/modules/chat/chat-bridge.service.ts b/apps/edr-freight-api/src/modules/chat/chat-bridge.service.ts index 75bbe9a29..55c98b897 100644 --- a/apps/edr-freight-api/src/modules/chat/chat-bridge.service.ts +++ b/apps/edr-freight-api/src/modules/chat/chat-bridge.service.ts @@ -1,28 +1,11 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; import type { ConfigType } from '@nestjs/config'; -import { NotificationType, type NotifyInput } from '@edr/types'; +import type { NotifyInput } from '@edr/types'; import chatConfig from '../../config/chat.config'; +import { ALERTS_ROOM } from './chat-provisioning.service'; import { MatrixClient } from './matrix.client'; -const FALLBACK_ROOM = { alias: 'freight-alerts', name: 'Freight Alerts' }; - -/** - * Best-effort per-type routing to an existing dept room. Anything not listed - * (including GENERIC) falls through to #freight-alerts — safer than a wrong - * guess at which department a type belongs to. Extend as real usage shows - * which types actually want a dept room instead of the shared feed. - * - * `name` matters only if this bridge is the very first thing to touch that - * alias (normally the nightly/on-demand reconcile creates dept rooms first, - * with the position's real name) — ensureRoom never renames an existing - * room, so this must match what ChatProvisioningService would have used. - */ -const ROOM_FOR_TYPE: Partial> = { - [NotificationType.REQUEST_SUBMITTED]: { alias: 'dept-operation', name: 'Operation' }, - [NotificationType.CLEARANCE_REVIEW]: { alias: 'dept-operation', name: 'Operation' }, -}; - /** * Mirrors BACKOFFICE-audience notifications into chat so staff see them * without having the inbox open. Hooked once into @@ -31,6 +14,15 @@ const ROOM_FOR_TYPE: Partial${escapeHtml(input.title)}
${escapeHtml(input.body)}${ input.link ? `
${escapeHtml(input.link)}` : '' diff --git a/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.spec.ts b/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.spec.ts index 935f03ab4..3b06cc518 100644 --- a/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.spec.ts +++ b/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.spec.ts @@ -5,34 +5,57 @@ import type { DataSource } from 'typeorm'; import { ChatProvisioningService } from './chat-provisioning.service'; import type { MatrixClient } from './matrix.client'; +const NAA = '03f5eb9e-23a0-4413-8d98-8de4b98b1be2'; +const SUPER_ADMIN = 'f1534714-fa4a-4780-a081-05d4c1f6c25f'; +const BOT = '@edrbot:m.test'; + +interface Holder { + positionKey: string; + positionName: string; + userId: string; + userName: string; +} + +const holder = ( + userId: string, + userName: string, + positionKey: string, + positionName = positionKey, +): Holder => ({ positionKey, positionName, userId, userName }); + +/** + * `members` maps a room id to who Matrix currently reports as joined, so a + * test can put a leaver in a room and watch what the reconcile does about it. + */ +function harness(holders: Holder[], members: Record = {}) { + const matrix = { + mxidFor: jest.fn( + (userId: string, name: string) => `@${name}.${userId.slice(0, 6)}:m.test`, + ), + whoami: jest.fn(async () => BOT), + ensureUser: jest.fn(async (_mxid: string, _name?: string) => undefined), + ensureRoom: jest.fn( + async (alias: string, _name?: string, _opts?: unknown) => `!${alias}:m.test`, + ), + ensureJoined: jest.fn(async (_roomId: string, _mxid: string) => undefined), + joinedMembers: jest.fn(async (roomId: string) => members[roomId] ?? [BOT]), + kick: jest.fn(async (_roomId: string, _mxid: string, _reason: string) => undefined), + lockUser: jest.fn(async (_mxid: string) => undefined), + }; + const dataSource = { query: jest.fn(async () => holders) }; + const service = new ChatProvisioningService( + dataSource as unknown as DataSource, + matrix as unknown as MatrixClient, + ); + return { service, matrix, dataSource }; +} + /** * `joinUserRooms` is the only thing standing between a first sign-in and an * empty Element — the reconcile that would otherwise fill the room list runs - * nightly. Both branches below are outages that actually happened on dev. + * nightly. */ describe('ChatProvisioningService.joinUserRooms', () => { - const NAA = '03f5eb9e-23a0-4413-8d98-8de4b98b1be2'; - const SUPER_ADMIN = 'f1534714-fa4a-4780-a081-05d4c1f6c25f'; - - function harness(holders: unknown[]) { - const matrix = { - mxidFor: jest.fn( - (userId: string, name: string) => `@${name}.${userId.slice(0, 6)}:m.test`, - ), - ensureUser: jest.fn(async (_mxid: string, _name?: string) => undefined), - ensureRoom: jest.fn( - async (alias: string, _name?: string, _opts?: unknown) => `!${alias}:m.test`, - ), - ensureJoined: jest.fn(async (_roomId: string, _mxid: string) => undefined), - }; - const dataSource = { query: jest.fn(async () => holders) }; - const service = new ChatProvisioningService( - dataSource as unknown as DataSource, - matrix as unknown as MatrixClient, - ); - return { service, matrix }; - } - it('creates nothing for a user holding no current position', async () => { // Super Admin on dev: three iam.employees rows, zero employee_positions. // Synapse still auto-registers the account on JWT login, so the only @@ -46,17 +69,12 @@ describe('ChatProvisioningService.joinUserRooms', () => { expect(matrix.ensureJoined).not.toHaveBeenCalled(); }); - it('joins a position holder to the space, #general and their dept room', async () => { + it('joins a holder to the space, #general, #freight-alerts and their dept room', async () => { const { service, matrix } = harness([ - { - positionKey: 'edr_freight_app/marketer', - positionName: 'Marketer', - userId: NAA, - userName: 'naa', - }, + holder(NAA, 'naa', 'edr_freight_app/marketer', 'Marketer'), ]); - await expect(service.joinUserRooms(NAA, 'naa')).resolves.toBe(2); + await expect(service.joinUserRooms(NAA, 'naa')).resolves.toBe(4); // The account has to exist before the admin join API will touch it — JWT // auto-registration happens after this runs. @@ -65,28 +83,112 @@ describe('ChatProvisioningService.joinUserRooms', () => { expect(matrix.ensureRoom.mock.calls.map(([alias]) => alias)).toEqual([ 'edr-freight', 'general', + 'freight-alerts', 'dept-edr_freight_app/marketer', ]); // The space itself is joined, not only the rooms under it: Element shows a // space in the left rail only to its members, so dropping this scatters - // every dept room loose into Home. + // every dept room loose into Home. #freight-alerts is joined here too, or + // a new hire sees no bridged notification until the nightly reconcile. expect(matrix.ensureJoined.mock.calls.map(([roomId]) => roomId)).toEqual([ '!edr-freight:m.test', '!general:m.test', + '!freight-alerts:m.test', '!dept-edr_freight_app/marketer:m.test', ]); }); it('scopes the position lookup to the one user', async () => { - const { service } = harness([]); + const { service, dataSource } = harness([]); await service.joinUserRooms(NAA, 'naa'); // Without the third parameter this would reconcile the whole unit on every // click of "Open EDR Chat". - const [sql, params] = (service as unknown as { - dataSource: { query: jest.Mock }; - }).dataSource.query.mock.calls[0]; + const [sql, params] = dataSource.query.mock.calls[0] as unknown as [ + string, + unknown[], + ]; expect(sql).toContain('AND e.user_id = $3'); expect(params).toEqual(['edr_freight', 'edr_freight_app', NAA]); }); }); + +describe('ChatProvisioningService.reconcile', () => { + it('aborts instead of emptying every room when the holder query returns nothing', async () => { + // Zero holders never means "every employee left at once" — it means the + // query failed, the org/unit keys drifted, or a migration is mid-flight. + // Acting on it would kick every member of every room and lock every + // account, which is exactly the outage this guard exists to prevent. + const { service, matrix } = harness([]); + + await expect(service.reconcile()).rejects.toThrow(/no current position holders/i); + + expect(matrix.kick).not.toHaveBeenCalled(); + expect(matrix.lockUser).not.toHaveBeenCalled(); + }); + + it('locks a departed member rather than deactivating them', async () => { + const leaver = '@gone.999999:m.test'; + const { service, matrix } = harness( + [holder(NAA, 'naa', 'marketer', 'Marketer')], + { + '!edr-freight:m.test': [BOT, '@naa.03f5eb:m.test', leaver], + '!general:m.test': [BOT, '@naa.03f5eb:m.test', leaver], + '!freight-alerts:m.test': [BOT, '@naa.03f5eb:m.test'], + '!dept-marketer:m.test': [BOT, '@naa.03f5eb:m.test'], + }, + ); + + const result = await service.reconcile(); + + expect(matrix.kick.mock.calls.map(([, mxid]) => mxid)).toEqual([leaver, leaver]); + // Locking is reversible; deactivation is not, and on a homeserver with no + // password login it cannot be undone at all. + expect(matrix.lockUser).toHaveBeenCalledTimes(1); + expect(matrix.lockUser).toHaveBeenCalledWith(leaver); + expect(result.locked).toBe(1); + }); + + it('does not lock someone who only moved between positions', async () => { + const naaMxid = '@naa.03f5eb:m.test'; + // naa holds `marketer` now; the room for their old position still lists them. + const { service, matrix } = harness( + [ + holder(NAA, 'naa', 'marketer', 'Marketer'), + holder('aaa04914-b7ee-47b3-9c63-4324046a26bd', 'nati', 'opn', 'Operation'), + ], + { '!dept-opn:m.test': [BOT, naaMxid, '@nati.aaa049:m.test'] }, + ); + + const result = await service.reconcile(); + + expect(matrix.kick).toHaveBeenCalledWith( + '!dept-opn:m.test', + naaMxid, + expect.any(String), + ); + // Kicked from one room, still current elsewhere — their account stays open. + expect(matrix.lockUser).not.toHaveBeenCalled(); + expect(result.locked).toBe(0); + }); + + it('refuses to empty a populated room when its desired set is empty', async () => { + // Per-room backstop for the paths the unit-level guard above cannot see. + const { service, matrix } = harness([holder(NAA, 'naa', 'marketer')], { + '!room:m.test': [BOT, '@naa.03f5eb:m.test', '@nati.aaa049:m.test'], + }); + + const diff = await ( + service as unknown as { + syncMembership: ( + roomId: string, + desired: Set, + bot: string, + ) => Promise<{ joined: number; kicked: string[] }>; + } + ).syncMembership('!room:m.test', new Set(), BOT); + + expect(diff).toEqual({ joined: 0, kicked: [] }); + expect(matrix.kick).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.ts b/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.ts index 23d05d8e6..102cb071a 100644 --- a/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.ts +++ b/apps/edr-freight-api/src/modules/chat/chat-provisioning.service.ts @@ -13,6 +13,11 @@ const UNIT_KEY = 'edr_freight_app'; const SPACE_ALIAS = 'edr-freight'; const GENERAL_ALIAS = 'general'; +/** Where ChatBridgeService mirrors backoffice notifications. Provisioned here, + * with every position holder in it, so bridged messages land somewhere staff + * actually are — the bridge only ever get-or-creates it as a safety net. */ +export const ALERTS_ROOM = { alias: 'freight-alerts', name: 'Freight Alerts' }; + interface PositionHolder { positionKey: string; positionName: string; @@ -24,7 +29,8 @@ export interface ReconcileResult { rooms: number; joined: number; kicked: number; - deactivated: number; + /** Departed accounts locked — reversible. See {@link MatrixClient.lockUser}. */ + locked: number; } /** @@ -55,7 +61,7 @@ export class ChatProvisioningService { const result = await this.reconcile(); this.logger.log( `Chat reconcile: ${result.rooms} room(s), ${result.joined} joined, ` + - `${result.kicked} kicked, ${result.deactivated} deactivated`, + `${result.kicked} kicked, ${result.locked} locked`, ); } catch (err) { // Never throws into the scheduler — chat provisioning must not be able @@ -122,6 +128,14 @@ export class ChatProvisioningService { parentSpaceId: spaceId, }); await this.matrix.ensureJoined(generalRoomId, mxid); + // Without this a new hire sees no bridged notification until the nightly + // reconcile puts them in the alerts room. + const alertsRoomId = await this.matrix.ensureRoom( + ALERTS_ROOM.alias, + ALERTS_ROOM.name, + { parentSpaceId: spaceId }, + ); + await this.matrix.ensureJoined(alertsRoomId, mxid); for (const position of positions) { const roomId = await this.matrix.ensureRoom( @@ -132,10 +146,10 @@ export class ChatProvisioningService { await this.matrix.ensureJoined(roomId, mxid); } - return positions.length + 1; + return positions.length + 3; // space + general + alerts } - /** Force-joins additions, kicks+deactivates users no longer entitled anywhere. */ + /** Force-joins additions, kicks users no longer entitled to this room. */ private async syncMembership( roomId: string, desiredUserIds: Set, @@ -144,6 +158,19 @@ export class ChatProvisioningService { const current = await this.matrix.joinedMembers(roomId); const currentSet = new Set(current.filter((id) => id !== botMxid)); + // An empty desired set against a populated room is not "everyone left" — + // it is a query that failed, a key that drifted, or a migration caught + // mid-flight. Acting on it would clear the room and then lock every + // account that was in it. {@link reconcile} guards the same shape at the + // unit level; this is the per-room backstop for the paths it cannot see. + if (desiredUserIds.size === 0 && currentSet.size > 0) { + this.logger.warn( + `Refusing to empty room ${roomId}: desired membership is empty while ` + + `${currentSet.size} member(s) are joined. Left untouched.`, + ); + return { joined: 0, kicked: [] }; + } + let joined = 0; for (const userId of desiredUserIds) { if (!currentSet.has(userId)) { @@ -165,6 +192,17 @@ export class ChatProvisioningService { async reconcile(): Promise { const holders = await this.currentHolders(); + // The desired state for the whole unit. Empty means the IAM query failed, + // the org/unit keys drifted, or a migration is mid-flight — it never means + // every employee left at once. Continuing would kick every member of every + // room and lock every account, so refuse the run and keep yesterday's + // state, which is wrong at worst by a day. + if (holders.length === 0) { + throw new Error( + `Chat reconcile aborted: no current position holders for ${ORG_KEY}/${UNIT_KEY}. ` + + 'Refusing to read that as "remove everyone".', + ); + } const botMxid = await this.matrix.whoami(); const spaceId = await this.matrix.ensureRoom(SPACE_ALIAS, 'EDR Freight', { @@ -173,6 +211,11 @@ export class ChatProvisioningService { const generalRoomId = await this.matrix.ensureRoom(GENERAL_ALIAS, 'General', { parentSpaceId: spaceId, }); + const alertsRoomId = await this.matrix.ensureRoom( + ALERTS_ROOM.alias, + ALERTS_ROOM.name, + { parentSpaceId: spaceId }, + ); const allUserIds = new Set( holders.map((h) => this.matrix.mxidFor(h.userId, h.userName)), @@ -189,12 +232,12 @@ export class ChatProvisioningService { await this.matrix.ensureUser(mxid, h.userName); } - let rooms = 2; // space + general + let rooms = 3; // space + general + alerts let joined = 0; let kicked = 0; // A user kicked from anything while holding zero current positions // anywhere in the unit (allUserIds spans every position) is a full - // leaver, not just moved between positions — deactivate their account. + // leaver, not just moved between positions — lock their account. const kickedUserIds = new Set(); // Space membership follows the org tree exactly like room membership — @@ -210,6 +253,11 @@ export class ChatProvisioningService { kicked += generalDiff.kicked.length; generalDiff.kicked.forEach((uid) => kickedUserIds.add(uid)); + const alertsDiff = await this.syncMembership(alertsRoomId, allUserIds, botMxid); + joined += alertsDiff.joined; + kicked += alertsDiff.kicked.length; + alertsDiff.kicked.forEach((uid) => kickedUserIds.add(uid)); + const byPosition = new Map }>(); for (const h of holders) { const entry = byPosition.get(h.positionKey) ?? { @@ -232,19 +280,19 @@ export class ChatProvisioningService { diff.kicked.forEach((uid) => kickedUserIds.add(uid)); } - let deactivated = 0; + let locked = 0; for (const userId of kickedUserIds) { if (allUserIds.has(userId)) continue; // moved position, still current elsewhere try { - await this.matrix.deactivateUser(userId); - deactivated += 1; + await this.matrix.lockUser(userId); + locked += 1; } catch (err) { this.logger.warn( - `Failed to deactivate departed user ${userId}: ${(err as Error).message}`, + `Failed to lock departed user ${userId}: ${(err as Error).message}`, ); } } - return { rooms, joined, kicked, deactivated }; + return { rooms, joined, kicked, locked }; } } diff --git a/apps/edr-freight-api/src/modules/chat/matrix.client.spec.ts b/apps/edr-freight-api/src/modules/chat/matrix.client.spec.ts index 73798bce9..22cf8afd8 100644 --- a/apps/edr-freight-api/src/modules/chat/matrix.client.spec.ts +++ b/apps/edr-freight-api/src/modules/chat/matrix.client.spec.ts @@ -121,6 +121,68 @@ describe('MatrixClient.verifyServerAdmin', () => { }); }); +describe('MatrixClient.ensureUser', () => { + it('lifts the lock on a returning employee', async () => { + // A previous reconcile locked them as a leaver. Force-joining them back + // into rooms while they still cannot log in is a silent half-restore. + fetchMock + .mockResolvedValueOnce( + response(200, { name: '@naa.03f5eb:matrix.test', locked: true }), + ) + .mockResolvedValueOnce(response(200, {})); + + await new MatrixClient(config).ensureUser('@naa.03f5eb:matrix.test', 'naa'); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const [url, init] = fetchMock.mock.calls[1] as [string, { body: string }]; + expect(String(url)).toContain('/_synapse/admin/v2/users/'); + expect(JSON.parse(init.body)).toEqual({ locked: false }); + }); + + it('leaves an account that is not locked alone', async () => { + fetchMock.mockResolvedValueOnce( + response(200, { name: '@naa.03f5eb:matrix.test', locked: false }), + ); + + await new MatrixClient(config).ensureUser('@naa.03f5eb:matrix.test', 'naa'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); + +describe('MatrixClient rate limiting', () => { + it('retries a 429 after the delay Synapse asks for', async () => { + // The dev outage: a reconcile is a burst of writes, Synapse throttled an + // m.space.child PUT, and one un-retried 429 threw the whole run away. + fetchMock + .mockResolvedValueOnce(response(200, { user_id: '@edrbot:matrix.test' })) + .mockResolvedValueOnce( + response(429, { + errcode: 'M_LIMIT_EXCEEDED', + error: 'Too Many Requests', + retry_after_ms: 1, + }), + ) + .mockResolvedValueOnce(response(200, { users: [] })); + + const check = await new MatrixClient(config).verifyServerAdmin(); + + expect(check.ok).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it('gives up rather than hanging on a homeserver that only ever 429s', async () => { + fetchMock.mockResolvedValue( + response(429, { errcode: 'M_LIMIT_EXCEEDED', retry_after_ms: 1 }), + ); + + const check = await new MatrixClient(config).verifyServerAdmin(); + + expect(check.ok).toBe(false); + expect(check.error).toContain('429'); + }); +}); + describe('MatrixClient.adminCheck', () => { it('does not re-hit Synapse on every readiness probe', async () => { fetchMock diff --git a/apps/edr-freight-api/src/modules/chat/matrix.client.ts b/apps/edr-freight-api/src/modules/chat/matrix.client.ts index fdfab052d..4da85b339 100644 --- a/apps/edr-freight-api/src/modules/chat/matrix.client.ts +++ b/apps/edr-freight-api/src/modules/chat/matrix.client.ts @@ -57,6 +57,9 @@ export class MatrixClient implements OnApplicationBootstrap { /** The token is a deploy-time fact and the readiness probe runs every few * seconds, so {@link adminCheck} memoises for this long. */ private static readonly ADMIN_CHECK_TTL_MS = 5 * 60_000; + /** Enough to ride out Synapse's limiter; short enough that a genuinely + * wedged homeserver still fails the run rather than hanging it. */ + private static readonly MAX_RATE_LIMIT_RETRIES = 5; private adminCheckCache?: { at: number; result: AdminCheck }; constructor( @@ -95,13 +98,43 @@ export class MatrixClient implements OnApplicationBootstrap { return this.config.enabled; } + /** + * Synapse answers a burst of writes with 429 + `retry_after_ms`, and a + * reconcile is nothing but a burst of writes — one run creates the space, + * #general and a room per position, then force-joins every holder into each. + * The first run against dev tripped the limiter on an `m.space.child` PUT, + * and because nothing retried, that single 429 threw the whole reconcile + * away mid-flight. On the sign-in path ChatSsoService swallows the throw, so + * the only visible symptom was an empty Element. + * + * Honour the delay Synapse asks for rather than guessing at one. + */ + private async fetchWithRetry( + url: string, + init: Parameters[1], + ): Promise>> { + for (let attempt = 0; ; attempt++) { + const res = await fetch(url, init); + if (res.status !== 429 || attempt >= MatrixClient.MAX_RATE_LIMIT_RETRIES) { + return res; + } + // Body is discarded either way — this response is being retried. + const body = (await res.json().catch(() => ({}))) as { + retry_after_ms?: number; + }; + await new Promise((resolve) => + setTimeout(resolve, (Number(body.retry_after_ms) || 1000) + 100), + ); + } + } + private async request( method: string, path: string, body?: unknown, token: string = this.config.adminToken, ): Promise { - const res = await fetch(`${this.config.baseUrl}${path}`, { + const res = await this.fetchWithRetry(`${this.config.baseUrl}${path}`, { method, headers: { 'Content-Type': 'application/json', @@ -125,7 +158,7 @@ export class MatrixClient implements OnApplicationBootstrap { path: string, body: unknown, ): Promise { - const res = await fetch(`${this.config.baseUrl}${path}`, { + const res = await this.fetchWithRetry(`${this.config.baseUrl}${path}`, { method, headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), @@ -145,7 +178,7 @@ export class MatrixClient implements OnApplicationBootstrap { path: string, token?: string, ): Promise { - const res = await fetch(`${this.config.baseUrl}${path}`, { + const res = await this.fetchWithRetry(`${this.config.baseUrl}${path}`, { method, headers: { Authorization: `Bearer ${token ?? this.config.adminToken}` }, }); @@ -333,11 +366,18 @@ export class MatrixClient implements OnApplicationBootstrap { * ("User not found") on an account that doesn't exist yet. */ async ensureUser(userId: string, displayName?: string): Promise { - const existing = await this.requestOrNull<{ name: string }>( + const existing = await this.requestOrNull<{ name: string; locked?: boolean }>( 'GET', `/_synapse/admin/v2/users/${encodeURIComponent(userId)}`, ); - if (existing) return; + if (existing) { + // A returning employee is still locked from the reconcile that saw them + // leave. Force-joining them into rooms while they cannot log in is a + // silent half-restore, and this is the one call that already knows the + // flag — so undo it here rather than making the caller ask again. + if (existing.locked) await this.setLocked(userId, false); + return; + } await this.request( 'PUT', `/_synapse/admin/v2/users/${encodeURIComponent(userId)}`, @@ -377,12 +417,30 @@ export class MatrixClient implements OnApplicationBootstrap { ); } - /** Deactivating (rather than just kicking) a leaver's account revokes all their sessions. */ - deactivateUser(userId: string): Promise { + /** + * Lock a departed employee out of chat — reversible, unlike deactivation. + * + * This used to call `/_synapse/admin/v1/deactivate`. That revokes sessions + * the same way but cannot be undone in any useful sense on this deployment: + * reactivation wants a password, and `password_config.enabled: false` means + * there is none to set. Room memberships do not come back either. One bad + * reconcile — a half-applied IAM migration, a renamed org key — would have + * destroyed every staff account that way, permanently. + * + * Locking blocks exactly the same access (Synapse rejects the account's + * tokens with M_USER_LOCKED and refuses new logins) and is undone with a + * single PUT — see {@link ensureUser}, which lifts it automatically when + * someone comes back. + */ + lockUser(userId: string): Promise { + return this.setLocked(userId, true); + } + + private setLocked(userId: string, locked: boolean): Promise { return this.request( - 'POST', - `/_synapse/admin/v1/deactivate/${encodeURIComponent(userId)}`, - { erase: false }, + 'PUT', + `/_synapse/admin/v2/users/${encodeURIComponent(userId)}`, + { locked }, ); } diff --git a/apps/edr-freight-web/backoffice/src/auth/cookies.ts b/apps/edr-freight-web/backoffice/src/auth/cookies.ts index 0292f7f38..dd7edc5a4 100644 --- a/apps/edr-freight-web/backoffice/src/auth/cookies.ts +++ b/apps/edr-freight-web/backoffice/src/auth/cookies.ts @@ -1,3 +1,5 @@ +import { POSITION_COOKIE } from "@/shared/utils/positionCookie"; + const DEFAULT_PATH = "/"; const SEVEN_DAYS_IN_SECONDS = 60 * 60 * 24 * 7; @@ -32,6 +34,8 @@ export const clearSessionCookies = () => { AUTH_TOKEN_COOKIE, REFRESH_TOKEN_COOKIE, AUTH_USER_COOKIE, + POSITION_COOKIE, + // Pre-rename name, still cleared so a stale value cannot outlive logout. "current-position-id", "selected-position-id", ].forEach(clearCookie); diff --git a/apps/edr-freight-web/backoffice/src/components/customers/ChangeRequestReview.tsx b/apps/edr-freight-web/backoffice/src/components/customers/ChangeRequestReview.tsx index 7f4cf30ce..88cbc1769 100644 --- a/apps/edr-freight-web/backoffice/src/components/customers/ChangeRequestReview.tsx +++ b/apps/edr-freight-web/backoffice/src/components/customers/ChangeRequestReview.tsx @@ -174,6 +174,10 @@ export function DiffRow({ from: string; to: string; }) { + // No real "before" (field went from unset straight to a value, e.g. the + // onboarding wizard's first save) — show the value alone rather than a + // fake "— → value" that implies a prior state that never existed. + const hadBefore = from !== "—"; const changed = from !== to; return ( @@ -181,24 +185,24 @@ export function DiffRow({ {label} - - {from} - - {changed && ( - <> - - → - - - {to} - - + {hadBefore && ( + + {from} + )} + {hadBefore && changed && ( + + → + + )} + + {to} + ); diff --git a/apps/edr-freight-web/backoffice/src/components/layout/FreightDashboardHeader.tsx b/apps/edr-freight-web/backoffice/src/components/layout/FreightDashboardHeader.tsx index 0144db306..45c2f36f1 100644 --- a/apps/edr-freight-web/backoffice/src/components/layout/FreightDashboardHeader.tsx +++ b/apps/edr-freight-web/backoffice/src/components/layout/FreightDashboardHeader.tsx @@ -22,6 +22,7 @@ import { type ReactNode } from "react"; import { useNavigate } from "react-router-dom"; import DocReviewAlertButton from "@/features/bookingWindows/DocReviewAlertButton"; +import { PositionSelect } from "@/record-management/components/positionSelection"; import NotificationBellContainer from "@/features/notifications/NotificationBellContainer"; import type { PageMeta } from "./types"; @@ -111,6 +112,10 @@ const FreightDashboardHeader = ({ renders only during a review phase that still has undecided requests, so it never competes for space otherwise. */} + {/* Staff holding two posts switch desks here. Renders nothing for the + single-position majority, so it costs the header no space. */} + + diff --git a/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx b/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx index b014425e5..6e904066a 100644 --- a/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx +++ b/apps/edr-freight-web/backoffice/src/record-management/components/positionSelection.tsx @@ -18,6 +18,10 @@ import { TooltipTrigger, } from "@/shared/common/ui/tooltip"; import { PositionName } from "../dto/delegation/delegationDto"; +import { + getPositionCookie, + setPositionCookie, +} from "@/shared/utils/positionCookie"; interface BasePosition { id: string; employeePositionId: string; @@ -55,7 +59,7 @@ export const PositionSelect = () => { ? unFilteredUserDetails.employee.flatMap((emp) => emp?.positions ?? []) : []; const selectablePositions = allPositions ?? []; - const currentPositionCookie = Cookies.get("current-position-id"); + const currentPositionCookie = getPositionCookie(); const delegatedPositionCookie = Cookies.get("delegatedPositionId"); const activePositionId = selectedPositionId || currentPositionCookie || delegatedPositionCookie; @@ -85,15 +89,18 @@ export const PositionSelect = () => { // (useAuthUser already self-heals this cookie for the same reason.) if ( currentPosition.employeePositionId && - Cookies.get("current-position-id") !== currentPosition.employeePositionId + getPositionCookie() !== currentPosition.employeePositionId ) { - Cookies.set("current-position-id", currentPosition.employeePositionId); + setPositionCookie(currentPosition.employeePositionId); } applyDelegationCookie(currentPosition); }, [currentPosition, isLoading, selectedPositionId, setSelectedPositionId]); - if (isLoading || selectablePositions.length === 0) return null; + // Below two desks there is nothing to switch between. This now sits in the + // main freight header, so a one-option dropdown would show for every + // single-desk staff member. + if (isLoading || selectablePositions.length < 2) return null; const handleChange = (value: string) => { const selected = selectablePositions.find((pos) => pos.id === value); @@ -102,7 +109,7 @@ export const PositionSelect = () => { // dropdown's own value is position.id, which the API does not match on. setSelectedPositionId(selected?.employeePositionId ?? value); if (selected?.employeePositionId) { - Cookies.set("current-position-id", selected.employeePositionId); + setPositionCookie(selected.employeePositionId); } applyDelegationCookie(selected); diff --git a/apps/edr-freight-web/backoffice/src/record-management/services/api/signatureAndTeeterService.ts b/apps/edr-freight-web/backoffice/src/record-management/services/api/signatureAndTeeterService.ts index bb9d8b274..cefeb94ef 100644 --- a/apps/edr-freight-web/backoffice/src/record-management/services/api/signatureAndTeeterService.ts +++ b/apps/edr-freight-web/backoffice/src/record-management/services/api/signatureAndTeeterService.ts @@ -12,9 +12,11 @@ import { } from "@/record-management/dto/userRecords/teetersAndSignatureDto"; import Cookies from "js-cookie"; +import { getPositionCookie } from "@/shared/utils/positionCookie"; + export const withHeaders = (passPosId: boolean = false) => { const unitId = Cookies.get("unit-id"); - const positionId = Cookies.get("current-position-id"); + const positionId = getPositionCookie(); const projectId = Cookies.get("current-project-id"); const delegatedPositionId = Cookies.get("delegatedPositionId"); diff --git a/apps/edr-freight-web/backoffice/src/record-management/services/api/withHeaders.tsx b/apps/edr-freight-web/backoffice/src/record-management/services/api/withHeaders.tsx index b735ce611..e00325c24 100644 --- a/apps/edr-freight-web/backoffice/src/record-management/services/api/withHeaders.tsx +++ b/apps/edr-freight-web/backoffice/src/record-management/services/api/withHeaders.tsx @@ -1,9 +1,11 @@ import Cookies from "js-cookie"; +import { getPositionCookie } from "@/shared/utils/positionCookie"; + export const withHeaders = () => { const tenantKey = Cookies.get("tenant-key"); const unitId = Cookies.get("unit-id"); - const positionId = Cookies.get("current-position-id"); + const positionId = getPositionCookie(); const projectId = Cookies.get("current-project-id"); const delegatedPositionId = Cookies.get("delegatedPositionId"); const headers: Record = {}; diff --git a/apps/edr-freight-web/backoffice/src/shared/hooks/useAuthUser.ts b/apps/edr-freight-web/backoffice/src/shared/hooks/useAuthUser.ts index af41dc2b7..bd589d51b 100644 --- a/apps/edr-freight-web/backoffice/src/shared/hooks/useAuthUser.ts +++ b/apps/edr-freight-web/backoffice/src/shared/hooks/useAuthUser.ts @@ -30,6 +30,10 @@ import { persistRememberMePreference, setAuthCookies, } from "@/shared/utils/authPersistence"; +import { + getPositionCookie, + setPositionCookie, +} from "@/shared/utils/positionCookie"; import { clearComplaintVerification } from "@/complaints/utils/complaintVerificationStorage"; interface LoginPayload { @@ -46,7 +50,7 @@ export const useAuthUser = () => { const { t } = useTranslation(); const { handleError } = useErrorHandler(t); const delegatedPositionId = Cookies.get("delegatedPositionId"); - const currentPositionId = Cookies.get("current-position-id"); + const currentPositionId = getPositionCookie(); const { setUser, @@ -96,7 +100,7 @@ export const useAuthUser = () => { userDetails.employee?.[0]?.positions?.[0]?.employeePositionId; if (firstPositionId) { setSelectedPositionId(firstPositionId); - Cookies.set("current-position-id", firstPositionId, cookieOptions); + setPositionCookie(firstPositionId, cookieOptions); } } @@ -141,7 +145,7 @@ export const useAuthUser = () => { if (fallbackId) { setSelectedPositionId(fallbackId); - Cookies.set("current-position-id", fallbackId); + setPositionCookie(fallbackId); } } else if (selectedPositionId) { // Self-heal stale cookies that were set to position.id instead of @@ -157,10 +161,7 @@ export const useAuthUser = () => { if (matchingPosition?.employeePositionId) { setSelectedPositionId(matchingPosition.employeePositionId); - Cookies.set( - "current-position-id", - matchingPosition.employeePositionId, - ); + setPositionCookie(matchingPosition.employeePositionId); } } } diff --git a/apps/edr-freight-web/backoffice/src/shared/utils/faydaAuthSession.ts b/apps/edr-freight-web/backoffice/src/shared/utils/faydaAuthSession.ts index 8200f6aba..8dce901b3 100644 --- a/apps/edr-freight-web/backoffice/src/shared/utils/faydaAuthSession.ts +++ b/apps/edr-freight-web/backoffice/src/shared/utils/faydaAuthSession.ts @@ -8,6 +8,7 @@ import { getAuthCookieOptions, setAuthCookies, } from "@/shared/utils/authPersistence"; +import { setPositionCookie } from "@/shared/utils/positionCookie"; import type { VerifiedCitizen } from "@/complaints/types/complaint.types"; function unwrapApiData(payload: T | { data?: T }): T { @@ -138,7 +139,7 @@ export async function persistFaydaRegistrationAuth( const firstPositionId = userDetails.employee?.[0]?.positions?.[0]?.employeePositionId; if (firstPositionId) { - Cookies.set("current-position-id", firstPositionId, cookieOptions); + setPositionCookie(firstPositionId, cookieOptions); } return userDetails; diff --git a/apps/edr-freight-web/backoffice/src/shared/utils/positionCookie.ts b/apps/edr-freight-web/backoffice/src/shared/utils/positionCookie.ts new file mode 100644 index 000000000..60c97c6ae --- /dev/null +++ b/apps/edr-freight-web/backoffice/src/shared/utils/positionCookie.ts @@ -0,0 +1,39 @@ +import Cookies from "js-cookie"; + +type CookieOptions = NonNullable[2]>; + +/** + * Freight's own active-position cookie. + * + * Smart Office is a separate app on the same IAM and it also writes a cookie + * named `current-position-id` — but it stores `position.id` where freight + * stores `employeePositionId`. The two are not interchangeable, so on a shared + * domain each app's login silently overwrote the other's desk selection and the + * loser fell back to `positions[0]`. Freight keeps its own cookie name so both + * can hold a selection at once. + */ +export const POSITION_COOKIE = "freight-current-position-id"; + +/** + * The pre-rename, shared-with-Smart-Office name. Still read so a session that + * is live across the deploy keeps its desk, and cleared on every write so the + * colliding cookie does not linger. + */ +const LEGACY_POSITION_COOKIE = "current-position-id"; + +/** The active `employeePositionId`, or undefined when no desk is selected. */ +export const getPositionCookie = (): string | undefined => + Cookies.get(POSITION_COOKIE) ?? Cookies.get(LEGACY_POSITION_COOKIE); + +export const setPositionCookie = ( + value: string, + options?: CookieOptions, +): void => { + Cookies.set(POSITION_COOKIE, value, options); + Cookies.remove(LEGACY_POSITION_COOKIE); +}; + +export const clearPositionCookie = (): void => { + Cookies.remove(POSITION_COOKIE); + Cookies.remove(LEGACY_POSITION_COOKIE); +};