diff --git a/integration/hello-world/e2e/middleware-global-prefix-exclude.spec.ts b/integration/hello-world/e2e/middleware-global-prefix-exclude.spec.ts index 7ed807341c..9d3adb5ee0 100644 --- a/integration/hello-world/e2e/middleware-global-prefix-exclude.spec.ts +++ b/integration/hello-world/e2e/middleware-global-prefix-exclude.spec.ts @@ -152,3 +152,76 @@ describe.each([ ); }, ); + +@Controller('health') +class ProbesController { + @Get() + live() { + return RETURN_VALUE; + } + + @Get('ready') + ready() { + return RETURN_VALUE; + } +} + +describe.each([ + ['Express', () => undefined], + ['Fastify', () => new FastifyAdapter()], +])( + 'Middleware - exclusion covering routes excluded from the global prefix (%s)', + (_, createAdapter) => { + let app: INestApplication; + + const createApp = async (exclusion: string, prefixExclusions: string[]) => { + @Module({ + controllers: [ProbesController, ReportsController, UsersController], + }) + class ProbesModule { + configure(consumer: MiddlewareConsumer) { + consumer + .apply((req, res, next) => res.end(MIDDLEWARE_VALUE)) + .exclude(exclusion) + .forRoutes('{*path}'); + } + } + + app = ( + await Test.createTestingModule({ imports: [ProbesModule] }).compile() + ).createNestApplication(createAdapter()); + app.setGlobalPrefix('api', { exclude: prefixExclusions }); + await listenOnLoopback(app); + }; + + afterEach(async () => { + await app.close(); + }); + + it.each(['health/{*path}', 'health/:check', 'health/(.*)'])( + `exclude('%s') should skip "/health/ready" excluded from the prefix`, + async exclusion => { + await createApp(exclusion, ['health', 'health/ready']); + await request(app.getHttpServer()) + .get('/health/ready') + .expect(200, RETURN_VALUE); + await request(app.getHttpServer()) + .get('/health') + .expect(200, MIDDLEWARE_VALUE); + await request(app.getHttpServer()) + .get('/api/users/1') + .expect(200, MIDDLEWARE_VALUE); + }, + ); + + it(`exclude('reports') should skip "/reports" excluded by a wildcard`, async () => { + await createApp('reports', ['reports/{*path}']); + await request(app.getHttpServer()) + .get('/reports') + .expect(200, RETURN_VALUE); + await request(app.getHttpServer()) + .get('/reports/1') + .expect(200, MIDDLEWARE_VALUE); + }); + }, +); diff --git a/integration/security/e2e/csrf-protection.spec.ts b/integration/security/e2e/csrf-protection.spec.ts index dfcca89053..143149a56d 100644 --- a/integration/security/e2e/csrf-protection.spec.ts +++ b/integration/security/e2e/csrf-protection.spec.ts @@ -332,6 +332,29 @@ describe.each(adapters)('CSRF protection (%s)', (_name, createAdapter) => { }); }); + describe('with an exclusion covering a route excluded from the global prefix', () => { + beforeEach(() => + init(app => { + app.setGlobalPrefix('api', { exclude: ['webhooks/stripe'] }); + app.enableCsrfProtection({ + exclude: [{ path: 'webhooks/*path', method: RequestMethod.POST }], + }); + }), + ); + + it('allows a cross-site POST to the route served without the prefix', async () => { + const post = (path: string) => + request(app.getHttpServer()) + .post(path) + .set('Host', HOST) + .set('Sec-Fetch-Site', 'cross-site') + .set('Origin', 'https://api.stripe.example'); + + await post('/webhooks/stripe').expect(201, { received: true }); + await post('/api/items').expect(403); + }); + }); + describe('with a global exception filter', () => { beforeEach(() => init(app => { diff --git a/packages/core/middleware/builder.ts b/packages/core/middleware/builder.ts index 8a39d9ee12..705143bc5d 100644 --- a/packages/core/middleware/builder.ts +++ b/packages/core/middleware/builder.ts @@ -61,18 +61,13 @@ export class MiddlewareBuilder implements MiddlewareConsumer { ): MiddlewareConfigProxy { this.excludedRoutes = [ ...this.excludedRoutes, - ...this.getRoutesFlatList(routes).reduce((excludedRoutes, route) => { - for (const routePath of this.routeInfoPathExtractor.extractPathFrom( - route, - )) { - excludedRoutes.push({ - ...route, - path: routePath, - }); - } - - return excludedRoutes; - }, [] as RouteInfo[]), + ...this.getRoutesFlatList(routes).flatMap(route => + this.routeInfoPathExtractor.extractExclusionsFrom(route, { + caseSensitive: + this.builder.getHttpAdapter().getRouteMatchingOptions?.() + ?.caseSensitive ?? true, + }), + ), ]; return this; diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index f4b61ab470..0786835c97 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -82,12 +82,15 @@ export class RouteInfoPathExtractor { * route's own path, which an exclusion ending in an optional segment such * as "reports/{*path}" takes out of the prefix (see * `isExcludedWithTrailingSlash()`). + * + * The route matches case-insensitively unless `caseSensitive` is set: a + * caller that skips something for the covered routes (an exclusion) must + * match them as strictly as the router does. */ - public extractExcludedRoutesCoveredBy({ - path, - method, - version, - }: RouteInfo): RouteInfo[] { + public extractExcludedRoutesCoveredBy( + { path, method, version }: RouteInfo, + { caseSensitive = false }: { caseSensitive?: boolean } = {}, + ): RouteInfo[] { if ( !Array.isArray(this.excludedGlobalPrefixRoutes) || this.isAWildcard(path) || @@ -99,6 +102,7 @@ export class RouteInfoPathExtractor { try { routeRegex = pathToRegexp( LegacyRouteConverter.tryConvert(addLeadingSlash(path), { logs: false }), + { sensitive: caseSensitive }, ).regexp; } catch { // The adapter reports a path it cannot mount @@ -134,6 +138,29 @@ export class RouteInfoPathExtractor { }); } + /** + * Returns the routes that an exclusion (`MiddlewareConsumer.exclude()`, or + * the `exclude` option of the CSRF protection) skips, with the paths they + * are served at: the route itself and the routes excluded from the global + * prefix that it covers, which the router serves without the prefix (see + * `extractExcludedRoutesCoveredBy()`). Exclusions match case-sensitively + * unless told otherwise, as matching too much skips the middleware. + */ + public extractExclusionsFrom( + route: RouteInfo, + { caseSensitive = true }: { caseSensitive?: boolean } = {}, + ): RouteInfo[] { + return [ + route, + ...this.extractExcludedRoutesCoveredBy(route, { caseSensitive }), + ].flatMap(excludedRoute => + this.extractPathFrom(excludedRoute).map(path => ({ + ...excludedRoute, + path, + })), + ); + } + /** * Returns the paths the router serves a route excluded from the global * prefix at: those of its pattern and, for a pattern ending in an optional diff --git a/packages/core/security/cross-origin-protection.ts b/packages/core/security/cross-origin-protection.ts index 5ee4c7755d..2d34315035 100644 --- a/packages/core/security/cross-origin-protection.ts +++ b/packages/core/security/cross-origin-protection.ts @@ -278,16 +278,20 @@ export class CrossOriginProtection { * of `RouteInfo.version` (with another versioning type, only the requests * for that version are excluded). Called by `app.init()`, once the global * prefix and versioning are final; until then nothing is excluded. + * The routes excluded from the global prefix that a route covers are + * excluded too, at the paths the router serves them at. */ public resolveExclusions(pathExtractor: RouteInfoPathExtractor): void { this.resolvedExclusions = this.excludedRoutes.flatMap(route => - pathExtractor.extractPathFrom(route).map(path => { - const exclusion = compileExclusion(path, route.method, true); - if (route.version) { - exclusion.version = route.version; - } - return exclusion; - }), + pathExtractor + .extractExclusionsFrom(route, { caseSensitive: true }) + .map(({ path, method }) => { + const exclusion = compileExclusion(path, method, true); + if (route.version) { + exclusion.version = route.version; + } + return exclusion; + }), ); this.ignoreTrailingSlash = !!this.httpAdapter.getRouteMatchingOptions?.()?.ignoreTrailingSlash; diff --git a/packages/core/test/middleware/builder.spec.ts b/packages/core/test/middleware/builder.spec.ts index a88229e818..b245058741 100644 --- a/packages/core/test/middleware/builder.spec.ts +++ b/packages/core/test/middleware/builder.spec.ts @@ -273,5 +273,60 @@ describe('MiddlewareBuilder', () => { }, ]); }); + + it('should also exclude the routes excluded from the global prefix that a route covers', () => { + const appConfig = new ApplicationConfig(); + appConfig.setGlobalPrefix('api'); + appConfig.setGlobalPrefixOptions({ + exclude: mapToExcludeRoute(['health', 'health/ready']), + }); + builder = new MiddlewareBuilder( + new RoutesMapper(new NestContainer(), appConfig), + new NoopHttpAdapter({}), + new RouteInfoPathExtractor(appConfig), + ); + const proxy: any = builder.apply().exclude('health/{*path}', 'health'); + + expect(proxy.getExcludedRoutes()).toEqual([ + { path: '/api/health/{*path}', method: -1 }, + { path: '/health/ready', method: -1 }, + { path: '/health', method: -1 }, + ]); + }); + + it('should match the routes excluded from the global prefix with the case sensitivity of the router', () => { + const appConfig = new ApplicationConfig(); + appConfig.setGlobalPrefix('api'); + appConfig.setGlobalPrefixOptions({ + exclude: mapToExcludeRoute(['health/ready']), + }); + const createBuilder = (caseSensitive: boolean) => { + const httpAdapter = new NoopHttpAdapter({}); + httpAdapter.getRouteMatchingOptions = () => ({ + caseSensitive, + ignoreTrailingSlash: false, + }); + return new MiddlewareBuilder( + new RoutesMapper(new NestContainer(), appConfig), + httpAdapter, + new RouteInfoPathExtractor(appConfig), + ); + }; + + const sensitive: any = createBuilder(true) + .apply() + .exclude('HEALTH/:check'); + expect(sensitive.getExcludedRoutes()).toEqual([ + { path: '/api/HEALTH/:check', method: -1 }, + ]); + + const insensitive: any = createBuilder(false) + .apply() + .exclude('HEALTH/:check'); + expect(insensitive.getExcludedRoutes()).toEqual([ + { path: '/api/HEALTH/:check', method: -1 }, + { path: '/health/ready', method: -1 }, + ]); + }); }); }); diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index 6dd1cfcc4f..e8ed5b9623 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -431,6 +431,111 @@ describe('RouteInfoPathExtractor', () => { }); }); + describe('extractExclusionsFrom', () => { + let routeInfoPathExtractor: RouteInfoPathExtractor; + + beforeEach(() => { + const appConfig = new ApplicationConfig(); + appConfig.setGlobalPrefix('api'); + appConfig.setGlobalPrefixOptions({ + exclude: mapToExcludeRoute([ + 'health', + 'health/ready', + { path: 'webhooks/stripe', method: RequestMethod.POST }, + 'reports/{*path}', + ]), + }); + routeInfoPathExtractor = new RouteInfoPathExtractor(appConfig); + }); + + it(`should also return the routes excluded from the global prefix that the route covers`, () => { + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/health/{*path}', + method: -1 as RequestMethod, + }), + ).toEqual([ + { path: '/api/health/{*path}', method: -1 }, + { path: '/health/ready', method: -1 }, + ]); + + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/health/(.*)', + method: -1 as RequestMethod, + }), + ).toEqual([ + { path: '/api/health/(.*)', method: -1 }, + { path: '/health/ready', method: -1 }, + ]); + + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/webhooks/:provider', + method: RequestMethod.ALL, + }), + ).toEqual([ + { path: '/api/webhooks/:provider', method: RequestMethod.ALL }, + { path: '/webhooks/stripe', method: RequestMethod.POST }, + ]); + }); + + it(`should not return the excluded routes of another request method`, () => { + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/webhooks/:provider', + method: RequestMethod.GET, + }), + ).toEqual([ + { path: '/api/webhooks/:provider', method: RequestMethod.GET }, + ]); + }); + + it(`should return the root route of a controller excluded by an optional wildcard`, () => { + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/reports', + method: -1 as RequestMethod, + }), + ).toEqual([ + { path: '/api/reports', method: -1 }, + { path: '/reports/', method: -1 }, + ]); + }); + + it(`should match the excluded routes case-sensitively unless told otherwise`, () => { + const route = { path: '/HEALTH/:check', method: -1 as RequestMethod }; + + expect(routeInfoPathExtractor.extractExclusionsFrom(route)).toEqual([ + { path: '/api/HEALTH/:check', method: -1 }, + ]); + expect( + routeInfoPathExtractor.extractExclusionsFrom(route, { + caseSensitive: false, + }), + ).toEqual([ + { path: '/api/HEALTH/:check', method: -1 }, + { path: '/health/ready', method: -1 }, + ]); + }); + + it(`should only return the route itself when it is excluded or a wildcard`, () => { + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/health/ready', + method: -1 as RequestMethod, + }), + ).toEqual([{ path: '/health/ready', method: -1 }]); + + expect( + routeInfoPathExtractor.extractExclusionsFrom({ + path: '/{*path}', + method: -1 as RequestMethod, + }), + ).toEqual([{ path: '/{*path}', method: -1 }]); + }); + }); + describe('extractPathsFrom with a route excluded by an optional wildcard', () => { it(`should also return the parent path of the excluded route`, () => { const appConfig = new ApplicationConfig(); diff --git a/packages/core/test/security/cross-origin-protection.spec.ts b/packages/core/test/security/cross-origin-protection.spec.ts index d9d0c26a83..ded4251533 100644 --- a/packages/core/test/security/cross-origin-protection.spec.ts +++ b/packages/core/test/security/cross-origin-protection.spec.ts @@ -473,6 +473,36 @@ describe('CrossOriginProtection', () => { expect(isExcluded(protection, '/api/health')).toBe(false); }); + it('also excludes the routes excluded from the global prefix that a route covers', () => { + const protection = createProtection(['webhooks/*path'], config => { + config.setGlobalPrefix('api'); + config.setGlobalPrefixOptions({ + exclude: mapToExcludeRoute([ + { path: 'webhooks/stripe', method: RequestMethod.POST }, + ]), + }); + }); + expect(isExcluded(protection, '/webhooks/stripe')).toBe(true); + expect(isExcluded(protection, '/webhooks/stripe', 'PUT')).toBe(false); + expect(isExcluded(protection, '/webhooks/github')).toBe(false); + expect(isExcluded(protection, '/api/webhooks/github')).toBe(true); + }); + + it('matches the routes excluded from the global prefix case-sensitively', () => { + const protection = createProtection( + [{ path: 'WEBHOOKS/*path', method: RequestMethod.POST }], + config => { + config.setGlobalPrefix('api'); + config.setGlobalPrefixOptions({ + exclude: mapToExcludeRoute(['webhooks/stripe']), + }); + }, + ); + expect(isExcluded(protection, '/webhooks/stripe')).toBe(false); + expect(isExcluded(protection, '/api/webhooks/github')).toBe(false); + expect(isExcluded(protection, '/api/WEBHOOKS/github')).toBe(true); + }); + it('adds the version segment of RouteInfo.version (URI versioning)', () => { const protection = createProtection( [{ path: 'webhooks', method: RequestMethod.POST, version: '1' }],