From 75dd46dcb451dccb17dc4763f3e5b8b040faf39f Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 13 Aug 2024 10:50:13 -0700 Subject: [PATCH] In create_oauth, link existing email account To elaborate, suppose we create an account dchen@kscale.dev PASSWORD in the username-password registration flow. Before this change, if you were to use the dchen@kscale.dev 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 dchen@kscale.dev 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 dchen@kscale.dev. 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 dchen@kscale.dev, and then use that to register for RoboList, and then change it on GitHub, RoboList would still be using the email dchen@kscale.dev. 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. --- store/app/crud/users.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/store/app/crud/users.py b/store/app/crud/users.py index 57b40692..16d09ad6 100644 --- a/store/app/crud/users.py +++ b/store/app/crud/users.py @@ -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