-
Notifications
You must be signed in to change notification settings - Fork 536
fix(module): filter header params that duplicate security schemes #3785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,142 @@ | ||||||||||||||||||||||||||||||||||||||||
| import { Controller, Get, Headers } from '@nestjs/common'; | ||||||||||||||||||||||||||||||||||||||||
| import { NestFactory } from '@nestjs/core'; | ||||||||||||||||||||||||||||||||||||||||
| import { DocumentBuilder, SwaggerModule } from '../lib'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| describe('SwaggerModule - Security scheme header filtering', () => { | ||||||||||||||||||||||||||||||||||||||||
| @Controller('test') | ||||||||||||||||||||||||||||||||||||||||
| class TestController { | ||||||||||||||||||||||||||||||||||||||||
| @Get('bearer') | ||||||||||||||||||||||||||||||||||||||||
| getWithAuthHeader(@Headers('Authorization') auth: string) { | ||||||||||||||||||||||||||||||||||||||||
| return auth; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| @Get('custom-header') | ||||||||||||||||||||||||||||||||||||||||
| getWithCustomHeader(@Headers('X-Custom') custom: string) { | ||||||||||||||||||||||||||||||||||||||||
| return custom; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| let app; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| beforeAll(async () => { | ||||||||||||||||||||||||||||||||||||||||
| app = await NestFactory.create( | ||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||
| module: class {}, | ||||||||||||||||||||||||||||||||||||||||
| controllers: [TestController] | ||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||
| { logger: false } | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| afterAll(async () => { | ||||||||||||||||||||||||||||||||||||||||
| await app.close(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| it('should filter out Authorization header parameter when bearer auth is configured', () => { | ||||||||||||||||||||||||||||||||||||||||
| const config = new DocumentBuilder() | ||||||||||||||||||||||||||||||||||||||||
| .setTitle('Test') | ||||||||||||||||||||||||||||||||||||||||
| .setVersion('1.0') | ||||||||||||||||||||||||||||||||||||||||
| .addBearerAuth() | ||||||||||||||||||||||||||||||||||||||||
| .addSecurityRequirements('bearer') | ||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const document = SwaggerModule.createDocument(app, config); | ||||||||||||||||||||||||||||||||||||||||
| const params = document.paths['/test/bearer']?.get?.parameters || []; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // The Authorization header should NOT appear as a parameter | ||||||||||||||||||||||||||||||||||||||||
| const authParam = params.find( | ||||||||||||||||||||||||||||||||||||||||
| (p: any) => p.in === 'header' && p.name === 'Authorization' | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| expect(authParam).toBeUndefined(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| it('should filter out Authorization header parameter when basic auth is configured', () => { | ||||||||||||||||||||||||||||||||||||||||
| const config = new DocumentBuilder() | ||||||||||||||||||||||||||||||||||||||||
| .setTitle('Test') | ||||||||||||||||||||||||||||||||||||||||
| .setVersion('1.0') | ||||||||||||||||||||||||||||||||||||||||
| .addBasicAuth() | ||||||||||||||||||||||||||||||||||||||||
| .addSecurityRequirements('basic') | ||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const document = SwaggerModule.createDocument(app, config); | ||||||||||||||||||||||||||||||||||||||||
| const params = document.paths['/test/bearer']?.get?.parameters || []; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const authParam = params.find( | ||||||||||||||||||||||||||||||||||||||||
| (p: any) => p.in === 'header' && p.name === 'Authorization' | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| expect(authParam).toBeUndefined(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| it('should filter out custom API key header parameter when apiKey scheme uses it', () => { | ||||||||||||||||||||||||||||||||||||||||
| const config = new DocumentBuilder() | ||||||||||||||||||||||||||||||||||||||||
| .setTitle('Test') | ||||||||||||||||||||||||||||||||||||||||
| .setVersion('1.0') | ||||||||||||||||||||||||||||||||||||||||
| .addApiKey({ type: 'apiKey', in: 'header', name: 'X-Custom' }, 'custom') | ||||||||||||||||||||||||||||||||||||||||
| .addSecurityRequirements('custom') | ||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const document = SwaggerModule.createDocument(app, config); | ||||||||||||||||||||||||||||||||||||||||
| const params = | ||||||||||||||||||||||||||||||||||||||||
| document.paths['/test/custom-header']?.get?.parameters || []; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const customParam = params.find( | ||||||||||||||||||||||||||||||||||||||||
| (p: any) => p.in === 'header' && p.name === 'X-Custom' | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| expect(customParam).toBeUndefined(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| it('should NOT filter out non-security header parameters', () => { | ||||||||||||||||||||||||||||||||||||||||
| const config = new DocumentBuilder() | ||||||||||||||||||||||||||||||||||||||||
| .setTitle('Test') | ||||||||||||||||||||||||||||||||||||||||
| .setVersion('1.0') | ||||||||||||||||||||||||||||||||||||||||
| .addBearerAuth() | ||||||||||||||||||||||||||||||||||||||||
| .addSecurityRequirements('bearer') | ||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const document = SwaggerModule.createDocument(app, config); | ||||||||||||||||||||||||||||||||||||||||
| const params = | ||||||||||||||||||||||||||||||||||||||||
| document.paths['/test/custom-header']?.get?.parameters || []; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // X-Custom header should still appear since it's not a security header | ||||||||||||||||||||||||||||||||||||||||
| const customParam = params.find( | ||||||||||||||||||||||||||||||||||||||||
| (p: any) => p.in === 'header' && p.name === 'X-Custom' | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| expect(customParam).toBeDefined(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| it('should not filter any headers when no security schemes are configured', () => { | ||||||||||||||||||||||||||||||||||||||||
| const config = new DocumentBuilder() | ||||||||||||||||||||||||||||||||||||||||
| .setTitle('Test') | ||||||||||||||||||||||||||||||||||||||||
| .setVersion('1.0') | ||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const document = SwaggerModule.createDocument(app, config); | ||||||||||||||||||||||||||||||||||||||||
| const params = document.paths['/test/bearer']?.get?.parameters || []; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Authorization should still appear since there are no security schemes | ||||||||||||||||||||||||||||||||||||||||
| const authParam = params.find( | ||||||||||||||||||||||||||||||||||||||||
| (p: any) => p.in === 'header' && p.name === 'Authorization' | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| expect(authParam).toBeDefined(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
| it('should not filter security headers when schemes are defined without requirements', () => { | |
| const config = new DocumentBuilder() | |
| .setTitle('Test') | |
| .setVersion('1.0') | |
| .addBearerAuth() | |
| .build(); | |
| const document = SwaggerModule.createDocument(app, config); | |
| const params = document.paths['/test/bearer']?.get?.parameters || []; | |
| // Authorization should still appear since bearer auth is defined | |
| // but no document-level or operation-level security requirement uses it | |
| const authParam = params.find( | |
| (p: any) => p.in === 'header' && p.name === 'Authorization' | |
| ); | |
| expect(authParam).toBeDefined(); | |
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filterSecuritySchemeHeaders()filters header parameters whenever any security scheme exists incomponents.securitySchemes, even if that scheme is never applied viadocument.securityoroperation.security. This can incorrectly remove legitimate@Headers('Authorization')/ apiKey header parameters on operations that are not secured (or when the project only defines schemes to show the Authorize dialog but doesn’t add security requirements). Consider filtering per-operation only when the effective security requirements (operation.security if defined, else document.security) are non-empty, and only for the scheme names referenced there (respectingoperation.security = []overriding global security).