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
731 stars 178 forks source link

Unexpected AuthException during supabase auth `verifyOTP` email change flow #981

Open rmshin opened 4 months ago

rmshin commented 4 months ago

Bug report

Describe the bug

As per the Supabase docs, the default behaviour when a user requests an email change is that a confirmation email is sent to both the old and new email addresses of the user.

For the case where an OTP code is used instead of a confirmation link, the user receives two distinct OTP codes to each of their given email addresses. Both these codes must subsequently be verified via the verifyOTP API in order for the email change to take effect.

However, at least in the supabase-flutter client, calling this endpoint with a valid OTP token raises an AuthException. This is due to the following code snippet within verifyOTP defined in gotrue_client.dart:

    final response = await _fetch
        .request('$_url/verify', RequestMethodType.post, options: fetchOptions);

    final authResponse = AuthResponse.fromJson(response);

    if (authResponse.session == null) {
      throw AuthException(
        'An error occurred on token verification.',
      );
    }

Here the function expects a successful user session to be returned within the /verify API response, but in the case of secure email change the returned response is:

{code: 200, msg: Confirmation link accepted. Please proceed to confirm link sent to the other email}

Essentially, the current implementation of verifyOtp doesn't account for the two-step verification flow of email change via OTP.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Update Email Templates for email change within Supabase dashboard to send {{ .Token }} rather than {{ .ConfirmationUrl }}.
  2. Request email change for a user via updateUser API:
    await supabase.auth.updateUser(
      UserAttributes(
        email: newEmail,
      ),
    );
  3. Copy OTP code from a confirmation email sent to either address and call verifyOtp:
      final res = await supabase.auth.verifyOTP(
        email: newEmail, // or oldEmail
        token: code, // from respective email
        type: OtpType.emailChange,
      );
  4. AuthException is raised:
    AuthException(message: An error occurred on token verification., statusCode: null)
  5. Further, when verifyOTP is called with the wrong code, an AuthRetryableFetchException is raised due to a server 500 response:
      // this raises `AuthRetryableFetchException` rather than proper error message
      final res = await supabase.auth.verifyOTP(
        email: email,
        token: wrongToken, // supply an incorrect OTP token
        type: OtpType.emailChange,
      );

Expected behavior

When a user is requesting an secure email change via OTP, calling verifyOTP with a correct first token should not result in an AuthException. Instead, confirmation of the successful verification should be returned so the client can relay the appropriate feedback to the user.

Providing an incorrect code, on the other hand, should return a meaningful error message about the invalid code rather than a generic AuthRetryableFetchException.

Screenshots

Screenshot 2024-06-04 at 14 59 57 Screenshot 2024-06-04 at 14 44 59

System information

Additional context

According to the logs shown within the Supabase dashboard, the issue of 500 responses being returned from /verify when the wrong OTP token is sent seems to stem from a runtime panic somewhere in the server logic. See screenshots for additional details, and the below raw output of the event log:

