From b061a39d5fb1106051e5076d6c40959f755f6a1a Mon Sep 17 00:00:00 2001 From: Ilya Kreymer Date: Wed, 24 Apr 2024 16:40:25 +0200 Subject: [PATCH] Handle registration when user already exists (#1744) Fixes https://github.com/webrecorder/browsertrix/issues/1743 On the backend, support adding user to new org and improved error messaging: - if user exists is not part of the org to be added to, add user to the registration org, return 201 - if user is already part of the org, return 400 with 'user_already_is_org_member' error - if user is not being added to a new org but already exists, return 'user_already_exists' frontend: - if user user same password, they will just be logged in and added to registration org (same as before) - if user uses wrong password, show alert message - handle both user_already_is_org_member and user_already_registered with alert message and link to log-in page. note: this means existing user is added to the registration org even if they provide wrong password, but they won't be able to login until they use correct password/reset/etc... --- backend/btrixcloud/models.py | 2 +- backend/btrixcloud/orgs.py | 9 ++++--- backend/btrixcloud/users.py | 15 +++++++++-- .../src/features/accounts/sign-up-form.ts | 26 ++++++++++++++++--- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/backend/btrixcloud/models.py b/backend/btrixcloud/models.py index bb7b7e90..3aa26b5c 100644 --- a/backend/btrixcloud/models.py +++ b/backend/btrixcloud/models.py @@ -50,7 +50,7 @@ class InvitePending(BaseMongoModel): inviterEmail: str fromSuperuser: Optional[bool] oid: Optional[UUID] - role: Optional[UserRole] = UserRole.VIEWER + role: UserRole = UserRole.VIEWER email: Optional[str] diff --git a/backend/btrixcloud/orgs.py b/backend/btrixcloud/orgs.py index 0740b372..f62d67c4 100644 --- a/backend/btrixcloud/orgs.py +++ b/backend/btrixcloud/orgs.py @@ -367,11 +367,12 @@ class OrgOps: await self.add_user_to_org(org, user.id, invite.role) return True - async def add_user_to_org( - self, org: Organization, userid: UUID, role: Optional[UserRole] = None - ): + async def add_user_to_org(self, org: Organization, userid: UUID, role: UserRole): """Add user to organization with specified role""" - org.users[str(userid)] = role or UserRole.OWNER + if str(userid) in org.users: + raise HTTPException(status_code=400, detail="user_already_is_org_member") + + org.users[str(userid)] = role await self.update_users(org) async def get_org_owners(self, org: Organization): diff --git a/backend/btrixcloud/users.py b/backend/btrixcloud/users.py index fa024ef5..92794af3 100644 --- a/backend/btrixcloud/users.py +++ b/backend/btrixcloud/users.py @@ -359,10 +359,17 @@ class UserManager: is_verified=is_verified, ) + user_already_exists = False + try: await self.users.insert_one(user.dict()) except DuplicateKeyError: - raise HTTPException(status_code=400, detail="user_already_exists") + maybe_user = await self.get_by_email(create.email) + # shouldn't happen since user should exist if we have duplicate key, but just in case! + if not maybe_user: + raise HTTPException(status_code=400, detail="user_missing") + user = maybe_user + user_already_exists = True add_to_org = False @@ -380,12 +387,16 @@ class UserManager: else: add_to_org = True - if not is_verified: + if not is_verified and not user_already_exists: asyncio.create_task(self.request_verify(user, request)) # org to auto-add user to, if any auto_add_org: Optional[Organization] = None + # if user already exists, and not adding to a new org, error + if user_already_exists and not add_to_org: + raise HTTPException(status_code=400, detail="user_already_exists") + # if add to default, then get default org if add_to_org: if self.register_to_org_id: diff --git a/frontend/src/features/accounts/sign-up-form.ts b/frontend/src/features/accounts/sign-up-form.ts index 080ca4c1..3545adc9 100644 --- a/frontend/src/features/accounts/sign-up-form.ts +++ b/frontend/src/features/accounts/sign-up-form.ts @@ -44,6 +44,9 @@ export class SignUpForm extends LiteElement { @state() private pwStrengthResults: null | ZxcvbnResult = null; + @state() + private showLoginLink = false; + protected firstUpdated() { void PasswordService.setOptions(); } @@ -55,8 +58,15 @@ export class SignUpForm extends LiteElement { serverError = html`
${this.serverError} + >${this.serverError} + ${this.showLoginLink + ? html`

+ Go to the + Log-In Page and try + again. +

` + : ``} +
`; } @@ -179,6 +189,7 @@ export class SignUpForm extends LiteElement { this.dispatchEvent(new CustomEvent("submit")); this.serverError = undefined; + this.showLoginLink = false; this.isSubmitting = true; const formData = new FormData(event.target as HTMLFormElement); @@ -229,7 +240,10 @@ export class SignUpForm extends LiteElement { const { detail } = (await resp.json()) as { detail: string & { code: string }; }; - if (detail === "user_already_exists") { + if ( + detail === "user_already_exists" || + detail === "user_already_is_org_member" + ) { shouldLogIn = true; } else if (detail.code && detail.code === "invalid_password") { this.serverError = msg( @@ -254,7 +268,11 @@ export class SignUpForm extends LiteElement { try { await this.logIn({ email, password }); } catch { - this.dispatchEvent(new CustomEvent("unauthenticated")); + this.serverError = msg( + "User is already registered, but with a different password.", + ); + this.showLoginLink = true; + //this.dispatchEvent(new CustomEvent("unauthenticated")); } } }