Skip to content

Commit 32821be

Browse files
authored
fix: allow schema router to config middlewares status codes (#7926)
1 parent 75942c7 commit 32821be

File tree

7 files changed

+219
-89
lines changed

7 files changed

+219
-89
lines changed

packages/core/src/middleware/koa-quota-guard.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,42 +10,30 @@ type Method = 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'COPY' | 'HEAD' | 'O
1010
type UsageGuardConfig = {
1111
key: Exclude<AllLimitKey, EntityBasedUsageKey>;
1212
quota: QuotaLibrary;
13-
/** Guard usage only for the specified method types. Guard all if not provided. */
14-
methods?: Method[];
1513
};
1614

1715
export function koaQuotaGuard<StateT, ContextT, ResponseBodyT>({
1816
key,
1917
quota,
20-
methods,
2118
}: UsageGuardConfig): MiddlewareType<StateT, ContextT, ResponseBodyT> {
22-
return async (ctx, next) => {
23-
// eslint-disable-next-line no-restricted-syntax
24-
if (!methods || methods.includes(ctx.method.toUpperCase() as Method)) {
25-
await quota.guardTenantUsageByKey(key);
26-
}
19+
return async (_, next) => {
20+
await quota.guardTenantUsageByKey(key);
2721
return next();
2822
};
2923
}
3024

3125
type UsageReportConfig = {
3226
key: keyof SubscriptionUsage;
3327
quota: QuotaLibrary;
34-
/** Report usage only for the specified method types. Report for all if not provided. */
35-
methods?: Method[];
3628
};
3729

3830
export function koaReportSubscriptionUpdates<StateT, ContextT, ResponseBodyT>({
3931
key,
4032
quota,
41-
methods = ['POST', 'PUT', 'DELETE'],
4233
}: UsageReportConfig): MiddlewareType<StateT, ContextT, Nullable<ResponseBodyT>> {
43-
return async (ctx, next) => {
34+
return async (_, next) => {
4435
await next();
4536

46-
// eslint-disable-next-line no-restricted-syntax
47-
if (methods.includes(ctx.method.toUpperCase() as Method)) {
48-
void quota.reportSubscriptionUpdatesUsage(key);
49-
}
37+
void quota.reportSubscriptionUpdatesUsage(key);
5038
};
5139
}

packages/core/src/routes/hook.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ export default function hookRoutes<T extends ManagementApiRouter>(
172172
koaReportSubscriptionUpdates({
173173
key: 'hooksLimit',
174174
quota,
175-
methods: ['POST'],
176175
}),
177176
async (ctx, next) => {
178177
const { event, events, enabled, ...rest } = ctx.guard.body;
@@ -266,7 +265,6 @@ export default function hookRoutes<T extends ManagementApiRouter>(
266265
koaReportSubscriptionUpdates({
267266
key: 'hooksLimit',
268267
quota,
269-
methods: ['DELETE'],
270268
}),
271269
async (ctx, next) => {
272270
const { id } = ctx.guard.params;

packages/core/src/routes/organization/index.ts

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,24 +39,19 @@ export default function organizationRoutes<T extends ManagementApiRouter>(
3939
const router = new SchemaRouter(Organizations, organizations, {
4040
middlewares: [
4141
{
42-
middlewares: [
43-
koaQuotaGuard({ key: 'organizationsLimit', quota, methods: ['POST', 'PUT'] }),
44-
],
45-
scope: {
46-
native: ['post', 'put'],
47-
},
42+
middleware: koaQuotaGuard({ key: 'organizationsLimit', quota }),
43+
scope: 'native',
44+
method: ['post', 'put'],
45+
// Throw 403 when quota exceeded
46+
status: [403],
4847
},
4948
{
50-
middlewares: [
51-
koaReportSubscriptionUpdates({
52-
key: 'organizationsLimit',
53-
quota,
54-
methods: ['POST', 'PUT', 'DELETE'],
55-
}),
56-
],
57-
scope: {
58-
native: ['post', 'put', 'delete'],
59-
},
49+
middleware: koaReportSubscriptionUpdates({
50+
key: 'organizationsLimit',
51+
quota,
52+
}),
53+
scope: 'native',
54+
method: ['post', 'put', 'delete'],
6055
},
6156
],
6257
errorHandler,

packages/core/src/routes/resource.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ export default function resourceRoutes<T extends ManagementApiRouter>(
9090
koaReportSubscriptionUpdates({
9191
key: 'resourcesLimit',
9292
quota,
93-
methods: ['POST'],
9493
}),
9594
async (ctx, next) => {
9695
const { body } = ctx.guard;
@@ -193,7 +192,6 @@ export default function resourceRoutes<T extends ManagementApiRouter>(
193192
koaReportSubscriptionUpdates({
194193
key: 'resourcesLimit',
195194
quota,
196-
methods: ['DELETE'],
197195
}),
198196
async (ctx, next) => {
199197
const { id } = ctx.guard.params;

packages/core/src/routes/sso-connector/index.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ export default function singleSignOnConnectorsRoutes<T extends ManagementApiRout
8383
koaReportSubscriptionUpdates({
8484
quota,
8585
key: 'enterpriseSsoLimit',
86-
methods: ['POST'],
8786
}),
8887
async (ctx, next) => {
8988
const { body } = ctx.guard;
@@ -233,7 +232,6 @@ export default function singleSignOnConnectorsRoutes<T extends ManagementApiRout
233232
koaReportSubscriptionUpdates({
234233
quota,
235234
key: 'enterpriseSsoLimit',
236-
methods: ['DELETE'],
237235
}),
238236
async (ctx, next) => {
239237
const { id } = ctx.guard.params;

packages/core/src/utils/SchemaRouter.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { type GeneratedSchema } from '@logto/schemas';
2+
import type { Middleware } from 'koa';
23
import { z } from 'zod';
34

45
import RequestError from '#src/errors/RequestError/index.js';
@@ -132,6 +133,120 @@ describe('SchemaRouter', () => {
132133
});
133134
});
134135

136+
describe('middlewares', () => {
137+
it('should allow status codes declared in middleware guard config with scope and method', async () => {
138+
// eslint-disable-next-line unicorn/consistent-function-scoping
139+
const throwingMiddleware: Middleware = async () => {
140+
throw new RequestError({ code: 'entity.not_found', status: 403 });
141+
};
142+
143+
const schemaRouterWithMiddleware = new SchemaRouter(schema, queries, {
144+
middlewares: [
145+
{
146+
middleware: throwingMiddleware,
147+
scope: 'native',
148+
method: ['get'],
149+
status: [403],
150+
},
151+
],
152+
});
153+
const requestWithMiddleware = createRequester({
154+
authedRoutes: (router) => router.use(schemaRouterWithMiddleware.routes()),
155+
});
156+
157+
const response = await requestWithMiddleware.get(baseRoute);
158+
159+
expect(response.status).toEqual(403);
160+
});
161+
162+
it('should apply middleware to all methods when method is not specified', async () => {
163+
const callTracker = jest.fn();
164+
165+
const trackingMiddleware: Middleware = async (ctx, next) => {
166+
callTracker(ctx.method);
167+
return next();
168+
};
169+
170+
const schemaRouterWithMiddleware = new SchemaRouter(schema, queries, {
171+
middlewares: [
172+
{
173+
middleware: trackingMiddleware,
174+
scope: 'native',
175+
},
176+
],
177+
});
178+
const requestWithMiddleware = createRequester({
179+
authedRoutes: (router) => router.use(schemaRouterWithMiddleware.routes()),
180+
});
181+
182+
await requestWithMiddleware.get(baseRoute);
183+
await requestWithMiddleware.post(baseRoute).send({});
184+
await requestWithMiddleware.patch(`${baseRoute}/test`).send({ name: 'test' });
185+
186+
expect(callTracker).toHaveBeenCalledTimes(3);
187+
expect(callTracker).toHaveBeenCalledWith('GET');
188+
expect(callTracker).toHaveBeenCalledWith('POST');
189+
expect(callTracker).toHaveBeenCalledWith('PATCH');
190+
});
191+
192+
it('should only apply middleware to specified methods', async () => {
193+
const callTracker = jest.fn();
194+
195+
const trackingMiddleware: Middleware = async (ctx, next) => {
196+
callTracker(ctx.method);
197+
return next();
198+
};
199+
200+
const schemaRouterWithMiddleware = new SchemaRouter(schema, queries, {
201+
middlewares: [
202+
{
203+
middleware: trackingMiddleware,
204+
scope: 'native',
205+
method: ['get', 'post'],
206+
},
207+
],
208+
});
209+
const requestWithMiddleware = createRequester({
210+
authedRoutes: (router) => router.use(schemaRouterWithMiddleware.routes()),
211+
});
212+
213+
await requestWithMiddleware.get(baseRoute);
214+
await requestWithMiddleware.post(baseRoute).send({});
215+
await requestWithMiddleware.patch(`${baseRoute}/test`).send({ name: 'test' });
216+
217+
expect(callTracker).toHaveBeenCalledTimes(2);
218+
expect(callTracker).toHaveBeenCalledWith('GET');
219+
expect(callTracker).toHaveBeenCalledWith('POST');
220+
expect(callTracker).not.toHaveBeenCalledWith('PATCH');
221+
});
222+
223+
it('should apply middleware without scope to all routes', async () => {
224+
const callTracker = jest.fn();
225+
226+
const trackingMiddleware: Middleware = async (ctx, next) => {
227+
callTracker();
228+
return next();
229+
};
230+
231+
const schemaRouterWithMiddleware = new SchemaRouter(schema, queries, {
232+
middlewares: [
233+
{
234+
middleware: trackingMiddleware,
235+
},
236+
],
237+
});
238+
const requestWithMiddleware = createRequester({
239+
authedRoutes: (router) => router.use(schemaRouterWithMiddleware.routes()),
240+
});
241+
242+
await requestWithMiddleware.get(baseRoute);
243+
await requestWithMiddleware.post(baseRoute).send({});
244+
245+
// Global middleware should be called for all routes
246+
expect(callTracker).toHaveBeenCalledTimes(2);
247+
});
248+
});
249+
135250
describe('disable routes', () => {
136251
it('should be able to disable routes', async () => {
137252
const disabledSchemaRouter = new SchemaRouter(schema, queries, {

0 commit comments

Comments
 (0)