supabase / supabase-flutter

Flutter integration for Supabase. This package makes it simple for developers to build secure and scalable products.
https://supabase.com/
MIT License
667 stars 156 forks source link

Redirect link in email change confirmation email wrong #961

Open Mr-Pepe opened 1 week ago

Mr-Pepe commented 1 week ago

Bug report

Describe the bug

I have a Flutter web app.

Calling updateUser with a new email sends two confirmation emails.

Both emails contain a confirmation link ending with &type=email_change&redirect_to=http://localhost:8000/#/settings?enableUserSignIn.

However, clicking one of the two links redirects to http://localhost:8000/#message=Confirmation+link+accepted.+Please+proceed+to+confirm+link+sent+to+the+other+email.

Clicking the other link (order doesn't seem to matter) redirects to http://localhost:8000/#/settings?enableUserSignIn#access_token=eyJhbGciblablabla&token_type=bearer&type=email_change which is also not a valid URL with the hashtag instead of an ampersand.

Expected behavior

Supabase honors the redirect link I provided and adds query parameters correctly.

System information

Additional context

I couldn't find any documentation on how the workflow should be implemented correctly. Not gonna lie, implementing basic user management functionalities with Supabase (Auth) and its Flutter client has been mostly disappointing so far due to stuff like https://github.com/supabase/supabase/issues/27554, https://github.com/supabase/supabase-flutter/issues/937, and https://github.com/supabase/auth/issues/1517.

j4w8n commented 1 week ago

The #access_token... is part of the implicit auth flow and should be automatically parsed by the supabase client - assuming there's a supabase client running on your /#/settings page.

Having said that, I've never tried it when it's preceded by a dev's query param, but I assume it ought to still work.

Mr-Pepe commented 6 days ago

assuming there's a supabase client running on your /#/settings page.

Yes, there is.

I am not sure whether the access_token gets parsed correctly, because accessing client.auth.currentUser?.email still yields the old email address. I'd expect it to be the new email.

The first redirect link completely dropping the path (#/settings) looks like a clear bug to me in any case.

encima commented 6 days ago

Can you provide the Redirect URLs you have set, as well as your Site URL?

Mr-Pepe commented 6 days ago

Authentication is not live yet, due to the issues I have faced with Supabase Auth. I can try to stitch together a minimal example though.

Mr-Pepe commented 6 days ago

Here you go.

  1. flutter create supabase_auth
  2. flutter pub add supabase_flutter
  3. flutter pub add go_router
  4. Paste the following into main.dart

    main.dart

    ```dart import 'package:flutter/material.dart'; import 'package:go_router/go_router.dart'; import 'package:supabase_flutter/supabase_flutter.dart'; final _router = GoRouter( routes: [ GoRoute( path: '/', builder: (context, state) => const HomePage(), ), GoRoute( path: "/settings", builder: (context, state) => const SettingsPage(), ) ], ); final supabase = Supabase.instance.client; Future main() async { await Supabase.initialize( url: const String.fromEnvironment("SUPABASE_URL"), anonKey: const String.fromEnvironment("SUPABASE_ANON_KEY"), ); runApp(const MyApp()); } class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return MaterialApp.router( routerConfig: _router, ); } } class HomePage extends StatefulWidget { const HomePage({super.key}); @override State createState() => _HomePageState(); } class _HomePageState extends State { @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(title: const Text("Home")), body: Center( child: ElevatedButton( child: const Text("Go to settings"), onPressed: () => context.go("/settings"), ), )); } } class SettingsPage extends StatefulWidget { const SettingsPage({super.key}); @override State createState() => _SettingsPageState(); } class _SettingsPageState extends State { final _signUpEmailController = TextEditingController(); final _signUpPasswordController = TextEditingController(); final _oldEmailController = TextEditingController(); final _newEmailController = TextEditingController(); @override void dispose() { _signUpEmailController.dispose(); _signUpPasswordController.dispose(); _oldEmailController.dispose(); _newEmailController.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(title: const Text("Settings")), body: Center( child: ConstrainedBox( constraints: const BoxConstraints(maxWidth: 400), child: Column( children: [ if (supabase.auth.currentSession != null) Text("Logged in as ${supabase.auth.currentUser!.email}") else const Text("Not logged in"), const SizedBox(height: 50), const Text("Sign up"), TextFormField( controller: _signUpEmailController, decoration: const InputDecoration(label: Text("Email")), ), TextFormField( controller: _signUpPasswordController, decoration: const InputDecoration(label: Text("Password")), ), const SizedBox(height: 20), ElevatedButton( onPressed: () async { await supabase.auth.signUp( email: _signUpEmailController.text.trim(), password: _signUpPasswordController.text.trim(), emailRedirectTo: "http://localhost:8000/#/settings"); }, child: const Text("Sign up"), ), const SizedBox(height: 50), const Text("Change email"), TextFormField( controller: _oldEmailController, decoration: const InputDecoration(label: Text("Old email")), ), TextFormField( controller: _newEmailController, decoration: const InputDecoration(label: Text("New email")), ), const SizedBox(height: 20), ElevatedButton( onPressed: () async { await supabase.auth.updateUser( UserAttributes(email: _newEmailController.text.trim()), emailRedirectTo: "http://localhost:8000/#/settings"); }, child: const Text("Change email")) ], ), ))); } } ```

  5. flutter run -d web-server --web-hostname localhost --web-port 8000 --dart-define=SUPABASE_URL=<your-supabase-url> --dart-define=SUPABASE_ANON_KEY=<your-anon-key>
  6. Go to http://localhost:8000
  7. Navigate to settings -> It says "Not logged in" at the top
  8. Sign up with some email address xxx
  9. Click the link in the confirmation email
  10. Get redirected to http://localhost:8000/?code=a6eb8fb0-3bcd-4ba8-9756-8ce3f3f1a8f8#/settings (already weird URL formatting) -> It says "Logged in as xxx" at the top
  11. Change email from xxx to yyy
  12. Click the link in the confirmation email sent to xxx
  13. Get redirected to http://localhost:8000/#message=Confirmation+link+accepted.+Please+proceed+to+confirm+link+sent+to+the+other+email -> #/settings got dropped, leading to the following UI:

    image

  14. Click the link in the confirmation email sent to yyy
  15. Get redirected to http://localhost:8000/#/settings#access_token=eyJhbGci....&expires_at=1719827394&expires_in=3600&refresh_token=G_sLgwu.....&token_type=bearer&type=email_change -> It still says "Logged in as xxx" at the top although the email has changed in auth.users
Mr-Pepe commented 6 days ago

For extra fun:

  1. Add a _passwordResetController
  2. Add the following to the list of widgets
    const SizedBox(height: 50),
    const Text("Reset password"),
    TextFormField(
      controller: _passwordResetController,
      decoration: const InputDecoration(label: Text("Email")),
    ),
    const SizedBox(height: 20),
    ElevatedButton(
        onPressed: () async {
          await supabase.auth.resetPasswordForEmail(
              _passwordResetController.text.trim(),
              redirectTo: "http://localhost:8000/#/settings");
        },
        child: const Text("Reset password"))
  3. Click the link in the password reset email once -> No issues
  4. Click the link in the password reset email again
  5. Get redirected to http://localhost:8000/?error=access_denied&error_code=403&error_description=Email+link+is+invalid+or+has+expired#error=access_denied&error_code=403&error_description=Email+link+is+invalid+or+has+expired -> This seems similar to step 13 from above, but in addition to dropping the #/settings path, it also adds all the parameters twice :man_shrugging:
encima commented 6 days ago

Not sure if this is "fun" but it certainly seems problematic :D Thanks for the detailed repro guide also, I will work on getting this reproduced and triaged!

encima commented 5 days ago

Transferring this to the flutter repo as I can reproduce and it seems to be 2 things (which may be Auth bugs):

  1. The SITE_URL or emailRedirectTo is not respected and redirects seem to default to root domain
  2. With secure email change enabled, this is a visible bug that is user facing, without it, it is less obvious