From c9db7c8cd7ce529cb0f1082f06a670828cc47b6f Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Fri, 10 Jul 2026 17:21:16 +0200 Subject: [PATCH] fix(auth): add username + passwordHash fields to users collection The production UsersRepository reads and writes username + passwordHash via the Payload local API, but the users collection never declared them, so production sign-up/sign-in was broken (audit finding B1). passwordHash uses access.read: () => false so credential material never serializes through any Payload API surface; the repository still reads it with overrideAccess: true. A contract-shaped test pins every repo-used field (USERS_REPOSITORY_FIELDS) against the collection config so drift fails at test time without a database. Co-Authored-By: Claude Fable 5 --- compliance/data-map.yml | 7 +++ .../repositories/users.repository.ts | 43 +++++++++++--- .../cms/collections/users.test.ts | 59 +++++++++++++++++++ .../src/integrations/cms/collections/users.ts | 29 +++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 packages/auth/src/integrations/cms/collections/users.test.ts diff --git a/compliance/data-map.yml b/compliance/data-map.yml index 00eb1df..e82aa74 100644 --- a/compliance/data-map.yml +++ b/compliance/data-map.yml @@ -32,4 +32,11 @@ collections: - transactional-notifications restrictable: true source: auth-default + - category: identification-username + exportable: true + field: username + purpose: + - service-delivery + restrictable: true + source: field-tag slug: users diff --git a/packages/auth/src/infrastructure/repositories/users.repository.ts b/packages/auth/src/infrastructure/repositories/users.repository.ts index dd23fb0..f8b8ecf 100644 --- a/packages/auth/src/infrastructure/repositories/users.repository.ts +++ b/packages/auth/src/infrastructure/repositories/users.repository.ts @@ -12,6 +12,14 @@ import { type User } from "../../entities/models/user"; const FEATURE = "auth" as const; const REPO = "users" as const; +/** + * Every users-collection field this repository reads or writes, besides the + * implicit `id`. Pinned against the collection config by + * `integrations/cms/collections/users.test.ts` so repo <-> collection drift + * fails at test time without a database (audit finding B1). + */ +export const USERS_REPOSITORY_FIELDS = ["username", "passwordHash"] as const; + export class UsersRepository implements IUsersRepository { private config: SanitizedConfig; private tracer: ITracer; @@ -40,7 +48,9 @@ export class UsersRepository implements IUsersRepository { }); const found = Boolean(result); span.setAttribute("found", found); - return result ? this.toDomain(result as Record) : undefined; + return result + ? this.toDomain(result as Record) + : undefined; } catch (err) { if ( err && @@ -54,7 +64,10 @@ export class UsersRepository implements IUsersRepository { this.logger.captureException(err, { tags: { feature: FEATURE, repo: REPO, method: "getUser" }, }); - span.setStatus("error", err instanceof Error ? err.message : String(err)); + span.setStatus( + "error", + err instanceof Error ? err.message : String(err), + ); throw err; } }, @@ -66,7 +79,11 @@ export class UsersRepository implements IUsersRepository { { name: "users.getUserByUsername", op: "repository", - attributes: { emailDomain: username.includes("@") ? (username.split("@")[1] ?? "(invalid)") : username }, + attributes: { + emailDomain: username.includes("@") + ? (username.split("@")[1] ?? "(invalid)") + : username, + }, }, async (span) => { try { @@ -79,12 +96,17 @@ export class UsersRepository implements IUsersRepository { }); const doc = docs[0]; span.setAttribute("found", Boolean(doc)); - return doc ? this.toDomain(doc as Record) : undefined; + return doc + ? this.toDomain(doc as Record) + : undefined; } catch (err) { this.logger.captureException(err, { tags: { feature: FEATURE, repo: REPO, method: "getUserByUsername" }, }); - span.setStatus("error", err instanceof Error ? err.message : String(err)); + span.setStatus( + "error", + err instanceof Error ? err.message : String(err), + ); throw err; } }, @@ -93,7 +115,11 @@ export class UsersRepository implements IUsersRepository { async createUser(input: User): Promise { return this.tracer.startSpan( - { name: "users.createUser", op: "repository", attributes: { id: input.id } }, + { + name: "users.createUser", + op: "repository", + attributes: { id: input.id }, + }, async (span) => { try { const payload = await getPayload({ config: this.config }); @@ -112,7 +138,10 @@ export class UsersRepository implements IUsersRepository { this.logger.captureException(err, { tags: { feature: FEATURE, repo: REPO, method: "createUser" }, }); - span.setStatus("error", err instanceof Error ? err.message : String(err)); + span.setStatus( + "error", + err instanceof Error ? err.message : String(err), + ); throw err; } }, diff --git a/packages/auth/src/integrations/cms/collections/users.test.ts b/packages/auth/src/integrations/cms/collections/users.test.ts new file mode 100644 index 0000000..a408c55 --- /dev/null +++ b/packages/auth/src/integrations/cms/collections/users.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from "vitest"; +import { users } from "@/integrations/cms/collections/users"; +import { USERS_REPOSITORY_FIELDS } from "@/infrastructure/repositories/users.repository"; + +type NamedField = { + name?: string; + type?: string; + required?: boolean; + unique?: boolean; + index?: boolean; + admin?: { hidden?: boolean }; + access?: { read?: (args: unknown) => boolean | Promise }; +}; + +function fieldByName(name: string): NamedField | undefined { + return (users.fields as NamedField[]).find((f) => f.name === name); +} + +// Contract-shaped drift guard (audit finding B1): the production +// UsersRepository reads/writes these fields via the Payload local API, so the +// collection config must declare every one of them. No database needed — +// we parse the collection object directly. +describe("users collection <-> UsersRepository field contract", () => { + it.each([...USERS_REPOSITORY_FIELDS])( + "declares the '%s' field the repository reads/writes", + (name) => { + expect(fieldByName(name)).toBeDefined(); + }, + ); + + it("username is a required, unique, indexed text field", () => { + const username = fieldByName("username"); + expect(username).toMatchObject({ + type: "text", + required: true, + unique: true, + index: true, + }); + }); + + it("passwordHash is required and hidden in the admin UI", () => { + const passwordHash = fieldByName("passwordHash"); + expect(passwordHash).toBeDefined(); + expect(passwordHash!.type).toBe("text"); + expect(passwordHash!.required).toBe(true); + expect(passwordHash!.admin?.hidden).toBe(true); + }); + + it("passwordHash is never readable through the Payload API", async () => { + const passwordHash = fieldByName("passwordHash"); + expect(passwordHash!.access?.read).toBeTypeOf("function"); + // Field-level read access must deny unconditionally — even for admins — + // so the hash never serializes into REST/GraphQL/admin responses. The + // repository bypasses this via the local API's overrideAccess: true. + await expect( + Promise.resolve(passwordHash!.access!.read!({ req: {} })), + ).resolves.toBe(false); + }); +}); diff --git a/packages/auth/src/integrations/cms/collections/users.ts b/packages/auth/src/integrations/cms/collections/users.ts index 7fd69b2..61129e6 100644 --- a/packages/auth/src/integrations/cms/collections/users.ts +++ b/packages/auth/src/integrations/cms/collections/users.ts @@ -18,6 +18,35 @@ export const users: CollectionConfig = { subject: { kind: "self", field: "id" }, }, fields: [ + { + // Read/written by the production UsersRepository (getUserByUsername, + // createUser). Pinned by collections/users.test.ts against + // USERS_REPOSITORY_FIELDS so repo <-> collection drift fails fast. + name: "username", + type: "text", + required: true, + unique: true, + index: true, + custom: { + pii: { + category: "identification-username", + purpose: ["service-delivery"], + exportable: true, + restrictable: true, + }, + }, + }, + { + // Credential material — must never leave the server. `access.read` + // returns false unconditionally so the field is stripped from every + // REST/GraphQL/admin API response; the auth repository still reads it + // through the local API with `overrideAccess: true`. + name: "passwordHash", + type: "text", + required: true, + admin: { hidden: true }, + access: { read: () => false }, + }, { name: "displayName", type: "text",