From 5d72e60178b534dcd748123fac12667d636aa248 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 14:05:39 +0100 Subject: [PATCH 1/5] Add author profile moderation endpoints --- .../extensions/v2/authors-database.ts | 184 ++++++++++++++ .../migrations/0002_add_author_approval.sql | 8 + src/services/extensions/v2/index.ts | 193 ++++++++++++++- src/services/extensions/v2/interfaces.ts | 8 + .../extensions/v2/submissions-database.ts | 8 +- test/services/extensions/v2/index.test.ts | 231 +++++++++++++++++- test/services/extensions/v2/mock-db.ts | 72 ++++++ 7 files changed, 696 insertions(+), 8 deletions(-) create mode 100644 src/services/extensions/v2/authors-database.ts create mode 100644 src/services/extensions/v2/db/migrations/0002_add_author_approval.sql diff --git a/src/services/extensions/v2/authors-database.ts b/src/services/extensions/v2/authors-database.ts new file mode 100644 index 0000000..0b2160a --- /dev/null +++ b/src/services/extensions/v2/authors-database.ts @@ -0,0 +1,184 @@ +import { DatabaseResult, IDatabase } from "../../../lib/interfaces"; +import { databaseError } from "./errors"; +import { Author, AuthorProfile } from "./interfaces"; + +function parseAuthorRow(row: Record): AuthorProfile { + return { + id: row.id as string, + type: row.type as AuthorProfile["type"], + name: row.name as string, + URL: (row.url as string | null) ?? undefined, + approved: row.approved_at !== null && row.approved_at !== undefined + }; +} + +export class AuthorsDatabase { + private db: IDatabase; + + constructor(db: IDatabase) { + this.db = db; + } + + async getOwn(userId: string): Promise> { + try { + const row = await this.db + .prepare("SELECT * FROM authors WHERE owner_user_id = ?") + .bind(userId) + .first>(); + return { data: row ? parseAuthorRow(row) : null, error: null }; + } catch (error) { + return databaseError("getOwn", error); + } + } + + async upsertOwn( + userId: string, + author: Author + ): Promise> { + try { + const existingOwn = await this.db + .prepare("SELECT * FROM authors WHERE owner_user_id = ?") + .bind(userId) + .first>(); + + const existingById = await this.db + .prepare("SELECT * FROM authors WHERE id = ?") + .bind(author.id) + .first>(); + + if (!existingOwn) { + if (existingById) { + return { + data: null, + error: { message: "Author id already exists", code: "CONFLICT" } + }; + } + + const result = await this.db + .prepare( + `INSERT INTO authors (id, type, name, url, owner_user_id, approved_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)` + ) + .bind(author.id, author.type, author.name, author.URL ?? null, userId) + .run(); + + if (!result.success) { + return databaseError( + "upsertOwn", + new Error(result.error || "Database query failed") + ); + } + } else { + if (author.id !== existingOwn.id) { + return { + data: null, + error: { + message: "Author id cannot be changed", + code: "CONFLICT" + } + }; + } + + // approved_at is always cleared here, even if nothing meaningful + // changed — the reviewed content just got overwritten, so the old + // approval no longer applies. Not worth diffing old vs. new values. + const result = await this.db + .prepare( + `UPDATE authors SET type = ?, name = ?, url = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP + WHERE id = ?` + ) + .bind(author.type, author.name, author.URL ?? null, author.id) + .run(); + + if (!result.success) { + return databaseError( + "upsertOwn", + new Error(result.error || "Database query failed") + ); + } + } + + return this.getById(author.id); + } catch (error) { + return databaseError("upsertOwn", error); + } + } + + private async getById(id: string): Promise> { + try { + const row = await this.db + .prepare("SELECT * FROM authors WHERE id = ?") + .bind(id) + .first>(); + if (!row) { + return { + data: null, + error: { + message: `Cannot find author by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + return { data: parseAuthorRow(row), error: null }; + } catch (error) { + return databaseError("getById", error); + } + } + + async listUnapproved(): Promise> { + let result; + try { + result = await this.db + .prepare( + "SELECT * FROM authors WHERE approved_at IS NULL ORDER BY created_at ASC" + ) + .all>(); + } catch (error) { + return databaseError("listUnapproved", error); + } + + if (!result.success) { + return databaseError( + "listUnapproved", + new Error(result.error || "Database query failed") + ); + } + + return { + data: (result.results ?? []).map(parseAuthorRow), + error: null + }; + } + + async approve( + id: string + ): Promise> { + let result; + try { + result = await this.db + .prepare( + "UPDATE authors SET approved_at = CURRENT_TIMESTAMP WHERE id = ?" + ) + .bind(id) + .run(); + } catch (error) { + return databaseError("approve", error); + } + + if (!result.success) { + return databaseError( + "approve", + new Error(result.error || "Database query failed") + ); + } + + if (!result.meta?.changes) { + return { + data: null, + error: { message: `Cannot find author by id: ${id}`, code: "NOT_FOUND" } + }; + } + + return { data: { id, approved: true }, error: null }; + } +} diff --git a/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql b/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql new file mode 100644 index 0000000..5b5b2a3 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql @@ -0,0 +1,8 @@ +-- v2: direct (unmoderated) developer-profile writes, with a moderator-set +-- "approved" trust flag. Adds to the v1-owned `authors` table. + +ALTER TABLE authors ADD COLUMN approved_at TEXT; +ALTER TABLE authors ADD COLUMN created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP; +ALTER TABLE authors ADD COLUMN updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP; + +CREATE INDEX IF NOT EXISTS idx_authors_approved ON authors(approved_at); diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index ae85427..7288410 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -6,6 +6,8 @@ import { Scalar } from "@scalar/hono-api-reference"; import { getPlatform } from "../../../lib/middleware"; import { getAuth, requireAuth } from "../../../lib/auth"; import { + AuthorProfileSchema, + AuthorSchema, ErrorResponseSchema, IdParamSchema, QueueQuerySchema, @@ -14,6 +16,7 @@ import { SubmissionPayloadSchema, SubmissionSchema } from "./interfaces"; +import { AuthorsDatabase } from "./authors-database"; import { SubmissionsDatabase } from "./submissions-database"; import { UsersDatabase } from "./users-database"; @@ -416,6 +419,185 @@ extensionsV2.openapi(rejectRoute, async (c) => { return c.json({ result: data }, 200); }); +const upsertOwnAuthorRoute = createRoute({ + method: "put", + path: "/authors/me", + tags: ["Authors"], + summary: "Create or update the caller's own developer profile", + security: [{ Bearer: [] }], + middleware: [requireAuth()] as const, + request: { + body: { + content: { "application/json": { schema: AuthorSchema } } + } + }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: AuthorProfileSchema }) + } + }, + description: "Developer profile created or updated and usable immediately" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 409: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: + "Author id already taken by someone else, or id was changed on an existing profile" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Payload failed validation" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(upsertOwnAuthorRoute, async (c) => { + const auth = getAuth(c); + const body = c.req.valid("json"); + const platform = getPlatform(c); + const db = new AuthorsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.upsertOwn(auth.userId, body); + if (error || !data) { + const status = error?.code === "CONFLICT" ? 409 : 500; + return c.json( + { + error: { + message: error?.message ?? "Unable to save developer profile", + code: error?.code ?? "DATABASE_ERROR" + } + }, + status + ); + } + + return c.json({ result: data }, 200); +}); + +const unapprovedAuthorsRoute = createRoute({ + method: "get", + path: "/authors/unapproved", + tags: ["Moderation"], + summary: "List developer profiles awaiting moderator review", + security: [{ Bearer: [] }], + middleware: [requireAuth(), requireModerator()] as const, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: z.array(AuthorProfileSchema) }) + } + }, + description: "Developer profiles not yet approved" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 403: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller is not a moderator" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(unapprovedAuthorsRoute, async (c) => { + const platform = getPlatform(c); + const db = new AuthorsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.listUnapproved(); + if (error || !data) { + return c.json( + { + error: { + message: error?.message ?? "Unable to load unapproved authors", + code: "DATABASE_ERROR" + } + }, + 500 + ); + } + + return c.json({ result: data }, 200); +}); + +const approveAuthorRoute = createRoute({ + method: "post", + path: "/authors/{id}/approve", + tags: ["Moderation"], + summary: "Mark a developer profile as reviewed/approved", + security: [{ Bearer: [] }], + middleware: [requireAuth(), requireModerator()] as const, + request: { params: IdParamSchema }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ id: z.string(), approved: z.literal(true) }) + }) + } + }, + description: "Developer profile marked approved" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 403: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller is not a moderator" + }, + 404: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "No author with that id" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "id param failed validation" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(approveAuthorRoute, async (c) => { + const { id } = c.req.valid("param"); + const platform = getPlatform(c); + const db = new AuthorsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.approve(id); + if (error || !data) { + const status = error?.code === "NOT_FOUND" ? 404 : 500; + return c.json( + { + error: { + message: error?.message ?? "Unable to approve author", + code: error?.code ?? "DATABASE_ERROR" + } + }, + status + ); + } + + return c.json({ result: data }, 200); +}); + extensionsV2.doc("/openapi.json", { openapi: "3.1.0", info: { @@ -427,9 +609,12 @@ extensionsV2.doc("/openapi.json", { servers: [{ url: "/extensions/v2" }] }); -extensionsV2.get("/docs", Scalar({ - url: "/extensions/v2/openapi.json", - pageTitle: 'FOSSBilling Extensions API (v2)', -})); +extensionsV2.get( + "/docs", + Scalar({ + url: "/extensions/v2/openapi.json", + pageTitle: "FOSSBilling Extensions API (v2)" + }) +); export default extensionsV2; diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 8a00ee1..d3d1ed7 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -38,6 +38,8 @@ export const AuthorSchema = z }) .openapi("Author"); +export type Author = z.infer; + export const ReleaseSchema = z .object({ tag: z.string().min(1), @@ -88,6 +90,12 @@ export const SubmissionPayloadSchema = z export type SubmissionPayload = z.infer; +export const AuthorProfileSchema = AuthorSchema.extend({ + approved: z.boolean() +}).openapi("AuthorProfile"); + +export type AuthorProfile = z.infer; + export const ReviewNoteOptionalSchema = z .object({ review_note: z.string().optional() diff --git a/src/services/extensions/v2/submissions-database.ts b/src/services/extensions/v2/submissions-database.ts index b061f43..8fc039b 100644 --- a/src/services/extensions/v2/submissions-database.ts +++ b/src/services/extensions/v2/submissions-database.ts @@ -77,10 +77,14 @@ export class SubmissionsDatabase { .bind(payload.author.id) .first<{ owner_user_id: string | null }>(); - if (payloadAuthor && payloadAuthor.owner_user_id !== callerId) { + if (!payloadAuthor || payloadAuthor.owner_user_id !== callerId) { return { data: null, - error: { message: "You do not own this author", code: "FORBIDDEN" } + error: { + message: + "You do not own this author, or it doesn't exist yet — create a developer profile first", + code: "FORBIDDEN" + } }; } diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 9b4e0a0..4b9e37f 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -60,6 +60,18 @@ function samplePayload(overrides?: { }; } +// Extension submissions now require the named author to already exist +// (created via PUT /authors/me) and be owned by the caller. +function seedAuthor(id: string, ownerUserId: string): void { + tables.authors.set(id, { + id, + type: "user", + name: "Author", + url: null, + owner_user_id: ownerUserId + }); +} + function seedOwnedExtension(): void { tables.authors.set("owner-author", { id: "owner-author", @@ -112,6 +124,35 @@ async function get(path: string, headers: Record) { return res; } +async function put( + path: string, + headers: Record, + body?: unknown +) { + const ctx = createExecutionContext(); + const res = await app.request( + path, + { + method: "PUT", + headers, + body: body !== undefined ? JSON.stringify(body) : undefined + }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return res; +} + +function sampleAuthor(overrides?: { id?: string; name?: string }) { + return { + id: overrides?.id ?? "dev-author", + type: "user", + name: overrides?.name ?? "Dev Author", + URL: "https://example.com" + }; +} + describe("Extensions API v2", () => { describe("POST /submissions", () => { it("requires auth", async () => { @@ -136,7 +177,8 @@ describe("Extensions API v2", () => { expect(data.error.code).toBe("VALIDATION_ERROR"); }); - it("creates a pending submission for a brand-new extension and author", async () => { + it("creates a pending submission for a brand-new extension under an existing author", async () => { + seedAuthor("new-author", "user-1"); const headers = await authHeaders("user-1"); const res = await post( "/extensions/v2/submissions", @@ -200,10 +242,25 @@ describe("Extensions API v2", () => { expect(res.status).toBe(403); }); + + it("rejects naming an author id that doesn't exist at all", async () => { + const headers = await authHeaders("user-1"); + + const res = await post( + "/extensions/v2/submissions", + headers, + samplePayload({ authorId: "no-such-author" }) + ); + + expect(res.status).toBe(403); + expect(tables.extension_submissions.size).toBe(0); + }); }); describe("GET /submissions/mine", () => { it("returns only the caller's own submissions", async () => { + seedAuthor("author-a", "user-1"); + seedAuthor("author-b", "user-2"); await post( "/extensions/v2/submissions", await authHeaders("user-1"), @@ -244,6 +301,7 @@ describe("Extensions API v2", () => { it("returns pending submissions for a moderator", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedAuthor("new-author", "user-1"); await post( "/extensions/v2/submissions", await authHeaders("user-1"), @@ -264,6 +322,7 @@ describe("Extensions API v2", () => { describe("approve / reject", () => { it("reverts to pending if the write-through fails after a successful claim", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedAuthor("new-author", "user-1"); const created = await post( "/extensions/v2/submissions", @@ -297,6 +356,7 @@ describe("Extensions API v2", () => { it("approves a submission and it becomes visible via the v1 read path", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedAuthor("new-author", "user-1"); const created = await post( "/extensions/v2/submissions", @@ -326,6 +386,7 @@ describe("Extensions API v2", () => { }); it("blocks non-moderators from approving", async () => { + seedAuthor("new-author", "user-1"); const created = await post( "/extensions/v2/submissions", await authHeaders("user-1"), @@ -343,6 +404,7 @@ describe("Extensions API v2", () => { it("rejects approving a submission that is not pending", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedAuthor("new-author", "user-1"); const created = await post( "/extensions/v2/submissions", await authHeaders("user-1"), @@ -412,6 +474,7 @@ describe("Extensions API v2", () => { it("requires a review_note to reject", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedAuthor("new-author", "user-1"); const created = await post( "/extensions/v2/submissions", await authHeaders("user-1"), @@ -429,6 +492,7 @@ describe("Extensions API v2", () => { it("rejects a submission with a note", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedAuthor("new-author", "user-1"); const created = await post( "/extensions/v2/submissions", await authHeaders("user-1"), @@ -448,6 +512,166 @@ describe("Extensions API v2", () => { }); }); + describe("PUT /authors/me", () => { + it("creates a new author profile, unapproved", async () => { + const res = await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { result: { approved: boolean } }; + expect(data.result.approved).toBe(false); + + const stored = tables.authors.get("dev-author"); + expect(stored).toBeDefined(); + expect(stored?.approved_at).toBeNull(); + }); + + it("updates an existing profile, still unapproved", async () => { + await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + + const res = await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor({ name: "Renamed Author" }) + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { name: string; approved: boolean }; + }; + expect(data.result.name).toBe("Renamed Author"); + expect(data.result.approved).toBe(false); + expect(tables.authors.get("dev-author")?.name).toBe("Renamed Author"); + }); + + it("rejects an id that already belongs to someone else", async () => { + await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + + const res = await put( + "/extensions/v2/authors/me", + await authHeaders("user-2"), + sampleAuthor() + ); + + expect(res.status).toBe(409); + }); + + it("rejects changing the id on an existing profile", async () => { + await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + + const res = await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor({ id: "different-id" }) + ); + + expect(res.status).toBe(409); + }); + + it("clears approval when an approved profile is edited", async () => { + await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + await post( + "/extensions/v2/authors/dev-author/approve", + await authHeaders("mod-1") + ); + + const res = await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor({ name: "Edited Again" }) + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { result: { approved: boolean } }; + expect(data.result.approved).toBe(false); + }); + }); + + describe("author moderation", () => { + it("approves an author and removes it from the unapproved list", async () => { + await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + + const approve = await post( + "/extensions/v2/authors/dev-author/approve", + await authHeaders("mod-1") + ); + expect(approve.status).toBe(200); + const approveBody = (await approve.json()) as { + result: { id: string; approved: boolean }; + }; + expect(approveBody.result).toEqual({ id: "dev-author", approved: true }); + + const unapproved = await get( + "/extensions/v2/authors/unapproved", + await authHeaders("mod-1") + ); + expect(unapproved.status).toBe(200); + const unapprovedBody = (await unapproved.json()) as { + result: Array<{ id: string }>; + }; + expect(unapprovedBody.result.map((a) => a.id)).not.toContain( + "dev-author" + ); + }); + + it("404s approving a nonexistent author", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + + const res = await post( + "/extensions/v2/authors/no-such-author/approve", + await authHeaders("mod-1") + ); + expect(res.status).toBe(404); + }); + + it("blocks non-moderators from listing unapproved authors", async () => { + const res = await get( + "/extensions/v2/authors/unapproved", + await authHeaders("user-1") + ); + expect(res.status).toBe(403); + }); + + it("blocks non-moderators from approving authors", async () => { + await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + + const res = await post( + "/extensions/v2/authors/dev-author/approve", + await authHeaders("user-1") + ); + expect(res.status).toBe(403); + }); + }); + describe("OpenAPI docs", () => { it("serves a generated OpenAPI document", async () => { const res = await get("/extensions/v2/openapi.json", {}); @@ -463,7 +687,10 @@ describe("Extensions API v2", () => { "/submissions/mine", "/submissions/queue", "/submissions/{id}/approve", - "/submissions/{id}/reject" + "/submissions/{id}/reject", + "/authors/me", + "/authors/unapproved", + "/authors/{id}/approve" ]) ); }); diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index b2f4832..d5d1bc3 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -110,6 +110,78 @@ class MockStatement implements D1PreparedStatement { return row ? [row] : []; } + if (q.startsWith("SELECT * FROM authors WHERE owner_user_id = ?")) { + const row = [...this.tables.authors.values()].find( + (r) => r.owner_user_id === p[0] + ); + return row ? [row] : []; + } + + if (q.startsWith("SELECT * FROM authors WHERE id = ?")) { + const row = this.tables.authors.get(String(p[0])); + return row ? [row] : []; + } + + if (q.startsWith("SELECT * FROM authors WHERE approved_at IS NULL")) { + return [...this.tables.authors.values()] + .filter((r) => (r.approved_at ?? null) === null) + .sort((a, b) => + String(a.created_at ?? "").localeCompare(String(b.created_at ?? "")) + ); + } + + if ( + q.startsWith( + "INSERT INTO authors (id, type, name, url, owner_user_id, approved_at, created_at, updated_at)" + ) + ) { + const [id, type, name, url, owner_user_id] = p; + const now = new Date().toISOString(); + this.tables.authors.set(String(id), { + id, + type, + name, + url, + owner_user_id, + approved_at: null, + created_at: now, + updated_at: now + }); + return []; + } + + if ( + q.startsWith( + "UPDATE authors SET type = ?, name = ?, url = ?, approved_at = NULL" + ) + ) { + const [type, name, url, id] = p; + const row = this.tables.authors.get(String(id)); + if (row) { + row.type = type; + row.name = name; + row.url = url; + row.approved_at = null; + row.updated_at = new Date().toISOString(); + this.changes = 1; + } + return []; + } + + if ( + q.startsWith( + "UPDATE authors SET approved_at = CURRENT_TIMESTAMP WHERE id = ?" + ) + ) { + const [id] = p; + const row = this.tables.authors.get(String(id)); + if (row) { + row.approved_at = new Date().toISOString(); + this.changes = 1; + } + return []; + } + if (q.startsWith("SELECT is_moderator FROM users WHERE id = ?")) { const row = this.tables.users.get(String(p[0])); return row ? [row] : []; From b157daea569f12ae4763f9fecf0751749c86cfef Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 14:18:39 +0100 Subject: [PATCH 2/5] Fix author timestamp migration defaults --- .../v2/db/migrations/0002_add_author_approval.sql | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql b/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql index 5b5b2a3..b32b4fe 100644 --- a/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql +++ b/src/services/extensions/v2/db/migrations/0002_add_author_approval.sql @@ -1,8 +1,16 @@ -- v2: direct (unmoderated) developer-profile writes, with a moderator-set -- "approved" trust flag. Adds to the v1-owned `authors` table. +-- +-- SQLite's ALTER TABLE ADD COLUMN rejects non-constant defaults (including +-- CURRENT_TIMESTAMP), so created_at/updated_at are added with a placeholder +-- default and backfilled immediately after. New rows always set these +-- explicitly (see authors-database.ts), so the placeholder is never seen +-- outside of this migration. ALTER TABLE authors ADD COLUMN approved_at TEXT; -ALTER TABLE authors ADD COLUMN created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP; -ALTER TABLE authors ADD COLUMN updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP; +ALTER TABLE authors ADD COLUMN created_at TEXT NOT NULL DEFAULT '1970-01-01T00:00:00.000Z'; +ALTER TABLE authors ADD COLUMN updated_at TEXT NOT NULL DEFAULT '1970-01-01T00:00:00.000Z'; + +UPDATE authors SET created_at = CURRENT_TIMESTAMP, updated_at = CURRENT_TIMESTAMP; CREATE INDEX IF NOT EXISTS idx_authors_approved ON authors(approved_at); From 21872cf1b17d7b9c15885d5dc8b3ba931814c3c5 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 14:41:56 +0100 Subject: [PATCH 3/5] Add author profile fields to extensions v2 --- .../extensions/v2/authors-database.ts | 30 ++++++++++-- .../0003_add_author_profile_fields.sql | 7 +++ src/services/extensions/v2/interfaces.ts | 5 +- test/services/extensions/v2/index.test.ts | 47 +++++++++++++++++++ test/services/extensions/v2/mock-db.ts | 23 +++++++-- 5 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 src/services/extensions/v2/db/migrations/0003_add_author_profile_fields.sql diff --git a/src/services/extensions/v2/authors-database.ts b/src/services/extensions/v2/authors-database.ts index 0b2160a..1859ed1 100644 --- a/src/services/extensions/v2/authors-database.ts +++ b/src/services/extensions/v2/authors-database.ts @@ -8,6 +8,9 @@ function parseAuthorRow(row: Record): AuthorProfile { type: row.type as AuthorProfile["type"], name: row.name as string, URL: (row.url as string | null) ?? undefined, + bio: (row.bio as string | null) ?? undefined, + avatar_url: (row.avatar_url as string | null) ?? undefined, + contact_email: (row.contact_email as string | null) ?? undefined, approved: row.approved_at !== null && row.approved_at !== undefined }; } @@ -56,10 +59,19 @@ export class AuthorsDatabase { const result = await this.db .prepare( - `INSERT INTO authors (id, type, name, url, owner_user_id, approved_at, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)` + `INSERT INTO authors (id, type, name, url, bio, avatar_url, contact_email, owner_user_id, approved_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)` + ) + .bind( + author.id, + author.type, + author.name, + author.URL ?? null, + author.bio ?? null, + author.avatar_url ?? null, + author.contact_email ?? null, + userId ) - .bind(author.id, author.type, author.name, author.URL ?? null, userId) .run(); if (!result.success) { @@ -84,10 +96,18 @@ export class AuthorsDatabase { // approval no longer applies. Not worth diffing old vs. new values. const result = await this.db .prepare( - `UPDATE authors SET type = ?, name = ?, url = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP + `UPDATE authors SET type = ?, name = ?, url = ?, bio = ?, avatar_url = ?, contact_email = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?` ) - .bind(author.type, author.name, author.URL ?? null, author.id) + .bind( + author.type, + author.name, + author.URL ?? null, + author.bio ?? null, + author.avatar_url ?? null, + author.contact_email ?? null, + author.id + ) .run(); if (!result.success) { diff --git a/src/services/extensions/v2/db/migrations/0003_add_author_profile_fields.sql b/src/services/extensions/v2/db/migrations/0003_add_author_profile_fields.sql new file mode 100644 index 0000000..7f33240 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0003_add_author_profile_fields.sql @@ -0,0 +1,7 @@ +-- v2: additional developer-profile fields for the public /developer/{id} +-- page (bio, avatar_url) and moderator/maintainer contact (contact_email, +-- never exposed on public reads). Adds to the v1-owned `authors` table. + +ALTER TABLE authors ADD COLUMN bio TEXT; +ALTER TABLE authors ADD COLUMN avatar_url TEXT; +ALTER TABLE authors ADD COLUMN contact_email TEXT; diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index d3d1ed7..b5dc09c 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -34,7 +34,10 @@ export const AuthorSchema = z id: lowercaseId("author"), type: z.enum(["user", "organization"]), name: z.string().min(1), - URL: httpUrl().optional() + URL: httpUrl().optional(), + bio: z.string().max(500).optional(), + avatar_url: httpUrl().optional(), + contact_email: z.string().email().optional() }) .openapi("Author"); diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 4b9e37f..58f5eb0 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -605,6 +605,53 @@ describe("Extensions API v2", () => { const data = (await res.json()) as { result: { approved: boolean } }; expect(data.result.approved).toBe(false); }); + + it("round-trips bio, avatar_url, and contact_email", async () => { + const headers = await authHeaders("user-1"); + const res = await put("/extensions/v2/authors/me", headers, { + ...sampleAuthor(), + bio: "I build FOSSBilling extensions.", + avatar_url: "https://example.com/avatar.png", + contact_email: "dev@example.com" + }); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { + bio: string; + avatar_url: string; + contact_email: string; + }; + }; + expect(data.result.bio).toBe("I build FOSSBilling extensions."); + expect(data.result.avatar_url).toBe("https://example.com/avatar.png"); + expect(data.result.contact_email).toBe("dev@example.com"); + + const stored = tables.authors.get("dev-author"); + expect(stored?.bio).toBe("I build FOSSBilling extensions."); + expect(stored?.avatar_url).toBe("https://example.com/avatar.png"); + expect(stored?.contact_email).toBe("dev@example.com"); + }); + + it("accepts a payload without bio, avatar_url, or contact_email", async () => { + const res = await put( + "/extensions/v2/authors/me", + await authHeaders("user-1"), + sampleAuthor() + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { + bio?: string; + avatar_url?: string; + contact_email?: string; + }; + }; + expect(data.result.bio).toBeUndefined(); + expect(data.result.avatar_url).toBeUndefined(); + expect(data.result.contact_email).toBeUndefined(); + }); }); describe("author moderation", () => { diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index d5d1bc3..a2109ba 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -132,16 +132,28 @@ class MockStatement implements D1PreparedStatement { if ( q.startsWith( - "INSERT INTO authors (id, type, name, url, owner_user_id, approved_at, created_at, updated_at)" + "INSERT INTO authors (id, type, name, url, bio, avatar_url, contact_email, owner_user_id, approved_at, created_at, updated_at)" ) ) { - const [id, type, name, url, owner_user_id] = p; + const [ + id, + type, + name, + url, + bio, + avatar_url, + contact_email, + owner_user_id + ] = p; const now = new Date().toISOString(); this.tables.authors.set(String(id), { id, type, name, url, + bio, + avatar_url, + contact_email, owner_user_id, approved_at: null, created_at: now, @@ -152,15 +164,18 @@ class MockStatement implements D1PreparedStatement { if ( q.startsWith( - "UPDATE authors SET type = ?, name = ?, url = ?, approved_at = NULL" + "UPDATE authors SET type = ?, name = ?, url = ?, bio = ?, avatar_url = ?, contact_email = ?, approved_at = NULL" ) ) { - const [type, name, url, id] = p; + const [type, name, url, bio, avatar_url, contact_email, id] = p; const row = this.tables.authors.get(String(id)); if (row) { row.type = type; row.name = name; row.url = url; + row.bio = bio; + row.avatar_url = avatar_url; + row.contact_email = contact_email; row.approved_at = null; row.updated_at = new Date().toISOString(); this.changes = 1; From 13e6c4bc445dc856b8ac3042014a6eeb10d185bb Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 26 Jul 2026 10:25:49 +0100 Subject: [PATCH 4/5] Address PR review findings on author profiles - Enforce one profile per owner at the DB level (unique index on authors.owner_user_id) and map the resulting conflict to 409, closing a race between concurrent first-time PUT /authors/me requests. - Reject bio/avatar_url/contact_email on extension submissions instead of silently discarding them on approval; those fields are direct-write-only via PUT /authors/me. - Remove unused AuthorsDatabase.getOwn(). - Strengthen tests: assert approval actually succeeded before checking it clears, cover the profile-fields update path, and add regression tests for the new conflict and validation behavior. Claude-Session: https://claude.ai/code/session_01LGdm9SjGTpdz9Gj7GcLs13 --- .../extensions/v2/authors-database.ts | 74 +++++++++++------- .../0004_add_authors_owner_unique.sql | 9 +++ src/services/extensions/v2/interfaces.ts | 15 +++- test/services/extensions/v2/index.test.ts | 78 ++++++++++++++++++- test/services/extensions/v2/mock-db.ts | 9 +++ 5 files changed, 155 insertions(+), 30 deletions(-) create mode 100644 src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql diff --git a/src/services/extensions/v2/authors-database.ts b/src/services/extensions/v2/authors-database.ts index 1859ed1..bbcd332 100644 --- a/src/services/extensions/v2/authors-database.ts +++ b/src/services/extensions/v2/authors-database.ts @@ -2,6 +2,13 @@ import { DatabaseResult, IDatabase } from "../../../lib/interfaces"; import { databaseError } from "./errors"; import { Author, AuthorProfile } from "./interfaces"; +// Matches the SQLite/D1 message for the idx_authors_owner_unique violation, +// which is how a lost race between two concurrent first-time PUT /authors/me +// requests (same caller, different ids) surfaces. +function isOwnerConflict(message: string | undefined): boolean { + return !!message && /UNIQUE constraint failed.*owner_user_id/i.test(message); +} + function parseAuthorRow(row: Record): AuthorProfile { return { id: row.id as string, @@ -22,18 +29,6 @@ export class AuthorsDatabase { this.db = db; } - async getOwn(userId: string): Promise> { - try { - const row = await this.db - .prepare("SELECT * FROM authors WHERE owner_user_id = ?") - .bind(userId) - .first>(); - return { data: row ? parseAuthorRow(row) : null, error: null }; - } catch (error) { - return databaseError("getOwn", error); - } - } - async upsertOwn( userId: string, author: Author @@ -57,24 +52,47 @@ export class AuthorsDatabase { }; } - const result = await this.db - .prepare( - `INSERT INTO authors (id, type, name, url, bio, avatar_url, contact_email, owner_user_id, approved_at, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)` - ) - .bind( - author.id, - author.type, - author.name, - author.URL ?? null, - author.bio ?? null, - author.avatar_url ?? null, - author.contact_email ?? null, - userId - ) - .run(); + let result; + try { + result = await this.db + .prepare( + `INSERT INTO authors (id, type, name, url, bio, avatar_url, contact_email, owner_user_id, approved_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)` + ) + .bind( + author.id, + author.type, + author.name, + author.URL ?? null, + author.bio ?? null, + author.avatar_url ?? null, + author.contact_email ?? null, + userId + ) + .run(); + } catch (error) { + if (isOwnerConflict(error instanceof Error ? error.message : "")) { + return { + data: null, + error: { + message: "You already have a developer profile", + code: "CONFLICT" + } + }; + } + throw error; + } if (!result.success) { + if (isOwnerConflict(result.error)) { + return { + data: null, + error: { + message: "You already have a developer profile", + code: "CONFLICT" + } + }; + } return databaseError( "upsertOwn", new Error(result.error || "Database query failed") diff --git a/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql b/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql new file mode 100644 index 0000000..6557070 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql @@ -0,0 +1,9 @@ +-- v2: enforce one profile per owner at the database level. Without this, +-- two concurrent first-time PUT /authors/me requests with different ids +-- (same caller) could both pass the app-level "do I already have a +-- profile?" check and both insert, leaving the caller with two profiles. +-- NULL owner_user_id (pre-v2 rows) is exempt: SQLite never treats NULLs as +-- equal in a unique index, so legacy unowned authors can coexist freely. + +DROP INDEX IF EXISTS idx_authors_owner; +CREATE UNIQUE INDEX IF NOT EXISTS idx_authors_owner_unique ON authors(owner_user_id); diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index b5dc09c..b694649 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -43,6 +43,19 @@ export const AuthorSchema = z export type Author = z.infer; +// Submissions go through moderation and only ever touch identity fields — +// profile fields (bio/avatar_url/contact_email) are direct-write-only via +// PUT /authors/me, so this schema rejects them instead of silently +// accepting-then-dropping them when a submission is approved. +export const SubmissionAuthorSchema = AuthorSchema.pick({ + id: true, + type: true, + name: true, + URL: true +}) + .strict() + .openapi("SubmissionAuthor"); + export const ReleaseSchema = z .object({ tag: z.string().min(1), @@ -86,7 +99,7 @@ export const ExtensionPayloadSchema = z export const SubmissionPayloadSchema = z .object({ - author: AuthorSchema, + author: SubmissionAuthorSchema, extension: ExtensionPayloadSchema }) .openapi("SubmissionPayload"); diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 58f5eb0..7ded365 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -177,6 +177,19 @@ describe("Extensions API v2", () => { expect(data.error.code).toBe("VALIDATION_ERROR"); }); + it("rejects profile fields (bio/avatar_url/contact_email) on a submission's author", async () => { + seedAuthor("new-author", "user-1"); + const headers = await authHeaders("user-1"); + const payload = samplePayload(); + const res = await post("/extensions/v2/submissions", headers, { + ...payload, + author: { ...payload.author, bio: "Should not be accepted here" } + }); + + expect(res.status).toBe(422); + expect(tables.extension_submissions.size).toBe(0); + }); + it("creates a pending submission for a brand-new extension under an existing author", async () => { seedAuthor("new-author", "user-1"); const headers = await authHeaders("user-1"); @@ -551,6 +564,30 @@ describe("Extensions API v2", () => { expect(tables.authors.get("dev-author")?.name).toBe("Renamed Author"); }); + it("only lets one of two concurrent first-time profile creations by the same caller win", async () => { + const headers = await authHeaders("user-1"); + const [resA, resB] = await Promise.all([ + put( + "/extensions/v2/authors/me", + headers, + sampleAuthor({ id: "author-a" }) + ), + put( + "/extensions/v2/authors/me", + headers, + sampleAuthor({ id: "author-b" }) + ) + ]); + + const statuses = [resA.status, resB.status].sort(); + expect(statuses).toEqual([200, 409]); + + const ownedAuthors = [...tables.authors.values()].filter( + (a) => a.owner_user_id === "user-1" + ); + expect(ownedAuthors).toHaveLength(1); + }); + it("rejects an id that already belongs to someone else", async () => { await put( "/extensions/v2/authors/me", @@ -590,10 +627,15 @@ describe("Extensions API v2", () => { sampleAuthor() ); tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); - await post( + const approved = await post( "/extensions/v2/authors/dev-author/approve", await authHeaders("mod-1") ); + expect(approved.status).toBe(200); + const approvedBody = (await approved.json()) as { + result: { approved: boolean }; + }; + expect(approvedBody.result.approved).toBe(true); const res = await put( "/extensions/v2/authors/me", @@ -633,6 +675,40 @@ describe("Extensions API v2", () => { expect(stored?.contact_email).toBe("dev@example.com"); }); + it("updates bio, avatar_url, and contact_email on an existing profile", async () => { + const headers = await authHeaders("user-1"); + await put("/extensions/v2/authors/me", headers, { + ...sampleAuthor(), + bio: "Old bio.", + avatar_url: "https://example.com/old.png", + contact_email: "old@example.com" + }); + + const res = await put("/extensions/v2/authors/me", headers, { + ...sampleAuthor(), + bio: "New bio.", + avatar_url: "https://example.com/new.png", + contact_email: "new@example.com" + }); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { + bio: string; + avatar_url: string; + contact_email: string; + }; + }; + expect(data.result.bio).toBe("New bio."); + expect(data.result.avatar_url).toBe("https://example.com/new.png"); + expect(data.result.contact_email).toBe("new@example.com"); + + const stored = tables.authors.get("dev-author"); + expect(stored?.bio).toBe("New bio."); + expect(stored?.avatar_url).toBe("https://example.com/new.png"); + expect(stored?.contact_email).toBe("new@example.com"); + }); + it("accepts a payload without bio, avatar_url, or contact_email", async () => { const res = await put( "/extensions/v2/authors/me", diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index a2109ba..a5db629 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -145,6 +145,15 @@ class MockStatement implements D1PreparedStatement { contact_email, owner_user_id ] = p; + // Mirrors idx_authors_owner_unique: one profile per non-null owner. + const ownerTaken = [...this.tables.authors.values()].some( + (r) => owner_user_id !== null && r.owner_user_id === owner_user_id + ); + if (ownerTaken) { + throw new Error( + "D1_ERROR: UNIQUE constraint failed: authors.owner_user_id" + ); + } const now = new Date().toISOString(); this.tables.authors.set(String(id), { id, From ea38e43c3995c90bf5d9396fa7c8c9cb9df5aa2e Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 26 Jul 2026 10:37:58 +0100 Subject: [PATCH 5/5] Handle pre-existing duplicate owners in the unique-index migration The race that idx_authors_owner_unique closes predates this migration, so a database could already have duplicate owner_user_id rows, which would make CREATE UNIQUE INDEX fail and block every later migration. Detach ownership from all but the most recently created row per owner before creating the constraint. Claude-Session: https://claude.ai/code/session_01LGdm9SjGTpdz9Gj7GcLs13 --- .../migrations/0004_add_authors_owner_unique.sql | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql b/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql index 6557070..6bb62fe 100644 --- a/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql +++ b/src/services/extensions/v2/db/migrations/0004_add_authors_owner_unique.sql @@ -4,6 +4,21 @@ -- profile?" check and both insert, leaving the caller with two profiles. -- NULL owner_user_id (pre-v2 rows) is exempt: SQLite never treats NULLs as -- equal in a unique index, so legacy unowned authors can coexist freely. +-- +-- The race this closes predates this migration, so a database may already +-- have duplicate owner_user_id rows — CREATE UNIQUE INDEX would fail on +-- those and block every migration after it. Detach ownership (not delete) +-- from all but the most recently created row per owner first, so the +-- constraint can always be created; any detached profile becomes unowned, +-- same as a legacy pre-v2 row, and needs manual reconciliation. +UPDATE authors +SET owner_user_id = NULL +WHERE owner_user_id IS NOT NULL + AND rowid NOT IN ( + SELECT MAX(rowid) FROM authors + WHERE owner_user_id IS NOT NULL + GROUP BY owner_user_id + ); DROP INDEX IF EXISTS idx_authors_owner; CREATE UNIQUE INDEX IF NOT EXISTS idx_authors_owner_unique ON authors(owner_user_id);