Merge pull request #715 from SUNET/lundberg_manacc_fixes

Backend fixes for managed accounts
SUNET · Nov 19, 2024 · c35040e · c35040e
2 parents 6852018 + 41a9e96
commit c35040e
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 10 deletions.
diff --git a/src/eduid/graphdb/groupdb/db.py b/src/eduid/graphdb/groupdb/db.py
@@ -388,6 +388,10 @@ def save(self, group: Group) -> Group:
         with self.db.driver.session(default_access_mode=WRITE_ACCESS) as session:
             try:
                 tx = session.begin_transaction()
+            except ClientError as e:
+                logger.error(e)
+                raise EduIDGroupDBError(e.message)
+            try:
                 self._remove_missing_users_and_groups(tx, group, Role.OWNER)
                 self._remove_missing_users_and_groups(tx, group, Role.MEMBER)
                 saved_group = self._create_or_update_group(tx, group)
@@ -396,9 +400,6 @@ def save(self, group: Group) -> Group:
             except ConstraintError as e:
                 logger.error(e)
                 raise VersionMismatch("Tried to save a group with wrong version")
-            except ClientError as e:
-                logger.error(e)
-                raise EduIDGroupDBError(e.message)
             finally:
                 if tx.closed():
                     logger.info("Group save successful")

diff --git a/src/eduid/maccapi/routers/users.py b/src/eduid/maccapi/routers/users.py
@@ -129,13 +129,12 @@ async def remove_user(
         )
         request.app.context.stats.count("maccapi_remove_user_success")
     except UserDoesNotExist as e:
-        request.app.context.logger.error(f"remove_user error: {e}")
+        request.app.context.logger.error(f"remove_user error: {e} - user already removed")
         remove_user_response = UserRemovedResponse(
-            status="error",
+            status="success",
             scope=request.app.context.config.default_eppn_scope,
         )
         request.app.context.stats.count("maccapi_remove_user_error")
-        response.status_code = status.HTTP_422_UNPROCESSABLE_ENTITY
 
     return remove_user_response
 
@@ -185,7 +184,7 @@ async def reset_password(
         )
         request.app.context.stats.count("maccapi_reset_password_error")
         if isinstance(e, UserDoesNotExist):
-            response.status_code = status.HTTP_422_UNPROCESSABLE_ENTITY
+            response.status_code = status.HTTP_404_NOT_FOUND
         else:
             response.status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
 

diff --git a/src/eduid/maccapi/tests/test_maccapi.py b/src/eduid/maccapi/tests/test_maccapi.py
@@ -152,12 +152,12 @@ def test_remove_error(self) -> None:
         headers["Authorization"] = f"Bearer {token}"
 
         response = self.client.post(url="/Users/remove", json={"eppn": "made_up"}, headers=headers)
-        assert response.status_code == 422
+        assert response.status_code == 200
 
     def test_reset_error(self) -> None:
         token = self._make_bearer_token(claims=self.claims)
 
         headers = self.headers
         headers["Authorization"] = f"Bearer {token}"
         response = self.client.post(url="/Users/reset_password", json={"eppn": "made_up"}, headers=headers)
-        assert response.status_code == 422
+        assert response.status_code == 404
diff --git a/src/eduid/userdb/scimapi/groupdb.py b/src/eduid/userdb/scimapi/groupdb.py
@@ -218,7 +218,7 @@ def update_group(self, update_request: GroupUpdateRequest, db_group: ScimApiGrou
             db_group.external_id = update_request.external_id
             logger.debug(f"Changed external id for group: {db_group.external_id} -> {update_request.external_id}")
 
-        # Check if there where new, changed or removed members
+        # Check if there were new, changed or removed members
         if db_group.graph.members != updated_members:
             changed = True
             db_group.graph = replace(db_group.graph, members=updated_members)