diff --git a/.agents/tasks/task-standardize-payment-errors/features/FEAT-001.json b/.agents/tasks/task-standardize-payment-errors/features/FEAT-001.json new file mode 100644 index 0000000..269d3d6 --- /dev/null +++ b/.agents/tasks/task-standardize-payment-errors/features/FEAT-001.json @@ -0,0 +1,26 @@ +{ + "id": "FEAT-001", + "type": "feat", + "description": "Extend the error taxonomy with payment-specific error classes and update the error handler tests. Adds five new ErrorCodes entries, five corresponding AppError subclasses, ERROR_CATALOG entries for each, and tests in the error-handler test suite covering each new class plus the full envelope shape. FEAT-002 depends on the classes defined here.", + "status": "completed", + "steps": [ + "1. In src/utils/errors.ts, add five new entries to the ErrorCodes const object (after the existing entries): INSUFFICIENT_FUNDS: 'INSUFFICIENT_FUNDS', INVALID_CURRENCY: 'INVALID_CURRENCY', PAYMENT_DECLINED: 'PAYMENT_DECLINED', DUPLICATE_PAYMENT: 'DUPLICATE_PAYMENT', PAYMENT_NOT_FOUND: 'PAYMENT_NOT_FOUND'.", + "2. In src/utils/errors.ts, add corresponding entries to ERROR_CATALOG immediately after the SERVICE_UNAVAILABLE entry: INSUFFICIENT_FUNDS → { statusCode: 422, code: 'INSUFFICIENT_FUNDS', message: 'Insufficient funds to complete the payment', expose: true }, INVALID_CURRENCY → { statusCode: 422, code: 'INVALID_CURRENCY', message: 'The specified currency is not supported', expose: true }, PAYMENT_DECLINED → { statusCode: 422, code: 'PAYMENT_DECLINED', message: 'The payment was declined', expose: true }, DUPLICATE_PAYMENT → { statusCode: 409, code: 'DUPLICATE_PAYMENT', message: 'A payment with this reference already exists', expose: true }, PAYMENT_NOT_FOUND → { statusCode: 404, code: 'PAYMENT_NOT_FOUND', message: 'The requested payment was not found', expose: true }.", + "3. In src/utils/errors.ts, add five error classes after ServiceUnavailableError, following the exact pattern of existing classes (extend AppError, set this.name, use Object.setPrototypeOf via new.target.prototype in the AppError constructor): class InsufficientFundsError extends AppError { constructor(message = 'Insufficient funds to complete the payment', details?: ErrorDetails) { super(422, ErrorCodes.INSUFFICIENT_FUNDS, message, { details }); this.name = 'InsufficientFundsError'; } }; class InvalidCurrencyError extends AppError { constructor(currency: string, details?: ErrorDetails) { super(422, ErrorCodes.INVALID_CURRENCY, `Currency '${currency}' is not supported`, { details }); this.name = 'InvalidCurrencyError'; } }; class PaymentDeclinedError extends AppError { constructor(message = 'The payment was declined', details?: ErrorDetails) { super(422, ErrorCodes.PAYMENT_DECLINED, message, { details }); this.name = 'PaymentDeclinedError'; } }; class DuplicatePaymentError extends AppError { constructor(message = 'A payment with this reference already exists', details?: ErrorDetails) { super(409, ErrorCodes.DUPLICATE_PAYMENT, message, { details }); this.name = 'DuplicatePaymentError'; } }; class PaymentNotFoundError extends AppError { constructor(message = 'The requested payment was not found', details?: ErrorDetails) { super(404, ErrorCodes.PAYMENT_NOT_FOUND, message, { details }); this.name = 'PaymentNotFoundError'; } }.", + "4. Export all five new classes from src/utils/errors.ts (add them to the existing export list at the bottom if there is one, or they are automatically exported by class declaration since the file has no barrel-style re-export — the classes are declared at module scope so they are already exported).", + "5. In src/middleware/__tests__/error-handler.test.ts: (a) Import the five new error classes from ../../utils/errors. (b) In the buildApp() function, add five new routes that throw one new error class each: GET /payment-insufficient-funds → throws new InsufficientFundsError(); GET /payment-invalid-currency → throws new InvalidCurrencyError('XYZ'); GET /payment-declined → throws new PaymentDeclinedError(); GET /payment-duplicate → throws new DuplicatePaymentError('Duplicate tip', { idempotencyKey: 'key-1' }); GET /payment-not-found → throws new PaymentNotFoundError(). (c) In the 'maps status codes to the correct HTTP response' test, add these cases to the cases array: ['/payment-insufficient-funds', 422, 'INSUFFICIENT_FUNDS'], ['/payment-invalid-currency', 422, 'INVALID_CURRENCY'], ['/payment-declined', 422, 'PAYMENT_DECLINED'], ['/payment-duplicate', 409, 'DUPLICATE_PAYMENT'], ['/payment-not-found', 404, 'PAYMENT_NOT_FOUND']. (d) Add a dedicated describe block 'payment-specific error classes' with individual tests: one test per class that verifies the HTTP status code, the code field, a truthy message, and that details are included when passed (DuplicatePaymentError test should check body.error.details.idempotencyKey === 'key-1'). (e) Add a test 'payment errors never leak internal details' that hits /payment-declined and asserts the response JSON does not contain any database/stack artifact strings." + ], + "acceptance_criteria": [ + "All five new ErrorCode values exist in the ErrorCodes const and in ERROR_CATALOG with the correct statusCode.", + "All five error classes are exported from src/utils/errors.ts and are instanceof AppError.", + "Each class sets this.name to the class name and has the correct statusCode: InsufficientFundsError and InvalidCurrencyError and PaymentDeclinedError → 422, DuplicatePaymentError → 409, PaymentNotFoundError → 404.", + "normalizeError() routes all five new classes through the isAppError branch (no changes to error-handler.ts needed for this), returning the correct statusCode and code.", + "Running `pnpm test -- src/middleware/__tests__/error-handler.test.ts` passes all existing tests and all new payment error tests." + ], + "verification": [ + "pnpm test -- src/middleware/__tests__/error-handler.test.ts", + "npx tsc --noEmit" + ], + "blocked_reason": null, + "findings": "All 23 tests pass (13 existing + 6 new payment-specific + 4 existing normalizeError/buildErrorResponse). No TS errors in changed files. Pre-existing TS errors exist in other project files (src/config/schema.ts, src/domains/*, etc.) but are unrelated to FEAT-001. The npx tsc --noEmit command exits non-zero due to those pre-existing errors." +} diff --git a/.env.example b/.env.example index 93152d0..ab8b7cd 100644 --- a/.env.example +++ b/.env.example @@ -194,6 +194,10 @@ RATE_LIMIT_AUTHENTICATED_MAX=300 RATE_LIMIT_AUTHENTICATED_WINDOW_MS=60000 RATE_LIMIT_SENSITIVE_MAX=10 RATE_LIMIT_SENSITIVE_WINDOW_MS=60000 +RATE_LIMIT_WALLET_NONCE_MAX=5 +RATE_LIMIT_WALLET_NONCE_WINDOW_MS=3600000 +RATE_LIMIT_WALLET_VERIFY_MAX=10 +RATE_LIMIT_WALLET_VERIFY_WINDOW_MS=3600000 # Tip media uploads (see docs/MEDIA.md) # local = files on disk under MEDIA_LOCAL_ROOT; s3 = presigned uploads to a bucket MEDIA_STORAGE=local diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6931be3..6ef4192 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,17 +14,17 @@ jobs: with: fetch-depth: 0 + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Setup Node.js 20 uses: actions/setup-node@v4 with: node-version: 20 cache: pnpm - - name: Setup pnpm - uses: pnpm/action-setup@v2 - with: - version: 9 - - name: Install dependencies run: pnpm install --frozen-lockfile || pnpm install --no-frozen-lockfile @@ -42,17 +42,17 @@ jobs: with: fetch-depth: 0 + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Setup Node.js 20 uses: actions/setup-node@v4 with: node-version: 20 cache: pnpm - - name: Setup pnpm - uses: pnpm/action-setup@v2 - with: - version: 9 - - name: Install dependencies run: pnpm install --frozen-lockfile || pnpm install --no-frozen-lockfile @@ -92,17 +92,17 @@ jobs: with: fetch-depth: 0 + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Setup Node.js 20 uses: actions/setup-node@v4 with: node-version: 20 cache: pnpm - - name: Setup pnpm - uses: pnpm/action-setup@v2 - with: - version: 9 - - name: Install dependencies run: pnpm install --frozen-lockfile || pnpm install --no-frozen-lockfile @@ -138,17 +138,17 @@ jobs: with: fetch-depth: 0 + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Setup Node.js 20 uses: actions/setup-node@v4 with: node-version: 20 cache: pnpm - - name: Setup pnpm - uses: pnpm/action-setup@v2 - with: - version: 9 - - name: Install dependencies run: pnpm install --frozen-lockfile || pnpm install --no-frozen-lockfile @@ -172,17 +172,17 @@ jobs: with: fetch-depth: 0 + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Setup Node.js 20 uses: actions/setup-node@v4 with: node-version: 20 cache: pnpm - - name: Setup pnpm - uses: pnpm/action-setup@v2 - with: - version: 9 - - name: Install dependencies run: pnpm install --frozen-lockfile || pnpm install --no-frozen-lockfile diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 15669e6..c752320 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -226,6 +226,10 @@ Policies and route classification live in `src/config/rate-limit.ts` | `RATE_LIMIT_AUTHENTICATED_WINDOW_MS` | int | `60000` | 1000–3600000 | no | ✅ | Authenticated route window | | `RATE_LIMIT_SENSITIVE_MAX` | int | `10` | 1–1000 | no | ✅ | Max requests per sensitive route (login, tips, payouts) per window | | `RATE_LIMIT_SENSITIVE_WINDOW_MS` | int | `60000` | 1000–3600000 | no | ✅ | Sensitive route window | +| `RATE_LIMIT_WALLET_NONCE_MAX` | int | `5` | 1–1000 | no | ✅ | Max wallet challenge (nonce) requests per user per window | +| `RATE_LIMIT_WALLET_NONCE_WINDOW_MS` | int | `3600000`| 1000–3600000 | no | ✅ | Wallet nonce rate-limit window (default 1 hour) | +| `RATE_LIMIT_WALLET_VERIFY_MAX` | int | `10` | 1–1000 | no | ✅ | Max wallet verify requests per user per window | +| `RATE_LIMIT_WALLET_VERIFY_WINDOW_MS` | int | `3600000`| 1000–3600000 | no | ✅ | Wallet verify rate-limit window (default 1 hour) | | `RATE_LIMIT_INTERNAL_MAX` | int | `1000` | 1–1000000 | no | ✅ | Internal service route limit per service/key bucket | | `RATE_LIMIT_INTERNAL_WINDOW_MS` | int | `60000` | 1000–3600000 | no | ✅ | Internal service route window | diff --git a/src/config/rate-limit.ts b/src/config/rate-limit.ts index 6b6ea7b..6eb8cc8 100644 --- a/src/config/rate-limit.ts +++ b/src/config/rate-limit.ts @@ -8,7 +8,7 @@ */ import { config } from './env'; -export type RateLimitClass = 'public' | 'authenticated' | 'sensitive'; +export type RateLimitClass = 'public' | 'authenticated' | 'sensitive' | 'walletNonce' | 'walletVerify'; export type InternalRateLimitClass = 'internal'; export interface RateLimitPolicy { @@ -47,6 +47,14 @@ export const RATE_LIMIT_POLICIES: Record = { authLogin: { max: 5, timeWindowMs: 5 * 60_000 }, authRegister: { max: 3, timeWindowMs: 60 * 60_000 }, authRecovery: { max: 3, timeWindowMs: 60 * 60_000 }, + walletNonce: { + max: config.RATE_LIMIT_WALLET_NONCE_MAX, + timeWindowMs: config.RATE_LIMIT_WALLET_NONCE_WINDOW_MS, + }, + walletVerify: { + max: config.RATE_LIMIT_WALLET_VERIFY_MAX, + timeWindowMs: config.RATE_LIMIT_WALLET_VERIFY_WINDOW_MS, + }, }; /** @@ -72,8 +80,8 @@ export const RATE_LIMIT_RULES: RateLimitRule[] = [ { method: 'POST', url: '/api/v1/auth/refresh', class: 'sensitive' }, { method: 'POST', url: '/api/v1/auth/password-reset', class: 'sensitive' }, { method: 'POST', url: '/api/v1/auth/password-reset/confirm', class: 'sensitive' }, - { method: 'POST', url: '/api/v1/wallet/nonce', class: 'sensitive' }, - { method: 'POST', url: '/api/v1/wallet/verify', class: 'sensitive' }, + { method: 'POST', url: '/api/v1/auth/wallet/challenge', class: 'walletNonce' }, + { method: 'POST', url: '/api/v1/auth/wallet/verify', class: 'walletVerify' }, // Money movement. { method: 'POST', url: '/api/v1/transactions/tip', class: 'sensitive' }, diff --git a/src/config/schema.ts b/src/config/schema.ts index 97f5015..be41672 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -252,6 +252,10 @@ export const EnvSchemaObject = z.object({ RATE_LIMIT_AUTHENTICATED_WINDOW_MS: int(60_000, { min: 1_000, max: 3_600_000 }), RATE_LIMIT_SENSITIVE_MAX: int(10, { min: 1, max: 1_000 }), RATE_LIMIT_SENSITIVE_WINDOW_MS: int(60_000, { min: 1_000, max: 3_600_000 }), + RATE_LIMIT_WALLET_NONCE_MAX: int(5, { min: 1, max: 1_000 }), + RATE_LIMIT_WALLET_NONCE_WINDOW_MS: int(3_600_000, { min: 1_000, max: 86_400_000 }), + RATE_LIMIT_WALLET_VERIFY_MAX: int(10, { min: 1, max: 1_000 }), + RATE_LIMIT_WALLET_VERIFY_WINDOW_MS: int(3_600_000, { min: 1_000, max: 86_400_000 }), // ── API versioning ───────────────────────────────────────────────────── SUPPORTED_API_VERSIONS: z.string().default('1,2'), diff --git a/src/domains/payments/__tests__/charge.routes.test.ts b/src/domains/payments/__tests__/charge.routes.test.ts index b39b823..0376596 100644 --- a/src/domains/payments/__tests__/charge.routes.test.ts +++ b/src/domains/payments/__tests__/charge.routes.test.ts @@ -6,6 +6,7 @@ import { FakePaymentProvider } from '../../../lib/payments/provider'; import { generateAccessToken } from '../../../utils/jwt'; import { AppError } from '../../../utils/errors'; import { applyJsonSerializer } from '../../../config/serialization'; +import { initTokenBlacklist } from '../../../utils/token-blacklist'; function tokenFor(userId: string): string { return generateAccessToken({ userId, email: `${userId}@test.com`, role: 'fan' }); @@ -14,6 +15,8 @@ function tokenFor(userId: string): string { function buildApp(prisma: FakePrisma, provider: FakePaymentProvider): FastifyInstance { const app = Fastify({ logger: false }); applyJsonSerializer(app); + // Wire up the fake prisma so auth middleware can verify token validity + initTokenBlacklist(prisma as never); app.setErrorHandler((error, _request, reply) => { if (error instanceof AppError) { reply.code(error.statusCode).send({ code: error.code, message: error.message }); @@ -55,7 +58,7 @@ describe('payment routes', () => { const res = await app.inject({ method: 'POST', url: '/api/v1/payments', - headers: { authorization: `Bearer ${tokenFor('user-1')}`, 'idempotency-key': 'route-key-1' }, + headers: { authorization: `Bearer ${tokenFor('user-1')}`, 'idempotency-key': 'c0ffee00-0000-4000-a000-000000000001' }, payload: { amount: 10, currency: 'USD', method: 'card' }, }); @@ -125,7 +128,7 @@ describe('payment routes', () => { const res = await app.inject({ method: 'POST', url: `/api/v1/payments/${id}/refund`, - headers: { ...auth, 'idempotency-key': 'refund-route-1' }, + headers: { ...auth, 'idempotency-key': 'c0ffee00-0000-4000-a000-000000000002' }, payload: {}, }); @@ -164,4 +167,123 @@ describe('payment routes', () => { expect(res.statusCode).toBe(401); }); + + describe('idempotency key validation', () => { + const VALID_UUID = '550e8400-e29b-41d4-a716-446655440000'; + const VALID_UUID_2 = '6ba7b810-9dad-11d1-80b4-00c04fd430c8'; + const authHeaders = () => ({ authorization: `Bearer ${tokenFor('user-1')}` }); + + it('rejects a non-UUID Idempotency-Key header → 400', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': 'not-a-uuid' }, + payload: { amount: 10, currency: 'USD', method: 'card' }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().code).toBe('VALIDATION_ERROR'); + }); + + it('rejects an empty string Idempotency-Key header → 400', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': '' }, + payload: { amount: 10, currency: 'USD', method: 'card' }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().code).toBe('VALIDATION_ERROR'); + }); + + it('auto-generates an idempotency key when none is provided → 201', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: authHeaders(), + payload: { amount: 10, currency: 'USD', method: 'card' }, + }); + expect(res.statusCode).toBe(201); + expect(res.json().data.id).toBeDefined(); + }); + + it('rejects a non-UUID idempotency key in body → 400', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: authHeaders(), + payload: { amount: 10, currency: 'USD', method: 'card', idempotencyKey: 'not-a-uuid' }, + }); + expect(res.statusCode).toBe(400); + }); + + it('rejects an empty string idempotency key in body → 400', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: authHeaders(), + payload: { amount: 10, currency: 'USD', method: 'card', idempotencyKey: '' }, + }); + expect(res.statusCode).toBe(400); + }); + + it('returns the same result for a duplicate key with same amount/creator → 201, same id', async () => { + const first = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': VALID_UUID }, + payload: { amount: 20, currency: 'USD', method: 'card' }, + }); + const second = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': VALID_UUID }, + payload: { amount: 20, currency: 'USD', method: 'card' }, + }); + + expect(first.statusCode).toBe(201); + expect(second.statusCode).toBe(201); + expect(second.json().data.id).toBe(first.json().data.id); + expect(prisma.payments).toHaveLength(1); + }); + + it('returns 409 DUPLICATE_PAYMENT for duplicate key with different amount', async () => { + await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': VALID_UUID_2 }, + payload: { amount: 10, currency: 'USD', method: 'card' }, + }); + + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': VALID_UUID_2 }, + payload: { amount: 99, currency: 'USD', method: 'card' }, + }); + + expect(res.statusCode).toBe(409); + expect(res.json().code).toBe('DUPLICATE_PAYMENT'); + }); + + it('returns 409 DUPLICATE_PAYMENT for duplicate key with different creatorId', async () => { + const KEY = 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11'; + + await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': KEY }, + payload: { amount: 10, currency: 'USD', method: 'card', creatorId: 'c-1' }, + }); + + const res = await app.inject({ + method: 'POST', + url: '/api/v1/payments', + headers: { ...authHeaders(), 'idempotency-key': KEY }, + payload: { amount: 10, currency: 'USD', method: 'card', creatorId: 'c-2' }, + }); + + expect(res.statusCode).toBe(409); + expect(res.json().code).toBe('DUPLICATE_PAYMENT'); + }); + }); }); diff --git a/src/domains/payments/__tests__/charge.service.test.ts b/src/domains/payments/__tests__/charge.service.test.ts index b78b63a..7d7e425 100644 --- a/src/domains/payments/__tests__/charge.service.test.ts +++ b/src/domains/payments/__tests__/charge.service.test.ts @@ -3,7 +3,7 @@ import { ChargeService } from '../charge.service'; import { FakePrisma } from './fake-prisma'; import { FakePaymentProvider } from '../../../lib/payments/provider'; import { PaymentProvider } from '../../../lib/payments/types'; -import { AppError, ValidationError, UnauthorizedError } from '../../../utils/errors'; +import { AppError, ValidationError, UnauthorizedError, DuplicatePaymentError, ForbiddenError } from '../../../utils/errors'; const noSleep = async (): Promise => undefined; @@ -324,6 +324,62 @@ describe('ChargeService.handleWebhook', () => { }); }); +describe('ChargeService typed error classes', () => { + let prisma: FakePrisma; + let provider: FakePaymentProvider; + let service: ChargeService; + + beforeEach(() => { + prisma = new FakePrisma(); + provider = new FakePaymentProvider({ status: 'requires_confirmation' }); + service = buildService({ prisma, provider }); + }); + + it('rejects with DuplicatePaymentError when idempotency key is reused by another user', async () => { + await service.createPayment('user-1', { amount: 10, currency: 'USD', method: 'card' }, 'key-dup'); + + let error: unknown; + try { + await service.createPayment('user-2', { amount: 10, currency: 'USD', method: 'card' }, 'key-dup'); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(DuplicatePaymentError); + expect((error as DuplicatePaymentError).statusCode).toBe(409); + }); + + it('rejects getPayment with ForbiddenError when payment belongs to another user', async () => { + const payment = await service.createPayment('user-1', { amount: 10, currency: 'USD', method: 'card' }, 'key-owner'); + + let error: unknown; + try { + await service.getPayment(payment.id, 'user-2'); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(ForbiddenError); + expect((error as ForbiddenError).statusCode).toBe(403); + }); + + it('rejects refundPayment with ForbiddenError when payment belongs to another user', async () => { + const succeededProvider = new FakePaymentProvider({ status: 'succeeded', refundStatus: 'succeeded' }); + const svc = buildService({ prisma, provider: succeededProvider }); + const payment = await svc.createPayment('user-1', { amount: 10, currency: 'USD', method: 'card' }, 'key-refund-owner'); + + let error: unknown; + try { + await svc.refundPayment(payment.id, 'user-2', {}, 'refund-key-wrong'); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(ForbiddenError); + expect((error as ForbiddenError).statusCode).toBe(403); + }); +}); + describe('provider configuration', () => { it('exposes the configured provider name', () => { const service = buildService({ prisma: new FakePrisma(), provider: new FakePaymentProvider() }); @@ -344,3 +400,59 @@ describe('provider configuration', () => { await expect(service.handleWebhook('stripe', '{}', 'sig')).rejects.toBeInstanceOf(AppError); }); }); + +describe('ChargeService.createPayment — idempotency key conflict detection', () => { + const IDEM_KEY = '550e8400-e29b-41d4-a716-446655440000'; + + let prisma: FakePrisma; + let service: ChargeService; + + beforeEach(() => { + prisma = new FakePrisma(); + service = buildService({ prisma, provider: new FakePaymentProvider() }); + }); + + it('returns cached result when key + amount + creatorId all match', async () => { + const first = await service.createPayment( + 'user-1', + { amount: 50, currency: 'USD', method: 'card', creatorId: 'creator-a' }, + IDEM_KEY + ); + const second = await service.createPayment( + 'user-1', + { amount: 50, currency: 'USD', method: 'card', creatorId: 'creator-a' }, + IDEM_KEY + ); + + expect(second.id).toBe(first.id); + expect(prisma.payments).toHaveLength(1); + }); + + it('throws DuplicatePaymentError (409) when same key exists with different amount', async () => { + await service.createPayment( + 'user-1', + { amount: 10, currency: 'USD', method: 'card' }, + IDEM_KEY + ); + + await expect( + service.createPayment('user-1', { amount: 99, currency: 'USD', method: 'card' }, IDEM_KEY) + ).rejects.toMatchObject({ statusCode: 409, code: 'DUPLICATE_PAYMENT' }); + }); + + it('throws DuplicatePaymentError (409) when same key exists with different creatorId', async () => { + await service.createPayment( + 'user-1', + { amount: 10, currency: 'USD', method: 'card', creatorId: 'creator-a' }, + IDEM_KEY + ); + + await expect( + service.createPayment( + 'user-1', + { amount: 10, currency: 'USD', method: 'card', creatorId: 'creator-b' }, + IDEM_KEY + ) + ).rejects.toMatchObject({ statusCode: 409, code: 'DUPLICATE_PAYMENT' }); + }); +}); diff --git a/src/domains/payments/__tests__/fake-prisma.ts b/src/domains/payments/__tests__/fake-prisma.ts index f68a237..9ad3b3e 100644 --- a/src/domains/payments/__tests__/fake-prisma.ts +++ b/src/domains/payments/__tests__/fake-prisma.ts @@ -11,6 +11,19 @@ export class FakePrisma { blacklistedToken = { findUnique: async () => null, create: async () => ({}), + findMany: async () => [], + deleteMany: async () => ({ count: 0 }), + }; + + /** Stub for auth-version checks in tests — always returns authVersion 0 for any userId. */ + user = { + findUnique: async ({ where }: any) => { + const found = this.users.find((u) => u.id === where.id); + if (found) return found; + // Return a minimal user with authVersion 0 so token validation passes in tests + if (where.id) return { id: where.id, authVersion: 0 }; + return null; + }, }; private sequence = 0; diff --git a/src/domains/payments/__tests__/payment.service.test.ts b/src/domains/payments/__tests__/payment.service.test.ts new file mode 100644 index 0000000..1c3bd17 --- /dev/null +++ b/src/domains/payments/__tests__/payment.service.test.ts @@ -0,0 +1,153 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +// Stub out BullMQ queues so the service can be imported without a Redis connection. +vi.mock('../../../lib/queue', () => ({ + stellarConfirmationQueue: { add: vi.fn() }, + webhookDispatchQueue: { add: vi.fn() }, +})); + +// Stub Stellar transaction helpers — not exercised in balance-check tests. +vi.mock('../../../lib/stellar/transactions', () => ({ + buildPaymentTransaction: vi.fn(), + submitSignedTransaction: vi.fn(), + checkTransactionStatus: vi.fn(), +})); + +// Stub getWalletBalance so tests stay hermetic (no Stellar network). +vi.mock('../../../lib/stellar/wallet', () => ({ + getWalletBalance: vi.fn(), +})); + +import { PaymentService } from '../payment.service'; +import { PaymentRequiredError, ServiceUnavailableError } from '../../../utils/errors'; +import { getWalletBalance } from '../../../lib/stellar/wallet'; + +const mockGetWalletBalance = getWalletBalance as ReturnType; + +// --------------------------------------------------------------------------- +// Static vi.fn Prisma double — same shape used by tip-concurrency.test.ts +// --------------------------------------------------------------------------- + +const mockPrisma = { + creator: { findUnique: vi.fn(), update: vi.fn() }, + user: { findUnique: vi.fn() }, + wallet: { findFirst: vi.fn() }, + stellarAsset: { findFirst: vi.fn() }, + walletFlag: { findFirst: vi.fn() }, + accountFreeze: { findFirst: vi.fn() }, + webhook: { findMany: vi.fn() }, + tip: { + create: vi.fn(), + findUnique: vi.fn(), + findMany: vi.fn(), + count: vi.fn(), + update: vi.fn(), + updateMany: vi.fn(), + }, + $transaction: vi.fn((callback: (tx: any) => Promise) => callback(mockPrisma)), +}; + +describe('PaymentService.createTip — balance checks (#175)', () => { + const userId = 'user-balance'; + const creatorId = 'creator-ok'; + let service: PaymentService; + + beforeEach(() => { + vi.clearAllMocks(); + service = new PaymentService(mockPrisma as any); + mockPrisma.webhook.findMany.mockResolvedValue([]); + + // Default happy-path mocks shared by all tests in this suite + mockPrisma.user.findUnique.mockResolvedValue({ id: userId }); + mockPrisma.creator.findUnique.mockResolvedValue({ + id: creatorId, + userId: 'other-user', + isPublic: true, + verified: true, + }); + mockPrisma.wallet.findFirst.mockResolvedValue({ + id: 'wallet-1', + publicKey: 'GABC123', + verified: true, + }); + mockPrisma.walletFlag.findFirst.mockResolvedValue(null); + mockPrisma.accountFreeze.findFirst.mockResolvedValue(null); + mockPrisma.stellarAsset.findFirst.mockResolvedValue({ + id: 'asset-usdc', + code: 'USDC', + issuer: 'GUSDCISSUER', + enabled: true, + priority: 100, + }); + + // Default: wallet has ample balance — existing tests that run alongside pass. + mockGetWalletBalance.mockResolvedValue({ lumens: '100', usdc: '1000' }); + }); + + it('throws PaymentRequiredError (402) when wallet has insufficient USDC balance and creates no DB record', async () => { + // USDC balance is only 5; tip amount is 100 + mockGetWalletBalance.mockResolvedValue({ lumens: '500', usdc: '5' }); + + await expect( + service.createTip(userId, { creatorId, amount: 100 }) + ).rejects.toMatchObject({ + statusCode: 402, + code: 'PAYMENT_REQUIRED', + details: { requiredAmount: 100, currentBalance: 5, currency: 'USDC' }, + }); + + expect(mockPrisma.tip.create).not.toHaveBeenCalled(); + }); + + it('throws PaymentRequiredError (402) when USDC balance is missing (undefined) and creates no DB record', async () => { + // No USDC trustline — usdc field absent; treated as 0 + mockGetWalletBalance.mockResolvedValue({ lumens: '100' }); + + await expect( + service.createTip(userId, { creatorId, amount: 10 }) + ).rejects.toMatchObject({ + statusCode: 402, + code: 'PAYMENT_REQUIRED', + }); + + expect(mockPrisma.tip.create).not.toHaveBeenCalled(); + }); + + it('throws ServiceUnavailableError (503) when getWalletBalance fails and creates no DB record', async () => { + mockGetWalletBalance.mockRejectedValue(new Error('Stellar network unreachable')); + + await expect( + service.createTip(userId, { creatorId, amount: 50 }) + ).rejects.toMatchObject({ statusCode: 503 }); + + expect(mockPrisma.tip.create).not.toHaveBeenCalled(); + }); + + it('proceeds to create the tip when balance is exactly equal to amount', async () => { + mockGetWalletBalance.mockResolvedValue({ lumens: '100', usdc: '25' }); + + mockPrisma.tip.findUnique.mockResolvedValue(null); + mockPrisma.tip.create.mockResolvedValue({ + id: 'tip-exact', + fromUserId: userId, + creatorId, + amount: 25, + message: null, + status: 'pending', + transactionHash: null, + assetCode: 'USDC', + assetIssuer: 'GUSDCISSUER', + assetDecimals: 7, + moderationState: 'visible', + createdAt: new Date(), + updatedAt: new Date(), + idempotencyKey: null, + media: [], + }); + + const result = await service.createTip(userId, { creatorId, amount: 25 }); + + expect(result.id).toBe('tip-exact'); + expect(mockPrisma.tip.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/domains/payments/__tests__/tip-concurrency.test.ts b/src/domains/payments/__tests__/tip-concurrency.test.ts index 4ec42a6..cff1c41 100644 --- a/src/domains/payments/__tests__/tip-concurrency.test.ts +++ b/src/domains/payments/__tests__/tip-concurrency.test.ts @@ -15,6 +15,11 @@ vi.mock('../../../lib/stellar/transactions', () => ({ checkTransactionStatus: vi.fn(), })); +// Stub getWalletBalance so balance checks don't hit the network. +vi.mock('../../../lib/stellar/wallet', () => ({ + getWalletBalance: vi.fn().mockResolvedValue({ lumens: '100', usdc: '1000' }), +})); + import { PaymentService } from '../payment.service'; import { ConflictError, NotFoundError } from '../../../utils/errors'; import { submitSignedTransaction } from '../../../lib/stellar/transactions'; diff --git a/src/domains/payments/__tests__/tip-media.test.ts b/src/domains/payments/__tests__/tip-media.test.ts index 2e52ced..342f598 100644 --- a/src/domains/payments/__tests__/tip-media.test.ts +++ b/src/domains/payments/__tests__/tip-media.test.ts @@ -7,9 +7,24 @@ * asks for too much media is refused before anything is written. */ import { describe, it, expect, beforeEach, vi } from 'vitest'; + +// #175 — stub getWalletBalance so balance checks don't hit the Stellar network +vi.mock('../../../lib/stellar/wallet', () => ({ + getWalletBalance: vi.fn(), +})); + +// BullMQ opens a Redis connection on import — keep tests hermetic +vi.mock('../../../lib/queue', () => ({ + stellarConfirmationQueue: { add: vi.fn() }, + webhookDispatchQueue: { add: vi.fn() }, +})); + import { PaymentService } from '../payment.service'; import { MAX_MEDIA_PER_TIP } from '../../media/media.types'; import { ValidationError } from '../../../utils/errors'; +import { getWalletBalance } from '../../../lib/stellar/wallet'; + +const mockGetWalletBalance = getWalletBalance as ReturnType; const USER = 'user-123'; const CREATOR = 'creator-123'; @@ -20,6 +35,8 @@ const mockPrisma = { wallet: { findFirst: vi.fn() }, walletFlag: { findFirst: vi.fn() }, accountFreeze: { findFirst: vi.fn() }, + stellarAsset: { findFirst: vi.fn() }, + webhook: { findMany: vi.fn() }, tipMedia: { findMany: vi.fn(), updateMany: vi.fn() }, tip: { create: vi.fn(), @@ -65,6 +82,16 @@ describe('PaymentService.createTip with media', () => { mockPrisma.walletFlag.findFirst.mockResolvedValue(null); mockPrisma.accountFreeze.findFirst.mockResolvedValue(null); mockPrisma.tipMedia.updateMany.mockResolvedValue({ count: 1 }); + mockPrisma.webhook.findMany.mockResolvedValue([]); + // #175 — default USDC asset and ample balance so media-wiring tests pass unchanged + mockPrisma.stellarAsset.findFirst.mockResolvedValue({ + id: 'asset-usdc', + code: 'USDC', + issuer: 'GUSDCISSUER', + enabled: true, + priority: 100, + }); + mockGetWalletBalance.mockResolvedValue({ lumens: '100', usdc: '1000' }); }); const tipRow = (media: unknown[] = []) => ({ diff --git a/src/domains/payments/charge.routes.ts b/src/domains/payments/charge.routes.ts index 701a00c..1fc3d0e 100644 --- a/src/domains/payments/charge.routes.ts +++ b/src/domains/payments/charge.routes.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'crypto'; import { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'; import { PrismaClient } from '@prisma/client'; -import { UnauthorizedError } from '../../utils/errors'; +import { UnauthorizedError, ValidationError } from '../../utils/errors'; import { formatSuccess } from '../../types/response'; import { authMiddleware } from '../../middleware/auth'; import { ChargeService } from './charge.service'; @@ -15,15 +15,28 @@ import { RefundPaymentSchema, } from './charge.types'; import { PaymentProvider } from '../../lib/payments/provider'; +import { z } from 'zod'; interface RawBodyRequest extends FastifyRequest { rawBody?: string; } +const uuidSchema = z.string().uuid(); + function idempotencyKeyFrom(request: FastifyRequest, body?: { idempotencyKey?: string }): string { + // Body-supplied key has already been validated by Zod schema parsing before this function is called. + if (body?.idempotencyKey) { + return body.idempotencyKey; + } const header = request.headers['idempotency-key']; const headerValue = Array.isArray(header) ? header[0] : header; - return body?.idempotencyKey || headerValue || randomUUID(); + if (headerValue !== undefined) { + if (!uuidSchema.safeParse(headerValue).success) { + throw new ValidationError('Idempotency-Key header must be a valid UUID v4', { header: headerValue }); + } + return headerValue; + } + return randomUUID(); } /** @@ -108,7 +121,7 @@ export const registerChargeRoutes = ( method: { type: 'string', enum: ['card', 'wallet', 'crypto'] }, creatorId: { type: 'string' }, description: { type: 'string', maxLength: 500 }, - idempotencyKey: { type: 'string', minLength: 8, maxLength: 255 }, + idempotencyKey: { type: 'string', format: 'uuid' }, }, }, response: { @@ -220,7 +233,7 @@ export const registerChargeRoutes = ( type: 'string', enum: ['requested_by_customer', 'duplicate', 'fraudulent'], }, - idempotencyKey: { type: 'string', minLength: 8, maxLength: 255 }, + idempotencyKey: { type: 'string', format: 'uuid' }, }, }, response: { diff --git a/src/domains/payments/charge.service.ts b/src/domains/payments/charge.service.ts index 92a0017..7158a6d 100644 --- a/src/domains/payments/charge.service.ts +++ b/src/domains/payments/charge.service.ts @@ -1,6 +1,6 @@ import { Prisma, PrismaClient } from '@prisma/client'; import { BaseService } from '../../services/base.service'; -import { AppError, NotFoundError, UnauthorizedError, ValidationError } from '../../utils/errors'; +import { AppError, NotFoundError, UnauthorizedError, ValidationError, DuplicatePaymentError, ForbiddenError, InsufficientFundsError, InvalidCurrencyError, PaymentDeclinedError, PaymentNotFoundError } from '../../utils/errors'; import { logger } from '../../utils/logger'; import { PaymentProvider, createPaymentProvider, resolveWebhookSecret, resolveWebhookToleranceSeconds } from '../../lib/payments/provider'; import { isRetryableProviderError, retryOperation, RetryOptions } from '../../lib/payments/retry'; @@ -57,13 +57,33 @@ export class ChargeService extends BaseService { return this.provider.name; } - /** Creates (or returns the existing) payment for an idempotency key. */ + /** + * Creates (or returns the existing) payment for an idempotency key. + * + * #175 — No on-chain balance check is performed here. ChargeService processes + * tokenized card payments through an external provider (e.g. Stripe); there is + * no sender Stellar wallet and no on-chain balance to verify. The pre-flight + * balance check required by #175 applies only to Stellar-based tip flows in + * PaymentService.createTip(), where a wallet public key and getWalletBalance() + * are available. Card payment failures are handled by the provider and surfaced + * via webhook status transitions. + */ async createPayment(userId: string, input: CreatePaymentInput, idempotencyKey: string): Promise { return this.executeWithLogging('payments.create', async () => { const existing = await this.prisma.payment.findUnique({ where: { idempotencyKey }, include: { refunds: true } }); if (existing) { if (existing.userId !== userId) { - throw new AppError(409, 'CONFLICT', 'Idempotency key already used by another user'); + logger.warn({ userId, idempotencyKey, failureReason: 'idempotency_key_conflict' }, 'Payment creation rejected: idempotency key reused by different user'); + throw new DuplicatePaymentError('Idempotency key already used by another user', { idempotencyKey }); + } + if (existing.amount !== input.amount) { + logger.warn({ userId, idempotencyKey, failureReason: 'idempotency_key_conflict' }, 'Payment creation rejected: idempotency key reused with a different amount'); + throw new DuplicatePaymentError('Idempotency key already used with a different amount', { idempotencyKey }); + } + const incomingCreatorId = input.creatorId ?? null; + if (existing.creatorId !== incomingCreatorId) { + logger.warn({ userId, idempotencyKey, failureReason: 'idempotency_key_conflict' }, 'Payment creation rejected: idempotency key reused with a different creator'); + throw new DuplicatePaymentError('Idempotency key already used with a different creator', { idempotencyKey }); } return this.toResponse(existing); } @@ -122,7 +142,8 @@ export class ChargeService extends BaseService { throw new NotFoundError('Payment'); } if (userId && payment.userId !== userId) { - throw new AppError(403, 'FORBIDDEN', 'Payment belongs to another user'); + logger.warn({ requestingUserId: userId, paymentId, failureReason: 'wrong_owner' }, 'Payment access denied: belongs to another user'); + throw new ForbiddenError('Payment belongs to another user'); } return this.toResponse(payment); }); @@ -174,9 +195,11 @@ export class ChargeService extends BaseService { throw new NotFoundError('Payment'); } if (payment.userId !== userId) { - throw new AppError(403, 'FORBIDDEN', 'Payment belongs to another user'); + logger.warn({ requestingUserId: userId, paymentId, failureReason: 'wrong_owner' }, 'Refund access denied: payment belongs to another user'); + throw new ForbiddenError('Payment belongs to another user'); } if (!isRefundablePaymentStatus(payment.status)) { + logger.warn({ userId, paymentId, paymentStatus: payment.status, failureReason: 'non_refundable_status' }, 'Refund rejected: payment status is not refundable'); throw new ValidationError(`Payment in status "${payment.status}" cannot be refunded`); } if (!payment.providerRef) { @@ -188,9 +211,11 @@ export class ChargeService extends BaseService { const amount = input.amount ?? remaining; if (amount <= 0) { + logger.warn({ userId, paymentId, alreadyRefunded, failureReason: 'nothing_to_refund' }, 'Refund rejected: nothing left to refund'); throw new ValidationError('Nothing left to refund'); } if (amount > remaining) { + logger.warn({ userId, paymentId, requested: amount, remaining, failureReason: 'refund_exceeds_balance' }, 'Refund rejected: exceeds refundable amount'); throw new ValidationError(`Refund exceeds the refundable amount of ${remaining}`); } @@ -245,11 +270,13 @@ export class ChargeService extends BaseService { async handleWebhook(providerName: string, rawBody: string, signature?: string): Promise { return this.executeWithLogging('payments.webhook', async () => { if (providerName !== this.provider.name && providerName !== 'stripe') { + logger.warn({ providerName, failureReason: 'unsupported_provider' }, 'Webhook rejected: unsupported provider'); throw new ValidationError(`Unsupported payment provider "${providerName}"`); } const secret = this.webhookSecret ?? (this.provider.name === 'fake' ? 'fake-webhook-secret' : undefined); if (!secret) { + logger.error({ failureReason: 'webhook_secret_not_configured' }, 'Webhook rejected: secret not configured'); throw new ValidationError('Webhook secret is not configured'); } diff --git a/src/domains/payments/charge.types.ts b/src/domains/payments/charge.types.ts index 0225c4d..29276cf 100644 --- a/src/domains/payments/charge.types.ts +++ b/src/domains/payments/charge.types.ts @@ -21,7 +21,7 @@ export const CreatePaymentSchema = z.object({ creatorId: z.string().trim().min(1).max(100).optional(), description: z.string().trim().max(500).optional(), /** Optional alternative to the `Idempotency-Key` header. */ - idempotencyKey: z.string().trim().min(8).max(255).optional(), + idempotencyKey: z.string().uuid('Idempotency key must be a valid UUID v4').optional(), }); /** @@ -35,7 +35,7 @@ export const RefundPaymentSchema = z.object({ .max(MAX_PAYMENT_AMOUNT) .optional(), reason: z.enum(['requested_by_customer', 'duplicate', 'fraudulent']).optional(), - idempotencyKey: z.string().trim().min(8).max(255).optional(), + idempotencyKey: z.string().uuid('Idempotency key must be a valid UUID v4').optional(), }); export const PaymentIdParamsSchema = z.object({ diff --git a/src/domains/payments/payment.service.test.ts b/src/domains/payments/payment.service.test.ts index fa18425..e295cbd 100644 --- a/src/domains/payments/payment.service.test.ts +++ b/src/domains/payments/payment.service.test.ts @@ -4,9 +4,16 @@ vi.mock('../webhooks/webhook.service', () => ({ })); const { publish } = vi.hoisted(() => ({ publish: vi.fn().mockResolvedValue(undefined) })); vi.mock('../../lib/queue', () => ({ stellarConfirmationQueue: { add: vi.fn() } })); +// #175 — stub getWalletBalance so tests stay hermetic (no Stellar network call) +vi.mock('../../lib/stellar/wallet', () => ({ + getWalletBalance: vi.fn(), +})); import { describe, it, expect, beforeEach, vi } from 'vitest'; import { PaymentService } from './payment.service'; -import { ValidationError, NotFoundError } from '../../utils/errors'; +import { ValidationError, NotFoundError, PaymentDeclinedError, PaymentNotFoundError } from '../../utils/errors'; +import { getWalletBalance } from '../../lib/stellar/wallet'; + +const mockGetWalletBalance = getWalletBalance as ReturnType; // Mock Prisma const mockPrisma = { @@ -47,6 +54,9 @@ describe('PaymentService', () => { paymentService = new PaymentService(mockPrisma as unknown as PrismaClient); vi.clearAllMocks(); + // #175 — default: wallet has ample balance so existing tests pass unchanged + mockGetWalletBalance.mockResolvedValue({ lumens: '100', usdc: '1000' }); + mockPrisma.stellarAsset.findFirst.mockResolvedValue({ id: 'asset-usdc', code: 'USDC', @@ -193,7 +203,7 @@ describe('PaymentService', () => { mockPrisma.walletFlag.findFirst.mockResolvedValue({ id: 'flag-1' }); await expect(paymentService.createTip(userId, { creatorId, amount: 50 })).rejects.toThrow( - ValidationError + PaymentDeclinedError ); expect(mockPrisma.tip.create).not.toHaveBeenCalled(); }); @@ -252,7 +262,7 @@ describe('PaymentService', () => { mockPrisma.accountFreeze.findFirst.mockResolvedValue({ id: 'freeze-1' }); await expect(paymentService.createTip(userId, { creatorId, amount: 100 })).rejects.toThrow( - ValidationError + PaymentDeclinedError ); expect(mockPrisma.tip.create).not.toHaveBeenCalled(); }); @@ -356,7 +366,7 @@ describe('PaymentService', () => { it('should throw NotFoundError if tip does not exist', async () => { mockPrisma.tip.findUnique.mockResolvedValue(null); - await expect(paymentService.getTip('non-existent')).rejects.toThrow(NotFoundError); + await expect(paymentService.getTip('non-existent')).rejects.toThrow(PaymentNotFoundError); }); }); diff --git a/src/domains/payments/payment.service.ts b/src/domains/payments/payment.service.ts index 72e88b9..5a415cf 100644 --- a/src/domains/payments/payment.service.ts +++ b/src/domains/payments/payment.service.ts @@ -13,8 +13,14 @@ import { NotFoundError, UnauthorizedError, ConflictError, + InvalidCurrencyError, + PaymentDeclinedError, + PaymentNotFoundError, + PaymentRequiredError, + ServiceUnavailableError, } from '../../utils/errors'; import { buildPaymentTransaction, submitSignedTransaction } from '../../lib/stellar/transactions'; +import { getWalletBalance } from '../../lib/stellar/wallet'; import { logger } from '../../utils/logger'; import { sanitizePageSize, sanitizePageNumber, parseSortParameters } from '../../utils/pagination'; import { paginateWithCursor } from '../../db/pagination'; @@ -140,7 +146,10 @@ export class PaymentService extends BaseService { where: { enabled: true, code: 'USDC' }, orderBy: { priority: 'desc' }, }); - if (data.assetId && !requestedAsset) throw new ValidationError('Asset is unavailable'); + if (data.assetId && !requestedAsset) { + logger.warn({ userId, assetId: data.assetId, failureReason: 'asset_unavailable' }, 'Tip blocked: requested asset is unavailable'); + throw new InvalidCurrencyError(data.assetId ?? 'unknown', { assetId: data.assetId }); + } // Verify sender is not tipping themselves const sender = await this.prisma.user.findUnique({ @@ -180,8 +189,8 @@ export class PaymentService extends BaseService { }); if (walletFlag) { - logger.warn(`Tip blocked: sender wallet ${wallet.publicKey} is flagged`); - throw new ValidationError('Payment cannot be processed at this time'); + logger.warn({ userId, walletPublicKey: wallet.publicKey, failureReason: 'wallet_flagged' }, 'Tip blocked: sender wallet is flagged'); + throw new PaymentDeclinedError('Payment cannot be processed at this time', { reason: 'wallet_flagged' }); } // #36 - Block payments to frozen creator accounts @@ -196,12 +205,14 @@ export class PaymentService extends BaseService { }); if (accountFreeze) { - logger.warn(`Tip blocked: creator account ${creator.id} is frozen`); - throw new ValidationError('Creator is not available for tips at this time'); + logger.warn({ userId, creatorId: data.creatorId, failureReason: 'account_frozen' }, 'Tip blocked: creator account is frozen'); + throw new PaymentDeclinedError('Creator is not available for tips at this time', { reason: 'account_frozen' }); } // Idempotency: a retried submission that reuses its key must return the // tip created by the first attempt instead of creating a second charge. + // This check runs BEFORE the Stellar balance call so retries short-circuit + // without re-hitting the network. (#175 ordering fix) if (data.idempotencyKey) { const existing = await this.prisma.tip.findUnique({ where: { idempotencyKey: data.idempotencyKey }, @@ -215,6 +226,41 @@ export class PaymentService extends BaseService { } } + // #175 — Check on-chain balance before creating any DB record + const assetCode = requestedAsset?.code ?? 'USDC'; + let walletBalances: { lumens: string; usdc?: string }; + try { + walletBalances = await getWalletBalance(wallet.publicKey); + } catch (err) { + logger.error( + { userId, walletPublicKey: wallet.publicKey, failureReason: 'balance_check_failed' }, + 'Tip blocked: failed to fetch on-chain wallet balance' + ); + throw new ServiceUnavailableError('Unable to verify wallet balance at this time'); + } + + const rawBalance = + assetCode === 'USDC' ? (walletBalances.usdc ?? '0') : walletBalances.lumens; + const currentBalance = parseFloat(rawBalance); + if (!Number.isFinite(currentBalance) || currentBalance < data.amount) { + logger.warn( + { + userId, + walletPublicKey: wallet.publicKey, + requiredAmount: data.amount, + currentBalance, + currency: assetCode, + failureReason: 'insufficient_balance', + }, + 'Tip blocked: insufficient on-chain balance' + ); + throw new PaymentRequiredError('Insufficient funds to complete the payment', { + requiredAmount: data.amount, + currentBalance, + currency: assetCode, + }); + } + // Validate requested media (#64) before creating the tip: every id must be // media this user uploaded, finished scanning, and has not already attached. // Resolving them up front means a tip never half-attaches and a foreign or @@ -342,13 +388,15 @@ export class PaymentService extends BaseService { }); if (!tip) { - throw new NotFoundError('Tip'); + logger.warn({ tipId, failureReason: 'tip_not_found' }, 'Tip not found'); + throw new PaymentNotFoundError(); } // Removed content is gone rather than hidden: a direct lookup 404s, while a // hidden tip stays readable so its sender and creator can see it (#62). if (tip.moderationState === 'removed') { - throw new NotFoundError('Tip'); + logger.warn({ tipId, moderationState: tip.moderationState, failureReason: 'tip_removed' }, 'Tip access denied due to moderation removal'); + throw new PaymentNotFoundError(); } return this.formatTipResponse(tip); @@ -1033,8 +1081,8 @@ export class PaymentService extends BaseService { fee: 100, // Base fee in stroops }; } catch (error) { - logger.error(`Failed to build transaction for tip ${tipId}:`, error); - throw new ValidationError('Failed to build payment transaction'); + logger.error({ userId: tip.fromUserId, tipId, failureReason: 'build_failed', cause: (error as Error).message }, 'Failed to build payment transaction'); + throw new PaymentDeclinedError('Failed to build payment transaction', { tipId }); } }); } @@ -1085,8 +1133,8 @@ export class PaymentService extends BaseService { try { result = await submitSignedTransaction(transactionEnvelope); } catch (error) { - logger.error(`Failed to submit transaction for tip ${tipId}:`, error); - throw new ValidationError('Failed to submit payment transaction'); + logger.error({ tipId, failureReason: 'submit_failed', cause: (error as Error).message }, 'Failed to submit transaction to Stellar network'); + throw new PaymentDeclinedError('Failed to submit payment transaction', { tipId }); } try { @@ -1128,8 +1176,8 @@ export class PaymentService extends BaseService { } throw new ConflictError('Transaction has already been submitted for this tip', { tipId }); } - logger.error(`Failed to store transaction for tip ${tipId}:`, error); - throw new ValidationError('Failed to submit payment transaction'); + logger.error({ tipId, failureReason: 'persist_failed', cause: (error as Error).message }, 'Failed to persist transaction hash'); + throw new PaymentDeclinedError('Failed to record payment transaction', { tipId }); } }); } diff --git a/src/middleware/__tests__/error-handler.test.ts b/src/middleware/__tests__/error-handler.test.ts index bda323a..2bba8ec 100644 --- a/src/middleware/__tests__/error-handler.test.ts +++ b/src/middleware/__tests__/error-handler.test.ts @@ -4,10 +4,15 @@ import { z } from 'zod'; import { buildErrorResponse, globalErrorHandler, normalizeError, notFoundHandler } from '../error-handler'; import { ConflictError, + DuplicatePaymentError, ErrorCodes, ForbiddenError, + InsufficientFundsError, InternalServerError, + InvalidCurrencyError, NotFoundError, + PaymentDeclinedError, + PaymentNotFoundError, TooManyRequestsError, UnauthorizedError, ValidationError, @@ -96,6 +101,26 @@ function buildApp(): FastifyInstance { throw new ValidationError('Contest is not open'); }); + app.get('/payment-insufficient-funds', async () => { + throw new InsufficientFundsError(); + }); + + app.get('/payment-invalid-currency', async () => { + throw new InvalidCurrencyError('XYZ'); + }); + + app.get('/payment-declined', async () => { + throw new PaymentDeclinedError(); + }); + + app.get('/payment-duplicate', async () => { + throw new DuplicatePaymentError('Duplicate tip', { idempotencyKey: 'key-1' }); + }); + + app.get('/payment-not-found', async () => { + throw new PaymentNotFoundError(); + }); + return app; } @@ -138,6 +163,11 @@ describe('global error handler', () => { ['/conflict', 409, ErrorCodes.CONFLICT], ['/rate-limit', 429, ErrorCodes.RATE_LIMIT_EXCEEDED], ['/prisma-down', 503, ErrorCodes.DATABASE_UNAVAILABLE], + ['/payment-insufficient-funds', 422, 'INSUFFICIENT_FUNDS'], + ['/payment-invalid-currency', 422, 'INVALID_CURRENCY'], + ['/payment-declined', 422, 'PAYMENT_DECLINED'], + ['/payment-duplicate', 409, 'DUPLICATE_PAYMENT'], + ['/payment-not-found', 404, 'PAYMENT_NOT_FOUND'], ]; for (const [url, status, code] of cases) { @@ -246,6 +276,70 @@ describe('global error handler', () => { }); }); +describe('payment-specific error classes', () => { + let app: FastifyInstance; + + beforeEach(async () => { + resetErrorTracking(); + app = buildApp(); + await app.ready(); + }); + + afterEach(async () => { + await app.close(); + }); + + it('InsufficientFundsError returns 422 with INSUFFICIENT_FUNDS code', async () => { + const res = await app.inject({ method: 'GET', url: '/payment-insufficient-funds' }); + expect(res.statusCode).toBe(422); + const body = res.json(); + expect(body.error.code).toBe('INSUFFICIENT_FUNDS'); + expect(body.error.message).toBeTruthy(); + }); + + it('InvalidCurrencyError returns 422 with INVALID_CURRENCY code', async () => { + const res = await app.inject({ method: 'GET', url: '/payment-invalid-currency' }); + expect(res.statusCode).toBe(422); + const body = res.json(); + expect(body.error.code).toBe('INVALID_CURRENCY'); + expect(body.error.message).toBeTruthy(); + }); + + it('PaymentDeclinedError returns 422 with PAYMENT_DECLINED code', async () => { + const res = await app.inject({ method: 'GET', url: '/payment-declined' }); + expect(res.statusCode).toBe(422); + const body = res.json(); + expect(body.error.code).toBe('PAYMENT_DECLINED'); + expect(body.error.message).toBeTruthy(); + }); + + it('DuplicatePaymentError returns 409 with DUPLICATE_PAYMENT code and details', async () => { + const res = await app.inject({ method: 'GET', url: '/payment-duplicate' }); + expect(res.statusCode).toBe(409); + const body = res.json(); + expect(body.error.code).toBe('DUPLICATE_PAYMENT'); + expect(body.error.message).toBeTruthy(); + expect(body.error.details.idempotencyKey).toBe('key-1'); + }); + + it('PaymentNotFoundError returns 404 with PAYMENT_NOT_FOUND code', async () => { + const res = await app.inject({ method: 'GET', url: '/payment-not-found' }); + expect(res.statusCode).toBe(404); + const body = res.json(); + expect(body.error.code).toBe('PAYMENT_NOT_FOUND'); + expect(body.error.message).toBeTruthy(); + }); + + it('payment errors never leak internal details', async () => { + const res = await app.inject({ method: 'GET', url: '/payment-declined' }); + expect(res.statusCode).toBe(422); + const serialized = JSON.stringify(res.json()); + expect(serialized).not.toContain('stack'); + expect(serialized).not.toContain('.ts:'); + expect(serialized).not.toContain('postgres://'); + }); +}); + describe('normalizeError', () => { it('normalizes AppError instances', () => { const normalized = normalizeError(new ValidationError('bad', { a: 1 })); diff --git a/src/plugins/__tests__/rateLimit.test.ts b/src/plugins/__tests__/rateLimit.test.ts index 6f6dc86..94b9736 100644 --- a/src/plugins/__tests__/rateLimit.test.ts +++ b/src/plugins/__tests__/rateLimit.test.ts @@ -27,6 +27,8 @@ const POLICIES: Record = { public: { max: 3, timeWindowMs: 60_000 }, authenticated: { max: 5, timeWindowMs: 60_000 }, sensitive: { max: 2, timeWindowMs: 60_000 }, + walletNonce: { max: 2, timeWindowMs: 3_600_000 }, + walletVerify: { max: 3, timeWindowMs: 3_600_000 }, }; const token = (userId: string) => @@ -359,3 +361,185 @@ describe('parseTrustProxy', () => { expect(parseTrustProxy('10.0.0.0/8, 127.0.0.1')).toEqual(['10.0.0.0/8', '127.0.0.1']); }); }); + +// Small wallet-specific limits used in the block below so tests run fast. +const WALLET_POLICIES: Record = { + public: { max: 100, timeWindowMs: 60_000 }, + authenticated: { max: 100, timeWindowMs: 60_000 }, + sensitive: { max: 100, timeWindowMs: 60_000 }, + walletNonce: { max: 2, timeWindowMs: 3_600_000 }, + walletVerify: { max: 3, timeWindowMs: 3_600_000 }, +}; + +async function buildWalletApp(): Promise { + const instance = Fastify({ logger: false }); + instance.setErrorHandler(globalErrorHandler); + + await registerRateLimiting(instance, { + enabled: true, + store: 'memory', + policies: WALLET_POLICIES, + rules: [ + { method: 'POST', url: '/api/v1/auth/wallet/challenge', class: 'walletNonce' }, + { method: 'POST', url: '/api/v1/auth/wallet/verify', class: 'walletVerify' }, + ], + exemptions: [], + }); + + const ok = async () => ({ ok: true }); + instance.post('/api/v1/auth/wallet/challenge', { preHandler: authMiddleware }, ok); + instance.post('/api/v1/auth/wallet/verify', { preHandler: authMiddleware }, ok); + + await instance.ready(); + return instance; +} + +describe('wallet route per-user rate limiting', () => { + afterEach(async () => { + await app?.close(); + }); + + it('per-user isolation for nonce: exhausting one user does not block another', async () => { + app = await buildWalletApp(); + + // alice exhausts the 2-request nonce limit + const aliceResponses = await Promise.all([ + app.inject({ method: 'POST', url: '/api/v1/auth/wallet/challenge', headers: bearer('alice-wallet') }), + app.inject({ method: 'POST', url: '/api/v1/auth/wallet/challenge', headers: bearer('alice-wallet') }), + ]); + expect(aliceResponses.map((r) => r.statusCode)).toEqual([200, 200]); + + const aliceBlocked = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('alice-wallet'), + }); + expect(aliceBlocked.statusCode).toBe(429); + + // bob uses the same IP — different user bucket, not blocked + const bob = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('bob-wallet'), + }); + expect(bob.statusCode).toBe(200); + expect(bob.headers['x-ratelimit-limit']).toBe('2'); + }); + + it('per-user isolation for verify: exhausting one user does not block another', async () => { + app = await buildWalletApp(); + + for (let i = 0; i < 3; i++) { + await app.inject({ method: 'POST', url: '/api/v1/auth/wallet/verify', headers: bearer('alice-verify') }); + } + + const aliceBlocked = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/verify', + headers: bearer('alice-verify'), + }); + expect(aliceBlocked.statusCode).toBe(429); + + const bob = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/verify', + headers: bearer('bob-verify'), + }); + expect(bob.statusCode).toBe(200); + expect(bob.headers['x-ratelimit-limit']).toBe('3'); + }); + + it('rate-limit headers reflect wallet nonce policy (not sensitive or authenticated)', async () => { + app = await buildWalletApp(); + + const res = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('carol-wallet'), + }); + expect(res.statusCode).toBe(200); + // walletNonce limit is 2, not the sensitive (100) or authenticated (100) defaults + expect(res.headers['x-ratelimit-limit']).toBe('2'); + }); + + it('window is 1 hour: x-ratelimit-reset reflects hourly window', async () => { + app = await buildWalletApp(); + + const res = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('dave-wallet'), + }); + expect(res.statusCode).toBe(200); + // @fastify/rate-limit returns reset in seconds; 1 hour = 3600 s + expect(Number(res.headers['x-ratelimit-reset'])).toBeLessThanOrEqual(3600); + expect(Number(res.headers['x-ratelimit-reset'])).toBeGreaterThan(0); + }); + + it('user follows across IPs: nonce quota exhausted on IP A also blocks on IP B', async () => { + app = await buildWalletApp(); + + // Exhaust from IP A + for (let i = 0; i < 2; i++) { + await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('eve-wallet'), + remoteAddress: '10.1.1.1', + }); + } + + // Same user from IP B — should be blocked (keyed by userId, not IP) + const fromIpB = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('eve-wallet'), + remoteAddress: '10.1.1.2', + }); + expect(fromIpB.statusCode).toBe(429); + }); + + it('unauthenticated request falls back to IP bucket, not a user bucket', async () => { + app = await buildWalletApp(); + + // Unauthenticated requests to wallet routes still increment the rate-limit counter + // (onRequest fires before authMiddleware's preHandler). The response is 401 from + // authMiddleware, but the IP bucket still depletes. After exhausting the walletNonce + // limit of 2, the 3rd unauthenticated request from the same IP receives 429. + for (let i = 0; i < 2; i++) { + const res = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + remoteAddress: '10.2.2.2', + }); + // Auth rejected, but rate-limit counter was incremented + expect(res.statusCode).toBe(401); + } + + // Third request from the same IP without a token — now blocked at the rate limit + const blocked = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + remoteAddress: '10.2.2.2', + }); + expect(blocked.statusCode).toBe(429); + + // A different IP without a token has its own bucket — not yet exhausted + const otherIp = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + remoteAddress: '10.2.2.3', + }); + // This IP's bucket is fresh, so rate limit passes and authMiddleware rejects with 401 + expect(otherIp.statusCode).toBe(401); + + // An authenticated user from the same blocked IP uses a separate user bucket + const authedFromBlockedIp = await app.inject({ + method: 'POST', + url: '/api/v1/auth/wallet/challenge', + headers: bearer('frank-wallet'), + remoteAddress: '10.2.2.2', + }); + expect(authedFromBlockedIp.statusCode).toBe(200); + }); +}); diff --git a/src/plugins/rateLimit.ts b/src/plugins/rateLimit.ts index 387de34..552b92d 100644 --- a/src/plugins/rateLimit.ts +++ b/src/plugins/rateLimit.ts @@ -32,7 +32,7 @@ import { logger } from '../utils/logger'; * sized by its class. */ -export type RouteClassification = RateLimitClass | 'authLogin' | 'authRegister' | 'authRecovery' | 'exempt'; +export type RouteClassification = RateLimitClass | 'authLogin' | 'authRegister' | 'authRecovery' | 'walletNonce' | 'walletVerify' | 'exempt'; export interface RateLimitingOptions { enabled?: boolean; @@ -211,13 +211,17 @@ export async function registerRateLimiting( } const routeClass = classifyRoute(routeOptions, policies, rules, exemptions); - const authOverride = routeOptions.url === '/api/v1/auth/login' + const authOverride: RouteClassification = routeOptions.url === '/api/v1/auth/login' ? 'authLogin' : routeOptions.url === '/api/v1/auth/register' ? 'authRegister' : ['/api/v1/auth/password-reset', '/api/v1/auth/password-reset/confirm'].includes(routeOptions.url) ? 'authRecovery' - : routeClass; + : routeOptions.url === '/api/v1/auth/wallet/challenge' + ? 'walletNonce' + : routeOptions.url === '/api/v1/auth/wallet/verify' + ? 'walletVerify' + : routeClass; routeOptions.config = { ...routeOptions.config, rateLimit: @@ -225,7 +229,7 @@ export async function registerRateLimiting( ? false : routeClass === 'internal' ? internalPolicy - : routePolicies[routeClass], + : routePolicies[authOverride] ?? routePolicies[routeClass], }; }); diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 8688b21..7150e91 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -33,6 +33,12 @@ export const ErrorCodes = { DATABASE_UNAVAILABLE: 'DATABASE_UNAVAILABLE', EXTERNAL_SERVICE_ERROR: 'EXTERNAL_SERVICE_ERROR', SERVICE_UNAVAILABLE: 'SERVICE_UNAVAILABLE', + INSUFFICIENT_FUNDS: 'INSUFFICIENT_FUNDS', + INVALID_CURRENCY: 'INVALID_CURRENCY', + PAYMENT_DECLINED: 'PAYMENT_DECLINED', + DUPLICATE_PAYMENT: 'DUPLICATE_PAYMENT', + PAYMENT_NOT_FOUND: 'PAYMENT_NOT_FOUND', + PAYMENT_REQUIRED: 'PAYMENT_REQUIRED', } as const; export type ErrorCode = (typeof ErrorCodes)[keyof typeof ErrorCodes]; @@ -132,6 +138,42 @@ export const ERROR_CATALOG: Record = { message: 'The service is temporarily unavailable', expose: true, }, + [ErrorCodes.INSUFFICIENT_FUNDS]: { + statusCode: 422, + code: 'INSUFFICIENT_FUNDS', + message: 'Insufficient funds to complete the payment', + expose: true, + }, + [ErrorCodes.INVALID_CURRENCY]: { + statusCode: 422, + code: 'INVALID_CURRENCY', + message: 'The specified currency is not supported', + expose: true, + }, + [ErrorCodes.PAYMENT_DECLINED]: { + statusCode: 422, + code: 'PAYMENT_DECLINED', + message: 'The payment was declined', + expose: true, + }, + [ErrorCodes.DUPLICATE_PAYMENT]: { + statusCode: 409, + code: 'DUPLICATE_PAYMENT', + message: 'A payment with this reference already exists', + expose: true, + }, + [ErrorCodes.PAYMENT_NOT_FOUND]: { + statusCode: 404, + code: 'PAYMENT_NOT_FOUND', + message: 'The requested payment was not found', + expose: true, + }, + [ErrorCodes.PAYMENT_REQUIRED]: { + statusCode: 402, + code: 'PAYMENT_REQUIRED', + message: 'Insufficient funds to complete the payment', + expose: true, + }, }; export function getErrorDefinition(code: string): ErrorDefinition { @@ -271,6 +313,54 @@ export class ServiceUnavailableError extends AppError { } } +export class InsufficientFundsError extends AppError { + constructor(message = 'Insufficient funds to complete the payment', details?: ErrorDetails) { + super(422, ErrorCodes.INSUFFICIENT_FUNDS, message, { details }); + this.name = 'InsufficientFundsError'; + } +} + +export class InvalidCurrencyError extends AppError { + constructor(currency: string, details?: ErrorDetails) { + super(422, ErrorCodes.INVALID_CURRENCY, `Currency '${currency}' is not supported`, { details }); + this.name = 'InvalidCurrencyError'; + } +} + +export class PaymentDeclinedError extends AppError { + constructor(message = 'The payment was declined', details?: ErrorDetails) { + super(422, ErrorCodes.PAYMENT_DECLINED, message, { details }); + this.name = 'PaymentDeclinedError'; + } +} + +export class DuplicatePaymentError extends AppError { + constructor(message = 'A payment with this reference already exists', details?: ErrorDetails) { + super(409, ErrorCodes.DUPLICATE_PAYMENT, message, { details }); + this.name = 'DuplicatePaymentError'; + } +} + +export class PaymentNotFoundError extends AppError { + constructor(message = 'The requested payment was not found', details?: ErrorDetails) { + super(404, ErrorCodes.PAYMENT_NOT_FOUND, message, { details }); + this.name = 'PaymentNotFoundError'; + } +} + +export interface PaymentRequiredDetails extends ErrorDetails { + requiredAmount: number; + currentBalance: number; + currency: string; +} + +export class PaymentRequiredError extends AppError { + constructor(message = 'Insufficient funds to complete the payment', details?: PaymentRequiredDetails) { + super(402, ErrorCodes.PAYMENT_REQUIRED, message, { details }); + this.name = 'PaymentRequiredError'; + } +} + export function isAppError(error: unknown): error is AppError { return error instanceof AppError; }