defaultMethodOptions should be selectively applied to CORS OPTIONS method

[whimzyLive]

Tl;DR

defaultMethodOptions should exclude options method by default or at least have an option to do that.

Read More

LambdaRestApi has defaultMethodOption, when it is specified, it gets applied to all methods for given api resource. which is awesome but doesn't work when CORS is involved.

Take this simple example,
where I want all the methods to require x-api-key api key so I specify

defaultMethodOption: {
  apiKeyRequired: true
}

On top of this I also want CORS to be enabled on all api resources so in LambdaRestApi I also specify this

defaultCorsPreflightOptions : {
  ... cors options 
} 

Because of defaultCorsPreflightOptions an additional method will be created for all my resources to support CORS but because defaultMethodOption applies on all methods, options will also have apiKeyRequired , which it shouldn't.

Environment

• CLI Version : 1.38
• Framework Version: 1.38
• Node.js Version: 12 -->
• OS :
• Language (Version): TypeScript (3.8.3) -->

This is 🐛 Bug Report

[nija-at]

Thanks for filing this issue @whimzyLive.

The 'OPTIONS' method created as part of CORS preflight setup will have to be adjusted such that only properties in defaultMethodOptions that apply in the CORS context are filtered.

For the time being, you should be able to work around this using escape hatches.

[asterikx]

I stumbled across this too.

For now, I have removed the API key requirement from the RestApi and added it to each resource instead. This isn't ideal though.

[whimzyLive]

@asterikx until issue is resolved correctly, you can use cdk escape hatches as also suggested by @nija-at.

For my case I did

    const corsMethods = this.apiV1.methods.filter(
      method => method.httpMethod === 'OPTIONS'
    );
    corsMethods.forEach(method => {
      const cfnMethod = method.node.defaultChild as CfnMethod;
      cfnMethod.addPropertyOverride('ApiKeyRequired', false);
      cfnMethod.addPropertyOverride('AuthorizationType', 'NONE');
      cfnMethod.addPropertyDeletionOverride('AuthorizerId');
    });

[asterikx]

@whimzyLive thanks for sharing! I wasn't sure how to do this.

[mhart]

There are extra issues with this and proxy support AFAICT. For example, using LambdaRestApi:

const api = new apigateway.LambdaRestApi(stack, 'DummyApi', {
  handler,
  defaultMethodOptions: { authorizationType: apigateway.AuthorizationType.IAM },
  defaultCorsPreflightOptions: { allowOrigins: apigateway.Cors.ALL_ORIGINS },
})

api.methods.filter(m => m.httpMethod === 'OPTIONS')
  .forEach(m => m.node.defaultChild.addPropertyOverride('AuthorizationType', 'NONE'))

This works in so much as the OPTIONS methods are changed correctly in the CFN output to not use auth – but I still get 403s when sending OPTIONS requests – and I believe (although happy to be wrong) it's because the auth on the ANY methods from the proxy is overriding the (lack of) auth on the OPTIONS methods on the same resources.

EDIT: this was wrong. I believe the root cause is that the CDK is failing to see the addPropertyOverride changes when calculating the hash for the Deployment – so when the CloudFormation stack is updated after adding overrides, the deployment appears not to have changed, and so CloudFormation doesn't deploy the auth changes (because CloudFormation has no auto-deploy mechanism)

I'm guessing addPropertyOverride changes are out of scope for determining the deployment hash? If not, I'll file a separate issue for that. But definitely something people should be aware of – you'll need to trigger the deployment to change some other way to get the stack to update.

EDIT2: Added an issue for this over at #9086