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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<string, unknown>) : undefined;
|
||||
return result
|
||||
? this.toDomain(result as Record<string, unknown>)
|
||||
: 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<string, unknown>) : undefined;
|
||||
return doc
|
||||
? this.toDomain(doc as Record<string, unknown>)
|
||||
: 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<User> {
|
||||
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;
|
||||
}
|
||||
},
|
||||
|
||||
59
packages/auth/src/integrations/cms/collections/users.test.ts
Normal file
59
packages/auth/src/integrations/cms/collections/users.test.ts
Normal file
@@ -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<boolean> };
|
||||
};
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user