From b964b98a8bb2125ac3890badb8237d0dbc0bd37a Mon Sep 17 00:00:00 2001 From: OpenClaw Date: Fri, 6 Mar 2026 20:06:04 +0100 Subject: [PATCH] fix(BUG-106): DB fallback for downgradeByCustomer and recover route - downgradeByCustomer now queries DB when key not in memory cache, preventing cancelled customers from keeping Pro access in multi-pod setups - recover/verify endpoint falls back to DB lookup when cache miss on email - Added TDD tests for both fallback paths (4 new tests) --- src/__tests__/keys-downgrade.test.ts | 75 ++++++++++++++ src/__tests__/recover-db-fallback.test.ts | 117 ++++++++++++++++++++++ src/routes/recover.ts | 22 +++- src/services/keys.ts | 28 +++++- 4 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/keys-downgrade.test.ts create mode 100644 src/__tests__/recover-db-fallback.test.ts diff --git a/src/__tests__/keys-downgrade.test.ts b/src/__tests__/keys-downgrade.test.ts new file mode 100644 index 0000000..3842323 --- /dev/null +++ b/src/__tests__/keys-downgrade.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Override the global setup.ts mock for keys — we need the REAL implementation +vi.unmock("../services/keys.js"); + +// Keep db mocked (setup.ts already does this, but be explicit about our mock) +vi.mock("../services/db.js", () => ({ + default: { query: vi.fn(), connect: vi.fn(), on: vi.fn(), end: vi.fn() }, + pool: { query: vi.fn(), connect: vi.fn(), on: vi.fn(), end: vi.fn() }, + queryWithRetry: vi.fn().mockResolvedValue({ rows: [], rowCount: 0 }), + connectWithRetry: vi.fn().mockResolvedValue(undefined), + initDatabase: vi.fn().mockResolvedValue(undefined), + cleanupStaleData: vi.fn(), + isTransientError: vi.fn(), +})); + +vi.mock("../services/logger.js", () => ({ + default: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, +})); + +import { queryWithRetry } from "../services/db.js"; +import { downgradeByCustomer } from "../services/keys.js"; + +const mockQuery = vi.mocked(queryWithRetry); + +describe("downgradeByCustomer DB fallback (BUG-106)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns true and updates DB when key is NOT in cache but IS in DB", async () => { + mockQuery + .mockResolvedValueOnce({ + rows: [ + { + key: "df_pro_abc123", + tier: "pro", + email: "user@example.com", + created_at: "2026-01-01T00:00:00.000Z", + stripe_customer_id: "cus_123", + }, + ], + rowCount: 1, + } as any) + .mockResolvedValueOnce({ rows: [], rowCount: 1 } as any); // UPDATE + + const result = await downgradeByCustomer("cus_123"); + + expect(result).toBe(true); + expect(mockQuery).toHaveBeenCalledWith( + expect.stringContaining("SELECT"), + expect.arrayContaining(["cus_123"]) + ); + expect(mockQuery).toHaveBeenCalledWith( + expect.stringContaining("UPDATE"), + expect.arrayContaining(["cus_123"]) + ); + }); + + it("returns false when key is NOT in cache AND NOT in DB", async () => { + mockQuery.mockResolvedValueOnce({ rows: [], rowCount: 0 } as any); + + const result = await downgradeByCustomer("cus_nonexistent"); + + expect(result).toBe(false); + expect(mockQuery).toHaveBeenCalledWith( + expect.stringContaining("SELECT"), + expect.arrayContaining(["cus_nonexistent"]) + ); + const updateCalls = mockQuery.mock.calls.filter((c) => + (c[0] as string).includes("UPDATE") + ); + expect(updateCalls).toHaveLength(0); + }); +}); diff --git a/src/__tests__/recover-db-fallback.test.ts b/src/__tests__/recover-db-fallback.test.ts new file mode 100644 index 0000000..2753d88 --- /dev/null +++ b/src/__tests__/recover-db-fallback.test.ts @@ -0,0 +1,117 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Unmock keys to use real getAllKeys (but we'll mock it ourselves) +vi.unmock("../services/keys.js"); +vi.unmock("../services/verification.js"); +vi.unmock("../services/email.js"); + +vi.mock("../services/db.js", () => ({ + default: { query: vi.fn(), connect: vi.fn(), on: vi.fn(), end: vi.fn() }, + pool: { query: vi.fn(), connect: vi.fn(), on: vi.fn(), end: vi.fn() }, + queryWithRetry: vi.fn().mockResolvedValue({ rows: [], rowCount: 0 }), + connectWithRetry: vi.fn().mockResolvedValue(undefined), + initDatabase: vi.fn().mockResolvedValue(undefined), + cleanupStaleData: vi.fn(), + isTransientError: vi.fn(), +})); + +vi.mock("../services/logger.js", () => ({ + default: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, +})); + +vi.mock("../services/verification.js", () => ({ + createPendingVerification: vi.fn(), + verifyCode: vi.fn(), +})); + +vi.mock("../services/email.js", () => ({ + sendVerificationEmail: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../services/keys.js", () => ({ + getAllKeys: vi.fn().mockReturnValue([]), + loadKeys: vi.fn().mockResolvedValue(undefined), + isValidKey: vi.fn(), + getKeyInfo: vi.fn(), + isProKey: vi.fn(), + createFreeKey: vi.fn(), + createProKey: vi.fn(), + downgradeByCustomer: vi.fn(), + findKeyByCustomerId: vi.fn(), + updateKeyEmail: vi.fn(), + updateEmailByCustomer: vi.fn(), +})); + +import { queryWithRetry } from "../services/db.js"; +import { getAllKeys } from "../services/keys.js"; +import { verifyCode } from "../services/verification.js"; +import express from "express"; +import request from "supertest"; +import { recoverRouter } from "../routes/recover.js"; + +const mockQuery = vi.mocked(queryWithRetry); +const mockGetAllKeys = vi.mocked(getAllKeys); +const mockVerifyCode = vi.mocked(verifyCode); + +function createApp() { + const app = express(); + app.use(express.json()); + app.use("/v1/recover", recoverRouter); + return app; +} + +describe("recover verify DB fallback", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("falls back to DB when cache has no matching email after verify succeeds", async () => { + mockVerifyCode.mockResolvedValueOnce({ status: "ok" } as any); + mockGetAllKeys.mockReturnValueOnce([]); // cache empty + + // DB fallback returns the key + mockQuery.mockResolvedValueOnce({ + rows: [ + { + key: "df_pro_xyz", + tier: "pro", + email: "user@test.com", + created_at: "2026-01-01T00:00:00.000Z", + stripe_customer_id: null, + }, + ], + rowCount: 1, + } as any); + + const app = createApp(); + const res = await request(app) + .post("/v1/recover/verify") + .send({ email: "user@test.com", code: "123456" }); + + expect(res.status).toBe(200); + expect(res.body.status).toBe("recovered"); + expect(res.body.apiKey).toBe("df_pro_xyz"); + expect(res.body.tier).toBe("pro"); + + expect(mockQuery).toHaveBeenCalledWith( + expect.stringContaining("SELECT"), + expect.arrayContaining(["user@test.com"]) + ); + }); + + it("returns 'no key found' when not in cache AND not in DB", async () => { + mockVerifyCode.mockResolvedValueOnce({ status: "ok" } as any); + mockGetAllKeys.mockReturnValueOnce([]); + mockQuery.mockResolvedValueOnce({ rows: [], rowCount: 0 } as any); + + const app = createApp(); + const res = await request(app) + .post("/v1/recover/verify") + .send({ email: "nobody@test.com", code: "123456" }); + + expect(res.status).toBe(200); + expect(res.body.status).toBe("recovered"); + expect(res.body.apiKey).toBeUndefined(); + expect(res.body.message).toContain("No API key found"); + }); +}); diff --git a/src/routes/recover.ts b/src/routes/recover.ts index 2b8ca7e..b1f4841 100644 --- a/src/routes/recover.ts +++ b/src/routes/recover.ts @@ -3,6 +3,7 @@ import rateLimit from "express-rate-limit"; import { createPendingVerification, verifyCode } from "../services/verification.js"; import { sendVerificationEmail } from "../services/email.js"; import { getAllKeys } from "../services/keys.js"; +import { queryWithRetry } from "../services/db.js"; import logger from "../services/logger.js"; const router = Router(); @@ -143,8 +144,27 @@ router.post("/verify", recoverLimiter, async (req: Request, res: Response) => { switch (result.status) { case "ok": { const keys = getAllKeys(); - const userKey = keys.find(k => k.email === cleanEmail); + let userKey = keys.find(k => k.email === cleanEmail); + // DB fallback: cache may be stale in multi-replica setups + if (!userKey) { + logger.info({ email: cleanEmail }, "recover verify: cache miss, falling back to DB"); + const dbResult = await queryWithRetry( + "SELECT key, tier, email, created_at, stripe_customer_id FROM api_keys WHERE email = $1 LIMIT 1", + [cleanEmail] + ); + if (dbResult.rows.length > 0) { + const row = dbResult.rows[0]; + userKey = { + key: row.key, + tier: row.tier as "free" | "pro", + email: row.email, + createdAt: row.created_at instanceof Date ? row.created_at.toISOString() : row.created_at, + stripeCustomerId: row.stripe_customer_id || undefined, + }; + } + } + if (userKey) { res.json({ status: "recovered", diff --git a/src/services/keys.ts b/src/services/keys.ts index f5356da..79cc841 100644 --- a/src/services/keys.ts +++ b/src/services/keys.ts @@ -134,7 +134,33 @@ export async function downgradeByCustomer(stripeCustomerId: string): Promise {