Merge pull request #109 from AustralianBioCommons/AAI-379-revoke-users-from-TSI-embargo

enable user removal from groups

Author: amandazhuyilan

auth0/client.py

@@ -161,6 +161,18 @@ def add_roles_to_user(self, user_id: str, role_id: str | list[str]):
         resp.raise_for_status()
         return True
 
+    def remove_roles_from_user(self, user_id: str, role_id: str | list[str]):
+        """
+        Remove one or more roles from a user.
+        """
+        url = f"https://{self.domain}/api/v2/users/{user_id}/roles"
+        if isinstance(role_id, str):
+            role_id = [role_id]
+        # httpx.Client.delete() no longer accepts json payloads (0.28+), so use request()
+        resp = self._client.request("DELETE", url, json={"roles": role_id})
+        resp.raise_for_status()
+        return True
+
     def search_users_by_email(self, email: str) -> list[Auth0UserData]:
         url = f"https://{self.domain}/api/v2/users-by-email"
         resp = self._client.get(url, params={"email": email})

db/models.py

@@ -1,6 +1,6 @@
 import uuid
 from datetime import datetime, timezone
-from typing import Self
+from typing import Optional, Self
 
 from pydantic import AwareDatetime
 from sqlalchemy import Column, String, UniqueConstraint
@@ -536,6 +536,39 @@ def grant_auth0_role(self, auth0_client: Auth0Client):
         auth0_client.add_roles_to_user(user_id=self.user_id, role_id=role.id)
         return True
 
