feat(portal-bff): user directory upserted at sign-in (#140)
CI / check (push) Successful in 3m2s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 2m54s
CI / a11y (push) Successful in 1m25s
CI / perf (push) Successful in 4m23s

## Summary

First PR of the **portal-admin User-list chantier** per [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"v1 scope — User list (read-only)". Ships the **write side** only:

1. A new `public.users` table that holds the BFF's local cache of identities seen sign in to either portal-shell or portal-admin.
2. A `UserDirectoryService.recordSignIn(user)` upsert called from `SessionEstablisher.establish` after the blocking audit write.

The read side (`GET /api/admin/users` + the admin viewer SPA screen) lands in two follow-up PRs of the same chantier.

## Schema

[`prisma/schema.prisma`](apps/portal-bff/prisma/schema.prisma) gains a `User` model in the `public` schema:

| Column | Type | Notes |
| --- | --- | --- |
| `oid` | TEXT, PK | Entra's stable per-user identifier inside the tenant. Per-tenant uniqueness is sufficient for v1's single-workforce-tenant design (ADR-0008). |
| `tid` | TEXT | Tenant id. Updated on every upsert because a dual-audience future may legitimately change it. |
| `audience` | TEXT | `'workforce'` \| `'customer'`. Hardcoded to `workforce` in v1 per ADR-0008's simplification; will read from session/claims when External ID activates. |
| `username` | TEXT | Updated on every upsert (Entra-side rename possible). |
| `display_name` | TEXT | Same. |
| `first_seen_at` | TIMESTAMPTZ | Set once at first sign-in via DEFAULT NOW(); **never overwritten** thereafter. Enables "users since <date>" without joining anything. |
| `last_seen_at` | TIMESTAMPTZ | Updated on every upsert. Enables "most recently active" without scanning `audit.events`. |

Indexes:
- `last_seen_at DESC` — admin default sort.
- `username` — prefix filtering.

Migration in [`prisma/migrations/20260514192014_users_directory/`](apps/portal-bff/prisma/migrations/20260514192014_users_directory/migration.sql).

## [`UserDirectoryService`](apps/portal-bff/src/users/user-directory.service.ts)

```ts
async recordSignIn(entry): Promise<void> {
  try {
    await prisma.user.upsert({
      where: { oid },
      create: { oid, tid, audience, username, displayName },
      update: { tid, audience, username, displayName, lastSeenAt: new Date() },
    });
  } catch (err) {
    // logged, never propagated
  }
}
```

**Best-effort write.** Catches its own errors, logs a Pino warn (`user_directory.record_sign_in_failed`), returns `undefined`. The directory is a convenience for admin browsing, not a security boundary — a Postgres hiccup must not lock a user out of sign-in. ADR-0013's "no audit ⇒ no action" applies to the audit module only.

## [`SessionEstablisher`](apps/portal-bff/src/auth/session-establisher.service.ts) wiring

The directory call lands right after the existing audit emission:

```ts
await this.audit.signIn({ actor: user, sessionId: req.sessionID }); // blocking per ADR-0013
await this.userDirectory.recordSignIn({ ...user, audience: 'workforce' }); // best-effort
this.logger.log(...);
```

Two invariants the tests pin:

1. **Audit-first**: when `audit.signIn` throws, `userDirectory.recordSignIn` is NOT called. The directory never holds a row for a sign-in the audit log doesn't carry.
2. **Awaited**: an admin who just signed in sees themselves on the user list immediately — no race between the upsert and the response.

## Module wiring

[`UsersModule`](apps/portal-bff/src/users/users.module.ts) is declared `@Global()` so `SessionEstablisher` (which lives in `AuthModule`) injects `UserDirectoryService` without forcing `AuthModule` to import `UsersModule`. The directory is a true cross-cutting concern: one writer (the auth callback) and one future reader (the admin endpoint).

Wired into [`AppModule`](apps/portal-bff/src/app/app.module.ts) alongside the other v1 modules. `auth.module.spec.ts` updated to also import `UsersModule` in its slice-of-graph compile (otherwise the test fails to resolve the new `SessionEstablisher` dep).

## Notes for the reviewer

- The directory write **awaits** (not fire-and-forget). The cost is one round-trip per sign-in on the response-critical path; the benefit is the no-race property called out above. If sign-in p95 becomes an issue we can revisit (e.g. background job) but the simpler shape is correct first.
- `firstSeenAt` is intentionally absent from the `update` payload. The Prisma upsert's `update` block is precisely what changes on conflict; omitting the field leaves it untouched at the column level (Postgres-side default doesn't refire on UPDATE).
- The model lives in `public`, not in a dedicated `identity` or `cms` schema. ADR-0020 enumerates `cms.*` for editorial data and `audit.*` for the audit ledger but doesn't require a separate schema for user-directory data. We can promote it to its own schema later if a role-isolation need emerges; the migration would be a `ALTER TABLE users SET SCHEMA …`.
- `audit.events.actor_id_hash` is **not** stored on `public.users`. A future admin endpoint that joins sign-in counts from `audit.events` can compute the hash on-the-fly via `HashUserIdService` — keeping the salted-hash invariant from ADR-0013 intact (the salt stays inside the audit module).

## Test plan

- [x] `pnpm nx test portal-bff` — **365 specs pass** (was 358; +7: UserDirectoryService 4, SessionEstablisher integration 3).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing rate-limit warnings + one unused-eslint-disable from PR #137 are unrelated).
- [x] Prisma `migrate diff` confirms the model matches the migration SQL.
- [ ] e2e — after merge: sign in via portal-shell or portal-admin, expect a row in `public.users` with the right `oid` / `last_seen_at`; sign in again, expect the same row's `last_seen_at` to advance and `first_seen_at` to stay put.

## What's next

The chantier sequence:

1. **This PR** — write side: schema + service + sign-in upsert.
2. **PR 2** — BFF `GET /api/admin/users` (paginated + filterable, gated by `@RequireAdmin`, emits `admin.users.query` audit).
3. **PR 3** — portal-admin `/users` screen (table + filter form), promote the sidebar entry from "Soon" badge to live link.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #140
This commit was merged in pull request #140.
This commit is contained in:
2026-05-14 19:30:12 +02:00
parent 96339cc99b
commit aca9e8d155
10 changed files with 389 additions and 1 deletions
+2
View File
@@ -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],
@@ -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(
@@ -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();
@@ -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<typeof makeLoggerStub>;
}
@@ -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();
@@ -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',
@@ -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<typeof makeLogger>;
} {
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<string, unknown>;
update: Record<string, unknown>;
};
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<string, unknown>;
};
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',
);
});
});
@@ -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<void> {
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',
);
}
}
}
+26
View File
@@ -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 {}