Add file-cache option for AWS Lambda

[vanhumbeecka]

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

The caching option where cache = true does not work on AWS Lambda environments, resulting in too many JWKS endpoint calls.

This PR extends the options object of passportJwtSecret with an extra option called useTmpFileCache (optional boolean, default = false).
When enabled, the caching layer will use local file-storage for caching under /tmp folder.

This plays nicely with serverless like AWS Lambda (tested), but also GCP App Engine (not tested), which both uses the /tmp location for local file storage which is shared between multiple instances of the same service.

Tests have been added to the tests folder.

Testing

See the tests folder. mock-fs has been used for mocking the file-system.

Checklist

• I have added documentation for new/changed functionality in this PR or in auth0.com/docs
• All active GitHub checks for tests, formatting, and security are passing
• The correct base branch is being used, if not master

[vanhumbeecka]

Feedback on this PR is welcome.
I've added documentation in the Readme.md for clarification.

[DaMo86]

would be a great improvement - waiting for the merge 👍

[oscarwest]

Looking forward to seeing this merged.

[iacoshoria]

Looking forward for this feature. 🚀

[davidpatrick]

@vanhumbeecka thanks for the terrific contribution. Are you able to update this PR with the changes from #149 introduced by @oscarwest ? Or should we favor that PR? Thanks 🙏

[vanhumbeecka]

Hi @davidpatrick, I'll update the PR based on feedback in this thread.
Should be able to do this in the next couple of days.

[vanhumbeecka]

@davidpatrick Ok, so I changed this PR by changing the writeFile to writeFileSync as requested. Thereby this change also required me to add try-catch blocks instead of callbacks.
The tests remained unaffected. (I've not changed the tests according to #149 , since there, the done callback is incorrectly called outside of the callbacks where the checks happen with expect.)

Not sure why the codecov project is failing though...

[davidpatrick]

Brilliant thanks @vanhumbeecka 🙇 I will take a look

[davidpatrick]

@vanhumbeecka thanks for having the patience on this PR. I recently had some more bandwidth to dive a little deeper into this library and discuss with the team some of the requests/issues surrounding this library.

It seems the crux of the issue is that in-memory caching doesn't work for environments with multiple nodes/servers.

We appreciate raising this issue, and the accompanying PR, however the proposed PR has a couple of issues:

• No cache invalidation, Once it is written to /tmp/jwks-cache there is no invalidation of the cache allowing a rotated set to come in
• No customization of tmpFileCache location
• Too tightly coupled to this use-case -- No flexibility of cache implementation

A more flexible approach we think would be to allow the user to replace the memoizer(in-memory) cache with their own cache implementation. Similair to our sister library and its dummy-cache.

An example of what a custom cache might look like:

// This would belong to the client/consumer of this library
class CustomCache = {
  get(kid, getSigningKey) {
    const key = this.has(kid);
    if (key) { return Promise.resolve(key); ****}

    return new Promise((resolve, reject) => {
      getSigningKey(kid, (err, key) => {
        if (err) { return reject(err); }

        this.set(kid, key);
        return resolve(key);
      }
      );
    });
  },
  has(kid) {
    // if cache invalid (expired?) -- this.get
    // if cache valid -- retrieveFromCacheStore(kid)
  },
  set(kid, key) {
    // saveInCacheStore(kid, key)
  }
};

const customCache = new CustomCache();
const client = jwksClient({ cache: true, customCache })

Then inside of the cache wrapper we could implement it like so:

// ./src/wrappers/cache.js
export default function(client, { customCache, cacheMaxEntries = 5, cacheMaxAge = ms('10m') } = options) {
  const logger = debug('jwks');
  const getSigningKey = client.getSigningKey;

  if (customCache) {
    return customCache.get(kid, getSigningKey)
      .then(key => cb(null, key))
      .catch(err => cb(err));
  }

  return memoizer({
    //..//
  });

Let me know what you think and if you can see any instances where this wouldn't work for you. I can take ownership of the implementation, or if this is something that you want to take on I can assist.

[vanhumbeecka]

Hi @davidpatrick, I can certainly agree with your assessment.
But I guess it's a choice between flexibility and somewhat ease-of-use. To respond to your statements:

• No cache invalidation, Once it is written to /tmp/jwks-cache there is no invalidation of the cache allowing a rotated set to come in
• Too tightly coupled to this use-case -- No flexibility of cache implementation

The current implementation is indeed specific to serverless environments. That also means that the /tmp location will automically be cleared from time to time (thereby invalidating it), although you don't really have control over the specific moments. (as is the case with AWS Lambda)

• No customization of tmpFileCache location

True, but again, I went for 'easy-of-use' in this implementation, meaning adding a single flag to 'true' worked without worrying about configuration.

That being said, I guess most developers actually do want more flexibility and control, so I can't really argue with your statements in that regard.

So feel free to take over from here if you want to 👍 .
I'm already happy this issue gets attention to get fixed!

[davidpatrick]

@vanhumbeecka perfect thanks for the quick response. I will be tackling this next week. Thanks again for raising the issue 🙇