Is it bad practice or major security risk to cache decoded auth tokens in my backend?

Question

I've set up a firebase passport strategy on a NestJS server which works fine, but I did not like the long load times it would incur on all requests that went through it. So I decided to cache decoded tokens until they are expired, and this this massively reduced load times for valid and unexpired tokens.

However, I am concerned that there might be security risks connected to this. Mostly because this seemed like such a simple addition to me, someone must have thought about it before. I assume the people who made the firebase sdk must have considered adding it as a feature, but why haven't they?

For reference, here's the code for my passport strategy:

@Injectable()
export class FirebaseAuthStrategy extends PassportStrategy(Strategy, 'firebase-auth') {
  private defaultApp: any;

  constructor(
    private configService: ConfigService,
    @Inject(CACHE_MANAGER) private cacheManager: Cache
  ) {
    super({
      jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken()
    });
    const config = this.configService.get<string>('FIREBASE_CONFIG');
    if (!config) {
      throw new Error('FIREBASE_CONFIG not available. Please ensure the variable is supplied in the `.env` file.');
    }
    const firebase_params = JSON.parse(config);

    this.defaultApp = firebase.initializeApp({
      credential: firebase.credential.cert(firebase_params)
    });
  }

  async validate(token: string) {
    const cachedFirebaseUser = await this.cacheManager.get(token);
    if (cachedFirebaseUser) return cachedFirebaseUser;

    const firebaseUser: any = await this.defaultApp
      .auth()
      .verifyIdToken(token, true)
      .catch((err) => {
        console.log(err);
        throw new UnauthorizedException(err.Message);
      });

    if (!firebaseUser) {
      throw new UnauthorizedException();
    }

    /**
     * input parameter for `Date` or `moment` constructor is in milliseconds for unix timestamps.
     * input * 1000 will instantiate correct Date or moment value.
     * See here for reference: https://stackoverflow.com/a/45370395/5472560
     */
    const exp = moment(+firebaseUser['exp'] * 1000);
    const now = moment.now();
    const ttl = exp.diff(now, 'seconds');

    await this.cacheManager.set(token, firebaseUser, { ttl });

    return firebaseUser;
  }
}

Answer 1

You seem to just store the access tokens server-side and use it's string value to determine whether it is cached to avoid validating the signature again. In theory I don't see anything wrong with this approach although it somehow wrecks the advantages you get from the statelessness provided by bearer tokens. In practice I see a few partially potential issues with the implementation.

 const cachedFirebaseUser = await this.cacheManager.get(token);

You should ensure that the token value you use as a key includes the signature, otherwise invalid tokens with the exact same header and body parameters will be erroneously validated. Also note that checking for any nonces is impossible with this approach.

if (cachedFirebaseUser) return cachedFirebaseUser;

I don't know how your implementation of removing expired tokens looks like, but you might have expired tokens slipped through if there is a huge time skew occurring in your cache.

    const exp = moment(+firebaseUser['exp'] * 1000);
    const now = moment.now();
    const ttl = exp.diff(now, 'seconds');

    await this.cacheManager.set(token, firebaseUser, { ttl });

I am not familiar with firebase, but this doesn't specify whether tokens are signed, encrypted or neither/both.

I assume the people who made the firebase sdk must have considered adding it as a feature

To reiterate on what I mentioned a the top, they probably did not implement it because it wrecks the idea behind bearer tokens not needing any stateful session management on the server side. I don't know what algorithms you are using, but token validation is not generally something that's prone to produce a bottleneck. Again, if implemented thoroughly the approach is not unsecure, but as usual rolling out your own security middleware possesses the risk of bugs slipping through and breaking the security. My recommendation would be to get back to validating tokens with APIs the community has been thoroughly improving on for years and rather focus on determining why the token validation runs so slow in your environment; maybe even try whether other APIs provide better performance.

Answer 2

The risks I see are;

1. If the credential is revoked, the cache will continue serving a response that is no longer valid. The revocation cannot invalidate your cache, you would have to still check the validity of the cached data if you want to avoid serving invalid data from cache

2. Caches have many common issues that your code simply setting a value as you have shown do not address at all; poisoning can easily be achieved through the use of a deterministic key that is derived from a user's data, which an attacker can leverage and it isn't hard to observe with no knowledge of your source code (because it is so simple and very common the way you wrote it). The best case is they over-write a value with harmless data, the bad outcome is the value they write is malicious and served to other users (Stored XSS)

3. Another is a cache stampede, you may have concurrent requests that come which will write to the cache and try to read before the first write persists so the second will also go fetch the source and try to save it too. There are many problems with not handling concurrency of keys and source look up when a cache is leveraged and this issue makes the cache redundant entirely

4. when a permission is changed on the source, the cached credential may not carry that through until it has expired

5. an exploit on a client side will be able to masquerade as the user, and without validating the action until the cached ttl is reached you allow this to go unnoticed and the purpose of the auth validation you bypassed was to stop this exact issue