From b7d9ddc518d8eef68eef5029f35f2f1eacd6f994 Mon Sep 17 00:00:00 2001 From: euniceamoni Date: Tue, 20 Oct 2026 07:10:33 +0000 Subject: [PATCH] fix(#39): normalise publisher email case to prevent duplicate accounts - Trim and lowercase email in publisherRegisterSchema (Zod transform) so the HTTP layer normalises at the boundary before reaching the service. - Same trim+lowercase applied in registerPublisher as defence-in-depth for callers that bypass the schema. - Migration 0015_lowercase_publisher_email: detects collisions, aborts with an error listing conflicting addresses if any exist, then lowercases all existing rows in-place. - New service-level tests cover uppercase, whitespace, combined, already- normalised inputs, and unique-constraint propagation. - Two new route-level tests: schema normalises mixed-case input before calling the service; a case-variant duplicate returns 409 CONFLICT. --- drizzle/0015_lowercase_publisher_email.sql | 34 ++++ drizzle/meta/_journal.json | 7 + src/routes/publishers.test.ts | 45 ++++++ src/schemas/requests.ts | 4 +- ...blisherService.email-normalisation.test.ts | 152 ++++++++++++++++++ src/services/publisherService.ts | 6 +- 6 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 drizzle/0015_lowercase_publisher_email.sql create mode 100644 src/services/publisherService.email-normalisation.test.ts diff --git a/drizzle/0015_lowercase_publisher_email.sql b/drizzle/0015_lowercase_publisher_email.sql new file mode 100644 index 0000000..09ac903 --- /dev/null +++ b/drizzle/0015_lowercase_publisher_email.sql @@ -0,0 +1,34 @@ +-- #39 Normalise publisher email case to prevent duplicate accounts +-- +-- This migration lowercases and trims existing email values in the publishers +-- table. If any collisions exist (two rows that will have the same normalised +-- email), the UPDATE will fail with a unique constraint violation. The error +-- message lists the conflicting emails so an administrator can manually merge +-- or delete duplicates before retrying. + +DO $$ +DECLARE + collision_count INT; + collision_list TEXT; +BEGIN + -- Check for collisions before updating. The subquery returns rows that will + -- collide after normalisation: groups of publishers whose LOWER(TRIM(email)) + -- values are identical and count > 1. + SELECT COUNT(*), STRING_AGG(DISTINCT LOWER(TRIM(email)), ', ') + INTO collision_count, collision_list + FROM publishers + GROUP BY LOWER(TRIM(email)) + HAVING COUNT(*) > 1; + + IF collision_count > 0 THEN + RAISE EXCEPTION 'Cannot normalise publisher emails: % collision(s) found for: %', + collision_count, collision_list; + END IF; + + -- No collisions detected; safe to normalise. + UPDATE publishers + SET email = LOWER(TRIM(email)) + WHERE email <> LOWER(TRIM(email)); + + RAISE NOTICE 'Publisher emails normalised. Rows updated: %', (SELECT COUNT(*) FROM publishers WHERE email <> LOWER(TRIM(email))); +END $$; diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index b62fab9..2726ce3 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -92,6 +92,13 @@ "when": 1761004800000, "tag": "0014_publisher_signing_secret", "breakpoints": true + }, + { + "idx": 13, + "version": "7", + "when": 1761177600000, + "tag": "0015_lowercase_publisher_email", + "breakpoints": true } ] } diff --git a/src/routes/publishers.test.ts b/src/routes/publishers.test.ts index 7bb94e6..e27523e 100644 --- a/src/routes/publishers.test.ts +++ b/src/routes/publishers.test.ts @@ -208,6 +208,51 @@ describe("POST /publishers — registration (#294)", () => { expect(res.status).toBe(500); }); + + // ── Email normalisation (#39) ────────────────────────────────────────────── + + it("normalises a mixed-case email before passing it to registerPublisher (#39)", async () => { + mockRegisterPublisher.mockResolvedValue({ + publisher: { + id: "pub-2", + name: "Ada", + email: "ada@x.com", + walletAddress: VALID_WALLET, + createdAt: new Date("2026-01-01"), + }, + apiKey: "mv_freshkey2", + }); + + const res = await request(createTestApp()).post("/publishers").send({ + name: "Ada", + email: "Ada@X.Com", + walletAddress: VALID_WALLET, + }); + + expect(res.status).toBe(201); + // The service must receive the normalised (lowercased, trimmed) email. + expect(mockRegisterPublisher).toHaveBeenCalledWith( + expect.objectContaining({ email: "ada@x.com" }), + ); + }); + + it("returns 409 when a differently-cased email already exists (#39)", async () => { + mockRegisterPublisher.mockRejectedValue( + Object.assign(new Error("duplicate key value violates unique constraint"), { + code: "23505", + }), + ); + + const res = await request(createTestApp()).post("/publishers").send({ + name: "Ada", + // Email is normalised to ada@x.com, which is already taken. + email: "ADA@X.COM", + walletAddress: VALID_WALLET, + }); + + expect(res.status).toBe(409); + expect(res.body.code).toBe("CONFLICT"); + }); }); describe("GET /publishers/me — auth enforcement (#294)", () => { diff --git a/src/schemas/requests.ts b/src/schemas/requests.ts index 42e2018..772c5ca 100644 --- a/src/schemas/requests.ts +++ b/src/schemas/requests.ts @@ -8,7 +8,9 @@ import { httpUrlSchema } from "../lib/externalUrlValidation.js"; export const publisherRegisterSchema = z .object({ name: z.string().min(1), - email: z.email(), + // Normalise email at the boundary: trim whitespace and lowercase so the + // unique constraint reliably catches case-insensitive duplicates (#39). + email: z.email().transform((e) => e.trim().toLowerCase()), walletAddress: stellarAddressSchema, }) .strict(); diff --git a/src/services/publisherService.email-normalisation.test.ts b/src/services/publisherService.email-normalisation.test.ts new file mode 100644 index 0000000..aaa330d --- /dev/null +++ b/src/services/publisherService.email-normalisation.test.ts @@ -0,0 +1,152 @@ +/** + * Unit tests for email normalisation in registerPublisher (#39). + * + * These tests exercise the service directly (not via the HTTP layer) so they + * verify that the normalisation happens inside the service itself — the + * defence-in-depth layer that protects against callers that bypass the Zod + * schema. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// ── DB mock ────────────────────────────────────────────────────────────────── + +const mockInsertValues = vi.fn(); +const mockInsertReturning = vi.fn(); + +vi.mock("../db/client.js", () => ({ + db: { + insert: () => ({ + values: (row: unknown) => { + mockInsertValues(row); + return { returning: mockInsertReturning }; + }, + }), + }, +})); + +vi.mock("../db/schema.js", () => ({ + publishers: { __table: "publishers" }, + resources: { __table: "resources" }, +})); + +// ── Crypto mock — return deterministic values so we can assert row shape ───── + +vi.mock("../utils/crypto.js", () => ({ + generateApiKey: () => "mv_testkey", + hashApiKey: () => "hashed_testkey", + generateSigningSecret: () => "test_signing_secret", + encryptSigningSecret: (s: string) => `encrypted:${s}`, +})); + +vi.mock("../config.js", () => ({ + config: { SIGNING_SECRET_KEY: undefined }, +})); + +// ── Import the module under test after mocks are in place ──────────────────── + +import { registerPublisher } from "./publisherService.js"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +const VALID_WALLET = "GDEJJLXV2CNHF5HEGX34MU4G6L4PRVUAGD4ANF77ZDYTQJKSDSDYVVI6"; + +function makePublisher(email: string) { + return { + id: "pub-1", + name: "Alice", + email, + walletAddress: VALID_WALLET, + apiKeyHash: "hashed_testkey", + createdAt: new Date("2026-01-01"), + }; +} + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("registerPublisher — email normalisation (#39)", () => { + beforeEach(() => { + mockInsertValues.mockReset(); + mockInsertReturning.mockReset(); + }); + + it("lowercases an uppercase email before inserting", async () => { + const normalised = "ada@x.com"; + mockInsertReturning.mockResolvedValue([makePublisher(normalised)]); + + await registerPublisher({ name: "Ada", email: "ADA@X.COM", walletAddress: VALID_WALLET }); + + const inserted = mockInsertValues.mock.calls[0][0]; + expect(inserted.email).toBe(normalised); + }); + + it("trims whitespace from the email before inserting", async () => { + const normalised = "ada@x.com"; + mockInsertReturning.mockResolvedValue([makePublisher(normalised)]); + + await registerPublisher({ name: "Ada", email: " ada@x.com ", walletAddress: VALID_WALLET }); + + const inserted = mockInsertValues.mock.calls[0][0]; + expect(inserted.email).toBe(normalised); + }); + + it("trims and lowercases together", async () => { + const normalised = "ada@x.com"; + mockInsertReturning.mockResolvedValue([makePublisher(normalised)]); + + await registerPublisher({ name: "Ada", email: " Ada@X.Com ", walletAddress: VALID_WALLET }); + + const inserted = mockInsertValues.mock.calls[0][0]; + expect(inserted.email).toBe(normalised); + }); + + it("leaves an already-normalised email unchanged", async () => { + const email = "ada@x.com"; + mockInsertReturning.mockResolvedValue([makePublisher(email)]); + + await registerPublisher({ name: "Ada", email, walletAddress: VALID_WALLET }); + + const inserted = mockInsertValues.mock.calls[0][0]; + expect(inserted.email).toBe(email); + }); + + it("returns the normalised email from the publisher row", async () => { + const normalised = "ada@x.com"; + mockInsertReturning.mockResolvedValue([makePublisher(normalised)]); + + const { publisher } = await registerPublisher({ + name: "Ada", + email: "Ada@X.Com", + walletAddress: VALID_WALLET, + }); + + expect(publisher.email).toBe(normalised); + }); +}); + +describe("publishers route — 409 on case-insensitive duplicate (#39)", () => { + /** + * The route already has a 409 test for exact-match duplicates. This block + * adds a complementary test that verifies the same 409 path is hit when the + * DB rejects an insert that collides with a differently-cased row — i.e. the + * service normalises the email, hits the unique constraint, and the route + * converts that into a 409. + * + * Because the route test mocks registerPublisher we test the mapping of the + * DB error → 409 here at the service boundary instead. + */ + beforeEach(() => { + mockInsertValues.mockReset(); + mockInsertReturning.mockReset(); + }); + + it("propagates a unique-constraint error when the normalised email already exists", async () => { + const dbError = Object.assign(new Error("duplicate key value violates unique constraint"), { + code: "23505", + }); + mockInsertReturning.mockRejectedValue(dbError); + + await expect( + registerPublisher({ name: "Ada", email: "Ada@X.Com", walletAddress: VALID_WALLET }), + ).rejects.toMatchObject({ code: "23505" }); + }); +}); diff --git a/src/services/publisherService.ts b/src/services/publisherService.ts index 01ad8f0..9468c43 100644 --- a/src/services/publisherService.ts +++ b/src/services/publisherService.ts @@ -9,6 +9,10 @@ export async function registerPublisher(data: { email: string; walletAddress: string; }) { + // Normalise the email so that Ada@x.com and ada@x.com are treated as the + // same address and the unique constraint reliably catches duplicates (#39). + const email = data.email.trim().toLowerCase(); + const apiKey = generateApiKey(); const apiKeyHash = hashApiKey(apiKey); @@ -25,7 +29,7 @@ export async function registerPublisher(data: { .insert(publishers) .values({ name: data.name, - email: data.email, + email, walletAddress: data.walletAddress, apiKeyHash, signingSecret: storedSigningSecret,