feat: add rejected approval status (AAI-526) (#141)

* feat: add rejected status to groupmembership

* fix: autolink group admin roles based on name

* fix: version inconsistency in migration script

Author: uwwint

db/models.py

@@ -535,6 +535,10 @@ class GroupMembership(SoftDeleteModel, table=True):
         default=None,
         sa_column=Column(String(1024), nullable=True)
     )
+    rejection_reason: str | None = Field(
+        default=None,
+        sa_column=Column(String(255), nullable=True),
+    )
 
     @classmethod
     def get_by_user_id(
@@ -620,13 +624,14 @@ def save_history(
             session.add(self)
         session.flush()
 
+        history_reason = self.rejection_reason if self.approval_status == ApprovalStatusEnum.REJECTED else self.revocation_reason
         history = GroupMembershipHistory(
             group_id=self.group_id,
             user_id=self.user_id,
             approval_status=self.approval_status,
             updated_at=self.updated_at,
             updated_by=self.updated_by,
-            reason=self.revocation_reason,
+            reason=history_reason,
         )
         session.add(history)
         if commit:
@@ -673,6 +678,23 @@ def revoke(
         self.save(session=session, commit=commit)
         return role_revoked
 
+    def reject(
+        self,
+        *,
+        reason: str,
+        updated_by: Optional["BiocommonsUser"],
+        session: Session,
+        commit: bool = True,
+    ) -> None:
+        if self.approval_status != ApprovalStatusEnum.PENDING:
+            raise ValueError("Can only reject pending group memberships.")
+        self.approval_status = ApprovalStatusEnum.REJECTED
+        self.rejection_reason = reason
+        self.revocation_reason = None
+        self.updated_at = datetime.now(timezone.utc)
+        self.updated_by = updated_by
+        self.save(session=session, commit=commit)
+
     def get_data(self) -> GroupMembershipData:
         """
         Get a data model for this membership, suitable for returning to the frontend.
@@ -689,6 +711,7 @@ def get_data(self) -> GroupMembershipData:
             approval_status=self.approval_status,
             updated_by=updated_by,
             revocation_reason=self.revocation_reason,
+            rejection_reason=self.rejection_reason,
         )
 
 
@@ -808,6 +831,10 @@ def get_by_name(cls, name: str, session: Session) -> Self | None:
             select(Auth0Role).where(Auth0Role.name == name)
         ).one_or_none()
 
+    @classmethod
+    def get_all(cls, session: Session) -> list["Auth0Role"]:
+        return session.exec(select(Auth0Role)).all()
+
 
 class BiocommonsGroup(SoftDeleteModel, table=True):
     # Name of the group / role name in Auth0, e.g. biocommons/group/tsi

db/types.py

@@ -14,6 +14,7 @@ class ApprovalStatusEnum(str, Enum):
     APPROVED = "approved"
     PENDING = "pending"
     REVOKED = "revoked"
+    REJECTED = "rejected"
 
 
 class EmailStatusEnum(str, Enum):
@@ -49,6 +50,7 @@ class GroupMembershipData(BaseModel):
     approval_status: ApprovalStatusEnum
     updated_by: str
     revocation_reason: str | None = None
+    rejection_reason: str | None = None
 
 
 class GroupEnum(str, Enum):

migrations/versions/c4c7a8e9b2d3_group_membership_rejection.py

@@ -0,0 +1,35 @@
+"""group-membership-rejection
+
+Revision ID: c4c7a8e9b2d3
+Revises: d6a5578732bc
+Create Date: 2025-12-02 12:00:00.000000
+
+"""
+from typing import Sequence, Union
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision: str = "c4c7a8e9b2d3"
+down_revision: Union[str, Sequence[str], None] = "d6a5578732bc"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    bind = op.get_bind()
+    dialect_name = bind.dialect.name
+
+    if dialect_name != "sqlite":
+        op.execute('ALTER TYPE "ApprovalStatusEnum" ADD VALUE \'REJECTED\'')
+
+    op.add_column(
+        "groupmembership",
+        sa.Column("rejection_reason", sa.String(length=255), nullable=True),
+    )
+
+
+def downgrade() -> None:
+    op.drop_column("groupmembership", "rejection_reason")

routers/admin.py

@@ -10,6 +10,7 @@
 from sqlalchemy import func, or_
 from sqlmodel import Session, select
 from sqlmodel.sql._expression_select_cls import SelectOfScalar
+from starlette import status
 
 from auth.user_permissions import (
     get_db_user,
@@ -134,6 +135,16 @@ def strip_reason(cls, value: str | None) -> str | None:
         return stripped or None
 
 
+class RejectServiceRequest(BaseModel):
+    reason: Annotated[str, Field(min_length=1, max_length=255)]
+
+    @field_validator("reason")
+    @classmethod
+    def strip_reason(cls, value: str) -> str:
+        stripped = value.strip()
+        if not stripped:
+            raise ValueError("Reason must not be empty")
+        return stripped
 
 def _membership_response() -> dict[str, object]:
     return {"status": "ok", "updated": True}
@@ -208,9 +219,15 @@ def _approve_group_membership(
         group_id=group.group_id,
         session=db_session,
     )
+    if membership.approval_status not in {ApprovalStatusEnum.PENDING, ApprovalStatusEnum.REVOKED}:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Only pending or revoked group memberships can be approved.",
+        )
     status_changed = membership.approval_status != ApprovalStatusEnum.APPROVED
     membership.approval_status = ApprovalStatusEnum.APPROVED
     membership.revocation_reason = None
+    membership.rejection_reason = None
     membership.updated_at = datetime.now(timezone.utc)
     membership.updated_by = admin_record
     membership.grant_auth0_role(auth0_client=client)
@@ -817,6 +834,39 @@ def approve_group_membership(user_id: Annotated[str, UserIdParam],
     return _membership_response()
 
 
+@router.post("/users/{user_id}/groups/{group_id:path}/reject",
+             dependencies=[Depends(require_admin_permission_for_group)])
+def reject_group_membership(user_id: Annotated[str, UserIdParam],
+                            group_id: Annotated[str, ServiceIdParam],
+                            payload: RejectServiceRequest,
+                            admin_record: Annotated[BiocommonsUser, Depends(get_db_user)],
+                            db_session: Annotated[Session, Depends(get_db_session)]):
+    membership = GroupMembership.get_by_user_id_and_group_id_or_404(
+        user_id=user_id,
+        group_id=group_id,
+        session=db_session,
+    )
+    if membership.approval_status != ApprovalStatusEnum.PENDING:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Only pending group requests can be rejected.",
+        )
+    membership.reject(
+        reason=payload.reason,
+        updated_by=admin_record,
+        session=db_session,
+        commit=False,
+    )
+    db_session.commit()
+    logger.info(
+        "Rejected group %s for user %s: %s",
+        group_id,
+        user_id,
+        payload.reason,
+    )
+    return _membership_response()
+
+
 @router.post("/users/{user_id}/groups/{group_id:path}/revoke",
              dependencies=[Depends(require_admin_permission_for_group)])
 def revoke_group_membership(user_id: Annotated[str, UserIdParam],

routers/biocommons_groups.py

@@ -1,4 +1,5 @@
 import logging
+from datetime import datetime, timezone
 from http import HTTPStatus
 from typing import Annotated
 
@@ -41,33 +42,45 @@ def request_group_access(
         settings: Annotated[Settings, Depends(get_settings)],
     ):
     """
-    Request access to a group. Assumes the user does not already have a
-    GroupMembership record for this group.
+    Request access to a group. Users can re-request access if their last
+    request was rejected; otherwise the request is rejected if a membership
+    already exists.
     """
     group_id = request_data.group_id
     existing_membership = GroupMembership.get_by_user_id_and_group_id(
         user_id=user.access_token.sub,
         group_id=group_id,
         session=db_session,
     )
+    membership: GroupMembership
     if existing_membership is not None:
-        raise HTTPException(
-            status_code=HTTPStatus.CONFLICT,
-            detail=f"User {user.access_token.sub} already has a membership for {group_id}"
+        if existing_membership.approval_status != ApprovalStatusEnum.REJECTED:
+            raise HTTPException(
+                status_code=HTTPStatus.CONFLICT,
+                detail=f"User {user.access_token.sub} already has a membership for {group_id}"
+            )
+        membership = existing_membership
+        membership.approval_status = ApprovalStatusEnum.PENDING
+        membership.rejection_reason = None
+        membership.updated_by = None
+        membership.updated_at = datetime.now(timezone.utc)
+        membership.save(session=db_session, commit=False)
+        logger.info("Re-requested group membership for %s(%s)", group_id, user.access_token.sub)
+    else:
+        group = BiocommonsGroup.get_by_id(group_id, db_session)
+        user_record = BiocommonsUser.get_or_create(
+            auth0_id=user.access_token.sub,
+            db_session=db_session,
+            auth0_client=auth0_client
         )
-    group = BiocommonsGroup.get_by_id(group_id, db_session)
-    user_record = BiocommonsUser.get_or_create(
-        auth0_id=user.access_token.sub,
-        db_session=db_session,
-        auth0_client=auth0_client
-    )
-    membership = GroupMembership(
-        group=group,
-        user=user_record,
-        approval_status=ApprovalStatusEnum.PENDING,
-        updated_by=None
-    )
-    membership.save(session=db_session, commit=False)
+        membership = GroupMembership(
+            group=group,
+            user=user_record,
+            approval_status=ApprovalStatusEnum.PENDING,
+            updated_by=None
+        )
+        membership.save(session=db_session, commit=False)
+        logger.info("Requested group membership for %s(%s)", group_id, user.access_token.sub)
     logger.info("Queueing emails to group admins for approval")
     admin_emails = membership.group.get_admins(auth0_client=auth0_client)
     for email in admin_emails:
@@ -113,8 +126,15 @@ def approve_group_access(
             status_code=HTTPStatus.NOT_FOUND,
             detail="No membership request found for this user"
     )
+    if membership.approval_status not in {ApprovalStatusEnum.PENDING, ApprovalStatusEnum.REVOKED}:
+        raise HTTPException(
+            status_code=HTTPStatus.BAD_REQUEST,
+            detail="Only pending or revoked group memberships can be approved.",
+        )
     membership.approval_status = ApprovalStatusEnum.APPROVED
     membership.updated_by = approving_user_record
+    membership.rejection_reason = None
+    membership.revocation_reason = None
     membership.grant_auth0_role(auth0_client=auth0_client)
     membership.save(session=db_session, commit=False)
     if membership.user is None:

scheduled_tasks/tasks.py

@@ -22,7 +22,13 @@
     PlatformMembership,
 )
 from db.setup import get_db_session
-from db.types import GROUP_NAMES, ApprovalStatusEnum, EmailStatusEnum, GroupEnum
+from db.types import (
+    GROUP_NAMES,
+    ApprovalStatusEnum,
+    EmailStatusEnum,
+    GroupEnum,
+    PlatformEnum,
+)
 from scheduled_tasks.email_retry import (
     EMAIL_JOB_ID_PREFIX,
     EMAIL_MAX_ATTEMPTS,
@@ -263,6 +269,7 @@ async def sync_auth0_roles():
 
     db_session = next(get_db_session())
     auth0_role_ids: set[str] = set()
+    db_roles_by_name: dict[str, Auth0Role] = {}
     try:
         for role in roles:
             logger.info(f"  Role: {role.name}")
@@ -292,18 +299,58 @@ async def sync_auth0_roles():
             if db_role.name != role.name or db_role.description != role.description:
                 db_role.name = role.name
                 db_role.description = role.description
+            db_roles_by_name[db_role.name] = db_role
         # Soft delete roles missing from Auth0
         db_session.flush()
         existing_roles = db_session.exec(select(Auth0Role)).all()
         for db_role in existing_roles:
             if db_role.id not in auth0_role_ids:
                 logger.info(f"    Soft deleting role {db_role.name} ({db_role.id}) absent from Auth0")
                 db_role.delete(db_session, commit=False)
+        link_admin_roles(db_session, db_roles_by_name)
         db_session.commit()
     finally:
         db_session.close()
 
 
+def link_admin_roles(session: Session, db_roles_by_name: dict[str, Auth0Role]) -> None:
+    """
+    Link admin roles to platforms/groups based on naming conventions:
+      - Platform admin roles: biocommons/role/{platform_id}/admin
+      - Group admin roles:    biocommons/role/{group_short_id}/admin where group_id is biocommons/group/{group_short_id}
+    """
+    platform_admin_pattern = re.compile(
+        r"^biocommons/role/(?P<platform_id>[a-z0-9_]+)/admin$", re.IGNORECASE
+    )
+    group_admin_pattern = re.compile(
+        r"^biocommons/role/(?P<group_short_id>[a-z0-9_]+)/admin$", re.IGNORECASE
+    )
+
+    for role_name, role in db_roles_by_name.items():
+        platform_match = platform_admin_pattern.match(role_name)
+        if platform_match:
+            pid = platform_match.group("platform_id").lower()
+            try:
+                platform_enum = PlatformEnum(pid)
+            except ValueError:
+                platform = None
+            else:
+                platform = Platform.get_by_id(platform_enum, session)
+            if platform:
+                if role not in platform.admin_roles:
+                    platform.admin_roles.append(role)
+                    session.add(platform)
+                continue
+
+        group_match = group_admin_pattern.match(role_name)
+        if group_match:
+            gid_short = group_match.group("group_short_id").lower()
+            full_group_id = f"biocommons/group/{gid_short}"
+            group = BiocommonsGroup.get_by_id(full_group_id, session)
+            if group and role not in group.admin_roles:
+                group.admin_roles.append(role)
+                session.add(group)
+
 class MembershipSyncStatus(BaseModel):
     created: bool
     restored: bool
@@ -498,6 +545,10 @@ async def populate_db_groups(groups=GroupEnum):
                 db_group = BiocommonsGroup(group_id=group.value, name=name, short_name=short_name)
                 db_session.add(db_group)
         db_session.commit()
+        # Ensure admin roles are linked now that groups exist (sync_auth0_roles may have run earlier)
+        roles_by_name = {role.name: role for role in Auth0Role.get_all(db_session)}
+        link_admin_roles(db_session, roles_by_name)
+        db_session.commit()
     finally:
         db_session.close()
 
@@ -528,6 +579,10 @@ async def populate_platforms_from_auth0():
                 else:
                     logger.info(f"  Updating platform {platform_id} (if needed)")
                     platform.update_from_auth0_role(db_role, db_session, commit=False)
+        db_session.commit()
+        roles_by_name = {role.name: role for role in Auth0Role.get_all(db_session)}
+        link_admin_roles(db_session, roles_by_name)
+        db_session.commit()
     finally:
         db_session.close()