{
  "event_message": "{\"component\":\"api\",\"level\":\"error\",\"method\":\"POST\",\"msg\":\"request panicked\",\"panic\":\"runtime error: invalid memory address or nil pointer dereference\",\"path\":\"/verify\",\"request_id\":\"88e772583362b345-PRG\",\"stack\":\"goroutine 252 [running]:\\nruntime/debug.Stack()\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/runtime/debug/stack.go:24 +0x64\\ngithub.com/supabase/auth/internal/api.recoverer.func1.1()\\n\\t/home/runner/work/auth/auth/internal/api/errors.go:157 +0x60\\npanic({0xaf7680?, 0x157abd0?})\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/runtime/panic.go:770 +0x124\\ngithub.com/supabase/auth/internal/api.NewAPIWithVersion.timeoutMiddleware.func5.1({0xffff42a55f60, 0x4000383b40}, 0x4000387b00)\\n\\t/home/runner/work/auth/auth/internal/api/middleware.go:362 +0x2cc\\nnet/http.HandlerFunc.ServeHTTP(0x40007cdbe0?, {0xffff42a55f60?, 0x4000383b40?}, 0xdfb258?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\ngithub.com/supabase/auth/internal/api.recoverer.func1({0xffff42a55f60?, 0x4000383b40?}, 0x40007cdbe0?)\\n\\t/home/runner/work/auth/auth/internal/api/errors.go:170 +0x78\\nnet/http.HandlerFunc.ServeHTTP(0x4000387b00?, {0xffff42a55f60?, 0x4000383b40?}, 0x8625fc?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\ngithub.com/sebest/xff.(*XFF).Handler-fm.(*XFF).Handler.func1({0xffff42a55f60, 0x4000383b40}, 0x4000387b00)\\n\\t/home/runner/go/pkg/mod/github.com/sebest/xff@v0.0.0-20160910043805-6c115e0ffa35/xff.go:128 +0x98\\nnet/http.HandlerFunc.ServeHTTP(0x1581b20?, {0xffff42a55f60?, 0x4000383b40?}, 0x20060?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\ngithub.com/go-chi/chi/v5/middleware.RequestLogger.func1.1({0xe0c120, 0x400080a2a0}, 0x4000387680)\\n\\t/home/runner/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.12/middleware/logger.go:55 +0x128\\nnet/http.HandlerFunc.ServeHTTP(0xe02f20?, {0xe0c120?, 0x400080a2a0?}, 0x5359dc?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\ngithub.com/supabase/auth/internal/api.NewAPIWithVersion.NewStructuredLogger.func3.1({0xe0c120, 0x400080a2a0}, 0x4000387680)\\n\\t/home/runner/work/auth/auth/internal/observability/request-logger.go:36 +0x124\\nnet/http.HandlerFunc.ServeHTTP(0x4000387560?, {0xe0c120?, 0x400080a2a0?}, 0x6d6200919c6759b6?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\ngithub.com/supabase/auth/internal/api.NewAPIWithVersion.AddRequestID.func4.1({0xe0c120, 0x400080a2a0}, 0x4000387560)\\n\\t/home/runner/work/auth/auth/internal/observability/request-logger.go:24 +0xd4\\nnet/http.HandlerFunc.ServeHTTP(0xe101d8?, {0xe0c120?, 0x400080a2a0?}, 0x157b9e0?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\ngithub.com/go-chi/chi/v5.(*Mux).ServeHTTP(0x40003f5ec0, {0xe0c120, 0x400080a2a0}, 0x40003705a0)\\n\\t/home/runner/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.12/mux.go:90 +0x280\\ngithub.com/supabase/auth/internal/api.(*router).ServeHTTP(0x400014c540?, {0xe0c120?, 0x400080a2a0?}, 0x40003705a0?)\\n\\t/home/runner/work/auth/auth/internal/api/router.go:55 +0x30\\ngithub.com/supabase/auth/internal/api.NewAPIWithVersion.(*Cors).Handler.func9({0xe0c120, 0x400080a2a0}, 0x40003705a0)\\n\\t/home/runner/go/pkg/mod/github.com/rs/cors@v1.9.0/cors.go:236 +0x19c\\nnet/http.HandlerFunc.ServeHTTP(0x400082fd80?, {0xe0c120?, 0x400080a2a0?}, 0x400080a2a0?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2166 +0x38\\nnet/http.serverHandler.ServeHTTP({0xe090a0?}, {0xe0c120?, 0x400080a2a0?}, 0x6?)\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:3137 +0xbc\\nnet/http.(*conn).serve(0x40000f0990, {0xe101a0, 0x400026e480})\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:2039 +0x508\\ncreated by net/http.(*Server).Serve in goroutine 1\\n\\t/opt/hostedtoolcache/go/1.22.3/x64/src/net/http/server.go:3285 +0x3f0\\n\",\"time\":\"2024-06-04T11:05:14Z\"}",
  "id": "0206335a-4708-4a43-8b90-b40e1a871456",
  "metadata": [
    {
      "message": null,
      "timestamp": null,
      "__MONOTONIC_TIMESTAMP": null,
      "CODE_FUNC": null,
      "instance_id": null,
      "status": null,
      "_CMDLINE": null,
      "method": "POST",
      "_SYSTEMD_CGROUP": null,
      "CODE_FILE": null,
      "EXECUTABLE": null,
      "_EXE": null,
      "UNIT": null,
      "level": "error",
      "_COMM": null,
      "duration": null,
      "issuer": null,
      "_LINE_BREAK": null,
      "_SOURCE_REALTIME_TIMESTAMP": null,
      "msg": "request panicked",
      "action": null,
      "login_method": null,
      "_UID": null,
      "host": "db-mvrjkxehxyapzfvxnaau",
      "PRIORITY": null,
      "_CAP_EFFECTIVE": null,
      "_PID": null,
      "INVOCATION_ID": null,
      "_SYSTEMD_UNIT": null,
      "source_type": null,
      "SYSLOG_FACILITY": null,
      "request_id": "88e772583362b345-PRG",
      "CODE_LINE": null,
      "path": "/verify",
      "component": "api",
      "project": null,
      "user_id": null,
      "auth_event": [],
      "args": [],
      "factor_id": null,
      "provider": null,
      "client_id": null,
      "_SYSTEMD_SLICE": null,
      "_SYSTEMD_INVOCATION_ID": null,
      "header": null,
      "_MACHINE_ID": null,
      "_AUDIT_LOGINUID": null,
      "_TRANSPORT": null,
      "_SELINUX_CONTEXT": null,
      "MESSAGE_ID": null,
      "__REALTIME_TIMESTAMP": null,
      "metadata": [],
      "_STREAM_ID": null,
      "metering": null,
      "time": null,
      "_GID": null,
      "_BOOT_ID": null,
      "SYSLOG_IDENTIFIER": null,
      "_AUDIT_SESSION": null,
      "error": null
    }
  ],
  "timestamp": 1717499114000000
}
Kraktoos commented 4 months ago

