supabase / auth

A JWT based API for managing users and issuing JWT tokens
https://supabase.com/docs/guides/auth
MIT License
1.29k stars 324 forks source link

Prevent MFA factor hijacking #1615

Open lauri865 opened 3 weeks ago

lauri865 commented 3 weeks ago

Bug report / security issue

Describe the bug

Currently, a new MFA can be enrolled at any time after AAL2 has been reached. Which leaves the door open to potential account hijackings if one gains access to a session ( / open browser tab) – just add a new factor and delete the old one.

It's generally a good practice to require re-verification of the 2nd factor close to making dangerous changes to the account from security POV.

I wish this was configurable in user land though, as it may be difficult to come up with a solid middle ground for all use cases here. What would be great is if GoTrue would set the request.jwt.claims current_setting with all its (authenticated) requests, similar to PostgREST, so developers could create custom triggers to customize the logic (e.g. raise an exception if the AMR claim ts is older than x seconds when DELETE'ing from auth.mfa_factors). This would open up a myriad of other use cases as well (we have at least 5 other use cases off the top of my head where we now have had to resort to contrived backend code and re-creating JWTs to add custom logic).

To Reproduce

  1. Enroll an MFA
  2. Leave your session open in a public computer
  3. Someone can enroll their own factor and unenroll yours, effectively locking you out of your account

Expected behavior

Re-verification of the 2nd factor should be required if x-amount of time has passed from initial verification.

lauri865 commented 2 weeks ago

Or if it's not possible to add request.jwt.claims to most calls, it would be useful to have it at least for the Custom Access Token hook. Especially useful for token refresh events, but I guess also for initial token creation for multi-tenant workflows, or just to lock the authentication flows to the server.

J0 commented 2 weeks ago

Hey @lauri865,

Thanks for taking time to file the feedback. To ensure I'm understanding the request correctly would the ideal scenario be a world where one of the following exist:

  1. There's an option to require re-verification of any 2nd factor before it is deleted via unenroll
  2. A way to enforce a jwt claim requirement per authentication action (e.g. for MFA unenroll the amr claim should have been done in the past 5 minutes)
  3. There's a way to access the claims in a Custom Access Token Hook in order to enforce JWT requirements during token refresh or similar

Let me know

lauri865 commented 2 weeks ago

Well, sorry for being unclear – what I initially meant is #2. First and foremost for unenroll, but there's likely other destructive actions that could benefit from that as well.

  1. is also something that would be very useful as well, but maybe a bit offtopic for the issue – e.g. preventing refreshing of old tokens after 2FA was turned on, but also other use cases. Or in our case, we add custom claims to the JWT with our backend, and would love to carry them forwards.

But the reason I arrived at 3. in the first place was more broadly speaking – if GoTrue postgres queries would expose the JWT claims of the authenticated user with e.g. unenroll calls, we could easily add our own custom triggers to control the behaviour of the 2nd option, with much less engineering effort for the Supabase team. E.g. raise an exception on deleting from auth.mfa_factors.