fief-dev / fief

Users and authentication management SaaS
https://www.fief.dev
Other
538 stars 44 forks source link

Unable to reauthenticate with Google OAuth #432

Closed BlackIsBlack closed 1 month ago

BlackIsBlack commented 1 month ago

Describe the bug

This issue was found back in late 2023, and brought up in this discussion: https://github.com/orgs/fief-dev/discussions/314

Initial Authentication: Users successfully authenticate with Google OAuth on their first attempt and are directed to Fief's registration page. Post-registration, they are able to log in to my service without any issues.

Re-authentication Concern: The issue arises when the same users try to re-authenticate using Google OAuth. Instead of being logged in, they are redirected to the registration page again. When they try to register, an error message indicates that the account already exists.

It then asks you to link the account with an email, but you can't since it already exists

As you can see, all the same account_id from google, but only 1 linked account. image

To Reproduce

Steps to reproduce the behavior:

  1. Login to a tenant with google
  2. Login to a different account with username and password auth to remove the access token
  3. Try to login to the original account with google oauth

Expected behavior

You are logged back into the original account, seamlessly

frankie567 commented 1 month ago

Thank you for the detailed report. Actually, what might happen is that we're storing "dangling" OAuthAccount (i.e. user_id NULL), in case the user doesn't end the registration process fully. Thus, we may have duplicate OAuthAccount, and depending on the DB behavior we may either get the properly linked one or the dangling when logging in.

Several things we need to do to solve this:

  1. Add a SQL unique constraint on the couple account_id/oauth_provider_id to make sure on DB level we don't have duplicate accounts
  2. In the OAuth2 callback logic, account for the case where we might get a dangling OAuthAccount. Basically:
    • If there is an OAuthAccount, update the tokens in any case
    • If there is a user linked, proceed with the login logic
    • If there is no user linked, proceed with the registration logic
    • If there is no OAuthAccount create it and proceed with the registration logic
  3. Related Our get_one_or_none method doesn't check if there is more than one result. This could have helped us detect the error above earlier. We probably should change the implementation to result.scalar_one_or_none().