search

supabase / cli

Supabase CLI. Manage postgres migrations, run Supabase locally, deploy edge functions. Postgres backups. Generating types from your database schema.
https://supabase.com/docs/reference/cli/about
MIT License
1.07k stars 209 forks source link

MFA enrollment not detecting AAL properly in CLI version #2404

Closed lauri865 closed 5 months ago

lauri865 commented 5 months ago

Describe the bug When enrolling a new MFA factor with Supabase CLI version of Gotrue/Auth, AAL is not checked properly, hence users with AAL1 can generate a new TOTP even if there's an active MFA factor and they haven't verified for AAL2. We thought we had a serious security vulnerability due to that (and wasted a ton of time debugging it), but seems like the cloud version luckily doesn't experience it. But we now have mismatching environments with dev/prod with regards to this.

To Reproduce Steps to reproduce the behavior:

  1. Add and verify an MFA token
  2. Sign out
  3. Sign in with email/oauth only
  4. supabase.auth.mfa.enroll succeeds regardless of AAL being AAL1 only

Expected behavior It should error: "AAL2 required to enroll a new factor", which it does in production/cloud environment, but not locally.

Screenshots If applicable, add screenshots to help explain your problem.

System information Rerun the failing command with --create-ticket flag.

lauri865 commented 5 months ago

Seems like we were stuck on an old version of the Gotrue even though we've been updating the CLI continously. Do we need to manually run supabase link all the time to keep the versions in sync?

sweatybridge commented 5 months ago

Do we need to manually run supabase link all the time to keep the versions in sync?

CLI prints a reminder when it finds outdated auth service version when you run supabase start. But generally yes, you need to manually link again to update.

lauri865 commented 5 months ago

Thanks @sweatybridge. It was a complete surprise for me, as I thought the discrepancy came from out of date CLI previously, and deps were update alongside the CLI automatically.

I'm failing to see why this should be the default behaviour though. I'm sure there are use cases, but I'd rather have them in sync by default and have a flag to change that behaviour. Reduces the amount of surprises imho.

I've also been conditioned by this warning, and learned to ignore it over time, because it's not always possible to get rid of it by linking if the CLI hasn't been updated (iirc), there were cases where I tried to link over and over again, but the warning still popped up, as CLI didn't have access to the same version of deps as cloud or something along the lines. I'm not sure if that has changed by now, but that's the main reason the warning has lost any information value to me.

sweatybridge commented 5 months ago

Hmm you are right, this does seem like a regression for users who upgrade CLI regularly but don't link as often. I will look into improving the auto update process further.