From 32ce64d7d75b48bed58a5e01a35fcd09bb39cf08 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 19 Mar 2026 09:44:19 +0100 Subject: [PATCH 1/2] fix(billing): use active/trialing status allowlist in requireQuota middleware --- .../middlewares/billing.requireQuota.js | 5 +- .../billing/tests/billing.quota.unit.tests.js | 52 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/modules/billing/middlewares/billing.requireQuota.js b/modules/billing/middlewares/billing.requireQuota.js index dc312f7a9..6faa90eae 100644 --- a/modules/billing/middlewares/billing.requireQuota.js +++ b/modules/billing/middlewares/billing.requireQuota.js @@ -37,9 +37,10 @@ function requireQuota(resource, action) { } try { - // Determine current plan — default to free when no subscription or past_due + // Determine current plan — default to free when subscription is missing or inactive const subscription = await SubscriptionRepository.findByOrganization(req.organization._id); - const plan = (!subscription || subscription.status === 'past_due') ? 'free' : (subscription.plan || 'free'); + const activeStatuses = ['active', 'trialing']; + const plan = (!subscription || !activeStatuses.includes(subscription.status)) ? 'free' : (subscription.plan || 'free'); // Look up quota limit from config const quotas = config.billing?.quotas; diff --git a/modules/billing/tests/billing.quota.unit.tests.js b/modules/billing/tests/billing.quota.unit.tests.js index 5778c8eca..049d60bc6 100644 --- a/modules/billing/tests/billing.quota.unit.tests.js +++ b/modules/billing/tests/billing.quota.unit.tests.js @@ -173,6 +173,58 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); }); + test('should treat canceled subscription as free plan', async () => { + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'canceled' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(429); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ + type: 'error', + code: 429, + })); + }); + + test('should treat unpaid subscription as free plan', async () => { + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'unpaid' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(429); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ + type: 'error', + code: 429, + })); + }); + + test('should treat incomplete subscription as free plan', async () => { + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'incomplete' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(429); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ + type: 'error', + code: 429, + })); + }); + + test('should use subscription plan when status is trialing', async () => { + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'trialing' }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 15 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + test('should treat zero usage as under quota', async () => { mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' }); mockBillingUsageService.get.mockResolvedValue({ counters: {} }); From 81b4a3922df9cfa715e034894e9b67f55c7e4cc9 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 19 Mar 2026 09:51:14 +0100 Subject: [PATCH 2/2] fix(billing): extract activeStatuses constant and use test.each for inactive statuses - Hoist activeStatuses to shared lib/constants.js to avoid duplication between requireQuota middleware and billing controller - Collapse individual inactive-status test cases into test.each table for easier maintenance and extensibility --- .../billing/controllers/billing.controller.js | 2 +- modules/billing/lib/constants.js | 5 ++ .../middlewares/billing.requireQuota.js | 2 +- .../billing/tests/billing.quota.unit.tests.js | 71 +++++-------------- 4 files changed, 23 insertions(+), 57 deletions(-) create mode 100644 modules/billing/lib/constants.js diff --git a/modules/billing/controllers/billing.controller.js b/modules/billing/controllers/billing.controller.js index 85b2c4466..bca8e0144 100644 --- a/modules/billing/controllers/billing.controller.js +++ b/modules/billing/controllers/billing.controller.js @@ -1,6 +1,7 @@ /** * Module dependencies */ +import { activeStatuses } from '../lib/constants.js'; import config from '../../../config/index.js'; import responses from '../../../lib/helpers/responses.js'; import BillingService from '../services/billing.service.js'; @@ -67,7 +68,6 @@ const getUsage = async (req, res) => { try { // Determine current plan via service layer const subscription = await BillingService.getSubscription(req.organization._id); - const activeStatuses = ['active', 'trialing']; const plan = (!subscription || !activeStatuses.includes(subscription.status)) ? 'free' : (subscription.plan || 'free'); // Get usage counters (includes month field) diff --git a/modules/billing/lib/constants.js b/modules/billing/lib/constants.js new file mode 100644 index 000000000..31a7a4068 --- /dev/null +++ b/modules/billing/lib/constants.js @@ -0,0 +1,5 @@ +/** + * Stripe subscription statuses that indicate an active subscription. + * Any status not in this list is treated as inactive (falls back to free plan). + */ +export const activeStatuses = ['active', 'trialing']; diff --git a/modules/billing/middlewares/billing.requireQuota.js b/modules/billing/middlewares/billing.requireQuota.js index 6faa90eae..915e6e281 100644 --- a/modules/billing/middlewares/billing.requireQuota.js +++ b/modules/billing/middlewares/billing.requireQuota.js @@ -4,6 +4,7 @@ import SubscriptionRepository from '../repositories/billing.subscription.repository.js'; import BillingUsageService from '../services/billing.usage.service.js'; +import { activeStatuses } from '../lib/constants.js'; import config from '../../../config/index.js'; import responses from '../../../lib/helpers/responses.js'; @@ -39,7 +40,6 @@ function requireQuota(resource, action) { try { // Determine current plan — default to free when subscription is missing or inactive const subscription = await SubscriptionRepository.findByOrganization(req.organization._id); - const activeStatuses = ['active', 'trialing']; const plan = (!subscription || !activeStatuses.includes(subscription.status)) ? 'free' : (subscription.plan || 'free'); // Look up quota limit from config diff --git a/modules/billing/tests/billing.quota.unit.tests.js b/modules/billing/tests/billing.quota.unit.tests.js index 049d60bc6..2779f7101 100644 --- a/modules/billing/tests/billing.quota.unit.tests.js +++ b/modules/billing/tests/billing.quota.unit.tests.js @@ -117,19 +117,22 @@ describe('requireQuota middleware:', () => { })); }); - test('should treat past_due subscription as free plan', async () => { - mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'past_due' }); - mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); - - await requireQuota('scraps', 'create')(req, res, next); - - expect(next).not.toHaveBeenCalled(); - expect(res.status).toHaveBeenCalledWith(429); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ - type: 'error', - code: 429, - })); - }); + test.each(['past_due', 'canceled', 'unpaid', 'incomplete', 'incomplete_expired', 'paused'])( + 'should treat %s subscription as free plan', + async (status) => { + mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status }); + mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); + + await requireQuota('scraps', 'create')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(429); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ + type: 'error', + code: 429, + })); + }, + ); test('should allow unlimited (Infinity) without checking usage', async () => { mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'pro', status: 'active' }); @@ -173,48 +176,6 @@ describe('requireQuota middleware:', () => { expect(next).toHaveBeenCalled(); }); - test('should treat canceled subscription as free plan', async () => { - mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'canceled' }); - mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); - - await requireQuota('scraps', 'create')(req, res, next); - - expect(next).not.toHaveBeenCalled(); - expect(res.status).toHaveBeenCalledWith(429); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ - type: 'error', - code: 429, - })); - }); - - test('should treat unpaid subscription as free plan', async () => { - mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'unpaid' }); - mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); - - await requireQuota('scraps', 'create')(req, res, next); - - expect(next).not.toHaveBeenCalled(); - expect(res.status).toHaveBeenCalledWith(429); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ - type: 'error', - code: 429, - })); - }); - - test('should treat incomplete subscription as free plan', async () => { - mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'incomplete' }); - mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } }); - - await requireQuota('scraps', 'create')(req, res, next); - - expect(next).not.toHaveBeenCalled(); - expect(res.status).toHaveBeenCalledWith(429); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ - type: 'error', - code: 429, - })); - }); - test('should use subscription plan when status is trialing', async () => { mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'trialing' }); mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 15 } });