From 85cd214101a3035423a64d0890a3ab7188de329a Mon Sep 17 00:00:00 2001
From: Ilya Kreymer <ikreymer@users.noreply.github.com>
Date: Fri, 24 May 2024 10:41:05 -0700
Subject: [PATCH] Fix regression to changing user roles via PATCH /user-role
 API (#1824)

clean up adding user vs changing role logic:
- when adding user, ensure user doesn't exist
- when changing roles, ensure user does exist

add test for changing roles of existing user

Fixes #1821
---
 backend/btrixcloud/orgs.py  | 15 +++++++++++----
 backend/btrixcloud/users.py |  7 +++----
 backend/test/test_users.py  | 11 +++++++++++
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/backend/btrixcloud/orgs.py b/backend/btrixcloud/orgs.py
index f62d67c4..ca4c7aed 100644
--- a/backend/btrixcloud/orgs.py
+++ b/backend/btrixcloud/orgs.py
@@ -375,6 +375,14 @@ class OrgOps:
         org.users[str(userid)] = role
         await self.update_users(org)
 
+    async def change_user_role(self, org: Organization, userid: UUID, role: UserRole):
+        """Change role of existing user in organization"""
+        if str(userid) not in org.users:
+            raise HTTPException(status_code=400, detail="no_such_user")
+
+        org.users[str(userid)] = role
+        await self.update_users(org)
+
     async def get_org_owners(self, org: Organization):
         """Return list of org's Owner users."""
         org_owners = []
@@ -838,7 +846,7 @@ def init_orgs_api(app, mdb, user_manager, invites, user_dep):
         if other_user.email == user.email:
             raise HTTPException(status_code=400, detail="Can't change own role!")
 
-        await ops.add_user_to_org(org, other_user.id, update.role)
+        await ops.change_user_role(org, other_user.id, update.role)
 
         return {"updated": True}
 
@@ -923,11 +931,10 @@ def init_orgs_api(app, mdb, user_manager, invites, user_dep):
         if not user.is_superuser:
             raise HTTPException(status_code=403, detail="Not Allowed")
 
-        await user_manager.create_non_super_user(
+        new_user = await user_manager.create_non_super_user(
             invite.email, invite.password, invite.name
         )
-        update_role = UpdateRole(role=invite.role, email=invite.email)
-        await set_role(update_role, org, user)
+        await ops.add_user_to_org(org, new_user.id, invite.role)
         return {"added": True}
 
     @router.get("/metrics", tags=["organizations"], response_model=OrgMetrics)
diff --git a/backend/btrixcloud/users.py b/backend/btrixcloud/users.py
index 92794af3..90735784 100644
--- a/backend/btrixcloud/users.py
+++ b/backend/btrixcloud/users.py
@@ -275,11 +275,10 @@ class UserManager:
         email: str,
         password: str,
         name: str = "New user",
-    ) -> None:
+    ) -> User:
         """create a regular user with given credentials"""
         if not email:
-            print("No user defined", flush=True)
-            return
+            raise HTTPException(status_code=400, detail="missing_user_email")
 
         if not password:
             password = generate_password()
@@ -294,7 +293,7 @@ class UserManager:
                 is_verified=True,
             )
 
-            await self._create(user_create)
+            return await self._create(user_create)
         except HTTPException as exc:
             print(f"User {email} already exists", flush=True)
             raise exc
diff --git a/backend/test/test_users.py b/backend/test/test_users.py
index f00af142..88670446 100644
--- a/backend/test/test_users.py
+++ b/backend/test/test_users.py
@@ -182,6 +182,17 @@ def test_register_user_valid_password(admin_auth_headers, default_org_id):
     assert r.status_code == 201
 
 
+def test_user_change_role(admin_auth_headers, default_org_id):
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/user-role",
+        headers=admin_auth_headers,
+        json={"email": VALID_USER_EMAIL, "role": 40},
+    )
+
+    assert r.status_code == 200
+    assert r.json()["updated"] == True
+
+
 def test_reset_invalid_password(admin_auth_headers):
     r = requests.put(
         f"{API_PREFIX}/users/me/password-change",