Skip to content

Commit

Permalink
fix(apigateway): CORS OPTIONS method should not require auth (#22402)
Browse files Browse the repository at this point in the history
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 #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*
  • Loading branch information
corymhall authored Oct 12, 2022
1 parent 239215f commit ef72089
Show file tree
Hide file tree
Showing 22 changed files with 2,688 additions and 733 deletions.
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).
```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;

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: '',
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

0 comments on commit ef72089

Please sign in to comment.