Skip to content

Commit

Permalink
fix: AwsSolutions-APIG4 triggers for CORS preflight endpoints (#1816)
Browse files Browse the repository at this point in the history
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"
  }
}
```
  • Loading branch information
georeeve authored Oct 24, 2024
1 parent c40f725 commit 0bd0271
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
25 changes: 24 additions & 1 deletion src/rules/apigw/APIGWAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,41 @@ 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
*/
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
Expand Down
24 changes: 19 additions & 5 deletions test/rules/APIGW.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
CfnRequestValidator,
CfnRestApi,
CfnStage,
Cors,
MethodLoggingLevel,
RestApi,
} from 'aws-cdk-lib/aws-apigateway';
Expand Down Expand Up @@ -182,29 +183,42 @@ 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',
authorizerId: 'baz',
});
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', () => {
Expand Down

0 comments on commit 0bd0271

Please sign in to comment.