fix: forbid cross-user balance deduction in billing deduct route

src/middleware/adminAuth.ts

@@ -1,59 +1,121 @@
 import { createHash } from 'crypto';
 import type { Request, Response, NextFunction } from 'express';
 import jwt from 'jsonwebtoken';
+
 import { InternalServerError, UnauthorizedError } from '../errors/index.js';
-import { ALLOWED_ALGORITHMS } from './requireAuth.js';
+import { ALLOWED_ALGORITHMS, requireAuth, type AuthenticatedLocals } from './requireAuth.js';
 import { getTokenRevocationService } from '../services/tokenRevocation.js';
 import { timingSafeStringEqual } from '../lib/timingSafe.js';
 
-interface AdminJwtPayload { role: string; [key: string]: unknown }
+interface AdminJwtPayload {
+  role: string;
+  [key: string]: unknown;
+}
 
 // #1266: constant-time comparison lives in src/lib/timingSafe.ts (SHA-256
 // digests compared with crypto.timingSafeEqual, so key length is not leaked).
 
-/** Require the configured admin API key or an admin-role JWT. */
-export function adminAuth(req: Request, res: Response, next: NextFunction): void {
+/**
+ * Resolve the admin actor for a request without terminating the middleware
+ * chain.
+ *
+ * Accepts the configured admin API key or a Bearer JWT carrying the `admin`
+ * role (with a valid `exp`, an optional `admin` audience, and not revoked).
+ *
+ * @returns The actor identity (`admin-api-key`, the JWT `sub`/`email`, or
+ * `admin-jwt`), or `null` when the caller is not an authenticated admin.
+ */
+export function resolveAdminActor(req: Request): string | null {
   const apiKey = req.header('x-admin-api-key');
   const configuredKey = process.env.ADMIN_API_KEY;
   if (apiKey && configuredKey && timingSafeStringEqual(apiKey, configuredKey)) {
-    res.locals.adminActor = 'admin-api-key';
-    next();
-    return;
+    return 'admin-api-key';
   }
 
   const authHeader = req.header('Authorization');
-  if (authHeader?.startsWith('Bearer ')) {
-    const secret = process.env.JWT_SECRET;
-    if (!secret) {
-      next(new InternalServerError('JWT_SECRET not configured'));
-      return;
+  if (!authHeader?.startsWith('Bearer ')) {
+    return null;
+  }
+
+  const secret = process.env.JWT_SECRET;
+  if (!secret) {
+    return null;
+  }
+
+  const token = authHeader.slice(7);
+  try {
+    const payload = jwt.verify(token, secret, { algorithms: ALLOWED_ALGORITHMS }) as AdminJwtPayload;
+
+    if (typeof payload.exp !== 'number') {
+      return null;
+    }
+
+    if (payload.aud !== undefined && payload.aud !== 'admin') {
+      return null;
     }
-    const token = authHeader.slice(7);
-    try {
-      const payload = jwt.verify(token, secret, { algorithms: ALLOWED_ALGORITHMS }) as AdminJwtPayload;
-
-      if (typeof payload.exp !== 'number') {
-        throw new Error('Token missing exp claim');
-      }
-
-      if (payload.aud !== undefined && payload.aud !== 'admin') {
-        throw new Error('Invalid audience');
-      }
-
-      const tokenHash = createHash('sha256').update(token).digest('hex');
-      if (getTokenRevocationService().isRevoked(tokenHash)) {
-        throw new Error('Token is revoked');
-      }
-
-      if (payload.role === 'admin') {
-        res.locals.adminActor = (payload.sub as string) || (payload.email as string) || 'admin-jwt';
-        next();
-        return;
-      }
-    } catch {
-      // Fall through to the standard unauthorized response.
+
+    const tokenHash = createHash('sha256').update(token).digest('hex');
+    if (getTokenRevocationService().isRevoked(tokenHash)) {
+      return null;
+    }
+
+    if (payload.role === 'admin') {
+      return (payload.sub as string) || (payload.email as string) || 'admin-jwt';
     }
+  } catch {
+    // Not a verifiable admin token.
+  }
+
+  return null;
+}
+
+/**
+ * Admin authentication middleware.
+ *
+ * Authenticates admin callers via an API key or a Bearer JWT with the
+ * `admin` role. On success it sets `authenticatedAdmin` and `adminActor` in
+ * `res.locals` so downstream routes can authorize cross-user actions and audit
+ * log the actor.
+ */
+export function adminAuth(req: Request, res: Response, next: NextFunction): void {
+  const actor = resolveAdminActor(req);
+  if (actor) {
+    res.locals.adminActor = actor;
+    res.locals.authenticatedAdmin = true;
+    next();
+    return;
+  }
+
+  // Preserve the explicit misconfiguration signal for an admin Bearer attempt.
+  if (req.header('Authorization')?.startsWith('Bearer ') && !process.env.JWT_SECRET) {
+    next(new InternalServerError('JWT_SECRET not configured'));
+    return;
   }
 
   next(new UnauthorizedError('Unauthorized: admin access required'));
 }
+
+/**
+ * Authenticate either an ordinary user/service principal (via {@link requireAuth})
+ * or an admin (admin API key or admin-role JWT).
+ *
+ * Admins are projected onto `authenticatedUser` with their actor id so a route
+ * can share a single code path, while `authenticatedAdmin` and `adminActor`
+ * stay set so privileged cross-user actions can be authorised and audited.
+ */
+export function requireAuthOrAdmin(
+  req: Request,
+  res: Response<unknown, AuthenticatedLocals>,
+  next: NextFunction,
+): void {
+  const actor = resolveAdminActor(req);
+  if (actor) {
+    res.locals.authenticatedAdmin = true;
+    res.locals.adminActor = actor;
+    res.locals.authenticatedUser = { id: actor };
+    next();
+    return;
+  }
+
+  requireAuth(req, res, next);
+}

src/middleware/requireAuth.ts

@@ -9,11 +9,42 @@ import { logger } from "../logger.js";
 // Re-export the locals shape for files that import it from this module
 export type AuthenticatedLocals = {
   authenticatedUser?: AuthenticatedUser;
+  authenticatedService?: AuthenticatedService;
+  authenticatedAdmin?: boolean;
+  adminActor?: string;
 };
 
 /** Restrict accepted signing algorithms to prevent algorithm-confusion attacks. */
 export const ALLOWED_ALGORITHMS: jwt.Algorithm[] = ["HS256"];
 
+/** Scope that authorises a service principal to deduct on a user's behalf. */
+export const BILLING_DEDUCT_SCOPE = "billing:deduct";
+
+/**
+ * Authenticated service principal derived from a bearer token.
+ * Service principals are not users; they carry explicit scopes.
+ */
+export interface AuthenticatedService {
+  id: string;
+  scopes: string[];
+  isService: true;
+}
+
+/**
+ * Normalise the `scopes`/`scope` claims of a verified JWT payload into a
+ * de-duplicated list of scope strings. Accepts an array or a space/comma
+ * separated string.
+ */
+function extractScopes(payload: Record<string, unknown>): string[] {
+  const raw = payload.scopes ?? payload.scope;
+  const scopes = Array.isArray(raw)
+    ? raw.filter((value): value is string => typeof value === "string")
+    : typeof raw === "string"
+      ? raw.split(/[\s,]+/)
+      : [];
+  return Array.from(new Set(scopes.filter((scope) => scope.length > 0)));
+}
+
 export interface ResolvedRequestUserId {
   userId?: string;
   error?: UnauthorizedError;
@@ -173,6 +204,46 @@ export function resolveRequestUserId(req: Request): ResolvedRequestUserId {
   return {};
 }
 
+/**
+ * Resolve an authenticated service principal from the Bearer token,
+ * if the token carries the `type: "service"` claim. Returns null for
+ * ordinary user tokens.
+ */
+export function resolveRequestService(req: Request): AuthenticatedService | null {
+  const authHeader = req.header("authorization");
+  if (!authHeader || !authHeader.startsWith("Bearer ")) {
+    return null;
+  }
+
+  const token = authHeader.slice("Bearer ".length).trim();
+  if (!token) return null;
+
+  const secret = process.env.JWT_SECRET;
+  if (!secret) return null;
+
+  try {
+    const decoded = jwt.verify(token, secret, {
+      algorithms: ALLOWED_ALGORITHMS,
+    });
+
+    if (typeof decoded === "string" || !decoded) return null;
+
+    const payload = decoded as Record<string, unknown>;
+    if (payload.type !== "service") return null;
+
+    const uid = payload.userId || payload.sub;
+    if (typeof uid !== "string" || uid.trim() === "") return null;
+
+    return {
+      id: uid,
+      scopes: extractScopes(payload),
+      isService: true,
+    };
+  } catch {
+    return null;
+  }
+}
+
 export const requireAuth = (
   req: Request,
   res: Response<unknown, AuthenticatedLocals>,
@@ -189,7 +260,12 @@ export const requireAuth = (
     return;
   }
 
+  const service = resolveRequestService(req);
+
   res.locals.authenticatedUser = { id: userId };
+  if (service) {
+    res.locals.authenticatedService = service;
+  }
   req.developerId = userId; // Keep req.developerId backwards compatibility since main branch router depends on it
   next();
 };

src/routes/billing/deduct.test.ts

@@ -226,6 +226,107 @@ describe('POST /api/billing/deduct - developerId validation', () => {
     expect(app.locals.billingService).toBe(fakeService);
   });
 
+  function buildAppWithService() {
+    const fakeService = {
+      deduct: jest.fn().mockResolvedValue({
+        success: true,
+        usageEventId: 'evt_1',
+        stellarTxHash: 'tx_1',
+        alreadyProcessed: false,
+        deductionApplied: true,
+        reconciliationRequired: false,
+      }),
+      getByRequestId: jest.fn(),
+    };
+    const app = buildApp(
+      { query: jest.fn() } as unknown as Pool,
+      fakeService as unknown as BillingService,
+    );
+    return { app, fakeService };
+  }
+
+  it('returns 403 and never invokes the service when developerId is another user', async () => {
+    const { app, fakeService } = buildAppWithService();
+
+    const res = await request(app)
+      .post('/api/billing/deduct')
+      .set('Authorization', `Bearer ${makeToken('user_123')}`)
+      .send({ ...validPayload, developerId: 'user_456' });
+
+    expect(res.status).toBe(403);
+    expect(res.body.error.code).toBe('FORBIDDEN');
+    expect(fakeService.deduct).not.toHaveBeenCalled();
+  });
+
+  it('deducts from the authenticated user when developerId matches', async () => {
+    const { app, fakeService } = buildAppWithService();
+
+    const res = await request(app)
+      .post('/api/billing/deduct')
+      .set('Authorization', `Bearer ${makeToken('user_123')}`)
+      .send({ ...validPayload, developerId: 'user_123' });
+
+    expect(res.status).toBe(200);
+    expect(fakeService.deduct).toHaveBeenCalledWith(
+      expect.objectContaining({ userId: 'user_123' }),
+    );
+  });
+
+  it('returns 403 when a service principal lacks the billing scope', async () => {
+    const { app, fakeService } = buildAppWithService();
+    const token = jwt.sign(
+      { userId: 'svc_1', type: 'service', scopes: ['billing:read'] },
+      JWT_SECRET,
+      { algorithm: 'HS256', expiresIn: '1h' },
+    );
+
+    const res = await request(app)
+      .post('/api/billing/deduct')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ ...validPayload, developerId: 'user_456' });
+
+    expect(res.status).toBe(403);
+    expect(fakeService.deduct).not.toHaveBeenCalled();
+  });
+
+  it('lets a service principal with the billing scope deduct on behalf of a user', async () => {
+    const { app, fakeService } = buildAppWithService();
+    const token = jwt.sign(
+      { userId: 'svc_1', type: 'service', scopes: ['billing:deduct'] },
+      JWT_SECRET,
+      { algorithm: 'HS256', expiresIn: '1h' },
+    );
+
+    const res = await request(app)
+      .post('/api/billing/deduct')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ ...validPayload, developerId: 'user_456' });
+
+    expect(res.status).toBe(200);
+    expect(fakeService.deduct).toHaveBeenCalledWith(
+      expect.objectContaining({ userId: 'user_456' }),
+    );
+  });
+
+  it('allows an admin API key to deduct on behalf of another user', async () => {
+    process.env.ADMIN_API_KEY = 'test-admin-key';
+    try {
+      const { app, fakeService } = buildAppWithService();
+
+      const res = await request(app)
+        .post('/api/billing/deduct')
+        .set('x-admin-api-key', 'test-admin-key')
+        .send({ ...validPayload, developerId: 'user_456' });
+
+      expect(res.status).toBe(200);
+      expect(fakeService.deduct).toHaveBeenCalledWith(
+        expect.objectContaining({ userId: 'user_456' }),
+      );
+    } finally {
+      delete process.env.ADMIN_API_KEY;
+    }
+  });
+
   it('creates the billing client only once when the app starts', async () => {
     const fakeClient: jest.Mocked<SorobanClient> = {
       getBalance: jest.fn().mockResolvedValue({ balance: '0' }),