diff --git a/apps/portal-bff/prisma/migrations/20260514192014_users_directory/migration.sql b/apps/portal-bff/prisma/migrations/20260514192014_users_directory/migration.sql new file mode 100644 index 0000000..cab3b40 --- /dev/null +++ b/apps/portal-bff/prisma/migrations/20260514192014_users_directory/migration.sql @@ -0,0 +1,29 @@ +-- Users directory (per ADR-0020 §"v1 scope — User list"). +-- +-- Persistent ledger of identities the BFF has seen sign in to either +-- portal-shell or portal-admin. Upserted by `UserDirectoryService` +-- at every sign-in. Read by the future `GET /api/admin/users` admin +-- endpoint. Entra ID is the source of truth for identity; this +-- table is the BFF's local cache so the admin UI does not have to +-- re-query the IdP at every page render. + +-- CreateTable +CREATE TABLE "users" ( + "oid" TEXT NOT NULL, + "tid" TEXT NOT NULL, + "audience" TEXT NOT NULL, + "username" TEXT NOT NULL, + "display_name" TEXT NOT NULL, + "first_seen_at" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "last_seen_at" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "users_pkey" PRIMARY KEY ("oid") +); + +-- DESC index supports the default admin sort ("most recently active first") +-- without a server-side sort. +CREATE INDEX "users_last_seen_at_idx" ON "users"("last_seen_at" DESC); + +-- Plain (default ASC) index supports prefix matching on the +-- admin-side username filter. +CREATE INDEX "users_username_idx" ON "users"("username"); diff --git a/apps/portal-bff/prisma/schema.prisma b/apps/portal-bff/prisma/schema.prisma index fb52710..ddfa345 100644 --- a/apps/portal-bff/prisma/schema.prisma +++ b/apps/portal-bff/prisma/schema.prisma @@ -49,6 +49,57 @@ enum AuditOutcome { @@schema("audit") } +// ============================================================ +// User directory (per ADR-0020 §"v1 scope — User list") +// ============================================================ +// +// Persistent ledger of every identity that has signed in to either +// portal-shell or portal-admin. Upserted at sign-in by +// `UserDirectoryService.recordSignIn` (called from +// `SessionEstablisher.establish`), read by the future +// `GET /api/admin/users` endpoint per ADR-0020 §"User list +// (read-only)". +// +// **Not the source of truth for identity.** Entra ID is. This table +// is a cache the BFF maintains so the admin UI can list "everyone +// who's ever signed in" without re-querying the directory at +// every render. Per-tenant `oid` is the join key on the audit side +// (combined with the salted hash) — never the BFF's primary actor +// identifier elsewhere. +// +// **No PII redaction on read.** Per ADR-0013 the audit module +// hashes the actor id to defend against an audit-log dump leaking +// who-did-what. This table is the *deliberate* PII storage: an +// admin browsing the user list explicitly wants display names and +// usernames. The trust boundary is the admin role gate +// (ADR-0020 §"Auth — `admin` role claim"). + +model User { + // Entra `oid` — stable per-user identifier inside the tenant. + // Used as the natural primary key. Per-tenant uniqueness is + // sufficient: the dual-audience design (ADR-0008) currently + // assumes single workforce tenant; cross-tenant collisions + // become a separate ADR when we onboard the second one. + oid String @id + tid String + audience String + username String + displayName String @map("display_name") + // `first_seen_at` is set once at first sign-in and never + // updated thereafter. Lets the admin list "users since + // " without joining anything. + firstSeenAt DateTime @default(now()) @map("first_seen_at") @db.Timestamptz(6) + // `last_seen_at` is set every time the upsert fires (one upsert + // per sign-in), so "most recently active" can be computed + // without scanning audit.events. + lastSeenAt DateTime @default(now()) @map("last_seen_at") @db.Timestamptz(6) + + @@map("users") + @@schema("public") + @@index([lastSeenAt(sort: Desc)]) + @@index([username]) +} + model AuditEvent { id String @id @default(uuid()) @db.Uuid createdAt DateTime @default(now()) @map("created_at") @db.Timestamptz(6) diff --git a/apps/portal-bff/src/app/app.module.ts b/apps/portal-bff/src/app/app.module.ts index e959b46..e746b2b 100644 --- a/apps/portal-bff/src/app/app.module.ts +++ b/apps/portal-bff/src/app/app.module.ts @@ -11,6 +11,7 @@ import { DownstreamModule } from '../downstream/downstream.module'; import { RedisModule } from '../redis/redis.module'; import { SecurityModule } from '../security/security.module'; import { SessionModule } from '../session/session.module'; +import { UsersModule } from '../users/users.module'; @Module({ imports: [ @@ -24,6 +25,7 @@ import { SessionModule } from '../session/session.module'; HealthModule, AdminModule, DownstreamModule, + UsersModule, ], controllers: [AppController], providers: [AppService], diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index 9b09437..5c5dd2a 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -2,6 +2,7 @@ import type { Request, Response } from 'express'; import type { Logger } from 'nestjs-pino'; import type { AuditWriter } from '../audit/audit.service'; import type { UserSessionIndexService } from '../session/user-session-index.service'; +import type { UserDirectoryService } from '../users/user-directory.service'; import { AuthController } from './auth.controller'; import { PRE_AUTH_COOKIE_NAME, PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie'; import { AuthCodeFlowException } from './auth.errors'; @@ -143,6 +144,9 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller sessionExpired: jest.fn().mockResolvedValue(undefined), }; const logger = makeLoggerStub(); + // The directory service is a best-effort dependency — a noop mock + // is sufficient for the controller's behavioural assertions. + const userDirectory = { recordSignIn: jest.fn().mockResolvedValue(undefined) }; // Real SessionEstablisher with the same mocks the legacy tests // already wire — keeps the behavioural assertions on session // fields / audit calls untouched after the controller refactor. @@ -150,6 +154,7 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller logger as unknown as Logger, userSessionIndex as unknown as UserSessionIndexService, audit as unknown as AuditWriter, + userDirectory as unknown as UserDirectoryService, ); return { controller: new AuthController( diff --git a/apps/portal-bff/src/auth/auth.module.spec.ts b/apps/portal-bff/src/auth/auth.module.spec.ts index 855d9ee..ac9c5c5 100644 --- a/apps/portal-bff/src/auth/auth.module.spec.ts +++ b/apps/portal-bff/src/auth/auth.module.spec.ts @@ -6,6 +6,7 @@ import { ClsService } from 'nestjs-cls'; import { LoggerModule } from 'nestjs-pino'; import { PrismaService } from 'nestjs-prisma'; import { AuditModule } from '../audit/audit.module'; +import { UsersModule } from '../users/users.module'; import { AuthModule } from './auth.module'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; import { MSAL_CLIENT } from './msal-client.token'; @@ -67,6 +68,7 @@ async function compile() { LoggerModule.forRoot({ pinoHttp: { level: 'silent' } }), TestStubsModule, AuditModule, + UsersModule, AuthModule, ], }).compile(); diff --git a/apps/portal-bff/src/auth/session-establisher.service.spec.ts b/apps/portal-bff/src/auth/session-establisher.service.spec.ts index 7645083..51eef13 100644 --- a/apps/portal-bff/src/auth/session-establisher.service.spec.ts +++ b/apps/portal-bff/src/auth/session-establisher.service.spec.ts @@ -2,6 +2,7 @@ import type { Request, Response } from 'express'; import type { Logger } from 'nestjs-pino'; import type { AuditWriter } from '../audit/audit.service'; import type { UserSessionIndexService } from '../session/user-session-index.service'; +import type { UserDirectoryService } from '../users/user-directory.service'; import type { AuthenticatedUser } from './auth.service'; import { SessionEstablisher } from './session-establisher.service'; @@ -50,6 +51,7 @@ interface Fixture { est: SessionEstablisher; index: { add: jest.Mock; remove: jest.Mock; list: jest.Mock }; audit: { signIn: jest.Mock; signOut: jest.Mock }; + directory: { recordSignIn: jest.Mock }; logger: ReturnType; } @@ -63,13 +65,17 @@ function makeFixture(): Fixture { signIn: jest.fn().mockResolvedValue(undefined), signOut: jest.fn().mockResolvedValue(undefined), }; + const directory = { + recordSignIn: jest.fn().mockResolvedValue(undefined), + }; const logger = makeLoggerStub(); const est = new SessionEstablisher( logger as unknown as Logger, index as unknown as UserSessionIndexService, audit as unknown as AuditWriter, + directory as unknown as UserDirectoryService, ); - return { est, index, audit, logger }; + return { est, index, audit, directory, logger }; } describe('SessionEstablisher.establish', () => { @@ -129,6 +135,49 @@ describe('SessionEstablisher.establish', () => { expect(audit.signIn).toHaveBeenCalledWith({ actor: USER, sessionId: 'sid-7' }); }); + it('records the user in the directory after the audit write (ADR-0020)', async () => { + const { est, audit, directory } = makeFixture(); + const req = makeReqStub({ sessionID: 'sid-7' }); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'user' }); + expect(directory.recordSignIn).toHaveBeenCalledWith({ + oid: USER.oid, + tid: USER.tid, + username: USER.username, + displayName: USER.displayName, + audience: 'workforce', + }); + // Order: audit MUST succeed first (blocking per ADR-0013). + // Compare the recorded `mock.invocationCallOrder` — Jest + // assigns a monotonic counter to every call across all mocks. + const auditOrder = audit.signIn.mock.invocationCallOrder[0] ?? Infinity; + const directoryOrder = directory.recordSignIn.mock.invocationCallOrder[0] ?? 0; + expect(auditOrder).toBeLessThan(directoryOrder); + }); + + it('does NOT call the directory when the audit write fails (audit-first invariant)', async () => { + const { est, audit, directory } = makeFixture(); + audit.signIn.mockRejectedValueOnce(new Error('audit_writer denied')); + const req = makeReqStub(); + const res = makeResStub(); + await expect(est.establish({ user: USER, req, res, surface: 'user' })).rejects.toThrow(); + expect(directory.recordSignIn).not.toHaveBeenCalled(); + }); + + it('writes the directory entry under the v1 hardcoded `workforce` audience (single-audience era)', async () => { + // Pins the ADR-0008 dual-audience design's v1 simplification: + // every signed-in user is workforce. The day External ID + // ships, the SessionEstablisher will read audience from the + // session/claims; the assertion will need to be updated then. + const { est, directory } = makeFixture(); + const req = makeReqStub(); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'admin' }); + expect(directory.recordSignIn).toHaveBeenCalledWith( + expect.objectContaining({ audience: 'workforce' }), + ); + }); + it('logs the success event with the surface tag', async () => { const { est, logger } = makeFixture(); const req = makeReqStub(); diff --git a/apps/portal-bff/src/auth/session-establisher.service.ts b/apps/portal-bff/src/auth/session-establisher.service.ts index 7fbb936..6c4bf2d 100644 --- a/apps/portal-bff/src/auth/session-establisher.service.ts +++ b/apps/portal-bff/src/auth/session-establisher.service.ts @@ -6,6 +6,7 @@ import { AuditWriter } from '../audit/audit.service'; import { csrfCookieName, csrfCookieOptions } from '../security/csrf-cookie'; import { readSessionTimeouts } from '../session/session-cookie'; import { UserSessionIndexService } from '../session/user-session-index.service'; +import { UserDirectoryService } from '../users/user-directory.service'; import type { AuthenticatedUser } from './auth.service'; export type AuthSurface = 'user' | 'admin'; @@ -45,6 +46,7 @@ export class SessionEstablisher { private readonly logger: Logger, private readonly userSessionIndex: UserSessionIndexService, private readonly audit: AuditWriter, + private readonly userDirectory: UserDirectoryService, ) {} async establish(opts: { @@ -93,6 +95,23 @@ export class SessionEstablisher { // the controller emits a 5xx via the StructuredErrorFilter. await this.audit.signIn({ actor: user, sessionId: req.sessionID }); + // Best-effort directory upsert (ADR-0020 §"User list"). MUST + // NOT fail the sign-in — the directory is a convenience for + // admin browsing, not a security boundary. The service catches + // its own errors and logs them; awaiting it means an admin + // that just signed in sees themselves on the user list + // immediately, without racing the response. + await this.userDirectory.recordSignIn({ + oid: user.oid, + tid: user.tid, + username: user.username, + displayName: user.displayName, + // v1: single workforce-tenant audience per ADR-0008. The + // dual-audience design ships a real value here when External + // ID is activated. + audience: 'workforce', + }); + this.logger.log( { event: 'auth.signed_in', diff --git a/apps/portal-bff/src/users/user-directory.service.spec.ts b/apps/portal-bff/src/users/user-directory.service.spec.ts new file mode 100644 index 0000000..dc758f3 --- /dev/null +++ b/apps/portal-bff/src/users/user-directory.service.spec.ts @@ -0,0 +1,112 @@ +import type { Logger } from 'nestjs-pino'; +import type { PrismaService } from 'nestjs-prisma'; +import { UserDirectoryService, type UserDirectoryEntry } from './user-directory.service'; + +interface PrismaStub { + user: { + upsert: jest.Mock; + }; +} + +function makeLogger() { + return { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as unknown as Logger & { + log: jest.Mock; + warn: jest.Mock; + error: jest.Mock; + }; +} + +function makeSubject(opts?: { upsert?: jest.Mock }): { + service: UserDirectoryService; + prisma: PrismaStub; + logger: ReturnType; +} { + const prisma: PrismaStub = { + user: { + upsert: opts?.upsert ?? jest.fn().mockResolvedValue(undefined), + }, + }; + const logger = makeLogger(); + const service = new UserDirectoryService(prisma as unknown as PrismaService, logger); + return { service, prisma, logger }; +} + +const ENTRY: UserDirectoryEntry = { + oid: 'user-oid', + tid: 'tenant-1', + username: 'jane.doe@apf.example', + displayName: 'Jane Doe', + audience: 'workforce', +}; + +describe('UserDirectoryService.recordSignIn', () => { + it('issues an upsert keyed by oid with the right create / update payloads', async () => { + const { service, prisma } = makeSubject(); + await service.recordSignIn(ENTRY); + expect(prisma.user.upsert).toHaveBeenCalledTimes(1); + const args = prisma.user.upsert.mock.calls[0]?.[0] as { + where: { oid: string }; + create: Record; + update: Record; + }; + expect(args.where).toEqual({ oid: 'user-oid' }); + expect(args.create).toEqual({ + oid: 'user-oid', + tid: 'tenant-1', + audience: 'workforce', + username: 'jane.doe@apf.example', + displayName: 'Jane Doe', + }); + // Update path: refreshes every mutable field + bumps lastSeenAt; + // crucially does NOT include `firstSeenAt` so it stays + // immutable after first sign-in. + expect(args.update).toEqual( + expect.objectContaining({ + tid: 'tenant-1', + audience: 'workforce', + username: 'jane.doe@apf.example', + displayName: 'Jane Doe', + }), + ); + expect(args.update['firstSeenAt']).toBeUndefined(); + expect(args.update['lastSeenAt']).toBeInstanceOf(Date); + }); + + it('updates `displayName` + `username` if either changed since the previous sign-in', async () => { + const { service, prisma } = makeSubject(); + await service.recordSignIn({ + ...ENTRY, + displayName: 'Jane Updated', + username: 'jane2@apf.example', + }); + const args = prisma.user.upsert.mock.calls[0]?.[0] as { + update: Record; + }; + expect(args.update['displayName']).toBe('Jane Updated'); + expect(args.update['username']).toBe('jane2@apf.example'); + }); + + it('swallows Prisma errors and logs them — never propagates (sign-in must succeed)', async () => { + const upsert = jest.fn().mockRejectedValue(new Error('connection refused')); + const { service, logger } = makeSubject({ upsert }); + await expect(service.recordSignIn(ENTRY)).resolves.toBeUndefined(); + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ + event: 'user_directory.record_sign_in_failed', + oid: 'user-oid', + reason: 'connection refused', + }), + 'UserDirectoryService', + ); + }); + + it('logs the underlying message verbatim when the rejection is not an Error', async () => { + const upsert = jest.fn().mockRejectedValue('some string'); + const { service, logger } = makeSubject({ upsert }); + await service.recordSignIn(ENTRY); + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ reason: 'some string' }), + 'UserDirectoryService', + ); + }); +}); diff --git a/apps/portal-bff/src/users/user-directory.service.ts b/apps/portal-bff/src/users/user-directory.service.ts new file mode 100644 index 0000000..cd417e4 --- /dev/null +++ b/apps/portal-bff/src/users/user-directory.service.ts @@ -0,0 +1,93 @@ +import { Injectable } from '@nestjs/common'; +import { Logger } from 'nestjs-pino'; +import { PrismaService } from 'nestjs-prisma'; + +/** + * The minimal slice of `AuthenticatedUser` the directory cares + * about. Defined here (rather than importing the full session-side + * type) so the directory service stays decoupled from the auth + * module's internal shape and the spec can hand-roll the input. + */ +export interface UserDirectoryEntry { + readonly oid: string; + readonly tid: string; + readonly username: string; + readonly displayName: string; + /** + * `workforce` for v1 (single workforce-tenant Entra audience per + * ADR-0008). External-ID / customer audience will land its own + * value here when activated. + */ + readonly audience: 'workforce' | 'customer'; +} + +/** + * `UserDirectoryService` — persistent ledger of identities the BFF + * has seen sign in. v1 is one upsert per sign-in, called from + * `SessionEstablisher.establish` AFTER the blocking audit write + * (so an audit failure short-circuits the sign-in before the + * directory entry is ever recorded). Read by the future + * `GET /api/admin/users` admin endpoint per ADR-0020 §"User list + * (read-only)". + * + * Best-effort write. A Postgres hiccup writing the directory entry + * MUST NOT fail the sign-in — the directory is a convenience for + * admin browsing, not a security boundary. ADR-0013's "no audit ⇒ + * no action" applies to the audit module; this is a separate + * write path with weaker invariants. Failures are logged via Pino + * so ops can spot a directory falling behind. + */ +@Injectable() +export class UserDirectoryService { + constructor( + private readonly prisma: PrismaService, + private readonly logger: Logger, + ) {} + + /** + * Upsert: on first sign-in, INSERT a new row with both + * `first_seen_at` and `last_seen_at` set to NOW(); on subsequent + * sign-ins, UPDATE `last_seen_at`, `username`, `display_name`, + * `audience`, `tid` (any of those five may legitimately change — + * a user's `displayName` can be edited tenant-side; a previously + * workforce-only user could appear under the customer audience + * once dual-audience ships). + * + * `first_seen_at` is NEVER overwritten — `@updatedAt` semantics + * don't apply; the field is set once via the DEFAULT clause on + * INSERT and the UPDATE branch leaves it alone. + */ + async recordSignIn(entry: UserDirectoryEntry): Promise { + try { + await this.prisma.user.upsert({ + where: { oid: entry.oid }, + create: { + oid: entry.oid, + tid: entry.tid, + audience: entry.audience, + username: entry.username, + displayName: entry.displayName, + }, + update: { + tid: entry.tid, + audience: entry.audience, + username: entry.username, + displayName: entry.displayName, + lastSeenAt: new Date(), + }, + }); + } catch (err) { + // Postgres hiccup, schema drift, or a row-locked race against + // a concurrent sign-in. Logged but not propagated — the + // admin UI tolerates a missing entry until the next sign-in. + this.logger.warn( + { + event: 'user_directory.record_sign_in_failed', + oid: entry.oid, + reason: err instanceof Error ? err.message : String(err), + }, + 'UserDirectoryService', + ); + } + } +} diff --git a/apps/portal-bff/src/users/users.module.ts b/apps/portal-bff/src/users/users.module.ts new file mode 100644 index 0000000..c01e72e --- /dev/null +++ b/apps/portal-bff/src/users/users.module.ts @@ -0,0 +1,26 @@ +import { Global, Module } from '@nestjs/common'; +import { UserDirectoryService } from './user-directory.service'; + +/** + * `UsersModule` — owns the persistent user directory per ADR-0020 + * §"v1 scope — User list". + * + * v1 ships `UserDirectoryService` only (the write path: upsert at + * every sign-in via `SessionEstablisher`). The future + * `GET /api/admin/users` read endpoint lands in a sibling PR + * alongside the SPA viewer screen — those add a controller and + * possibly a read-side service, but the storage layer is here. + * + * Declared `@Global()` so `SessionEstablisher` (which lives in the + * auth module) can inject `UserDirectoryService` without re-routing + * the module graph through an import. The directory is a true + * cross-cutting concern: it has one writer (the auth callback) and + * one reader (admin), neither of which "owns" identity in a + * domain-module sense. + */ +@Global() +@Module({ + providers: [UserDirectoryService], + exports: [UserDirectoryService], +}) +export class UsersModule {}