From 7b629e0b12036077b0e3dc4524649dab95e06339 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 18 Aug 2025 14:48:22 +1000 Subject: [PATCH 1/3] Implement Biocommons user registration endpoint and associated schemas - Added biocommons_register router for user registration. - Created BiocommonsRegistrationRequest schema for registration data validation. - Implemented user record creation logic in the database. - Added email notification for new user registrations. - Updated tests for biocommons user registration functionality. --- db/admin.py | 60 +++++++-- db/models.py | 183 +++++++++++++++++++------- main.py | 13 +- routers/biocommons_register.py | 140 ++++++++++++++++++++ schemas/biocommons.py | 67 +++++++--- schemas/biocommons_register.py | 16 +++ tests/datagen.py | 15 ++- tests/test_biocommons_register.py | 206 ++++++++++++++++++++++++++++++ 8 files changed, 621 insertions(+), 79 deletions(-) create mode 100644 routers/biocommons_register.py create mode 100644 schemas/biocommons_register.py create mode 100644 tests/test_biocommons_register.py diff --git a/db/admin.py b/db/admin.py index b0731221..3793f750 100644 --- a/db/admin.py +++ b/db/admin.py @@ -13,6 +13,7 @@ from db.models import ( Auth0Role, BiocommonsGroup, + BiocommonsUser, GroupMembership, GroupMembershipHistory, ) @@ -32,11 +33,11 @@ def setup_oauth(): api_base_url=f"https://{settings.auth0_domain}", access_token_url=f"https://{settings.auth0_domain}/oauth/token", authorize_url=f"https://{settings.auth0_domain}/authorize", - server_metadata_url=f'https://{settings.auth0_domain}/.well-known/openid-configuration', + server_metadata_url=f"https://{settings.auth0_domain}/.well-known/openid-configuration", client_kwargs={ "scope": "openid profile email", - "audience": settings.auth0_audience - } + "audience": settings.auth0_audience, + }, ) return oauth.create_client("auth0") @@ -63,7 +64,7 @@ async def authenticate(self, request: Request) -> Union[bool, RedirectResponse]: roles = request.session.get("biocommons_roles") if not roles: print("Redirecting to Auth0 for login.") - redirect_uri = request.url_for('login_auth0') + redirect_uri = request.url_for("login_auth0") return await self.auth0_client.authorize_redirect(request, redirect_uri) for role in roles: if role in settings.admin_roles: @@ -71,6 +72,14 @@ async def authenticate(self, request: Request) -> Union[bool, RedirectResponse]: return False +class BiocommonsUserAdmin(ModelView, model=BiocommonsUser): + can_edit = False + can_create = False + can_delete = False + column_list = ["id", "username", "email", "created_at"] + column_default_sort = ("created_at", True) + + class GroupAdmin(ModelView, model=BiocommonsGroup): can_edit = False can_create = False @@ -88,10 +97,18 @@ class GroupMembershipAdmin(ModelView, model=GroupMembership): can_edit = False can_create = False can_delete = False - column_list = ["name", "group_id", "user_email", "user_id", "approval_status", "updated_at", "updated_by_email"] + column_list = [ + "name", + "group_id", + "user_email", + "user_id", + "approval_status", + "updated_at", + "updated_by_email", + ] column_filters = [ AllUniqueStringValuesFilter(GroupMembership.group_id), - AllUniqueStringValuesFilter(GroupMembership.approval_status) + AllUniqueStringValuesFilter(GroupMembership.approval_status), ] @@ -99,7 +116,15 @@ class GroupMembershipHistoryAdmin(ModelView, model=GroupMembershipHistory): can_edit = False can_create = False can_delete = False - column_list = ["name", "group_id", "user_email", "user_id", "approval_status", "updated_at", "updated_by_email"] + column_list = [ + "name", + "group_id", + "user_email", + "user_id", + "approval_status", + "updated_at", + "updated_by_email", + ] column_default_sort = ("updated_at", True) @@ -107,7 +132,14 @@ class DatabaseAdmin: """ Sets up the Admin app for the database. """ - views = (GroupAdmin, Auth0RoleAdmin, GroupMembershipAdmin, GroupMembershipHistoryAdmin,) + + views = ( + BiocommonsUserAdmin, + GroupAdmin, + Auth0RoleAdmin, + GroupMembershipAdmin, + GroupMembershipHistoryAdmin, + ) def __init__(self, app: FastAPI, secret_key: str): self.app = app @@ -116,8 +148,10 @@ def __init__(self, app: FastAPI, secret_key: str): app, engine=get_engine(), base_url="/db_admin", - authentication_backend=AdminAuth(secret_key=secret_key, auth0_client=self.auth0_client), - title="AAI Backend Admin" + authentication_backend=AdminAuth( + secret_key=secret_key, auth0_client=self.auth0_client + ), + title="AAI Backend Admin", ) self.admin.app.router.add_route("/auth/auth0", self.login_auth0) @@ -138,6 +172,8 @@ async def login_auth0(self, request: Request) -> Response: if not payload: raise HTTPException(status_code=401, detail="Could not verify JWT.") if not payload.has_admin_role(settings): - raise HTTPException(status_code=401, detail="User does not have admin role.") - request.session['biocommons_roles'] = payload.biocommons_roles + raise HTTPException( + status_code=401, detail="User does not have admin role." + ) + request.session["biocommons_roles"] = payload.biocommons_roles return RedirectResponse(request.url_for("admin:index"), status_code=302) diff --git a/db/models.py b/db/models.py index 77b6c2ae..965c9cb0 100644 --- a/db/models.py +++ b/db/models.py @@ -33,15 +33,17 @@ class BiocommonsUser(BaseModel, table=True): # Use a separate data model to validate this email: str = Field(unique=True) username: str = Field(unique=True) - created_at: AwareDatetime = Field(default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime) + created_at: AwareDatetime = Field( + default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime + ) platform_memberships: list["PlatformMembership"] = Relationship( back_populates="user", - sa_relationship_kwargs={"foreign_keys": "PlatformMembership.user_id"} + sa_relationship_kwargs={"foreign_keys": "PlatformMembership.user_id"}, ) group_memberships: list["GroupMembership"] = Relationship( back_populates="user", - sa_relationship_kwargs={"foreign_keys": "GroupMembership.user_id"} + sa_relationship_kwargs={"foreign_keys": "GroupMembership.user_id"}, ) @classmethod @@ -57,14 +59,12 @@ def from_auth0_data(cls, data: Auth0UserData) -> Self: """ Create a new BiocommonsUser object from Auth0 user data (no API call). """ - return cls( - id=data.user_id, - email=data.email, - username=data.username - ) + return cls(id=data.user_id, email=data.email, username=data.username) @classmethod - def get_or_create(cls, auth0_id: str, db_session: Session, auth0_client: Auth0Client) -> Self: + def get_or_create( + cls, auth0_id: str, db_session: Session, auth0_client: Auth0Client + ) -> Self: """ Get the user from the DB, or create it from Auth0 data if it doesn't exist. """ @@ -75,17 +75,34 @@ def get_or_create(cls, auth0_id: str, db_session: Session, auth0_client: Auth0Cl db_session.commit() return user - def add_platform_membership(self, platform: PlatformEnum, db_session: Session, auto_approve: bool = False) -> "PlatformMembership": + def add_platform_membership( + self, platform: PlatformEnum, db_session: Session, auto_approve: bool = False + ) -> "PlatformMembership": membership = PlatformMembership( platform_id=platform, user=self, - approval_status=ApprovalStatusEnum.APPROVED if auto_approve else ApprovalStatusEnum.PENDING, + approval_status=ApprovalStatusEnum.APPROVED + if auto_approve + else ApprovalStatusEnum.PENDING, updated_by=None, ) db_session.add(membership) membership.save_history(db_session) return membership + def add_group_membership( + self, group_id: str, db_session: Session, auto_approve: bool = False + ) -> "GroupMembership": + membership = GroupMembership( + group_id=group_id, + user_id=self.id, + approval_status=ApprovalStatusEnum.PENDING, + updated_by_id=None, + ) + db_session.add(membership) + membership.save_history(db_session) + return membership + class PlatformMembership(BaseModel, table=True): __table_args__ = ( @@ -94,15 +111,32 @@ class PlatformMembership(BaseModel, table=True): id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) platform_id: PlatformEnum = Field(sa_type=DbEnum(PlatformEnum, name="PlatformEnum")) user_id: str = Field(foreign_key="biocommons_user.id") - user: "BiocommonsUser" = Relationship(back_populates="platform_memberships", - sa_relationship_kwargs={"foreign_keys": "PlatformMembership.user_id",}) - approval_status: ApprovalStatusEnum = Field(sa_type=DbEnum(ApprovalStatusEnum, name="ApprovalStatusEnum")) - updated_at: AwareDatetime = Field(default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime) + user: "BiocommonsUser" = Relationship( + back_populates="platform_memberships", + sa_relationship_kwargs={ + "foreign_keys": "PlatformMembership.user_id", + }, + ) + approval_status: ApprovalStatusEnum = Field( + sa_type=DbEnum(ApprovalStatusEnum, name="ApprovalStatusEnum") + ) + updated_at: AwareDatetime = Field( + default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime + ) # Nullable: some memberships are automatically approved updated_by_id: str | None = Field(foreign_key="biocommons_user.id", nullable=True) - updated_by: "BiocommonsUser" = Relationship(sa_relationship_kwargs={"foreign_keys": "PlatformMembership.updated_by_id",}) + updated_by: "BiocommonsUser" = Relationship( + sa_relationship_kwargs={ + "foreign_keys": "PlatformMembership.updated_by_id", + } + ) + + def save_history(self, session: Session) -> "PlatformMembershipHistory": + # Make sure this object is in the session before accessing relationships + if self not in session: + session.add(self) + session.flush() # Ensure relationships are loaded - def save_history(self, session: Session) -> 'PlatformMembershipHistory': history = PlatformMembershipHistory( platform_id=self.platform_id, user=self.user, @@ -114,17 +148,27 @@ def save_history(self, session: Session) -> 'PlatformMembershipHistory': return history - - class PlatformMembershipHistory(BaseModel, table=True): id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) platform_id: PlatformEnum = Field(sa_type=DbEnum(PlatformEnum, name="PlatformEnum")) user_id: str = Field(foreign_key="biocommons_user.id") - user: "BiocommonsUser" = Relationship(sa_relationship_kwargs={"foreign_keys": "PlatformMembershipHistory.user_id",}) - approval_status: ApprovalStatusEnum = Field(sa_type=DbEnum(ApprovalStatusEnum, name="ApprovalStatusEnum")) - updated_at: AwareDatetime = Field(default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime) + user: "BiocommonsUser" = Relationship( + sa_relationship_kwargs={ + "foreign_keys": "PlatformMembershipHistory.user_id", + } + ) + approval_status: ApprovalStatusEnum = Field( + sa_type=DbEnum(ApprovalStatusEnum, name="ApprovalStatusEnum") + ) + updated_at: AwareDatetime = Field( + default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime + ) updated_by_id: str | None = Field(foreign_key="biocommons_user.id", nullable=True) - updated_by: "BiocommonsUser" = Relationship(sa_relationship_kwargs={"foreign_keys": "PlatformMembershipHistory.updated_by_id",}) + updated_by: "BiocommonsUser" = Relationship( + sa_relationship_kwargs={ + "foreign_keys": "PlatformMembershipHistory.updated_by_id", + } + ) class GroupMembership(BaseModel, table=True): @@ -133,6 +177,7 @@ class GroupMembership(BaseModel, table=True): Note: only one row per user/group, the approval history is kept separately in the GroupMembershipHistory table """ + __table_args__ = ( UniqueConstraint("group_id", "user_id", name="user_group_pairing"), ) @@ -140,22 +185,34 @@ class GroupMembership(BaseModel, table=True): group_id: str = Field(foreign_key="biocommonsgroup.group_id") group: "BiocommonsGroup" = Relationship(back_populates="members") user_id: str = Field(foreign_key="biocommons_user.id") - user: "BiocommonsUser" = Relationship(back_populates="group_memberships", - sa_relationship_kwargs={"foreign_keys": "GroupMembership.user_id",}) + user: "BiocommonsUser" = Relationship( + back_populates="group_memberships", + sa_relationship_kwargs={ + "foreign_keys": "GroupMembership.user_id", + }, + ) approval_status: ApprovalStatusEnum = Field( sa_type=DbEnum(ApprovalStatusEnum, name="ApprovalStatusEnum") ) - updated_at: AwareDatetime = Field(default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime) + updated_at: AwareDatetime = Field( + default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime + ) # Nullable: some memberships are automatically approved updated_by_id: str | None = Field(foreign_key="biocommons_user.id", nullable=True) - updated_by: "BiocommonsUser" = Relationship(sa_relationship_kwargs={"foreign_keys": "GroupMembership.updated_by_id",}) + updated_by: "BiocommonsUser" = Relationship( + sa_relationship_kwargs={ + "foreign_keys": "GroupMembership.updated_by_id", + } + ) @classmethod - def get_by_user_id(cls, user_id: str, group_id: str, session: Session) -> Self | None: + def get_by_user_id( + cls, user_id: str, group_id: str, session: Session + ) -> Self | None: return session.exec( - select(GroupMembership) - .where(GroupMembership.user_id == user_id, - GroupMembership.group_id == group_id) + select(GroupMembership).where( + GroupMembership.user_id == user_id, GroupMembership.group_id == group_id + ) ).one_or_none() def save(self, session: Session, commit: bool = True) -> Self: @@ -169,7 +226,14 @@ def save(self, session: Session, commit: bool = True) -> Self: session.commit() return self - def save_history(self, session: Session, commit: bool = True) -> 'GroupMembershipHistory': + def save_history( + self, session: Session, commit: bool = True + ) -> "GroupMembershipHistory": + # Make sure this object is in the session before accessing relationships + if self not in session: + session.add(self) + session.flush() + history = GroupMembershipHistory( group=self.group, user=self.user, @@ -194,25 +258,40 @@ class GroupMembershipHistory(BaseModel, table=True): """ Stores the full history of approval decisions for each user """ + id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) group_id: str = Field(foreign_key="biocommonsgroup.group_id") group: "BiocommonsGroup" = Relationship(back_populates="approval_history") user_id: str = Field(foreign_key="biocommons_user.id") - user: "BiocommonsUser" = Relationship(sa_relationship_kwargs={"foreign_keys": "GroupMembershipHistory.user_id",}) + user: "BiocommonsUser" = Relationship( + sa_relationship_kwargs={ + "foreign_keys": "GroupMembershipHistory.user_id", + } + ) approval_status: ApprovalStatusEnum = Field( sa_type=DbEnum(ApprovalStatusEnum, name="ApprovalStatusEnum") ) - updated_at: AwareDatetime = Field(default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime) + updated_at: AwareDatetime = Field( + default_factory=lambda: datetime.now(timezone.utc), sa_type=DateTime + ) # Nullable: some memberships are automatically approved updated_by_id: str | None = Field(foreign_key="biocommons_user.id", nullable=True) - updated_by: "BiocommonsUser" = Relationship(sa_relationship_kwargs={"foreign_keys": "GroupMembershipHistory.updated_by_id",}) + updated_by: "BiocommonsUser" = Relationship( + sa_relationship_kwargs={ + "foreign_keys": "GroupMembershipHistory.updated_by_id", + } + ) @classmethod - def get_by_user_id(cls, user_id: str, group_id: str, session: Session) -> list[Self] | None: + def get_by_user_id( + cls, user_id: str, group_id: str, session: Session + ) -> list[Self] | None: return session.exec( select(GroupMembershipHistory) - .where(GroupMembershipHistory.user_id == user_id, - GroupMembershipHistory.group_id == group_id) + .where( + GroupMembershipHistory.user_id == user_id, + GroupMembershipHistory.group_id == group_id, + ) .order_by(GroupMembershipHistory.updated_at.desc()) ).all() @@ -226,10 +305,14 @@ class Auth0Role(BaseModel, table=True): id: str = Field(primary_key=True, unique=True) name: str description: str = Field(default="") - admin_groups: list["BiocommonsGroup"] = Relationship(back_populates="admin_roles", link_model=GroupRoleLink) + admin_groups: list["BiocommonsGroup"] = Relationship( + back_populates="admin_roles", link_model=GroupRoleLink + ) @classmethod - def get_or_create_by_id(cls, auth0_id: str, session: Session, auth0_client: Auth0Client) -> Self: + def get_or_create_by_id( + cls, auth0_id: str, session: Session, auth0_client: Auth0Client + ) -> Self: # Try to get from the DB role = session.get(Auth0Role, auth0_id) if role is not None: @@ -237,18 +320,20 @@ def get_or_create_by_id(cls, auth0_id: str, session: Session, auth0_client: Auth # Try to get from the API and save to the DB role_data = auth0_client.get_role_by_id(role_id=auth0_id) role = cls( - id=role_data.id, - name=role_data.name, - description=role_data.description + id=role_data.id, name=role_data.name, description=role_data.description ) session.add(role) session.commit() return role @classmethod - def get_or_create_by_name(cls, name: str, session: Session, auth0_client: Auth0Client = None) -> Self: + def get_or_create_by_name( + cls, name: str, session: Session, auth0_client: Auth0Client = None + ) -> Self: # Try to get from the DB - role = session.exec(select(Auth0Role).where(Auth0Role.name == name)).one_or_none() + role = session.exec( + select(Auth0Role).where(Auth0Role.name == name) + ).one_or_none() if role is not None: return role # Try to get from the API and save to the DB @@ -265,9 +350,13 @@ class BiocommonsGroup(BaseModel, table=True): # Human-readable name for the group name: str = Field(unique=True) # List of roles that are allowed to approve group membership - admin_roles: list[Auth0Role] = Relationship(back_populates="admin_groups", link_model=GroupRoleLink) + admin_roles: list[Auth0Role] = Relationship( + back_populates="admin_groups", link_model=GroupRoleLink + ) members: list[GroupMembership] = Relationship(back_populates="group") - approval_history: list[GroupMembershipHistory] = Relationship(back_populates="group") + approval_history: list[GroupMembershipHistory] = Relationship( + back_populates="group" + ) def get_admins(self, auth0_client: Auth0Client) -> set[str]: """ diff --git a/main.py b/main.py index 016ce1ff..064fcb46 100644 --- a/main.py +++ b/main.py @@ -8,7 +8,15 @@ # This has to be imported even if unused from db import models # noqa: F401 from db.admin import DatabaseAdmin -from routers import admin, biocommons_groups, bpa_register, galaxy_register, user, utils +from routers import ( + admin, + biocommons_groups, + biocommons_register, + bpa_register, + galaxy_register, + user, + utils, +) # Load .env to get CORS_ALLOWED_ORIGINS. # Note that for most env variables, we use pydantic-settings @@ -26,10 +34,12 @@ async def lifespan(app: FastAPI): # NOTE: we only create the database and tables automatically in development: # we assume that if the DB is an sqlite DB, we are in dev. from db.setup import create_db_and_tables + create_db_and_tables() DatabaseAdmin.setup(app=app, secret_key=SECRET_KEY) yield + app = FastAPI(lifespan=lifespan) app.add_middleware(SessionMiddleware, secret_key=SECRET_KEY) app.add_middleware( @@ -48,6 +58,7 @@ def public_route(): app.include_router(admin.router) app.include_router(user.router) +app.include_router(biocommons_register.router) app.include_router(bpa_register.router) app.include_router(galaxy_register.router) app.include_router(utils.router) diff --git a/routers/biocommons_register.py b/routers/biocommons_register.py new file mode 100644 index 00000000..55587c04 --- /dev/null +++ b/routers/biocommons_register.py @@ -0,0 +1,140 @@ +import logging +from typing import Any, Dict + +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException +from httpx import HTTPStatusError +from sqlmodel import Session + +from auth.ses import EmailService +from auth0.client import Auth0Client, get_auth0_client +from config import Settings, get_settings +from db.models import BiocommonsGroup, BiocommonsUser, PlatformEnum +from db.setup import get_db_session +from schemas.biocommons import Auth0UserData, BiocommonsRegisterData +from schemas.biocommons_register import BiocommonsRegistrationRequest + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/biocommons", tags=["biocommons", "registration"]) + + +def send_approval_email(registration: BiocommonsRegistrationRequest): + """Send email notification about new biocommons registration.""" + email_service = EmailService() + approver_email = "aai-dev@biocommons.org.au" + subject = "New BioCommons User Registration" + + body_html = f""" +

