feat: MFA Support

[Vinzent03]

close supabase/supabase-flutter#238

I'm currently using the part of language feature to extract the MFA methods into its own class. Don't know if we want to keep that, since it's not recommended by Dart.

[Vinzent03]

@dshukertjr Do you know what listFactors should exactly do? The documentation on the interface in js says you have to manually refresh the user's data, but the code uses getUser, which does exactly that. I think one of these is outdated.

In addition, on the website, there's only documentation about the method for admin, but not for the client.

[dshukertjr]

@Vinzent03 Sorry for the late reply here. Was on a short vacation, and am finally caught up. Thanks for getting started with this, it's awesome!

The documentation on the interface in js says you have to manually refresh the user's data, but the code uses getUser, which does exactly that. I think one of these is outdated.

I think you are right! Let me ping them to check it out.

In addition, on the website, there's only documentation about the method for admin, but not for the client.

Thanks for this too. Will ping them about this one as well!

[Vinzent03]

@dshukertjr After launch week is vacation, probably a good idea. I wanted to write some tests (which gotrue-js btw doesn't have 🤨), but the schema is too old, and I wanted to work with some seed data. Do you know if the auth team has an updated ready to go postgres schema? I tried working with gotrue/migrations, but I got many issues while applying the migrations.

[dshukertjr]

@Vinzent03 I think all the migrations inside gotrue/migrations will be applied if you run docker, but does it not?

[Vinzent03]

@dshukertjr They are, but for me it doesn't work. Some tables don't get recognized or relations are created multiple times. First of all, I had to replace all {{ index .Options "Namespace" }} with auth. Maybe this is the root of the issue, because that env var is needed by gotrue somewhere else than in the replacements in the migrations. I couldn't find a way to declare that variable.

[dshukertjr]

@Vinzent03 Are you trying to add some files from gotrue/migrations to this repo and running docker?

What I meant was that since the docker file is set to use the latest version of gotrue, when running docker-compose up it should run all the migrations and create all the necessary table schema change to bring the state the same as latest Supabase projects and there is no need to change anything from the current setup.

[Vinzent03]

@dshukertjr I don't think so, because we already create the sql schema ourselves here. I connected via psql to the local postgres instance and I could only find the tables created in the 00-schema.sql.

[dshukertjr]

@Vinzent03 I can confirm that migrations will run automatically if you can get the latest version running on your docker with the attached screenshot of my local database. You can maybe try one of these solutions to see if you can get the true latest version running on your machine!

[Vinzent03]

None of them worked, but by changing the version manually to the latest tag v2.40.1, I'm able to see them.

I think I found the issue, following this article the latest tag doesn't work as expected, there is actually an old image tagged with latest. From my understanding, the tag should be updated. What do you think ?

[dshukertjr]

@Vinzent03 Ah, nice!

Yeah, I think the latest tagged docker image should be updated. Let me leave a feedback to the auth team about it!

[Vinzent03]

@dshukertjr I'm thinking about how to test the new mfa integration and improve the tests in general supabase/supabase-flutter#236.

I would like some preexisting data, like already registered users and mfa parts. I found two approaches:

1. Insert them on database startup, which would make writing tests hard, because you'd need to restart the database all the time.
2. Write a reset postgres function, which first clears all tables and inserts the required data. I would call the function before each test, so that each test can be run without other tests to be run before. I think this requires the postgrest docker image to be added, in order to call the reset function.

What do you think?

[dshukertjr]

@Vinzent03 I like the second plan!

[dshukertjr]

@Vinzent03 Apparently we stopped updating the latest tag for security reasons, so we can probably update the docker-compose.yml file to just specify a version instead of using the latest tag.

[Vinzent03]

I think I'm finished now :tada: The testing became more complex than I thought, but I think it's worth and is a good base to improve the existing tests in another pr.

[Vinzent03]

I now remember the listFactors() discussion. What do we want now? Should we update the user or should we use the cached version? For refreshing the user data, the only way I found is using refreshSession() correct?

[dshukertjr]

@Vinzent03 Amazing! Thanks for all the hard work! I will review it when I have some time!

Should we update the user or should we use the cached version? For refreshing the user data, the only way I found is using refreshSession() correct?

Yeah, refreshSession() sounds like will do the job!

[Vinzent03]

@dshukertjr Should be everything ready now, right? Next step for me will be improving the existing gotrue tests.

[Vinzent03]

After adding the linter rule, dart fix --apply is your best friend 😉