Skip to content

Commit a17b8e5

Browse files
fix(billing): requirePlan gates on subscription.status + top-level errorCode
Closes #3679. Two related drifts fixed in one change: 1. Logic: canceled / past_due / unpaid / incomplete subscriptions no longer pass requirePlan('growth') just because subscription.plan === 'growth'. Only status ∈ {active, trialing} counts; everything else falls back to 'free' plan resolution. Pre-existing gap — middleware had zero route usages in devkit until trawl_node wired it on apiKeys / webhooks / org-invites for plan-entitlement enforcement. 2. Response shape: surfaces { type: 'error', message: 'Forbidden', code: 403, status: 403, errorCode: 'PLAN_REQUIRED', description, requiredPlans, currentPlan } at the top level of the JSON body (was: nested { type: 'PLAN_REQUIRED' } under responses.error meta). Upstreams the existing trawl_node #1183 patch so /update-stack --theirs no longer silently wipes the downstream contract. ORG_CONTEXT_REQUIRED also surfaces at top level for consistency. 14 unit tests total (6 baseline updated for new shape + 8 new for status gating and response-shape assertions). Drops responses.js import (no longer needed).
1 parent 5e166d7 commit a17b8e5

2 files changed

Lines changed: 203 additions & 13 deletions

File tree

modules/billing/middlewares/billing.requirePlan.js

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,23 @@
33
*/
44
import SubscriptionRepository from '../repositories/billing.subscription.repository.js';
55

6-
import responses from '../../../lib/helpers/responses.js';
6+
/**
7+
* Statuses considered "active" for plan-entitlement checks.
8+
* canceled / past_due / unpaid / incomplete fall back to 'free' resolution.
9+
* @type {Set<string>}
10+
*/
11+
const ACTIVE_STATUSES = new Set(['active', 'trialing']);
712

