From e4d0ff413ebc4cd001185c4541caf185bd3bc304 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 30 Apr 2026 19:46:11 +0200 Subject: [PATCH 1/3] fix(billing): stripe tax customer_update + admin/exempt org bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add customer_update: { address: 'auto', name: 'auto' } to both createCheckout and createExtrasCheckout to prevent customer_tax_location_invalid when customer has no address - Add meterExempt field (Boolean, default false) to Organization model (Mongoose + Zod) for explicit meter bypass - requireQuota middleware now short-circuits for admin users (req.user.roles includes 'admin') and meterExempt orgs — works in both legacy and meter mode, backward-compat path unchanged - Extend unit tests: customer_update assertion on both checkout paths, admin bypass + meterExempt bypass cases in legacy and meter mode --- .../middlewares/billing.requireQuota.js | 4 ++ modules/billing/services/billing.service.js | 2 + .../tests/billing.checkout.unit.tests.js | 19 +++++ .../billing/tests/billing.quota.unit.tests.js | 71 +++++++++++++++++++ .../billing.service.extras.unit.tests.js | 18 +++++ .../models/organizations.model.mongoose.js | 5 ++ .../models/organizations.schema.js | 1 + 7 files changed, 120 insertions(+) diff --git a/modules/billing/middlewares/billing.requireQuota.js b/modules/billing/middlewares/billing.requireQuota.js index 06fbd9c36..2273a5741 100644 --- a/modules/billing/middlewares/billing.requireQuota.js +++ b/modules/billing/middlewares/billing.requireQuota.js @@ -44,6 +44,10 @@ function requireQuota(resource, action) { return responses.error(res, 403, 'Forbidden', 'Organization context is required to check quota')(); } + // Bypass for admin users or meter-exempt organizations + if (req.user?.roles?.includes('admin')) return next(); + if (req.organization.meterExempt === true) return next(); + try { // ── Meter mode (meterMode: true) ────────────────────────────────────── if (config.billing?.meterMode === true) { diff --git a/modules/billing/services/billing.service.js b/modules/billing/services/billing.service.js index b18e11ee0..c5b3fb4cc 100644 --- a/modules/billing/services/billing.service.js +++ b/modules/billing/services/billing.service.js @@ -105,6 +105,7 @@ const createCheckout = async (organization, priceId, successUrl, cancelUrl) => { success_url: successUrl, cancel_url: cancelUrl, automatic_tax: { enabled: true }, + customer_update: { address: 'auto', name: 'auto' }, metadata: { organizationId: String(organization._id), plan: matchedPlan.planId, @@ -221,6 +222,7 @@ const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl) success_url: successUrl, cancel_url: cancelUrl, automatic_tax: { enabled: true }, + customer_update: { address: 'auto', name: 'auto' }, metadata: { organizationId: String(organization._id), packId, diff --git a/modules/billing/tests/billing.checkout.unit.tests.js b/modules/billing/tests/billing.checkout.unit.tests.js index 0c8352e02..f6151e209 100644 --- a/modules/billing/tests/billing.checkout.unit.tests.js +++ b/modules/billing/tests/billing.checkout.unit.tests.js @@ -235,6 +235,7 @@ describe('Billing service unit tests:', () => { success_url: 'http://ok', cancel_url: 'http://cancel', automatic_tax: { enabled: true }, + customer_update: { address: 'auto', name: 'auto' }, metadata: { organizationId: orgId, plan: 'starter', @@ -243,6 +244,24 @@ describe('Billing service unit tests:', () => { { idempotencyKey: `sub_checkout_${orgId}_price_starter_m` }, ); }); + + test('should include customer_update in checkout session params', async () => { + jest.unstable_mockModule('../../../config/index.js', () => ({ + default: { stripe: { secretKey: 'sk_test_cu' } }, + })); + + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ + stripeCustomerId: 'cus_cu_test', + }); + + const mod = await import('../services/billing.service.js'); + BillingService = mod.default; + + await BillingService.createCheckout(mockOrganization, 'price_starter_m', 'http://ok', 'http://cancel'); + + const callArgs = mockStripeInstance.checkout.sessions.create.mock.calls[0][0]; + expect(callArgs.customer_update).toEqual({ address: 'auto', name: 'auto' }); + }); }); describe('createPortalSession', () => { diff --git a/modules/billing/tests/billing.quota.unit.tests.js b/modules/billing/tests/billing.quota.unit.tests.js index 06768583a..f6cd6252b 100644 --- a/modules/billing/tests/billing.quota.unit.tests.js +++ b/modules/billing/tests/billing.quota.unit.tests.js @@ -195,6 +195,53 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); }); + // ── Admin bypass ────────────────────────────────────────────────────────── + + test('should bypass quota for admin user (legacy mode)', async () => { + req.user = { roles: ['admin'] }; + // No subscription/usage lookups needed — should short-circuit + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + test('should NOT bypass quota for non-admin user (legacy mode)', async () => { + req.user = { roles: ['user'] }; + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(429); + }); + + test('should bypass quota for meterExempt organization', async () => { + req.organization = { _id: '507f1f77bcf86cd799439011', meterExempt: true }; + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + test('should NOT bypass quota when meterExempt is false', async () => { + req.organization = { _id: '507f1f77bcf86cd799439011', meterExempt: false }; + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(429); + }); + // ── Meter mode (meterMode: true) ─────────────────────────────────────────── describe('meter mode (meterMode: true)', () => { @@ -383,6 +430,30 @@ describe('requireQuota middleware:', () => { expect(res.status).toHaveBeenCalledWith(402); }); + test('should bypass meter quota for admin user', async () => { + req.user = { roles: ['admin'] }; + // getMeter would return exhausted, but admin should bypass + mockBillingUsageService.getMeter.mockResolvedValue({ meterUsed: 5000, meterQuota: 5000 }); + mockBillingExtraBalanceRepository.getBalance.mockResolvedValue(0); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + test('should bypass meter quota for meterExempt organization', async () => { + req.organization = { _id: '507f1f77bcf86cd799439011', meterExempt: true }; + // getMeter would return exhausted, but exempt org should bypass + mockBillingUsageService.getMeter.mockResolvedValue({ meterUsed: 5000, meterQuota: 5000 }); + mockBillingExtraBalanceRepository.getBalance.mockResolvedValue(0); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + test('degraded J+5: still blocks if meter is exhausted', async () => { const fiveDaysAgo = new Date(Date.now() - 5 * 24 * 60 * 60 * 1000); mockSubscriptionRepository.findByOrganization.mockResolvedValue({ diff --git a/modules/billing/tests/billing.service.extras.unit.tests.js b/modules/billing/tests/billing.service.extras.unit.tests.js index bfa834cae..94b9aaf4f 100644 --- a/modules/billing/tests/billing.service.extras.unit.tests.js +++ b/modules/billing/tests/billing.service.extras.unit.tests.js @@ -235,6 +235,24 @@ describe('BillingService.createExtrasCheckout unit tests:', () => { expect(mockStripeInstance.checkout.sessions.create).toHaveBeenCalled(); }); + test('should include customer_update in extras checkout session params', async () => { + jest.unstable_mockModule('../../../config/index.js', () => ({ + default: makeConfig(), + })); + + const mod = await import('../services/billing.service.js'); + BillingService = mod.default; + + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ stripeCustomerId: 'cus_existing' }); + + await BillingService.createExtrasCheckout( + mockOrganization, 'pack_500k', 'http://success', 'http://cancel', + ); + + const [params] = mockStripeInstance.checkout.sessions.create.mock.calls[0]; + expect(params.customer_update).toEqual({ address: 'auto', name: 'auto' }); + }); + test('should handle 11000 duplicate key on subscription create gracefully', async () => { jest.unstable_mockModule('../../../config/index.js', () => ({ default: makeConfig(), diff --git a/modules/organizations/models/organizations.model.mongoose.js b/modules/organizations/models/organizations.model.mongoose.js index b2acbd929..e3461691e 100644 --- a/modules/organizations/models/organizations.model.mongoose.js +++ b/modules/organizations/models/organizations.model.mongoose.js @@ -39,6 +39,11 @@ const OrganizationMongoose = new Schema( enum: config.billing.plans, default: 'free', }, + meterExempt: { + type: Boolean, + default: false, + sparse: true, + }, createdBy: { type: Schema.ObjectId, ref: 'User', diff --git a/modules/organizations/models/organizations.schema.js b/modules/organizations/models/organizations.schema.js index 70fab208a..feb498625 100644 --- a/modules/organizations/models/organizations.schema.js +++ b/modules/organizations/models/organizations.schema.js @@ -14,6 +14,7 @@ const Organization = z.object({ slug: z.string().trim().min(1).toLowerCase().optional(), domain: z.string().trim().default(''), plan: z.enum(config.billing.plans).default('free'), + meterExempt: z.boolean().default(false).optional(), }); const OrganizationUpdate = Organization.partial(); From f786f28a4555ed9ad820b78cf25a439bc8cb5ef4 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 30 Apr 2026 19:48:50 +0200 Subject: [PATCH 2/3] fix(billing): remove meaningless sparse index on boolean meterExempt field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sparse:true on a Boolean with default:false has no effect — sparse indexes only skip null/undefined documents. Remove to avoid misleading maintainers. --- modules/organizations/models/organizations.model.mongoose.js | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/organizations/models/organizations.model.mongoose.js b/modules/organizations/models/organizations.model.mongoose.js index e3461691e..e1c1410bd 100644 --- a/modules/organizations/models/organizations.model.mongoose.js +++ b/modules/organizations/models/organizations.model.mongoose.js @@ -42,7 +42,6 @@ const OrganizationMongoose = new Schema( meterExempt: { type: Boolean, default: false, - sparse: true, }, createdBy: { type: Schema.ObjectId, From 10ca70e09842c83e0fffbcb883881ff5b083f791 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 30 Apr 2026 19:54:12 +0200 Subject: [PATCH 3/3] fix(billing): fix mass-assignment risk + strengthen bypass short-circuit tests - Remove meterExempt from OrganizationUpdate Zod schema to prevent any authenticated user from setting it via PUT /api/organizations/:id. Field stays in Mongoose model for admin-level DB access. - Add not.toHaveBeenCalled() assertions to all 4 bypass tests (legacy + meter mode, admin + meterExempt) to lock the short-circuit contract. --- modules/billing/tests/billing.quota.unit.tests.js | 14 ++++++++++++++ .../organizations/models/organizations.schema.js | 1 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/modules/billing/tests/billing.quota.unit.tests.js b/modules/billing/tests/billing.quota.unit.tests.js index f6cd6252b..b74412f8d 100644 --- a/modules/billing/tests/billing.quota.unit.tests.js +++ b/modules/billing/tests/billing.quota.unit.tests.js @@ -207,6 +207,9 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); expect(res.status).not.toHaveBeenCalled(); + // Short-circuit: no downstream quota lookups + expect(mockSubscriptionRepository.findByOrganization).not.toHaveBeenCalled(); + expect(mockBillingUsageService.get).not.toHaveBeenCalled(); }); test('should NOT bypass quota for non-admin user (legacy mode)', async () => { @@ -229,6 +232,9 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); expect(res.status).not.toHaveBeenCalled(); + // Short-circuit: no downstream quota lookups + expect(mockSubscriptionRepository.findByOrganization).not.toHaveBeenCalled(); + expect(mockBillingUsageService.get).not.toHaveBeenCalled(); }); test('should NOT bypass quota when meterExempt is false', async () => { @@ -440,6 +446,10 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); expect(res.status).not.toHaveBeenCalled(); + // Short-circuit: no downstream meter/subscription lookups + expect(mockSubscriptionRepository.findByOrganization).not.toHaveBeenCalled(); + expect(mockBillingUsageService.getMeter).not.toHaveBeenCalled(); + expect(mockBillingExtraBalanceRepository.getBalance).not.toHaveBeenCalled(); }); test('should bypass meter quota for meterExempt organization', async () => { @@ -452,6 +462,10 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); expect(res.status).not.toHaveBeenCalled(); + // Short-circuit: no downstream meter/subscription lookups + expect(mockSubscriptionRepository.findByOrganization).not.toHaveBeenCalled(); + expect(mockBillingUsageService.getMeter).not.toHaveBeenCalled(); + expect(mockBillingExtraBalanceRepository.getBalance).not.toHaveBeenCalled(); }); test('degraded J+5: still blocks if meter is exhausted', async () => { diff --git a/modules/organizations/models/organizations.schema.js b/modules/organizations/models/organizations.schema.js index feb498625..70fab208a 100644 --- a/modules/organizations/models/organizations.schema.js +++ b/modules/organizations/models/organizations.schema.js @@ -14,7 +14,6 @@ const Organization = z.object({ slug: z.string().trim().min(1).toLowerCase().optional(), domain: z.string().trim().default(''), plan: z.enum(config.billing.plans).default('free'), - meterExempt: z.boolean().default(false).optional(), }); const OrganizationUpdate = Organization.partial();