v1.x: setAuthCookies crashes on some Firebase Auth errors and swallows the root cause exception

[valeriangalliat]

Describe the bug

In my particular case, although it could happen in other situations, admin.auth().verifyIdToken(token) throws with:

auth/argument-error FirebaseAuthError: Firebase ID token has incorrect "aud" (audience) claim. Expected "emulator" but got "my-project".

This error is not the reason I'm opening the issue - that's for me to fix on my setup. The problem is that this error was swallowed and I had no visibility until I added debugging instructions to the minified source code of next-firebase-auth to see what's going on internally. 😄

The error is caught and results in returning an unauthenticated user object (without calling onVerifyTokenError, so we don't have visibility on the error).

In getCustomIdAndRefreshTokens (called by setAuthCookies), we require the user to be authenticated:

https://github.com/gladly-team/next-firebase-auth/blob/v1.x/src/firebaseAdmin.js#L144-L149

admin.auth().createCustomToken(AuthUser.id) is called with an unauthenticated user where id is null, resulting in the following error:

FirebaseAuthError: `uid` argument must be a non-empty string uid.

This error is a bit obscure and it would be more useful to have visibility on the underlying Firebase Auth error

Versions

next-firebase-auth version: v1.x (1.0.0-canary.9 specifically but the issue is present in the v1.x branch as of time of writing)
Firebase JS SDK: 9.8.3
Next.js: 12.1.6

To Reproduce

To be fair I'm still unclear why I'm getting JWT with the wrong aud when running in emulator mode, but for sure having access to the underlying exception from Firebase Auth would have been helpful to debug!

Expected behavior

Since getCustomIdAndRefreshTokens requires an authenticated user, it should handle the case where verifyIdToken returns an unauthenticated user and propagate the original exception to make debugging easier.

[kmjennison]

Thanks for the issue! This makes sense.

Do you think it would have solved your headache if we called onVerifyTokenError when 'auth/argument-error' isn't handled here?

We also probably should verify a user has an ID before calling createCustomToken here—if not, we can throw, then handle the error in setAuthCookies to to set unauthed cookies or skip setting cookies.

I'm also thinking more debugging logs could help so that you'd be able to solve this with debug: true rather than digging through source.

Let me know if this all makes sense to you.

As an aside, people have seemed to have problems with the emulator, which could be affecting you:
#411

[valeriangalliat]

Thanks for the quick reply!!

Definitely calling onVerifyTokenError on auth/argument-error would make it easier to debug 😄

We also probably should verify a user has an ID before calling createCustomToken

Agreed! I was thinking of somehow re-throwing the original error from verifyIdToken but it seems like it would require a bit of refactoring, or an alternative version of verifyIdToken that throws instead of returning an unauthenticated user... it's probably easier to rely on onVerifyTokenError and then throw a separate exception if the user is unauthenticated

Also thanks for the pointer to #411, in the end it turns out that I had configured projectId: 'emulator' but my emulator was running as my-project (the default firebase use for that directory) and I had to do firebase --project emulator emulators:start to make everything work 😇

[kmjennison]

Closed in #540.

@valeriangalliat These changes are live in v1.0.0-canary.16. If you have a few minutes, could you reproduce your prior error on the new version to see if the improved error logging is helpful or if you have any other recommendations? Thanks!

[valeriangalliat]

Awesome, thank you! I can confirm the root cause error is reported in the logs now, without cascading down to empty uid error 🙇