Skip to content
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

fix(apigateway): CORS OPTIONS method should not require auth #22402

Merged
merged 3 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion packages/@aws-cdk/aws-apigateway/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,51 @@ books.addMethod('GET', new apigateway.HttpIntegration('http://amazon.com'), {

A full working example is shown below.

[Full token authorizer example](test/authorizers/integ.token-authorizer.lit.ts).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not mind the link, since code is better represented as such, instead of READMEs, but I appreciate the lack of clicking around.

```ts
import * as path from 'path';
import * as lambda from '@aws-cdk/aws-lambda';
import { App, Stack } from '@aws-cdk/core';
import { MockIntegration, PassthroughBehavior, RestApi, TokenAuthorizer, Cors } from '../../lib';

/// !show
const app = new App();
const stack = new Stack(app, 'TokenAuthorizerInteg');

const authorizerFn = new lambda.Function(stack, 'MyAuthorizerFunction', {
runtime: lambda.Runtime.NODEJS_14_X,
handler: 'index.handler',
code: lambda.AssetCode.fromAsset(path.join(__dirname, 'integ.token-authorizer.handler')),
});

const authorizer = new TokenAuthorizer(stack, 'MyAuthorizer', {
handler: authorizerFn,
});

const restapi = new RestApi(stack, 'MyRestApi', {
cloudWatchRole: true,
defaultMethodOptions: {
authorizer,
},
defaultCorsPreflightOptions: {
allowOrigins: Cors.ALL_ORIGINS,
},
});


restapi.root.addMethod('ANY', new MockIntegration({
integrationResponses: [
{ statusCode: '200' },
],
passthroughBehavior: PassthroughBehavior.NEVER,
requestTemplates: {
'application/json': '{ "statusCode": 200 }',
},
}), {
methodResponses: [
{ statusCode: '200' },
],
});
```

By default, the `TokenAuthorizer` looks for the authorization token in the request header with the key 'Authorization'. This can,
however, be modified by changing the `identitySource` property.
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/lib/method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class Method extends Resource {

const defaultMethodOptions = props.resource.defaultMethodOptions || {};
const authorizer = options.authorizer || defaultMethodOptions.authorizer;
const authorizerId = authorizer?.authorizerId;
const authorizerId = authorizer?.authorizerId ? authorizer.authorizerId : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pardon my lack of coffee, but what's the difference?

Answer: authorizerId being "" would still lead to an undefined.

Follow-up: is that needed?


const authorizationTypeOption = options.authorizationType || defaultMethodOptions.authorizationType;
const authorizationType = authorizer?.authorizationType || authorizationTypeOption || AuthorizationType.NONE;
Expand Down
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-apigateway/lib/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { CfnResource, CfnResourceProps } from './apigateway.generated';
import { Cors, CorsOptions } from './cors';
import { Integration } from './integration';
import { MockIntegration } from './integrations';
import { Method, MethodOptions } from './method';
import { Method, MethodOptions, AuthorizationType } from './method';
import { IRestApi, RestApi } from './restapi';

export interface IResource extends IResourceBase {
Expand Down Expand Up @@ -296,6 +296,12 @@ export abstract class ResourceBase extends ResourceConstruct implements IResourc
{ statusCode: `${statusCode}`, responseParameters: integrationResponseParams, responseTemplates: renderResponseTemplate() },
],
}), {
authorizer: {
authorizerId: '',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per comment above, this would still lead to an undefined, why specify it?

authorizationType: AuthorizationType.NONE,
},
apiKeyRequired: false,
authorizationType: AuthorizationType.NONE,
methodResponses: [
{ statusCode: `${statusCode}`, responseParameters: methodResponseParams },
],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import * as path from 'path';
import * as lambda from '@aws-cdk/aws-lambda';
import { App, Stack, Duration } from '@aws-cdk/core';
import { IntegTest, ExpectedResult, Match } from '@aws-cdk/integ-tests';
import { MockIntegration, PassthroughBehavior, RestApi, TokenAuthorizer, Cors } from '../../lib';

const app = new App();
const stack = new Stack(app, 'TokenAuthorizerInteg');

const authorizerFn = new lambda.Function(stack, 'MyAuthorizerFunction', {
runtime: lambda.Runtime.NODEJS_14_X,
handler: 'index.handler',
code: lambda.AssetCode.fromAsset(path.join(__dirname, 'integ.token-authorizer.handler')),
});

const authorizer = new TokenAuthorizer(stack, 'MyAuthorizer', {
handler: authorizerFn,
});

const restapi = new RestApi(stack, 'MyRestApi', {
cloudWatchRole: true,
defaultMethodOptions: {
authorizer,
},
defaultCorsPreflightOptions: {
allowOrigins: Cors.ALL_ORIGINS,
},
});


restapi.root.addMethod('ANY', new MockIntegration({
integrationResponses: [
{ statusCode: '200' },
],
passthroughBehavior: PassthroughBehavior.NEVER,
requestTemplates: {
'application/json': '{ "statusCode": 200 }',
},
}), {
methodResponses: [
{ statusCode: '200' },
],
});

const integ = new IntegTest(app, 'apigw-token-auth', {
testCases: [stack],
});
const hostName = `${restapi.restApiId}.execute-api.${stack.region}.${stack.urlSuffix}`;
const testFunc = new lambda.Function(stack, 'InvokeFunction', {
memorySize: 250,
timeout: Duration.seconds(10),
code: lambda.Code.fromInline(`
const https = require('https');
const options = {
hostname: '${hostName}',
path: '/${restapi.deploymentStage.stageName}',
};
exports.handler = async function(event) {
console.log(event);
options.method = event.method;
if ('authorization' in event) {
options.headers = {
Authorization: event.authorization,
};
}
let dataString = '';
const response = await new Promise((resolve, reject) => {
const req = https.request(options, (res) => {
res.on('data', data => {
dataString += data;
})
res.on('end', () => {
resolve({
statusCode: res.statusCode,
body: dataString,
});
})
});
req.on('error', err => {
reject({
statusCode: 500,
body: JSON.stringify({
cause: 'Something went wrong',
error: err,
})
});
});
req.end();
});
return response;
}
`),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_16_X,
});

const invokeGet = integ.assertions.invokeFunction({
functionName: testFunc.functionName,
payload: JSON.stringify({
method: 'GET',
authorization: 'allow',
}),
});
invokeGet.expect(ExpectedResult.objectLike({
Payload: Match.stringLikeRegexp('200'),
}));

const invokeGetDeny = integ.assertions.invokeFunction({
functionName: testFunc.functionName,
payload: JSON.stringify({
method: 'GET',
authorization: 'deny',
}),
});
invokeGetDeny.expect(ExpectedResult.objectLike({
Payload: Match.stringLikeRegexp('User is not authorized to access this resource with an explicit deny'),
}));

const invokeOptions = integ.assertions.invokeFunction({
functionName: testFunc.functionName,
payload: JSON.stringify({
method: 'OPTIONS',
}),
});
invokeOptions.expect(ExpectedResult.objectLike({
Payload: Match.stringLikeRegexp('204'),
}));
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"files": {
"fec8e8354e12687c5a4b843b4e269741f53dec634946869b276f7fd1017845c3": {
"source": {
Expand All @@ -14,15 +14,15 @@
}
}
},
"d121ee9744a20c9af43e516c8fb4fe93d1ed9b26130e2db68ed9534c7104c866": {
"d48b90b340d35b9bc726b78e652d17148e2449f6f756e4377428635071f68d09": {
"source": {
"path": "TokenAuthorizerInteg.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "d121ee9744a20c9af43e516c8fb4fe93d1ed9b26130e2db68ed9534c7104c866.json",
"objectKey": "d48b90b340d35b9bc726b78e652d17148e2449f6f756e4377428635071f68d09.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Loading