Skip to content

Commit ef72089

Browse files
authored
fix(apigateway): CORS OPTIONS method should not require auth (aws#22402)
When you create a RestApi and you provide `defaultCorsPreflightOptions` we automatically create a CORS OPTIONS method for each method. If you also provide `defaultMethodOptions` then those default options get passed through to the CORS OPTION method as well. In the case of authentication options this should not be the case. This PR explicitly sets the authentication related options to NONE values which overrides whatever is provided in `defaultMethodOptions`. I've updated an integration tests to assert that an OPTIONS call is successful (I also tested before the fix to assert that it failed). fixes aws#8615 ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 239215f commit ef72089

File tree

22 files changed

+2688
-733
lines changed

22 files changed

+2688
-733
lines changed

packages/@aws-cdk/aws-apigateway/README.md

+45-1
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,51 @@ books.addMethod('GET', new apigateway.HttpIntegration('http://amazon.com'), {
740740

741741
A full working example is shown below.
742742

743-
[Full token authorizer example](test/authorizers/integ.token-authorizer.lit.ts).
743+
```ts
744+
import * as path from 'path';
745+
import * as lambda from '@aws-cdk/aws-lambda';
746+
import { App, Stack } from '@aws-cdk/core';
747+
import { MockIntegration, PassthroughBehavior, RestApi, TokenAuthorizer, Cors } from '../../lib';
748+
749+
/// !show
750+
const app = new App();
751+
const stack = new Stack(app, 'TokenAuthorizerInteg');
752+
753+
const authorizerFn = new lambda.Function(stack, 'MyAuthorizerFunction', {
754+
runtime: lambda.Runtime.NODEJS_14_X,
755+
handler: 'index.handler',
756+
code: lambda.AssetCode.fromAsset(path.join(__dirname, 'integ.token-authorizer.handler')),
757+
});
758+
759+
const authorizer = new TokenAuthorizer(stack, 'MyAuthorizer', {
760+
handler: authorizerFn,
761+
});
762+
763+
const restapi = new RestApi(stack, 'MyRestApi', {
764+
cloudWatchRole: true,
765+
defaultMethodOptions: {
766+
authorizer,
767+
},
768+
defaultCorsPreflightOptions: {
769+
allowOrigins: Cors.ALL_ORIGINS,
770+
},
771+
});
772+
773+
774+
restapi.root.addMethod('ANY', new MockIntegration({
775+
integrationResponses: [
776+
{ statusCode: '200' },
777+
],
778+
passthroughBehavior: PassthroughBehavior.NEVER,
779+
requestTemplates: {
780+
'application/json': '{ "statusCode": 200 }',
781+
},
782+
}), {
783+
methodResponses: [
784+
{ statusCode: '200' },
785+
],
786+
});
787+
```
744788

745789
By default, the `TokenAuthorizer` looks for the authorization token in the request header with the key 'Authorization'. This can,
746790
however, be modified by changing the `identitySource` property.

packages/@aws-cdk/aws-apigateway/lib/method.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ export class Method extends Resource {
186186

187187
const defaultMethodOptions = props.resource.defaultMethodOptions || {};
188188
const authorizer = options.authorizer || defaultMethodOptions.authorizer;
189-
const authorizerId = authorizer?.authorizerId;
189+
const authorizerId = authorizer?.authorizerId ? authorizer.authorizerId : undefined;
190190

191191
const authorizationTypeOption = options.authorizationType || defaultMethodOptions.authorizationType;
192192
const authorizationType = authorizer?.authorizationType || authorizationTypeOption || AuthorizationType.NONE;

packages/@aws-cdk/aws-apigateway/lib/resource.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { CfnResource, CfnResourceProps } from './apigateway.generated';
44
import { Cors, CorsOptions } from './cors';
55
import { Integration } from './integration';
66
import { MockIntegration } from './integrations';
7-
import { Method, MethodOptions } from './method';
7+
import { Method, MethodOptions, AuthorizationType } from './method';
88
import { IRestApi, RestApi } from './restapi';
99

1010
export interface IResource extends IResourceBase {
@@ -296,6 +296,12 @@ export abstract class ResourceBase extends ResourceConstruct implements IResourc
296296
{ statusCode: `${statusCode}`, responseParameters: integrationResponseParams, responseTemplates: renderResponseTemplate() },
297297
],
298298
}), {
299+
authorizer: {
300+
authorizerId: '',
301+
authorizationType: AuthorizationType.NONE,
302+
},
303+
apiKeyRequired: false,
304+
authorizationType: AuthorizationType.NONE,
299305
methodResponses: [
300306
{ statusCode: `${statusCode}`, responseParameters: methodResponseParams },
301307
],

packages/@aws-cdk/aws-apigateway/test/authorizers/integ.token-authorizer.lit.ts

-41
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import * as path from 'path';
2+
import * as lambda from '@aws-cdk/aws-lambda';
3+
import { App, Stack, Duration } from '@aws-cdk/core';
4+
import { IntegTest, ExpectedResult, Match } from '@aws-cdk/integ-tests';
5+
import { MockIntegration, PassthroughBehavior, RestApi, TokenAuthorizer, Cors } from '../../lib';
6+
7+
const app = new App();
8+
const stack = new Stack(app, 'TokenAuthorizerInteg');
9+
10+
const authorizerFn = new lambda.Function(stack, 'MyAuthorizerFunction', {
11+
runtime: lambda.Runtime.NODEJS_14_X,
12+
handler: 'index.handler',
13+
code: lambda.AssetCode.fromAsset(path.join(__dirname, 'integ.token-authorizer.handler')),
14+
});
15+
16+
const authorizer = new TokenAuthorizer(stack, 'MyAuthorizer', {
17+
handler: authorizerFn,
18+
});
19+
20+
const restapi = new RestApi(stack, 'MyRestApi', {
21+
cloudWatchRole: true,
22+
defaultMethodOptions: {
23+
authorizer,
24+
},
25+
defaultCorsPreflightOptions: {
26+
allowOrigins: Cors.ALL_ORIGINS,
27+
},
28+
});
29+
30+
31+
restapi.root.addMethod('ANY', new MockIntegration({
32+
integrationResponses: [
33+
{ statusCode: '200' },
34+
],
35+
passthroughBehavior: PassthroughBehavior.NEVER,
36+
requestTemplates: {
37+
'application/json': '{ "statusCode": 200 }',
38+
},
39+
}), {
40+
methodResponses: [
41+
{ statusCode: '200' },
42+
],
43+
});
44+
45+
const integ = new IntegTest(app, 'apigw-token-auth', {
46+
testCases: [stack],
47+
});
48+
const hostName = `${restapi.restApiId}.execute-api.${stack.region}.${stack.urlSuffix}`;
49+
const testFunc = new lambda.Function(stack, 'InvokeFunction', {
50+
memorySize: 250,
51+
timeout: Duration.seconds(10),
52+
code: lambda.Code.fromInline(`
53+
const https = require('https');
54+
const options = {
55+
hostname: '${hostName}',
56+
path: '/${restapi.deploymentStage.stageName}',
57+
};
58+
exports.handler = async function(event) {
59+
console.log(event);
60+
options.method = event.method;
61+
if ('authorization' in event) {
62+
options.headers = {
63+
Authorization: event.authorization,
64+
};
65+
}
66+
let dataString = '';
67+
const response = await new Promise((resolve, reject) => {
68+
const req = https.request(options, (res) => {
69+
res.on('data', data => {
70+
dataString += data;
71+
})
72+
res.on('end', () => {
73+
resolve({
74+
statusCode: res.statusCode,
75+
body: dataString,
76+
});
77+
})
78+
});
79+
req.on('error', err => {
80+
reject({
81+
statusCode: 500,
82+
body: JSON.stringify({
83+
cause: 'Something went wrong',
84+
error: err,
85+
})
86+
});
87+
});
88+
req.end();
89+
});
90+
return response;
91+
}
92+
`),
93+
handler: 'index.handler',
94+
runtime: lambda.Runtime.NODEJS_16_X,
95+
});
96+
97+
const invokeGet = integ.assertions.invokeFunction({
98+
functionName: testFunc.functionName,
99+
payload: JSON.stringify({
100+
method: 'GET',
101+
authorization: 'allow',
102+
}),
103+
});
104+
invokeGet.expect(ExpectedResult.objectLike({
105+
Payload: Match.stringLikeRegexp('200'),
106+
}));
107+
108+
const invokeGetDeny = integ.assertions.invokeFunction({
109+
functionName: testFunc.functionName,
110+
payload: JSON.stringify({
111+
method: 'GET',
112+
authorization: 'deny',
113+
}),
114+
});
115+
invokeGetDeny.expect(ExpectedResult.objectLike({
116+
Payload: Match.stringLikeRegexp('User is not authorized to access this resource with an explicit deny'),
117+
}));
118+
119+
const invokeOptions = integ.assertions.invokeFunction({
120+
functionName: testFunc.functionName,
121+
payload: JSON.stringify({
122+
method: 'OPTIONS',
123+
}),
124+
});
125+
invokeOptions.expect(ExpectedResult.objectLike({
126+
Payload: Match.stringLikeRegexp('204'),
127+
}));
+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "20.0.0",
2+
"version": "21.0.0",
33
"files": {
44
"fec8e8354e12687c5a4b843b4e269741f53dec634946869b276f7fd1017845c3": {
55
"source": {
@@ -14,15 +14,15 @@
1414
}
1515
}
1616
},
17-
"d121ee9744a20c9af43e516c8fb4fe93d1ed9b26130e2db68ed9534c7104c866": {
17+
"d48b90b340d35b9bc726b78e652d17148e2449f6f756e4377428635071f68d09": {
1818
"source": {
1919
"path": "TokenAuthorizerInteg.template.json",
2020
"packaging": "file"
2121
},
2222
"destinations": {
2323
"current_account-current_region": {
2424
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
25-
"objectKey": "d121ee9744a20c9af43e516c8fb4fe93d1ed9b26130e2db68ed9534c7104c866.json",
25+
"objectKey": "d48b90b340d35b9bc726b78e652d17148e2449f6f756e4377428635071f68d09.json",
2626
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
2727
}
2828
}

0 commit comments

Comments
 (0)