A new user has registered for the BioCommons platform.

+

User: {registration.first_name} {registration.last_name} ({registration.email})

+

Username: {registration.username}

+

Selected Bundle: {registration.bundle}

+

Requested Access: BPA Data Portal & Galaxy Australia

+

Please log into the AAI Admin Portal to review and approve access.

+ """ + + email_service.send(approver_email, subject, body_html) + + +@router.post( + "/register", + response_model=Dict[str, Any], + responses={ + 400: {"description": "Bad Request - Validation error"}, + 409: {"description": "Conflict - User already exists"}, + 500: {"description": "Internal server error"}, + }, +) +async def register_biocommons_user( + registration: BiocommonsRegistrationRequest, + background_tasks: BackgroundTasks, + settings: Settings = Depends(get_settings), + db_session: Session = Depends(get_db_session), + auth0_client: Auth0Client = Depends(get_auth0_client), +) -> Dict[str, Any]: + """Register a new BioCommons user.""" + + # Create Auth0 user data + user_data = BiocommonsRegisterData.from_biocommons_registration(registration) + + try: + logger.info("Registering user with Auth0") + auth0_user_data = auth0_client.create_user(user_data) + + logger.info("Adding user to DB") + _create_biocommons_user_record(auth0_user_data, registration, db_session) + + # Send approval email in background + if settings.send_email: + background_tasks.add_task(send_approval_email, registration) + + logger.info( + f"Successfully registered biocommons user: {auth0_user_data.user_id}" + ) + return { + "message": "User registered successfully", + "user": auth0_user_data.model_dump(mode="json"), + } + + except HTTPStatusError as e: + logger.error(f"Auth0 registration failed: {e}") + error_detail = "Registration failed" + if e.response.status_code == 409: + error_detail = "User already exists" + elif e.response.status_code == 400: + error_detail = "Invalid registration data" + raise HTTPException(status_code=e.response.status_code, detail=error_detail) + except Exception as e: + logger.error(f"Unexpected error during registration: {e}") + raise HTTPException(status_code=500, detail="Internal server error") + + +def _create_biocommons_user_record( + auth0_user_data: Auth0UserData, + registration: BiocommonsRegistrationRequest, + session: Session, +) -> BiocommonsUser: + """Create a BioCommons user record in the database with group membership based on selected bundle.""" + db_user = BiocommonsUser.from_auth0_data(data=auth0_user_data) + session.add(db_user) + session.flush() + + # Map bundle to group information + bundle_group_map = { + "bpa-galaxy": { + "id": "biocommons/group/bpa_galaxy", + "name": "BPA Data Portal & Galaxy Access", + }, + "tsi": {"id": "biocommons/group/tsi", "name": "Threatened Species Initiative"}, + } + + group_info = bundle_group_map[registration.bundle] + + # Create or get the BiocommonsGroup record + db_group = session.get(BiocommonsGroup, group_info["id"]) + if not db_group: + db_group = BiocommonsGroup( + group_id=group_info["id"], + name=group_info["name"], + admin_roles=[], + ) + session.add(db_group) + session.flush() + + # Create group membership + group_membership = db_user.add_group_membership( + group_id=group_info["id"], db_session=session, auto_approve=False + ) + session.add(group_membership) + + # Add platform memberships based on bundle + platforms_to_add = [PlatformEnum.BPA_DATA_PORTAL, PlatformEnum.GALAXY] + + for platform in platforms_to_add: + platform_membership = db_user.add_platform_membership( + platform=platform, db_session=session, auto_approve=False + ) + session.add(platform_membership) + + session.commit() + return db_user diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 453f76fb..2128c83f 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -4,6 +4,7 @@ These are the core schemas we use for storing/representing users and their metadata """ + import re from datetime import datetime, timezone from typing import Annotated, List, Literal, Optional, Self @@ -16,11 +17,17 @@ # From Auth0 password settings ALLOWED_SPECIAL_CHARS = "!@#$%^&*" -VALID_PASSWORD_REGEX = re.compile(f"^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[{ALLOWED_SPECIAL_CHARS}]).{{8,}}$") +VALID_PASSWORD_REGEX = re.compile( + f"^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[{ALLOWED_SPECIAL_CHARS}]).{{8,}}$" +) AppId = Literal["biocommons", "galaxy", "bpa"] -BiocommonsUsername = Annotated[str, StringConstraints(min_length=3, max_length=128, pattern='^[-_a-z0-9]+$')] -BiocommonsPassword = Annotated[str, StringConstraints(min_length=8, max_length=128, pattern=VALID_PASSWORD_REGEX)] +BiocommonsUsername = Annotated[ + str, StringConstraints(min_length=3, max_length=128, pattern="^[-_a-z0-9]+$") +] +BiocommonsPassword = Annotated[ + str, StringConstraints(min_length=8, max_length=128, pattern=VALID_PASSWORD_REGEX) +] class BPAMetadata(BaseModel): @@ -32,6 +39,7 @@ class BiocommonsUserMetadata(BaseModel): User metadata we use for user-changeable data like preferred usernames """ + bpa: Optional[BPAMetadata] = None @@ -41,6 +49,7 @@ class BiocommonsAppMetadata(BaseModel): Note we expect all app_metadata from Auth0 to match this format (if not empty). """ + groups: List[Group] = Field(default_factory=list) services: List[Service] = Field(default_factory=list) registration_from: Optional[AppId] = None @@ -69,7 +78,9 @@ def get_service_by_id(self, service_id: str) -> Optional[Service]: """Get a service by its ID.""" return next((s for s in self.services if s.id == service_id), None) - def get_resource_by_id(self, service_id: str, resource_id: str) -> Optional[Resource]: + def get_resource_by_id( + self, service_id: str, resource_id: str + ) -> Optional[Resource]: """Get a resource by its ID.""" service = self.get_service_by_id(service_id) if service: @@ -96,7 +107,9 @@ def approve_resource(self, service_id: str, resource_id: str, updated_by: str): resource = service.get_resource_by_id(resource_id) if not resource: - raise ValueError(f"Resource '{resource_id}' not found in service '{service_id}'.") + raise ValueError( + f"Resource '{resource_id}' not found in service '{service_id}'." + ) resource.status = "approved" resource.last_updated = datetime.now(timezone.utc) @@ -107,6 +120,7 @@ class BiocommonsRegisterData(BaseModel): """ Data we send to the /api/v2/users endpoint to register a user """ + email: EmailStr email_verified: bool = False password: BiocommonsPassword @@ -116,11 +130,17 @@ class BiocommonsRegisterData(BaseModel): user_metadata: Optional[BiocommonsUserMetadata] = None app_metadata: BiocommonsAppMetadata + def model_dump(self, **kwargs): + """Override model_dump to exclude user_metadata when it's None""" + data = super().model_dump(**kwargs) + if data.get("user_metadata") is None: + data.pop("user_metadata", None) + return data + @classmethod def from_bpa_registration( - cls, - registration: 'schemas.bpa.BPARegistrationRequest', - bpa_service: Service) -> Self: + cls, registration: "schemas.bpa.BPARegistrationRequest", bpa_service: Service + ) -> Self: return cls( email=registration.email, password=registration.password, @@ -130,15 +150,15 @@ def from_bpa_registration( bpa=BPAMetadata(registration_reason=registration.reason), ), app_metadata=BiocommonsAppMetadata( - services=[bpa_service], - registration_from="bpa" + services=[bpa_service], registration_from="bpa" ), ) @classmethod def from_galaxy_registration( - cls, - registration: 'schemas.galaxy.GalaxyRegistrationData',): + cls, + registration: "schemas.galaxy.GalaxyRegistrationData", + ): # Galaxy registration is approved automatically galaxy_service = Service( name="Galaxy Australia", @@ -146,7 +166,7 @@ def from_galaxy_registration( initial_request_time=datetime.now(), status="approved", last_updated=datetime.now(), - updated_by="" + updated_by="", ) return BiocommonsRegisterData( email=registration.email, @@ -155,8 +175,24 @@ def from_galaxy_registration( email_verified=False, connection="Username-Password-Authentication", app_metadata=BiocommonsAppMetadata( - services=[galaxy_service], - registration_from='galaxy' + services=[galaxy_service], registration_from="galaxy" + ), + ) + + @classmethod + def from_biocommons_registration( + cls, + registration: "schemas.biocommons_register.BiocommonsRegistrationRequest", + ): + return BiocommonsRegisterData( + email=registration.email, + username=registration.username, + password=registration.password, + name=f"{registration.first_name} {registration.last_name}", + email_verified=False, + connection="Username-Password-Authentication", + app_metadata=BiocommonsAppMetadata( + registration_from="biocommons", ), ) @@ -166,6 +202,7 @@ class Auth0UserData(BaseModel): Represents the user data we get back from Auth0 for Biocommons users (with our user and app metadata, if defined). """ + created_at: datetime email: EmailStr username: BiocommonsUsername diff --git a/schemas/biocommons_register.py b/schemas/biocommons_register.py new file mode 100644 index 00000000..4186cb4e --- /dev/null +++ b/schemas/biocommons_register.py @@ -0,0 +1,16 @@ +from typing import Literal + +from pydantic import BaseModel, EmailStr + +from schemas.biocommons import BiocommonsPassword, BiocommonsUsername + +BundleType = Literal["bpa-galaxy", "tsi"] + + +class BiocommonsRegistrationRequest(BaseModel): + first_name: str + last_name: str + email: EmailStr + username: BiocommonsUsername + password: BiocommonsPassword + bundle: BundleType diff --git a/tests/datagen.py b/tests/datagen.py index b3bcd992..3c72a2a1 100644 --- a/tests/datagen.py +++ b/tests/datagen.py @@ -5,6 +5,7 @@ from polyfactory.factories.pydantic_factory import ModelFactory from schemas.biocommons import Auth0UserData, BiocommonsAppMetadata +from schemas.biocommons_register import BiocommonsRegistrationRequest from schemas.bpa import BPARegistrationRequest from schemas.galaxy import GalaxyRegistrationData from schemas.tokens import AccessTokenPayload @@ -12,11 +13,11 @@ def random_auth0_id() -> str: - return "auth0|" + ''.join(random.choices('0123456789abcdef', k=24)) + return "auth0|" + "".join(random.choices("0123456789abcdef", k=24)) def random_auth0_role_id() -> str: - return "rol_" + ''.join(random.choices(ascii_letters + digits, k=16)) + return "rol_" + "".join(random.choices(ascii_letters + digits, k=16)) class AccessTokenPayloadFactory(ModelFactory[AccessTokenPayload]): @@ -31,14 +32,12 @@ class SessionUserFactory(ModelFactory[SessionUser]): ... class Auth0UserDataFactory(ModelFactory[Auth0UserData]): - @classmethod def user_id(cls) -> str: return random_auth0_id() class GalaxyRegistrationDataFactory(ModelFactory[GalaxyRegistrationData]): - @post_generated @classmethod def password_confirmation(cls, password: str) -> str: @@ -62,3 +61,11 @@ def get_default_organizations(cls) -> dict: class AppMetadataFactory(ModelFactory[BiocommonsAppMetadata]): ... + + +class BiocommonsRegistrationDataFactory(ModelFactory[BiocommonsRegistrationRequest]): + """Factory for generating BioCommons registration test data.""" + + @classmethod + def bundle(cls) -> str: + return "bpa-galaxy" diff --git a/tests/test_biocommons_register.py b/tests/test_biocommons_register.py new file mode 100644 index 00000000..980b3933 --- /dev/null +++ b/tests/test_biocommons_register.py @@ -0,0 +1,206 @@ +import pytest + +from schemas.biocommons import BiocommonsRegisterData +from schemas.biocommons_register import BiocommonsRegistrationRequest +from tests.datagen import Auth0UserDataFactory + + +def test_biocommons_registration_data_excludes_null_user_metadata(): + """Test that user_metadata is excluded when None and basic Auth0 data is correct""" + req = BiocommonsRegistrationRequest( + first_name="Test", + last_name="User", + email="test@example.com", + username="testuser", + password="StrongPass1!", + bundle="bpa-galaxy", + ) + + user_data = BiocommonsRegisterData.from_biocommons_registration(req) + + assert user_data.user_metadata is None + + dumped = user_data.model_dump(mode="json") + assert "user_metadata" not in dumped + + # Test that all required fields are present + assert dumped["email"] == "test@example.com" + assert dumped["username"] == "testuser" + assert dumped["name"] == "Test User" + assert dumped["password"] == "StrongPass1!" + assert dumped["connection"] == "Username-Password-Authentication" + assert dumped["email_verified"] is False + + assert "app_metadata" in dumped + app_metadata = dumped["app_metadata"] + assert app_metadata["registration_from"] == "biocommons" + assert app_metadata.get("services", []) == [] + assert app_metadata.get("groups", []) == [] + + +def test_biocommons_registration_tsi_bundle(): + """Test TSI bundle registration creates correct basic Auth0 data""" + req = BiocommonsRegistrationRequest( + first_name="TSI", + last_name="User", + email="tsi@example.com", + username="tsiuser", + password="StrongPass1!", + bundle="tsi", + ) + + user_data = BiocommonsRegisterData.from_biocommons_registration(req) + dumped = user_data.model_dump(mode="json") + + assert dumped["name"] == "TSI User" + assert dumped["app_metadata"]["registration_from"] == "biocommons" + assert dumped["app_metadata"].get("groups", []) == [] + assert dumped["app_metadata"].get("services", []) == [] + + +def test_create_biocommons_user_record_bpa_galaxy_bundle(test_db_session): + """Test database record creation for bpa-galaxy bundle""" + from db.models import PlatformEnum + from routers.biocommons_register import _create_biocommons_user_record + + registration = BiocommonsRegistrationRequest( + first_name="BPA", + last_name="Galaxy", + email="bpa.galaxy@example.com", + username="bpagalaxy", + password="StrongPass1!", + bundle="bpa-galaxy", + ) + + auth0_data = Auth0UserDataFactory.build( + email="bpa.galaxy@example.com", + username="bpagalaxy", + name="BPA Galaxy", + user_id="auth0|bpagalaxy123", + ) + + # Create user record + user = _create_biocommons_user_record(auth0_data, registration, test_db_session) + + # Verify user basic data + assert user.username == "bpagalaxy" + assert user.email == "bpa.galaxy@example.com" + assert user.id == "auth0|bpagalaxy123" + + # Verify group membership + assert len(user.group_memberships) == 1 + group_membership = user.group_memberships[0] + assert group_membership.group_id == "biocommons/group/bpa_galaxy" + assert group_membership.approval_status.value == "pending" + + # Verify platform memberships (should have both BPA and Galaxy) + assert len(user.platform_memberships) == 2 + platform_ids = {pm.platform_id for pm in user.platform_memberships} + assert PlatformEnum.BPA_DATA_PORTAL in platform_ids + assert PlatformEnum.GALAXY in platform_ids + + # Verify all platform memberships are pending + for pm in user.platform_memberships: + assert pm.approval_status.value == "pending" + + +def test_create_biocommons_user_record_tsi_bundle(test_db_session): + """Test database record creation for tsi bundle""" + from db.models import PlatformEnum + from routers.biocommons_register import _create_biocommons_user_record + + registration = BiocommonsRegistrationRequest( + first_name="TSI", + last_name="User", + email="tsi.user@example.com", + username="tsiuser", + password="StrongPass1!", + bundle="tsi", + ) + + auth0_data = Auth0UserDataFactory.build( + email="tsi.user@example.com", + username="tsiuser", + name="TSI User", + user_id="auth0|tsiuser123", + ) + + # Create user record + user = _create_biocommons_user_record(auth0_data, registration, test_db_session) + + # Verify user basic data + assert user.username == "tsiuser" + assert user.email == "tsi.user@example.com" + + # Verify group membership (should be TSI group) + assert len(user.group_memberships) == 1 + group_membership = user.group_memberships[0] + assert group_membership.group_id == "biocommons/group/tsi" + assert group_membership.approval_status.value == "pending" + + # Verify platform memberships (should still have both BPA and Galaxy) + assert len(user.platform_memberships) == 2 + platform_ids = {pm.platform_id for pm in user.platform_memberships} + assert PlatformEnum.BPA_DATA_PORTAL in platform_ids + assert PlatformEnum.GALAXY in platform_ids + + +def test_biocommons_group_creation(test_db_session): + """Test that BiocommonsGroup records are created when they don't exist""" + from db.models import BiocommonsGroup + from routers.biocommons_register import _create_biocommons_user_record + + registration = BiocommonsRegistrationRequest( + first_name="New", + last_name="Group", + email="new.group@example.com", + username="newgroup", + password="StrongPass1!", + bundle="bpa-galaxy", + ) + + auth0_data = Auth0UserDataFactory.build( + email="new.group@example.com", username="newgroup", user_id="auth0|newgroup123" + ) + + # Verify group doesn't exist initially + group = test_db_session.get(BiocommonsGroup, "biocommons/group/bpa_galaxy") + assert group is None + + # Create user record (should create group) + _create_biocommons_user_record(auth0_data, registration, test_db_session) + + # Verify group was created + group = test_db_session.get(BiocommonsGroup, "biocommons/group/bpa_galaxy") + assert group is not None + assert group.group_id == "biocommons/group/bpa_galaxy" + assert group.name == "BPA Data Portal & Galaxy Access" + assert group.admin_roles == [] + + +def test_bundle_validation(): + """Test that only valid bundles are accepted""" + with pytest.raises(ValueError): + BiocommonsRegistrationRequest( + first_name="Invalid", + last_name="Bundle", + email="invalid@example.com", + username="invalid", + password="StrongPass1!", + bundle="invalid-bundle", + ) + + +def test_biocommons_registration_name_formatting(): + """Test that first_name and last_name are properly combined""" + req = BiocommonsRegistrationRequest( + first_name="John", + last_name="Doe-Smith", + email="john.doe@example.com", + username="johndoe", + password="StrongPass1!", + bundle="tsi", + ) + + user_data = BiocommonsRegisterData.from_biocommons_registration(req) + assert user_data.name == "John Doe-Smith" From fc36622e8200c61dc742a47d550e6df82ca96f64 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Wed, 20 Aug 2025 11:27:53 +1000 Subject: [PATCH 2/3] Refactor bundle configuration and improve group membership validation in user registration --- routers/biocommons_register.py | 45 ++++--- tests/test_biocommons_register.py | 211 +++++++++++++++++++++++++----- 2 files changed, 200 insertions(+), 56 deletions(-) diff --git a/routers/biocommons_register.py b/routers/biocommons_register.py index 55587c04..50777c27 100644 --- a/routers/biocommons_register.py +++ b/routers/biocommons_register.py @@ -15,6 +15,18 @@ logger = logging.getLogger(__name__) +# Bundle configuration mapping bundle names to their groups and included platforms +BUNDLES = { + "bpa-galaxy": { + "group_id": "biocommons/group/bpa_galaxy", + "platforms": [PlatformEnum.BPA_DATA_PORTAL, PlatformEnum.GALAXY], + }, + "tsi": { + "group_id": "biocommons/group/tsi", + "platforms": [PlatformEnum.BPA_DATA_PORTAL, PlatformEnum.GALAXY], + }, +} + router = APIRouter(prefix="/biocommons", tags=["biocommons", "registration"]) @@ -99,38 +111,25 @@ def _create_biocommons_user_record( session.add(db_user) session.flush() - # Map bundle to group information - bundle_group_map = { - "bpa-galaxy": { - "id": "biocommons/group/bpa_galaxy", - "name": "BPA Data Portal & Galaxy Access", - }, - "tsi": {"id": "biocommons/group/tsi", "name": "Threatened Species Initiative"}, - } + # Get bundle configuration + bundle_config = BUNDLES[registration.bundle] + group_id = bundle_config["group_id"] - group_info = bundle_group_map[registration.bundle] - - # Create or get the BiocommonsGroup record - db_group = session.get(BiocommonsGroup, group_info["id"]) + # Verify the group exists (this will raise an error if it doesn't) + db_group = session.get(BiocommonsGroup, group_id) if not db_group: - db_group = BiocommonsGroup( - group_id=group_info["id"], - name=group_info["name"], - admin_roles=[], + raise ValueError( + f"Group '{group_id}' not found. Groups must be pre-configured in the database." ) - session.add(db_group) - session.flush() # Create group membership group_membership = db_user.add_group_membership( - group_id=group_info["id"], db_session=session, auto_approve=False + group_id=group_id, db_session=session, auto_approve=False ) session.add(group_membership) - # Add platform memberships based on bundle - platforms_to_add = [PlatformEnum.BPA_DATA_PORTAL, PlatformEnum.GALAXY] - - for platform in platforms_to_add: + # Add platform memberships based on bundle configuration + for platform in bundle_config["platforms"]: platform_membership = db_user.add_platform_membership( platform=platform, db_session=session, auto_approve=False ) diff --git a/tests/test_biocommons_register.py b/tests/test_biocommons_register.py index 980b3933..b8375ea5 100644 --- a/tests/test_biocommons_register.py +++ b/tests/test_biocommons_register.py @@ -23,7 +23,6 @@ def test_biocommons_registration_data_excludes_null_user_metadata(): dumped = user_data.model_dump(mode="json") assert "user_metadata" not in dumped - # Test that all required fields are present assert dumped["email"] == "test@example.com" assert dumped["username"] == "testuser" assert dumped["name"] == "Test User" @@ -60,9 +59,17 @@ def test_biocommons_registration_tsi_bundle(): def test_create_biocommons_user_record_bpa_galaxy_bundle(test_db_session): """Test database record creation for bpa-galaxy bundle""" - from db.models import PlatformEnum + from db.models import BiocommonsGroup, PlatformEnum from routers.biocommons_register import _create_biocommons_user_record + group = BiocommonsGroup( + group_id="biocommons/group/bpa_galaxy", + name="BPA Data Portal & Galaxy Access", + admin_roles=[], + ) + test_db_session.add(group) + test_db_session.commit() + registration = BiocommonsRegistrationRequest( first_name="BPA", last_name="Galaxy", @@ -79,36 +86,39 @@ def test_create_biocommons_user_record_bpa_galaxy_bundle(test_db_session): user_id="auth0|bpagalaxy123", ) - # Create user record user = _create_biocommons_user_record(auth0_data, registration, test_db_session) - # Verify user basic data assert user.username == "bpagalaxy" assert user.email == "bpa.galaxy@example.com" assert user.id == "auth0|bpagalaxy123" - # Verify group membership assert len(user.group_memberships) == 1 group_membership = user.group_memberships[0] assert group_membership.group_id == "biocommons/group/bpa_galaxy" assert group_membership.approval_status.value == "pending" - # Verify platform memberships (should have both BPA and Galaxy) assert len(user.platform_memberships) == 2 platform_ids = {pm.platform_id for pm in user.platform_memberships} assert PlatformEnum.BPA_DATA_PORTAL in platform_ids assert PlatformEnum.GALAXY in platform_ids - # Verify all platform memberships are pending for pm in user.platform_memberships: assert pm.approval_status.value == "pending" def test_create_biocommons_user_record_tsi_bundle(test_db_session): """Test database record creation for tsi bundle""" - from db.models import PlatformEnum + from db.models import BiocommonsGroup, PlatformEnum from routers.biocommons_register import _create_biocommons_user_record + group = BiocommonsGroup( + group_id="biocommons/group/tsi", + name="Threatened Species Initiative", + admin_roles=[], + ) + test_db_session.add(group) + test_db_session.commit() + registration = BiocommonsRegistrationRequest( first_name="TSI", last_name="User", @@ -125,67 +135,88 @@ def test_create_biocommons_user_record_tsi_bundle(test_db_session): user_id="auth0|tsiuser123", ) - # Create user record user = _create_biocommons_user_record(auth0_data, registration, test_db_session) - # Verify user basic data assert user.username == "tsiuser" assert user.email == "tsi.user@example.com" - # Verify group membership (should be TSI group) assert len(user.group_memberships) == 1 group_membership = user.group_memberships[0] assert group_membership.group_id == "biocommons/group/tsi" assert group_membership.approval_status.value == "pending" - # Verify platform memberships (should still have both BPA and Galaxy) assert len(user.platform_memberships) == 2 platform_ids = {pm.platform_id for pm in user.platform_memberships} assert PlatformEnum.BPA_DATA_PORTAL in platform_ids assert PlatformEnum.GALAXY in platform_ids -def test_biocommons_group_creation(test_db_session): - """Test that BiocommonsGroup records are created when they don't exist""" - from db.models import BiocommonsGroup +def test_biocommons_group_must_exist(test_db_session): + """Test that registration fails when the required group doesn't exist""" + import pytest + from routers.biocommons_register import _create_biocommons_user_record registration = BiocommonsRegistrationRequest( first_name="New", - last_name="Group", - email="new.group@example.com", - username="newgroup", + last_name="User", + email="new.user@example.com", + username="newuser", password="StrongPass1!", bundle="bpa-galaxy", ) auth0_data = Auth0UserDataFactory.build( - email="new.group@example.com", username="newgroup", user_id="auth0|newgroup123" + email="new.user@example.com", username="newuser", user_id="auth0|newuser123" ) - # Verify group doesn't exist initially - group = test_db_session.get(BiocommonsGroup, "biocommons/group/bpa_galaxy") - assert group is None + with pytest.raises( + ValueError, match="Group 'biocommons/group/bpa_galaxy' not found" + ): + _create_biocommons_user_record(auth0_data, registration, test_db_session) - # Create user record (should create group) - _create_biocommons_user_record(auth0_data, registration, test_db_session) - # Verify group was created - group = test_db_session.get(BiocommonsGroup, "biocommons/group/bpa_galaxy") - assert group is not None - assert group.group_id == "biocommons/group/bpa_galaxy" - assert group.name == "BPA Data Portal & Galaxy Access" - assert group.admin_roles == [] +def test_biocommons_group_membership_with_existing_group(test_db_session): + """Test that user is assigned to group when group exists""" + from db.models import BiocommonsGroup + from routers.biocommons_register import _create_biocommons_user_record + + group = BiocommonsGroup( + group_id="biocommons/group/bpa_galaxy", + name="BPA Data Portal & Galaxy Access", + admin_roles=[], + ) + test_db_session.add(group) + test_db_session.commit() + + registration = BiocommonsRegistrationRequest( + first_name="Test", + last_name="User", + email="test.user@example.com", + username="testuser", + password="StrongPass1!", + bundle="bpa-galaxy", + ) + + auth0_data = Auth0UserDataFactory.build( + email="test.user@example.com", username="testuser", user_id="auth0|testuser123" + ) + + user = _create_biocommons_user_record(auth0_data, registration, test_db_session) + + assert len(user.group_memberships) == 1 + assert user.group_memberships[0].group_id == "biocommons/group/bpa_galaxy" + assert user.group_memberships[0].approval_status.value == "pending" def test_bundle_validation(): """Test that only valid bundles are accepted""" with pytest.raises(ValueError): BiocommonsRegistrationRequest( - first_name="Invalid", + first_name="Test", last_name="Bundle", - email="invalid@example.com", - username="invalid", + email="test@example.com", + username="test", password="StrongPass1!", bundle="invalid-bundle", ) @@ -204,3 +235,117 @@ def test_biocommons_registration_name_formatting(): user_data = BiocommonsRegisterData.from_biocommons_registration(req) assert user_data.name == "John Doe-Smith" + + +def test_successful_biocommons_registration_endpoint( + test_client_with_email, mock_auth0_client, test_db_session, mocker +): + """Test successful biocommons registration via HTTP endpoint""" + from db.models import BiocommonsGroup, BiocommonsUser, PlatformEnum + from tests.datagen import random_auth0_id + + group = BiocommonsGroup( + group_id="biocommons/group/bpa_galaxy", + name="BPA Data Portal & Galaxy Access", + admin_roles=[], + ) + test_db_session.add(group) + test_db_session.commit() + + user_id = random_auth0_id() + mock_auth0_client.create_user.return_value = Auth0UserDataFactory.build( + user_id=user_id, email="test@example.com", username="testuser" + ) + + mock_email_cls = mocker.patch( + "routers.biocommons_register.EmailService", autospec=True + ) + mock_email_cls.return_value.send.return_value = None + + registration_data = { + "first_name": "Test", + "last_name": "User", + "email": "test@example.com", + "username": "testuser", + "password": "StrongPass1!", + "bundle": "bpa-galaxy", + } + + response = test_client_with_email.post( + "/biocommons/register", json=registration_data + ) + + assert response.status_code == 200 + assert response.json()["message"] == "User registered successfully" + assert "user" in response.json() + + db_user = test_db_session.get(BiocommonsUser, user_id) + assert db_user is not None + assert db_user.username == "testuser" + assert db_user.email == "test@example.com" + + assert len(db_user.group_memberships) == 1 + assert db_user.group_memberships[0].group_id == "biocommons/group/bpa_galaxy" + + assert len(db_user.platform_memberships) == 2 + platform_ids = {pm.platform_id for pm in db_user.platform_memberships} + assert PlatformEnum.BPA_DATA_PORTAL in platform_ids + assert PlatformEnum.GALAXY in platform_ids + + mock_email_cls.return_value.send.assert_called_once() + + +def test_biocommons_registration_auth0_conflict_error( + test_client, mock_auth0_client, test_db_session +): + """Test handling of Auth0 conflict error (user already exists)""" + from httpx import HTTPStatusError, Request, Response + + from db.models import BiocommonsGroup + + group = BiocommonsGroup( + group_id="biocommons/group/bpa_galaxy", + name="BPA Data Portal & Galaxy Access", + admin_roles=[], + ) + test_db_session.add(group) + test_db_session.commit() + + response = Response(409, json={"error": "user_exists"}) + request = Request("POST", "https://example.com") + mock_auth0_client.create_user.side_effect = HTTPStatusError( + "User already exists", request=request, response=response + ) + + registration_data = { + "first_name": "Test", + "last_name": "User", + "email": "existing@example.com", + "username": "existinguser", + "password": "StrongPass1!", + "bundle": "bpa-galaxy", + } + + response = test_client.post("/biocommons/register", json=registration_data) + + assert response.status_code == 409 + assert response.json()["detail"] == "User already exists" + + +def test_biocommons_registration_missing_group_error(test_client, mock_auth0_client): + """Test error when required group doesn't exist in database""" + registration_data = { + "first_name": "Test", + "last_name": "User", + "email": "test@example.com", + "username": "testuser", + "password": "StrongPass1!", + "bundle": "bpa-galaxy", + } + + mock_auth0_client.create_user.return_value = Auth0UserDataFactory.build() + + response = test_client.post("/biocommons/register", json=registration_data) + + assert response.status_code == 500 + assert response.json()["detail"] == "Internal server error" From 492c50a10e23bf32193f42907445096f50b7b863 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Wed, 20 Aug 2025 12:42:47 +1000 Subject: [PATCH 3/3] Update bundle configuration to auto-approve platform memberships and adjust tests accordingly --- routers/biocommons_register.py | 9 ++++++--- tests/test_biocommons_register.py | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/routers/biocommons_register.py b/routers/biocommons_register.py index 50777c27..be45825e 100644 --- a/routers/biocommons_register.py +++ b/routers/biocommons_register.py @@ -11,12 +11,15 @@ from db.models import BiocommonsGroup, BiocommonsUser, PlatformEnum from db.setup import get_db_session from schemas.biocommons import Auth0UserData, BiocommonsRegisterData -from schemas.biocommons_register import BiocommonsRegistrationRequest +from schemas.biocommons_register import BiocommonsRegistrationRequest, BundleType logger = logging.getLogger(__name__) # Bundle configuration mapping bundle names to their groups and included platforms -BUNDLES = { +# Note: Platforms listed here are auto-approved upon registration, +# while group memberships require manual approval +# Currently BPA Data Portal and Galaxy are auto-approved for all bundles +BUNDLES: dict[BundleType, dict] = { "bpa-galaxy": { "group_id": "biocommons/group/bpa_galaxy", "platforms": [PlatformEnum.BPA_DATA_PORTAL, PlatformEnum.GALAXY], @@ -131,7 +134,7 @@ def _create_biocommons_user_record( # Add platform memberships based on bundle configuration for platform in bundle_config["platforms"]: platform_membership = db_user.add_platform_membership( - platform=platform, db_session=session, auto_approve=False + platform=platform, db_session=session, auto_approve=True ) session.add(platform_membership) diff --git a/tests/test_biocommons_register.py b/tests/test_biocommons_register.py index b8375ea5..5bacb881 100644 --- a/tests/test_biocommons_register.py +++ b/tests/test_biocommons_register.py @@ -103,7 +103,7 @@ def test_create_biocommons_user_record_bpa_galaxy_bundle(test_db_session): assert PlatformEnum.GALAXY in platform_ids for pm in user.platform_memberships: - assert pm.approval_status.value == "pending" + assert pm.approval_status.value == "approved" def test_create_biocommons_user_record_tsi_bundle(test_db_session):