Skip to content

Commit

Permalink
In create_oauth, link existing email account
Browse files Browse the repository at this point in the history
To elaborate, suppose we create an account

[email protected]
PASSWORD

in the username-password registration flow.

Before this change, if you were to use the

[email protected]

Google OAuth, then you would create a new OAuth account. This is clearly
undesireable behavior.

There are two approaches we can take to address this.

- We can reject the registration, explaining in depth that the email
  [email protected] is already taken by an email account. This is likely
  to be frustrating for the user, but it doesn't do any "sneaky action
  at a distance".
- We can silently merge the email and Google OAuth accounts if we detect
  that there is already an account with the email [email protected]. I
  believe this is the better approach to take.

  This might be a worse approach in something like an online game, where
  people might want to have multiple, separate accounts. But RoboList
  does not fall into that category. For here, users are merely a set of
  *permissions* associated with a set of *authentication* procedures,
  and taking the union of both is maximally convenient for the user. So
  merging accounts is almost always good.

Now, there is an odd thing that can happen. If I were to register on
GitHub with [email protected], and then use that to register for
RoboList, and then change it on GitHub, RoboList would still be using
the email [email protected]. This might be surprising for some users, but
it will be fine so long as we build the settings page and are very clear
to the user about what email is associated with their account.

My philosophy for the "read email from OAuth token" is merely to
populate the email field with an *educated guess*. Because having
multiple registration/login flows is always going to be a little
finnicky by nature, we are not going to try and perpetually tie the
user's email account with their GitHub/Google email.
  • Loading branch information
chennisden committed Aug 13, 2024
1 parent 19fcd71 commit 75dd46d
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions store/app/crud/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,18 @@ async def _create_user_from_email(self, email: str, password: str) -> User:
"""OAuth sign up, creates user and links OAuthKey"""

async def _create_user_from_oauth(self, email: str, provider: str, user_token: str) -> User:
user = User.create(email=email, password=None)
if provider == "github":
user.github_id = user_token
user = await self.get_user_from_email(email)
if user is None:
user = User.create(email=email, password=None)
if provider == "github":
user.github_id = user_token
elif provider == "google":
user.google_id = user_token
await self._add_item(user, unique_fields=["email"])
elif provider == "github":
await self._update_item(user.id, User, {"github_id": user_token})
elif provider == "google":
user.google_id = user_token
await self._add_item(user, unique_fields=["email"])
await self._update_item(user.id, User, {"google_id": user_token})
oauth_key = OAuthKey.create(user_id=user.id, provider=provider, user_token=user_token)
await self._add_item(oauth_key, unique_fields=["user_token"])
return user
Expand Down

0 comments on commit 75dd46d

Please sign in to comment.