From 0bd0271fe117ed32022cad0ad4bd7d3fea349de6 Mon Sep 17 00:00:00 2001 From: George Reeve <116298340+georeeve@users.noreply.github.com> Date: Thu, 24 Oct 2024 20:03:21 +0100 Subject: [PATCH] fix: AwsSolutions-APIG4 triggers for CORS preflight endpoints (#1816) Fixes #1815. Currently this PR just marks all `OPTIONS` methods as compliant. I'm not sure the best way to narrow this down to specifically CORS preflight endpoints. We could try and match the template that CDK generates through its `addCorsPreflight` method, but that won't include any custom preflight endpoints which people may be using, and any changes to the CDK template may break our checks here. I took a look at the [MDN docs for the `OPTIONS` method](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/OPTIONS), and it seems like it has limited use cases outside of CORS. Perhaps we can mark them all as compliant? Here is the CloudFormation CfnMethod template which CDK generates when using the following code: ```typescript new apigateway.RestApi(this, 'TestRestApi', { restApiName: 'Test', defaultCorsPreflightOptions: { allowOrigins: apigateway.Cors.ALL_ORIGINS, }, }); ``` ```json { "apiKeyRequired": false, "authorizationType": "NONE", "httpMethod": "OPTIONS", "integration": { "type": "MOCK", "requestTemplates": { "application/json": "{ statusCode: 200 }" }, "integrationResponses": [ { "statusCode": "204", "responseParameters": { "method.response.header.Access-Control-Allow-Headers": "'Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token,X-Amz-User-Agent'", "method.response.header.Access-Control-Allow-Origin": "'*'", "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'" } } ] }, "methodResponses": [ { "statusCode": "204", "responseParameters": { "method.response.header.Access-Control-Allow-Headers": true, "method.response.header.Access-Control-Allow-Origin": true, "method.response.header.Access-Control-Allow-Methods": true } } ], "resourceId": { "Fn::GetAtt": ["Test", "RootResourceId"] }, "restApiId": { "Ref": "Test" } } ``` --- src/rules/apigw/APIGWAuthorization.ts | 25 ++++++++++++++++++++++++- test/rules/APIGW.test.ts | 24 +++++++++++++++++++----- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/rules/apigw/APIGWAuthorization.ts b/src/rules/apigw/APIGWAuthorization.ts index 01e621a789..114cb875db 100644 --- a/src/rules/apigw/APIGWAuthorization.ts +++ b/src/rules/apigw/APIGWAuthorization.ts @@ -3,11 +3,28 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: Apache-2.0 */ import { parse } from 'path'; -import { CfnResource } from 'aws-cdk-lib'; +import { CfnResource, Stack } from 'aws-cdk-lib'; import { AuthorizationType, CfnMethod } from 'aws-cdk-lib/aws-apigateway'; import { CfnRoute } from 'aws-cdk-lib/aws-apigatewayv2'; import { NagRuleCompliance, NagRules } from '../../nag-rules'; +function checkCORSMethodResponses(node: CfnMethod): boolean { + const methodResponses: CfnMethod.MethodResponseProperty[] = Stack.of( + node + ).resolve(node.methodResponses); + return methodResponses?.every((response) => { + const hasCORSResponseParameter = Object.entries( + response.responseParameters || {} + ).some( + ([key, value]) => + key.startsWith('method.response.header.Access-Control-Allow-') && + value === true + ); + + return response.statusCode === '204' && hasCORSResponseParameter; + }); +} + /** * APIs implement authorization * @param node the CfnResource to check @@ -15,6 +32,12 @@ import { NagRuleCompliance, NagRules } from '../../nag-rules'; export default Object.defineProperty( (node: CfnResource): NagRuleCompliance => { if (node instanceof CfnMethod || node instanceof CfnRoute) { + if (node instanceof CfnMethod) { + const httpMethod = NagRules.resolveIfPrimitive(node, node.httpMethod); + if (httpMethod === 'OPTIONS' && checkCORSMethodResponses(node)) { + return NagRuleCompliance.NOT_APPLICABLE; + } + } const authorizationType = NagRules.resolveIfPrimitive( node, node.authorizationType diff --git a/test/rules/APIGW.test.ts b/test/rules/APIGW.test.ts index d45ef110f5..a204c93e43 100644 --- a/test/rules/APIGW.test.ts +++ b/test/rules/APIGW.test.ts @@ -8,6 +8,7 @@ import { CfnRequestValidator, CfnRestApi, CfnStage, + Cors, MethodLoggingLevel, RestApi, } from 'aws-cdk-lib/aws-apigateway'; @@ -182,22 +183,26 @@ describe('Amazon API Gateway', () => { describe('APIGWAuthorization: APIs implement authorization', () => { const ruleId = 'APIGWAuthorization'; test('Noncompliance 1', () => { - new RestApi(stack, 'rRestApi').root.addMethod('ANY'); + new RestApi(stack, 'RestApi').root.addMethod('ANY'); validateStack(stack, ruleId, TestType.NON_COMPLIANCE); }); test('Noncompliance 2', () => { - new CfnRoute(stack, 'rRoute', { + new CfnRoute(stack, 'Route', { apiId: 'foo', routeKey: 'ANY /bar', authorizationType: 'NONE', }); validateStack(stack, ruleId, TestType.NON_COMPLIANCE); }); - test('Compliance', () => { - new RestApi(stack, 'rRestApi', { + test('Noncompliance 3', () => { + new RestApi(stack, 'RestApi').root.addMethod('OPTIONS'); + validateStack(stack, ruleId, TestType.NON_COMPLIANCE); + }); + test('Compliance 1', () => { + new RestApi(stack, 'RestApi', { defaultMethodOptions: { authorizationType: AuthorizationType.CUSTOM }, }).root.addMethod('ANY'); - new CfnRoute(stack, 'rRoute', { + new CfnRoute(stack, 'Route', { apiId: 'foo', routeKey: 'ANY /bar', authorizationType: 'CUSTOM', @@ -205,6 +210,15 @@ describe('Amazon API Gateway', () => { }); validateStack(stack, ruleId, TestType.COMPLIANCE); }); + test('Compliance 2', () => { + new RestApi(stack, 'RestApi').root.addCorsPreflight({ + allowOrigins: Cors.ALL_ORIGINS, + allowHeaders: Cors.DEFAULT_HEADERS, + allowMethods: Cors.ALL_METHODS, + allowCredentials: true, + }); + validateStack(stack, ruleId, TestType.COMPLIANCE); + }); }); describe('APIGWCacheEnabledAndEncrypted: API Gateway stages have caching enabled and encrypted for all methods', () => {