claudiajs / claudia

Deploy Node.js projects to AWS Lambda and API Gateway easily
https://claudiajs.com
MIT License
3.8k stars 274 forks source link

Custom authorizer resultTtl default value is not valid #166

Closed sartios closed 6 years ago

sartios commented 6 years ago

Deploy a Lambda with a custom authorizer without setting a resultTtl value should disable the custom authorizer cache.

The TTL value of the custom authorizer is undefined which results in caching the policy document for the default value of 300 seconds.

In order to solve the issue, I have to define a resultTtl with 1 as a value at registerAuthorizer configuration.

api.registerAuthorizer('customAuth', {
  lambdaName: 'customAuth',
  lambdaVersion: true,
  resultTtl: 1,
});

I have tried with zero as resultTtl value but results in set the value again as undefined.

gojko commented 6 years ago

Can I just check if I understand this correctly? Are you saying that if the value is not provided, the AWS default gets applied, or does it not work at all?

sartios commented 6 years ago

If the value is not provided or is 0, at the AWS is set as undefined, which results in caching the policy for 300 seconds

gojko commented 6 years ago

I guess the offending line would be this one:

https://github.com/claudiajs/claudia/blob/387e4dd3eb0bb70ee80bad1d00a79ce98749cd5b/src/tasks/register-authorizers.js#L81

I think the default should stay as the AWS default, otherwise we'll break a lot of backwards compatibility, but you should be able to explicitly set it to 0.

The fix would probably be to allow setting the resultTtl if it is explicitly set to 0, not just to check if it's truthy. If you'd like to submit a patch for that, I'll merge it. Please add a test to prove that 0 ttl authorisers can be set as well, it should likely be similar to this:

https://github.com/claudiajs/claudia/blob/387e4dd3eb0bb70ee80bad1d00a79ce98749cd5b/spec/register-authorizers-spec.js#L210

gojko commented 6 years ago

I'm closing this due to a week of inactivity -- if anyone wants to pick this change up later and submit a pull request, please reopen it.