813
/**
9-
* Returns Express middleware that gates access based on subscription plan.
10-
* This middleware is orthogonal to CASL — CASL gates by role, requirePlan
11-
* gates by subscription plan.
14+
* Returns Express middleware that gates access based on subscription plan AND status.
15+
* Orthogonal to CASL — CASL gates by role, requirePlan gates by subscription plan.
16+
*
17+
* A plan is only effective when subscription.status ∈ {active, trialing}.
18+
* canceled / past_due / unpaid / incomplete are treated as 'free'.
19+
*
20+
* Response shape on deny matches the downstream trawl_node contract (top-level errorCode):
21+
* { type: 'error', message: 'Forbidden', code: 403, status: 403,
22+
* errorCode: 'PLAN_REQUIRED', description, requiredPlans: string[], currentPlan: string }
1223
*
1324
* Expects `req.organization` to be set by resolveOrganization upstream.
1425
*
@@ -18,17 +29,30 @@ import responses from '../../../lib/helpers/responses.js';
1829
function requirePlan(...plans) {
1930
return async function requirePlanMiddleware(req, res, next) {
2031
if (!req.organization) {
21-
return responses.error(res, 403, 'Forbidden', 'Organization context is required to check subscription plan')();
32+
return res.status(403).json({
33+
type: 'error',
34+
message: 'Forbidden',
35+
code: 403,
36+
status: 403,
37+
errorCode: 'ORG_CONTEXT_REQUIRED',
38+
description: 'Organization context is required to check subscription plan',
39+
});
2240
}
2341

2442
try {
2543
const subscription = await SubscriptionRepository.findByOrganization(req.organization._id);
26-
const currentPlan = subscription?.plan || 'free';
44+
const isEffective = subscription && ACTIVE_STATUSES.has(subscription.status);
45+
const currentPlan = isEffective ? subscription.plan : 'free';
2746

2847
if (plans.includes(currentPlan)) return next();
2948

30-
return responses.error(res, 403, 'Forbidden', 'Your current plan does not allow access to this resource')({
31-
type: 'PLAN_REQUIRED',
49+
return res.status(403).json({
50+
type: 'error',
51+
message: 'Forbidden',
52+
code: 403,
53+
status: 403,
54+
errorCode: 'PLAN_REQUIRED',
55+
description: 'Your current plan does not allow access to this resource',
3256
requiredPlans: plans,
3357
currentPlan,
3458
});

modules/billing/tests/billing.requirePlan.unit.tests.js

Lines changed: 171 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('requirePlan middleware unit tests:', () => {
4646
});
4747

4848
test('should call next when subscription plan matches the required plan', async () => {
49-
mockFindByOrganization.mockResolvedValue({ plan: 'pro' });
49+
mockFindByOrganization.mockResolvedValue({ plan: 'pro', status: 'active' });
5050

5151
const middleware = requirePlan('pro');
5252
const req = mockReq();
@@ -60,7 +60,7 @@ describe('requirePlan middleware unit tests:', () => {
6060
});
6161

6262
test('should call next when subscription plan is in multiple allowed plans', async () => {
63-
mockFindByOrganization.mockResolvedValue({ plan: 'starter' });
63+
mockFindByOrganization.mockResolvedValue({ plan: 'starter', status: 'active' });
6464

6565
const middleware = requirePlan('starter', 'pro', 'enterprise');
6666
const req = mockReq();
@@ -74,7 +74,7 @@ describe('requirePlan middleware unit tests:', () => {
7474
});
7575

7676
test('should return 403 when subscription plan does not match', async () => {
77-
mockFindByOrganization.mockResolvedValue({ plan: 'free' });
77+
mockFindByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
7878

7979
const middleware = requirePlan('pro', 'enterprise');
8080
const req = mockReq();
@@ -85,7 +85,9 @@ describe('requirePlan middleware unit tests:', () => {
8585

8686
expect(next).not.toHaveBeenCalled();
8787
expect(res.status).toHaveBeenCalledWith(403);
88-
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ type: 'error', message: 'Forbidden' }));
88+
expect(res.json).toHaveBeenCalledWith(
89+
expect.objectContaining({ type: 'error', message: 'Forbidden', errorCode: 'PLAN_REQUIRED' }),
90+
);
8991
});
9092

9193
test('should default to free plan when no subscription exists', async () => {
@@ -126,7 +128,171 @@ describe('requirePlan middleware unit tests:', () => {
126128

127129
expect(next).not.toHaveBeenCalled();
128130
expect(res.status).toHaveBeenCalledWith(403);
129-
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ type: 'error', message: 'Forbidden' }));
131+
expect(res.json).toHaveBeenCalledWith(
132+
expect.objectContaining({ type: 'error', message: 'Forbidden', errorCode: 'ORG_CONTEXT_REQUIRED' }),
133+
);
130134
expect(mockFindByOrganization).not.toHaveBeenCalled();
131135
});
132136
});
137+
138+
describe('requirePlan — subscription.status gating:', () => {
139+
const fakeOrgId = new mongoose.Types.ObjectId();
140+
141+
function mockReq(overrides = {}) {
142+
return {
143+
organization: { _id: fakeOrgId, name: 'Test Org' },
144+
...overrides,
145+
};
146+
}
147+
148+
function mockRes() {
149+
const res = {};
150+
res.status = jest.fn().mockReturnValue(res);
151+
res.json = jest.fn().mockReturnValue(res);
152+
return res;
153+
}
154+
155+
beforeEach(() => {
156+
jest.clearAllMocks();
157+
});
158+
159+
test('treats a canceled growth subscription as free (denies access)', async () => {
160+
mockFindByOrganization.mockResolvedValue({ plan: 'growth', status: 'canceled' });
161+
162+
const req = mockReq();
163+
const res = mockRes();
164+
const next = jest.fn();
165+
166+
await requirePlan('growth', 'pro')(req, res, next);
167+
168+
expect(next).not.toHaveBeenCalled();
169+
expect(res.status).toHaveBeenCalledWith(403);
170+
const body = res.json.mock.calls[0][0];
171+
expect(body.currentPlan).toBe('free');
172+
});
173+
174+
test('treats a past_due growth subscription as free (denies access)', async () => {
175+
mockFindByOrganization.mockResolvedValue({ plan: 'growth', status: 'past_due' });
176+
177+
const req = mockReq();
178+
const res = mockRes();
179+
const next = jest.fn();
180+
181+
await requirePlan('growth')(req, res, next);
182+
183+
expect(next).not.toHaveBeenCalled();
184+
expect(res.status).toHaveBeenCalledWith(403);
185+
const body = res.json.mock.calls[0][0];
186+
expect(body.currentPlan).toBe('free');
187+
});
188+
189+
test('treats unpaid subscription as free (denies access)', async () => {
190+
mockFindByOrganization.mockResolvedValue({ plan: 'pro', status: 'unpaid' });
191+
192+
const req = mockReq();
193+
const res = mockRes();
194+
const next = jest.fn();
195+
196+
await requirePlan('pro')(req, res, next);
197+
198+
expect(next).not.toHaveBeenCalled();
199+
expect(res.status).toHaveBeenCalledWith(403);
200+
const body = res.json.mock.calls[0][0];
201+
expect(body.currentPlan).toBe('free');
202+
});
203+
204+
test('treats incomplete subscription as free (denies access)', async () => {
205+
mockFindByOrganization.mockResolvedValue({ plan: 'pro', status: 'incomplete' });
206+
207+
const req = mockReq();
208+
const res = mockRes();
209+
const next = jest.fn();
210+
211+
await requirePlan('pro')(req, res, next);
212+
213+
expect(next).not.toHaveBeenCalled();
214+
expect(res.status).toHaveBeenCalledWith(403);
215+
const body = res.json.mock.calls[0][0];
216+
expect(body.currentPlan).toBe('free');
217+
});
218+
219+
test('passes an active growth subscription', async () => {
220+
mockFindByOrganization.mockResolvedValue({ plan: 'growth', status: 'active' });
221+
222+
const req = mockReq();
223+
const res = mockRes();
224+
const next = jest.fn();
225+
226+
await requirePlan('growth')(req, res, next);
227+
228+
expect(next).toHaveBeenCalledWith();
229+
expect(res.status).not.toHaveBeenCalled();
230+
});
231+
232+
test('passes a trialing pro subscription', async () => {
233+
mockFindByOrganization.mockResolvedValue({ plan: 'pro', status: 'trialing' });
234+
235+
const req = mockReq();
236+
const res = mockRes();
237+
const next = jest.fn();
238+
239+
await requirePlan('pro')(req, res, next);
240+
241+
expect(next).toHaveBeenCalledWith();
242+
expect(res.status).not.toHaveBeenCalled();
243+
});
244+
});
245+
246+
describe('requirePlan — response shape (top-level errorCode):', () => {
247+
const fakeOrgId = new mongoose.Types.ObjectId();
248+
249+
function mockReq(overrides = {}) {
250+
return {
251+
organization: { _id: fakeOrgId, name: 'Test Org' },
252+
...overrides,
253+
};
254+
}
255+
256+
function mockRes() {
257+
const res = {};
258+
res.status = jest.fn().mockReturnValue(res);
259+
res.json = jest.fn().mockReturnValue(res);
260+
return res;
261+
}
262+
263+
beforeEach(() => {
264+
jest.clearAllMocks();
265+
});
266+
267+
test('returns top-level errorCode, requiredPlans, currentPlan on PLAN_REQUIRED', async () => {
268+
mockFindByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
269+
270+
const req = mockReq();
271+
const res = mockRes();
272+
const next = jest.fn();
273+
274+
await requirePlan('growth', 'pro')(req, res, next);
275+
276+
const body = res.json.mock.calls[0][0];
277+
expect(body.errorCode).toBe('PLAN_REQUIRED');
278+
expect(body.requiredPlans).toEqual(['growth', 'pro']);
279+
expect(body.currentPlan).toBe('free');
280+
expect(body.type).toBe('error');
281+
expect(body.message).toBe('Forbidden');
282+
expect(body.code).toBe(403);
283+
expect(body.status).toBe(403);
284+
});
285+
286+
test('returns ORG_CONTEXT_REQUIRED errorCode when req.organization is absent', async () => {
287+
const req = mockReq({ organization: undefined });
288+
const res = mockRes();
289+
const next = jest.fn();
290+
291+
await requirePlan('growth')(req, res, next);
292+
293+
const body = res.json.mock.calls[0][0];
294+
expect(body.errorCode).toBe('ORG_CONTEXT_REQUIRED');
295+
expect(res.status).toHaveBeenCalledWith(403);
296+
expect(body.type).toBe('error');
297+
});
298+
});

0 commit comments

Comments
 (0)