+    def revoke_auth0_role(self, auth0_client: Auth0Client):
+        """
+        Remove the Auth0 role backing this group membership when access is revoked.
+        """
+        if self.approval_status != ApprovalStatusEnum.APPROVED:
+            return False
+        role = auth0_client.get_role_by_name(self.group_id)
+        auth0_client.remove_roles_from_user(user_id=self.user_id, role_id=role.id)
+        return True
+
+    def revoke(
+        self,
+        *,
+        auth0_client: Auth0Client,
+        reason: str | None,
+        updated_by: Optional["BiocommonsUser"],
+        session: Session,
+        commit: bool = True,
+    ) -> bool:
+        """
+        Revoke this membership by removing its Auth0 role (when applicable) and
+        persisting the revoked status in the database.
+
+        :return: True when an Auth0 role removal call was performed.
+        """
+        role_revoked = self.revoke_auth0_role(auth0_client)
+        self.approval_status = ApprovalStatusEnum.REVOKED
+        self.revocation_reason = reason
+        self.updated_at = datetime.now(timezone.utc)
+        self.updated_by = updated_by
+        self.save(session=session, commit=commit)
+        return role_revoked
+
     def get_data(self) -> GroupMembershipData:
         """
         Get a data model for this membership, suitable for returning to the frontend.

routers/admin.py

@@ -204,19 +204,26 @@ def _revoke_group_membership(
     reason: str | None,
     admin_record: BiocommonsUser,
     db_session: Session,
+    client: Auth0Client,
 ) -> None:
     membership = GroupMembership.get_by_user_id_and_group_id_or_404(
         user_id=user_id,
         group_id=group.group_id,
         session=db_session,
     )
-    membership.approval_status = ApprovalStatusEnum.REVOKED
-    membership.revocation_reason = reason
-    membership.updated_at = datetime.now(timezone.utc)
-    membership.updated_by = admin_record
-    membership.save(session=db_session, commit=True)
+    role_revoked = membership.revoke(
+        auth0_client=client,
+        reason=reason,
+        updated_by=admin_record,
+        session=db_session,
+    )
     db_session.refresh(membership)
-    logger.info("Revoked group %s for user %s", group.group_id, user_id)
+    logger.info(
+        "Revoked group %s for user %s%s",
+        group.group_id,
+        user_id,
+        "" if role_revoked else " (no Auth0 role assigned)",
+    )
 
 
 @router.get("/filters")
@@ -623,6 +630,7 @@ def approve_group_membership(user_id: Annotated[str, UserIdParam],
 def revoke_group_membership(user_id: Annotated[str, UserIdParam],
                             group_id: Annotated[str, ServiceIdParam],
                             payload: RevokeServiceRequest,
+                            client: Annotated[Auth0Client, Depends(get_auth0_client)],
                             admin_record: Annotated[BiocommonsUser, Depends(get_db_user)],
                             db_session: Annotated[Session, Depends(get_db_session)]):
     group_record = BiocommonsGroup.get_by_id_or_404(group_id, session=db_session)
@@ -632,6 +640,7 @@ def revoke_group_membership(user_id: Annotated[str, UserIdParam],
         reason=payload.reason,
         admin_record=admin_record,
         db_session=db_session,
+        client=client,
     )
     return _membership_response()
 

tests/db/test_models.py

@@ -665,6 +665,91 @@ def test_group_membership_grant_auth0_role_not_approved(status, test_auth0_clien
         membership_request.grant_auth0_role(test_auth0_client)
 
 
+@respx.mock
+def test_group_membership_revoke_auth0_role(test_auth0_client, persistent_factories):
+    group = BiocommonsGroupFactory.create_sync(group_id="biocommons/group/tsi", admin_roles=[])
+    user = BiocommonsUserFactory.create_sync(group_memberships=[])
+    membership = GroupMembershipFactory.create_sync(group=group, user=user, approval_status=ApprovalStatusEnum.APPROVED.value)
+    role_data = RoleDataFactory.build(name=group.group_id)
+    role_lookup = respx.get(
+        "https://auth0.example.com/api/v2/roles",
+        params={"name_filter": group.group_id}
+    ).respond(status_code=200, json=[role_data.model_dump(mode="json")])
+    route = respx.delete(f"https://auth0.example.com/api/v2/users/{user.id}/roles").respond(status_code=200)
+    assert membership.revoke_auth0_role(test_auth0_client)
+    assert role_lookup.called
+    assert route.called
+
+
+@respx.mock
+def test_group_membership_revoke_updates_state(test_db_session, test_auth0_client, persistent_factories):
+    group = BiocommonsGroupFactory.create_sync(group_id="biocommons/group/tsi", admin_roles=[])
+    admin = BiocommonsUserFactory.create_sync()
+    user = BiocommonsUserFactory.create_sync(group_memberships=[])
+    membership = GroupMembershipFactory.create_sync(group=group, user=user, approval_status=ApprovalStatusEnum.APPROVED.value)
+    role_data = RoleDataFactory.build(name=group.group_id)
+    respx.get(
+        "https://auth0.example.com/api/v2/roles",
+        params={"name_filter": group.group_id}
+    ).respond(status_code=200, json=[role_data.model_dump(mode="json")])
+    route = respx.delete(f"https://auth0.example.com/api/v2/users/{user.id}/roles").respond(status_code=200)
+
+    result = membership.revoke(
+        auth0_client=test_auth0_client,
+        reason="No longer required",
+        updated_by=admin,
+        session=test_db_session,
+    )
+
+    assert result is True
+    assert route.called
+    test_db_session.refresh(membership)
+    assert membership.approval_status == ApprovalStatusEnum.REVOKED
+    assert membership.revocation_reason == "No longer required"
+    assert membership.updated_by_id == admin.id
+
+
+@pytest.mark.parametrize("status", ["pending", "revoked"])
+@respx.mock
+def test_group_membership_revoke_skips_auth0_when_not_approved(status, test_db_session, test_auth0_client, persistent_factories):
+    group = BiocommonsGroupFactory.create_sync(group_id="biocommons/group/tsi", admin_roles=[])
+    admin = BiocommonsUserFactory.create_sync()
+    user = BiocommonsUserFactory.create_sync(group_memberships=[])
+    membership = GroupMembershipFactory.create_sync(group=group, user=user, approval_status=status)
+    role_lookup = respx.get(
+        "https://auth0.example.com/api/v2/roles",
+        params={"name_filter": group.group_id}
+    ).respond(status_code=200, json=[])
+
+    result = membership.revoke(
+        auth0_client=test_auth0_client,
+        reason="Policy update",
+        updated_by=admin,
+        session=test_db_session,
+    )
+
+    assert result is False
+    assert not role_lookup.called
+    test_db_session.refresh(membership)
+    assert membership.approval_status == ApprovalStatusEnum.REVOKED
+    assert membership.revocation_reason == "Policy update"
+    assert membership.updated_by_id == admin.id
+
+
+@pytest.mark.parametrize("status", ["pending", "revoked"])
+@respx.mock
+def test_group_membership_revoke_auth0_role_not_approved(status, test_auth0_client, persistent_factories):
+    group = BiocommonsGroupFactory.create_sync(group_id="biocommons/group/tsi", admin_roles=[])
+    user = Auth0UserDataFactory.build()
+    membership = GroupMembershipFactory.create_sync(group=group, user_id=user.user_id, approval_status=status)
+    role_lookup = respx.get(
+        "https://auth0.example.com/api/v2/roles",
+        params={"name_filter": group.group_id}
+    ).respond(status_code=200, json=[])
+    assert membership.revoke_auth0_role(test_auth0_client) is False
+    assert not role_lookup.called
+
+
 def test_group_membership_save_with_history(test_db_session, persistent_factories):
     group = BiocommonsGroupFactory.create_sync()
     membership = GroupMembershipFactory.build(group_id=group.group_id)

tests/test_admin.py

@@ -14,6 +14,7 @@
 from db.types import ApprovalStatusEnum, GroupEnum, PlatformEnum
 from main import app
 from routers.admin import PaginationParams
+from tests.biocommons.datagen import RoleDataFactory
 from tests.datagen import (
     AccessTokenPayloadFactory,
     Auth0UserDataFactory,
@@ -770,6 +771,7 @@ def test_revoke_group_membership_records_reason(
     admin_user,
     tsi_group,
     persistent_factories,
+    mock_auth0_client,
 ):
     user = BiocommonsUserFactory.create_sync(group_memberships=[])
     membership = GroupMembershipFactory.create_sync(
@@ -784,6 +786,9 @@ def test_revoke_group_membership_records_reason(
     )
     test_db_session.commit()
 
+    mock_role = RoleDataFactory.build(name=tsi_group.group_id)
+    mock_auth0_client.get_role_by_name.return_value = mock_role
+
     reason = "Access no longer required"
     resp = test_client.post(
         f"/admin/users/{user.id}/groups/{tsi_group.group_id}/revoke",
@@ -797,13 +802,19 @@ def test_revoke_group_membership_records_reason(
     assert membership.approval_status == ApprovalStatusEnum.REVOKED
     assert membership.revocation_reason == reason
     assert membership.updated_by_id == admin_db_user.id
+    mock_auth0_client.get_role_by_name.assert_called_once_with(tsi_group.group_id)
+    mock_auth0_client.remove_roles_from_user.assert_called_once_with(
+        user_id=user.id,
+        role_id=mock_role.id,
+    )
 
 
 def test_revoke_group_membership_forbidden_without_group_role(
     test_client,
     test_db_session,
     tsi_group,
     persistent_factories,
+    mock_auth0_client,
 ):
     user = BiocommonsUserFactory.create_sync(group_memberships=[])
     membership = GroupMembershipFactory.create_sync(
@@ -843,6 +854,8 @@ def test_revoke_group_membership_forbidden_without_group_role(
     test_db_session.refresh(membership)
     assert membership.approval_status == original_status
     assert membership.revocation_reason == original_reason
+    mock_auth0_client.get_role_by_name.assert_not_called()
+    mock_auth0_client.remove_roles_from_user.assert_not_called()
 
 
 def test_resend_verification_email(test_client, as_admin_user, test_db_session, galaxy_platform, mock_auth0_client):

tests/test_auth0_client.py

@@ -142,6 +142,20 @@ def test_add_roles_to_user(test_auth0_client):
     assert call_data == b'{"roles":["' +role_id.encode() + b'"]}'
 
 
+@respx.mock
+def test_remove_roles_from_user(test_auth0_client):
+    """
+    Test we can remove roles from a user in Auth0 API
+    """
+    user_id = random_auth0_id()
+    role_id = random_auth0_role_id()
+    route = respx.delete(f"https://auth0.example.com/api/v2/users/{user_id}/roles").respond(204)
+    test_auth0_client.remove_roles_from_user(user_id, role_id)
+    assert route.called
+    call_data = route.calls[0].request.content
+    assert call_data == b'{"roles":["' + role_id.encode() + b'"]}'
+
+
 @respx.mock
 def test_create_user(test_auth0_client):
     """