Have you found a fix? I am getting [AuthApiError: Error confirm email] when using the js sdk for the same purpose, when confirming the new email (the old one works fine when using verifyOtp).

rmshin commented 4 months ago

hey @Kraktoos , not sure I understand your issue as your errors look a bit different to mine. Are you confirming the new email via OTP as well? What I did as a workaround/fix is essentially:

  1. Call verifyOTP with old email + OTP code from old email
  2. This call will fail with an AuthException. I simply catch this error and ignore it.
  3. Call verifyOTP with new email + OTP code from new email
  4. This call succeeds now that both email OTP codes have been verified. Email should now be changed and a user session returned.

Hope that helps!

ryanhanks-bestow commented 1 month ago

I'm able to reproduce this against the local setup. My scenario is slightly different than above:

  1. Sign-in anonymous user
        final emailId = Uuid().v4();
        final email = '$emailId@example.com';
        await auth.signInAnonymously();
        await auth.updateUser(
          UserAttributes(email: email),
          emailRedirectTo: 'https:localhost/a',
        );
  2. Grab token from email and verify
        await auth.verifyOTP(
            type: OtpType.emailChange, token: token, email: email);

This raises the following exception:

AuthException(message: Only the token_hash and type should be provided, statusCode: 400, errorCode: validation_failed)

And corresponding error in the container log:

{"component":"api","error":"400: Only the token_hash and type should be provided","level":"info","method":"POST","msg":"400: Only the token_hash and type should be provided","path":"/verify","referer":"http://127.0.0.1:3000","remote_addr":"192.168.65.1","request_id":"d8d3d2a2-cac8-4b0a-9381-a78df66efb2b","time":"2024-09-09T04:52:56Z"}

These lines prevent us from submitting what appears to be a valid request:

https://github.com/supabase/supabase-flutter/blob/main/packages/gotrue/lib/src/gotrue_client.dart#L519-L520

Removing that assertion and making the request without the email succeeds:

        await auth.verifyOTP(
          type: OtpType.emailChange,
          tokenHash: tokenHash,
        );

Not sure what a proper fix looks like, but if someone else runs into this you can use this hack to get around it: https://github.com/ryanhanks-bestow/supabase-flutter/tree/hack-fix-verify-otp

# in your pubspec
dependency_overrides:
  gotrue:
    git:
      url: https://github.com/ryanhanks-bestow/supabase-flutter
      ref: hack-fix-verify-otp
      path: packages/gotrue