From 60918ef17173206d61297d7ac19299874e1b87b1 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 10:34:33 +1000 Subject: [PATCH 01/32] feat: allow filtering users by connection when searching by email --- auth0/client.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/auth0/client.py b/auth0/client.py index b879b6d7..11d9a596 100644 --- a/auth0/client.py +++ b/auth0/client.py @@ -395,10 +395,22 @@ def remove_roles_from_user(self, user_id: str, role_id: str | list[str]): resp.raise_for_status() return True - def search_users_by_email(self, email: str) -> list[Auth0UserData]: + def search_users_by_email(self, email: str, connection: str | None = None) -> list[Auth0UserData]: + """ + Search users by email, optionally filtering by connection name. + """ url = f"https://{self.domain}/api/v2/users-by-email" resp = self._client.get(url, params={"email": email}) - return self._convert_users(resp) + users = self._convert_users(resp) + if connection is not None: + filtered_users = [] + for user in users: + in_connection = any(ident.connection == connection for ident in user.identities) + if in_connection: + filtered_users.append(user) + return filtered_users + else: + return users def _search_users(self, query: str, page: Optional[int] = None, per_page: Optional[int] = None) -> list[Auth0UserData]: params = {"q": query, "search_engine": "v3"} From b8418f42d61d3eb735200a8333ee0e0c46dcdf98 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 10:39:55 +1000 Subject: [PATCH 02/32] feat: updated schemas for AAF info --- schemas/auth0.py | 14 ++++++++++++++ schemas/biocommons.py | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/schemas/auth0.py b/schemas/auth0.py index de6c662b..57070774 100644 --- a/schemas/auth0.py +++ b/schemas/auth0.py @@ -1,5 +1,7 @@ import re +from pydantic import BaseModel, EmailStr + PLATFORM_ROLE_PATTERN = re.compile(r"biocommons/platform/(?P[a-z0-9_-]+)") GROUP_ROLE_PATTERN = re.compile(r"biocommons/group/(?P[a-z0-9_-]+)") @@ -16,3 +18,15 @@ def get_group_id_from_role_name(role_name: str) -> str | None: if match: return match.group("group_id") return None + + +class Auth0ActionToken(BaseModel): + """ + Token payload that we use in Auth0 actions - signed with a shared secret. + + Any future actions we create should use this payload. + """ + user_id: str + email: EmailStr + client_id: str + purpose: str diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 52009045..979fb10a 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -192,6 +192,10 @@ class BiocommonsAppMetadata(BaseModel): old_emails: Optional[list[OldEmailRecord]] = None user_needs_migration: Optional[bool] = None account_type: BiocommonsUserAccountType + aaf_only: Optional[bool] = None + checked_email: Optional[EmailStr] = None + linking_completed: Optional[bool] = None + linking_completed_at: Optional[bool] = None model_config = { "extra": "ignore" @@ -217,6 +221,10 @@ class BiocommonsAppMetadataUpdate(BaseModel): old_emails: Optional[list[OldEmailRecord]] = None user_needs_migration: Optional[bool] = None account_type: Optional[BiocommonsUserAccountType] = None + aaf_only: Optional[bool] = None + checked_email: Optional[EmailStr] = None + linking_completed: Optional[bool] = None + linking_completed_at: Optional[bool] = None model_config = { "extra": "ignore" From d36f484f289eac037f4e8376e93341a5afc643aa Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 15:44:41 +1000 Subject: [PATCH 03/32] feat: link_identity method for Auth0Client --- auth0/client.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/auth0/client.py b/auth0/client.py index 11d9a596..c37ccd9a 100644 --- a/auth0/client.py +++ b/auth0/client.py @@ -121,6 +121,18 @@ class IdentityData(BaseModel): provider: str +class LinkIdentityRequest(BaseModel): + """ + POST data sent when linking identities. + + Note: user_id is the secondary ID here. Primary ID goes in the + URL + """ + provider: str + connection_id: str | None = None + user_id: str + + class EmailVerificationRequest(BaseModel): user_id: str client_id: Optional[str] = None @@ -596,6 +608,21 @@ def trigger_password_change(self, user_email: str, client_id: str, settings: Set resp.raise_for_status() return True + def link_identity(self, primary_user_id: str, secondary_user_id: str, secondary_provider: str) -> bool: + """ + Link an identity to a primary account + """ + url = f"https://{self.domain}/users/{primary_user_id}/identities" + secondary_connection = self.get_connection_by_name(secondary_provider) + payload = LinkIdentityRequest( + provider=secondary_provider, + connection_id=secondary_connection.id, + user_id=secondary_user_id, + ) + resp = self._client.post(url, json=payload.model_dump(mode="json", exclude_none=True)) + resp.raise_for_status() + return True + def get_auth0_client(settings: Settings = Depends(get_settings), management_token: str = Depends(get_management_token)): From 917e625aff942cb50a1e5bcdf1b5b6c70e6bbcb2 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:05:22 +1000 Subject: [PATCH 04/32] feat: record other user ID for users --- db/models.py | 3 ++ .../versions/98fbefd9f191_other_user_id.py | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 migrations/versions/98fbefd9f191_other_user_id.py diff --git a/db/models.py b/db/models.py index 64203d7f..eaaeefeb 100644 --- a/db/models.py +++ b/db/models.py @@ -40,6 +40,9 @@ class BiocommonsUser(SoftDeleteModel, table=True): ) # Auth0 ID id: str = Field(primary_key=True) + # User ID from other providers, i.e. AAF. Don't want to rely on this in the DB + # (Auth0 should handle linked identities), but good for tracking user info + other_user_id: str | None = Field(default=None, nullable=True) account_type: BiocommonsUserAccountType = Field( sa_type=DbEnum( BiocommonsUserAccountType, diff --git a/migrations/versions/98fbefd9f191_other_user_id.py b/migrations/versions/98fbefd9f191_other_user_id.py new file mode 100644 index 00000000..35dda555 --- /dev/null +++ b/migrations/versions/98fbefd9f191_other_user_id.py @@ -0,0 +1,31 @@ +"""other_user_id + +Revision ID: 98fbefd9f191 +Revises: 0c91366532ae +Create Date: 2026-08-24 15:43:31.428645 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel + + +# revision identifiers, used by Alembic. +revision: str = '98fbefd9f191' +down_revision: Union[str, None] = '0c91366532ae' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('biocommons_user', sa.Column('other_user_id', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('biocommons_user', 'other_user_id') + # ### end Alembic commands ### From e07815aa28e38a11388e822b20fc441fbc89255f Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:06:27 +1000 Subject: [PATCH 05/32] feat: method to link account for BiocommonsUser --- db/models.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/db/models.py b/db/models.py index eaaeefeb..4438c81f 100644 --- a/db/models.py +++ b/db/models.py @@ -366,6 +366,21 @@ def update_username(self, new_username: str, session: Session, updated_by: Self if commit: session.commit() + def link_aaf_account(self, aaf_user_id: str, session: Session, updated_by: Self | None = None, commit: bool = False) -> None: + """ + Set an existing account as an AAF account with a linked ID. + + Auth0 linking is not handled here and must be done separately. + """ + # Save history before update + self.save_history(session=session, change="aaf_account_linking", updated_by=updated_by, commit=False) + self.account_type = BiocommonsUserAccountType.AAF + self.other_user_id = aaf_user_id + session.add(self) + if commit: + session.commit() + + def save_history(self, session: Session, change: str | None = None, reason: str | None = None, updated_by: Self | None = None, commit: bool = False) -> 'BiocommonsUserHistory': From fc85223968fa19b59d160bba797c75d53cf8f3bf Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:09:43 +1000 Subject: [PATCH 06/32] fix: linking_completed_at should be datetime --- schemas/biocommons.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 979fb10a..432a9d56 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -15,6 +15,7 @@ from fastapi import Path from pydantic import ( AfterValidator, + AwareDatetime, BaseModel, EmailStr, Field, @@ -195,7 +196,7 @@ class BiocommonsAppMetadata(BaseModel): aaf_only: Optional[bool] = None checked_email: Optional[EmailStr] = None linking_completed: Optional[bool] = None - linking_completed_at: Optional[bool] = None + linking_completed_at: Optional[AwareDatetime] = None model_config = { "extra": "ignore" @@ -224,7 +225,7 @@ class BiocommonsAppMetadataUpdate(BaseModel): aaf_only: Optional[bool] = None checked_email: Optional[EmailStr] = None linking_completed: Optional[bool] = None - linking_completed_at: Optional[bool] = None + linking_completed_at: Optional[AwareDatetime] = None model_config = { "extra": "ignore" From ebe6de1d904f1285d1a63f0e8ae73df36b46a175 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:12:37 +1000 Subject: [PATCH 07/32] feat: dependency to check Auth0 action tokens --- dependencies/auth.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 dependencies/auth.py diff --git a/dependencies/auth.py b/dependencies/auth.py new file mode 100644 index 00000000..9b939d9a --- /dev/null +++ b/dependencies/auth.py @@ -0,0 +1,21 @@ +from typing import Annotated + +from fastapi import Depends, HTTPException + +from auth.validator import verify_action_token +from config import Settings, get_settings +from schemas.auth0 import Auth0ActionToken + + +def require_action_token(purpose: str | None = None): + """ + Dependency that verifies the session_token (from query params), + and checks for the specified purpose, if present + """ + def check_action_token(session_token: str, settings: Annotated[Settings, Depends(get_settings)]): + payload = verify_action_token(session_token, settings=settings) + if purpose is not None: + if payload.get("purpose", None) != purpose: + raise HTTPException(status_code=401, detail="invalid purpose") + return Auth0ActionToken(**payload) + return check_action_token From cb208b7c53e88510a500655b38e7af14e014f6c0 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:15:44 +1000 Subject: [PATCH 08/32] feat: router with initial /aaf/check-link endpoint --- routers/aaf.py | 105 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 routers/aaf.py diff --git a/routers/aaf.py b/routers/aaf.py new file mode 100644 index 00000000..15f37e30 --- /dev/null +++ b/routers/aaf.py @@ -0,0 +1,105 @@ +import logging +from datetime import datetime, timezone +from http import HTTPStatus +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel +from sqlmodel import Session + +from auth0.client import Auth0Client, UpdateUserData, get_auth0_client +from config import Settings, get_settings +from db.models import BiocommonsUser +from db.setup import get_db_session +from dependencies.auth import require_action_token +from schemas.auth0 import Auth0ActionToken +from schemas.biocommons import ( + Auth0UserData, + BiocommonsAppMetadataUpdate, + BiocommonsUserAccountType, +) + +router = APIRouter( + prefix="/aaf", tags=["aaf"] +) +logger = logging.getLogger("uvicorn.error") + + +class AccountLinkResponse(BaseModel): + link: bool + aaf_only: bool = False + primary_id: str + + +def link_aaf_account(db_user_id: str, aaf_user_id: str, auth0_client: Auth0Client, session: Session): + def _get_aaf_identity(aaf_user: Auth0UserData): + for identity in aaf_user.identities: + if identity.connection == "AAF": + return identity + return None + aaf_user_info = auth0_client.get_user(user_id=aaf_user_id) + aaf_identity = _get_aaf_identity(aaf_user_info) + if aaf_identity is None: + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Couldn't get AAF provider information") + + logger.info("Linking AAF account to existing database account") + now = datetime.now(tz=timezone.utc) + auth0_client.link_identity(primary_user_id=db_user_id, secondary_user_id=aaf_user_id, + secondary_provider=aaf_identity.provider) + auth0_client.update_user( + db_user_id, + update_data=UpdateUserData( + app_metadata=BiocommonsAppMetadataUpdate( + account_type=BiocommonsUserAccountType.AAF, + linking_completed=True, + linking_completed_at=now + ) + ) + ) + logger.info("Updating DB record") + db_user = BiocommonsUser.get_by_id_or_404(db_user_id, session=session) + db_user.link_aaf_account(aaf_user_id=aaf_user_id, session=session, updated_by=db_user, commit=True) + return db_user + + + + + + +@router.get("/check-link", response_model=AccountLinkResponse) +def check_aaf_account_link( + token: Annotated[Auth0ActionToken, Depends(require_action_token(purpose="aaf_link"))], + session: Annotated[Session, Depends(get_db_session)], + auth0_client: Annotated[Auth0Client, Depends(get_auth0_client)], + settings: Annotated[Settings, Depends(get_settings)], +): + email = token.email + aaf_user_id = token.user_id + auth0_matches = auth0_client.search_users_by_email( + email, + connection=settings.auth0_db_connection + ) + # No existing account: no need to link + if not auth0_matches: + # TODO: update app_metadata + return AccountLinkResponse(link=False, aaf_only=True, primary_id=token.user_id) + + existing_account = None + for user in auth0_matches: + if user.email.lower() == email.lower(): + existing_account = user + break + # No exact match: are we safe to say there's no existing account here? + if not existing_account: + return AccountLinkResponse(link=False, aaf_only=True, primary_id=email) + + if existing_account.blocked: + raise HTTPException(status_code=403, detail="Existing account is blocked.") + + link_aaf_account( + db_user_id=existing_account.user_id, + aaf_user_id=aaf_user_id, + auth0_client=auth0_client, + session=session, + ) + return AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id) From 3e416e1d69e984f960cebbb3bf85a23f476e5a4c Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:16:16 +1000 Subject: [PATCH 09/32] chore: include aaf router in main --- main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main.py b/main.py index e69c06d3..e6fb760c 100644 --- a/main.py +++ b/main.py @@ -11,6 +11,7 @@ from db import models # noqa: F401 from db.st_admin import setup_starlette_admin from routers import ( + aaf, admin, biocommons_admin, biocommons_register, @@ -78,6 +79,7 @@ def public_route(): CURRENT_ENVIRONMENT = read_setting("ENVIRONMENT", "dev") +app.include_router(aaf.router) app.include_router(admin.router) app.include_router(biocommons_admin.router) app.include_router(user.router) From 5902ee98c1b2cc72d1f436aa7148eb95f07275b2 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:52:22 +1000 Subject: [PATCH 10/32] refactor: use a base URL for Auth0 API calls --- auth0/client.py | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/auth0/client.py b/auth0/client.py index c37ccd9a..b35db74a 100644 --- a/auth0/client.py +++ b/auth0/client.py @@ -190,6 +190,7 @@ class Auth0Client: def __init__(self, domain: str, management_token: str): self.domain = domain + self.api_base = f"https://{domain}/api/v2" self.management_token = management_token self._client = httpx.Client(headers={"Authorization": f"Bearer {management_token}"}) @@ -230,7 +231,7 @@ def get_users(self, page: Optional[int] = None, per_page: Optional[int] = None, if q is not None: params["q"] = q params["search_engine"] = "v3" - url = f"https://{self.domain}/api/v2/users" + url = f"{self.api_base}/users" resp = self._client.get(url, params=params) resp.raise_for_status() if include_totals: @@ -238,13 +239,13 @@ def get_users(self, page: Optional[int] = None, per_page: Optional[int] = None, return self._convert_users(resp) def get_user(self, user_id: str) -> Auth0UserData: - url = f"https://{self.domain}/api/v2/users/{user_id}" + url = f"{self.api_base}/users/{user_id}" resp = self._client.get(url) resp.raise_for_status() return Auth0UserData(**resp.json()) def create_user(self, user: BiocommonsRegisterData) -> Auth0UserData: - url = f"https://{self.domain}/api/v2/users" + url = f"{self.api_base}/users" # Exclude None values to avoid validation errors. resp = self._client.post(url, json=user.model_dump(mode="json", exclude_none=True)) resp.raise_for_status() @@ -254,7 +255,7 @@ def update_user(self, user_id: str, update_data: UpdateUserData) -> Auth0UserDat """ Send a PATCH request to the /users/{user_id} endpoint to update the included fields. """ - url = f"https://{self.domain}/api/v2/users/{user_id}" + url = f"{self.api_base}/users/{user_id}" # Make sure we exclude None to not update fields with null values. data = update_data.model_dump(mode="json", exclude_none=True) try: @@ -273,7 +274,7 @@ def start_user_export( """ Start a user export job, return the job ID. """ - url = f"https://{self.domain}/api/v2/jobs/users-exports" + url = f"{self.api_base}/jobs/users-exports" if fields is None: fields = [ {"name": "user_id"}, @@ -288,7 +289,7 @@ def start_user_export( return resp.json()["id"] def get_job_status(self, job_id: str) -> JobStatus: - url = f"https://{self.domain}/api/v2/jobs/{job_id}" + url = f"{self.api_base}/jobs/{job_id}" resp = self._client.get(url) resp.raise_for_status() return JobStatus(**resp.json()) @@ -345,9 +346,6 @@ def export_and_download_users( download_path.write_text(csv_data, encoding="utf-8") return download_path - - - def check_user_password(self, email: str, password: str, settings: Settings) -> bool: """ Verify a user's password by using the password-realm grant type. @@ -379,7 +377,7 @@ def delete_user_refresh_tokens(self, user_id: str) -> bool: """ Delete all refresh tokens for a user. """ - url = f"https://{self.domain}/api/v2/users/{user_id}/refresh-tokens" + url = f"{self.api_base}/users/{user_id}/refresh-tokens" resp = self._client.delete(url) resp.raise_for_status() return True @@ -388,7 +386,7 @@ def add_roles_to_user(self, user_id: str, role_id: str | list[str]) -> bool: """ Add one or more roles to a user. The role(s) must already exist. """ - url = f"https://{self.domain}/api/v2/users/{user_id}/roles" + url = f"{self.api_base}/users/{user_id}/roles" if isinstance(role_id, str): role_id = [role_id] resp = self._client.post(url, json={"roles": role_id}) @@ -399,7 +397,7 @@ 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" + url = f"{self.api_base}/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() @@ -411,7 +409,7 @@ def search_users_by_email(self, email: str, connection: str | None = None) -> li """ Search users by email, optionally filtering by connection name. """ - url = f"https://{self.domain}/api/v2/users-by-email" + url = f"{self.api_base}/users-by-email" resp = self._client.get(url, params={"email": email}) users = self._convert_users(resp) if connection is not None: @@ -432,7 +430,7 @@ def _search_users(self, query: str, page: Optional[int] = None, per_page: Option params["page"] = page if per_page is not None: params["per_page"] = per_page - url = f"https://{self.domain}/api/v2/users" + url = f"{self.api_base}/users" # TODO: set primary_order=false for faster search? # https://auth0.com/docs/manage-users/user-search/user-search-best-practices resp = self._client.get( @@ -447,7 +445,7 @@ def _search_users(self, query: str, page: Optional[int] = None, per_page: Option return self._convert_users(resp) def get_connections(self) -> list[Auth0Connection]: - url = f"https://{self.domain}/api/v2/connections" + url = f"{self.api_base}/connections" resp = self._client.get(url, params={"take": 10}) resp.raise_for_status() converted = ConnectionsWithCheckpoint(**resp.json()) @@ -477,7 +475,7 @@ def get_roles(self, params["page"] = page if per_page is not None: params["per_page"] = per_page - url = f"https://{self.domain}/api/v2/roles" + url = f"{self.api_base}/roles" resp = self._client.get(url, params=params) resp.raise_for_status() if include_totals: @@ -513,7 +511,7 @@ def get_role_by_name(self, name: str) -> RoleData: return roles[0] def get_role_by_id(self, role_id: str) -> RoleData: - url = f"https://{self.domain}/api/v2/roles/{role_id}" + url = f"{self.api_base}/roles/{role_id}" resp = self._client.get(url) resp.raise_for_status() return RoleData(**resp.json()) @@ -525,7 +523,7 @@ def get_role_users(self, include_totals: Optional[bool] = False, take: Optional[int] = None, checkpoint: Optional[str] = None) -> list[RoleUserData] | RoleUsersWithTotals | RoleUsersWithCheckpoint: - url = f"https://{self.domain}/api/v2/roles/{role_id}/users" + url = f"{self.api_base}/roles/{role_id}/users" params = {} if page is not None: params["page"] = page @@ -576,7 +574,7 @@ def get_all_role_users_generator(self, role_id: str) -> Iterator[list[RoleUserDa checkpoint = page_users.next def create_role(self, name: str, description: str) -> RoleData: - url = f"https://{self.domain}/api/v2/roles" + url = f"{self.api_base}/roles" resp = self._client.post(url, json={"name": name, "description": description}) resp.raise_for_status() return RoleData(**resp.json()) @@ -589,7 +587,7 @@ def get_or_create_role(self, name: str, description: str) -> RoleData: return role def resend_verification_email(self, user_id: str) -> EmailVerificationResponse: - url = f"https://{self.domain}/api/v2/jobs/verification-email" + url = f"{self.api_base}/jobs/verification-email" request_body = EmailVerificationRequest(user_id=user_id) resp = self._client.post(url, json=request_body.model_dump(mode="json", exclude_none=True)) resp.raise_for_status() @@ -612,7 +610,7 @@ def link_identity(self, primary_user_id: str, secondary_user_id: str, secondary_ """ Link an identity to a primary account """ - url = f"https://{self.domain}/users/{primary_user_id}/identities" + url = f"{self.api_base}/users/{primary_user_id}/identities" secondary_connection = self.get_connection_by_name(secondary_provider) payload = LinkIdentityRequest( provider=secondary_provider, From 809cb517095f4237c1b1c68ddffd7ad0a53fadf9 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Mon, 24 Aug 2026 16:52:42 +1000 Subject: [PATCH 11/32] test: unit tests of new Auth0Client methods --- tests/test_auth0_client.py | 51 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/test_auth0_client.py b/tests/test_auth0_client.py index 11bca468..5b3677cc 100644 --- a/tests/test_auth0_client.py +++ b/tests/test_auth0_client.py @@ -93,6 +93,57 @@ def test_get_connection_by_name(test_auth0_client): +@respx.mock +def test_get_connection_by_name_not_found(test_auth0_client): + other = Auth0ConnectionFactory.build(name="other") + resp = ConnectionsWithCheckpointFactory.build(connections=[other]) + respx.get("https://auth0.example.com/api/v2/connections").respond(200, json=resp.model_dump(mode="json")) + + result = test_auth0_client.get_connection_by_name("db_connection") + + assert result is None + + +@respx.mock +def test_link_identity(test_auth0_client): + """ + Test linking a secondary identity to a primary account. + + Per https://auth0.com/docs/api/management/v2/users/post-identities, + when linking with an API v2 token the request body has provider/user_id/ + connection_id, and the response is a list of identity objects. + """ + primary_user_id = random_auth0_id() + secondary_user_id = random_auth0_id() + connection = Auth0ConnectionFactory.build(name="google-oauth2") + connections_resp = ConnectionsWithCheckpointFactory.build(connections=[connection]) + respx.get("https://auth0.example.com/api/v2/connections").respond( + 200, json=connections_resp.model_dump(mode="json") + ) + route = respx.post(f"https://auth0.example.com/api/v2/users/{primary_user_id}/identities").respond( + 201, + json=[ + { + "connection": connection.name, + "provider": "google-oauth2", + "user_id": secondary_user_id, + "isSocial": True, + } + ], + ) + + result = test_auth0_client.link_identity(primary_user_id, secondary_user_id, "google-oauth2") + + assert route.called + assert result is True + call_data = json.loads(route.calls.last.request.content) + assert call_data == { + "provider": "google-oauth2", + "connection_id": connection.id, + "user_id": secondary_user_id, + } + + @respx.mock def test_get_role_users(test_auth0_client): """ From 6380271a6945959670cb9684117df99f1a56d01e Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 09:54:26 +1000 Subject: [PATCH 12/32] test: unit tests of AAF link function --- tests/test_aaf.py | 88 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 tests/test_aaf.py diff --git a/tests/test_aaf.py b/tests/test_aaf.py new file mode 100644 index 00000000..ee989808 --- /dev/null +++ b/tests/test_aaf.py @@ -0,0 +1,88 @@ +from http import HTTPStatus +from unittest.mock import MagicMock + +import pytest +from fastapi import HTTPException + +from auth0.client import Auth0Client +from routers.aaf import link_aaf_account +from schemas.biocommons import Auth0Identity, BiocommonsUserAccountType +from tests.datagen import Auth0UserDataFactory, random_auth0_id +from tests.db.datagen import BiocommonsUserFactory + + +def test_link_aaf_account_links_identity_updates_metadata_and_db(test_db_session, persistent_factories): + db_user = BiocommonsUserFactory.create_sync(account_type=BiocommonsUserAccountType.AUTH0, other_user_id=None) + test_db_session.commit() + + aaf_user_id = random_auth0_id() + aaf_user_data = Auth0UserDataFactory.build( + identities=[Auth0Identity(connection="AAF", provider="aaf-connection", user_id=aaf_user_id, isSocial=False)] + ) + auth0_client = MagicMock(spec=Auth0Client) + auth0_client.get_user.return_value = aaf_user_data + + result = link_aaf_account( + db_user_id=db_user.id, + aaf_user_id=aaf_user_id, + auth0_client=auth0_client, + session=test_db_session, + ) + + auth0_client.get_user.assert_called_once_with(user_id=aaf_user_id) + auth0_client.link_identity.assert_called_once_with( + primary_user_id=db_user.id, + secondary_user_id=aaf_user_id, + secondary_provider="aaf-connection", + ) + + auth0_client.update_user.assert_called_once() + call_args, call_kwargs = auth0_client.update_user.call_args + assert call_args[0] == db_user.id + app_metadata = call_kwargs["update_data"].app_metadata + assert app_metadata.account_type == BiocommonsUserAccountType.AAF + assert app_metadata.linking_completed is True + assert app_metadata.linking_completed_at is not None + + assert result.id == db_user.id + assert result.account_type == BiocommonsUserAccountType.AAF + assert result.other_user_id == aaf_user_id + + test_db_session.refresh(db_user) + assert db_user.account_type == BiocommonsUserAccountType.AAF + assert db_user.other_user_id == aaf_user_id + + +def test_link_aaf_account_raises_404_when_no_aaf_identity(test_db_session, persistent_factories): + db_user = BiocommonsUserFactory.create_sync(account_type=BiocommonsUserAccountType.AUTH0, other_user_id=None) + test_db_session.commit() + + aaf_user_id = random_auth0_id() + aaf_user_data = Auth0UserDataFactory.build( + identities=[ + Auth0Identity( + connection="Username-Password-Authentication", + provider="auth0", + user_id=aaf_user_id, + isSocial=False, + ) + ] + ) + auth0_client = MagicMock(spec=Auth0Client) + auth0_client.get_user.return_value = aaf_user_data + + with pytest.raises(HTTPException) as exc_info: + link_aaf_account( + db_user_id=db_user.id, + aaf_user_id=aaf_user_id, + auth0_client=auth0_client, + session=test_db_session, + ) + + assert exc_info.value.status_code == HTTPStatus.NOT_FOUND + auth0_client.link_identity.assert_not_called() + auth0_client.update_user.assert_not_called() + + test_db_session.refresh(db_user) + assert db_user.account_type == BiocommonsUserAccountType.AUTH0 + assert db_user.other_user_id is None From 7c71e2e2861ad36985f7a5255a74cba481b34b92 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 11:12:19 +1000 Subject: [PATCH 13/32] test: tests of require_action_token --- tests/test_dependencies.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/test_dependencies.py diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py new file mode 100644 index 00000000..6ee38290 --- /dev/null +++ b/tests/test_dependencies.py @@ -0,0 +1,37 @@ +from datetime import datetime, timedelta, timezone + +import jwt +import pytest +from fastapi import HTTPException + +from dependencies.auth import require_action_token +from schemas.auth0 import Auth0ActionToken +from tests.datagen import random_auth0_id + + +def create_signed_action_token(payload: Auth0ActionToken, secret: str) -> str: + claims = payload.model_dump(mode="json") + claims["exp"] = datetime.now(timezone.utc) + timedelta(minutes=5) + return jwt.encode(claims, key=secret, algorithm="HS256") + + +def test_require_action_token(mock_settings): + payload = Auth0ActionToken(user_id=random_auth0_id(), + email="test@example.com", + client_id="abc123", + purpose="action_purpose") + signed_token = create_signed_action_token(payload, secret=mock_settings.auth0_management_secret) + checker = require_action_token(purpose="action_purpose") + checked_payload = checker(signed_token, mock_settings) + assert checked_payload == payload + + +def test_require_action_token_wrong_purpose(mock_settings): + payload = Auth0ActionToken(user_id=random_auth0_id(), + email="test@example.com", + client_id="abc123", + purpose="wrong_purpose") + signed_token = create_signed_action_token(payload, secret=mock_settings.auth0_management_secret) + checker = require_action_token(purpose="action_purpose") + with pytest.raises(HTTPException): + checker(signed_token, mock_settings) From 04551d501cad23657959f67ed467affeb526ce2d Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 11:19:48 +1000 Subject: [PATCH 14/32] test: unit tests of account linking endpoint --- tests/test_aaf.py | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/test_aaf.py b/tests/test_aaf.py index ee989808..bbbdca8f 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -86,3 +86,69 @@ def test_link_aaf_account_raises_404_when_no_aaf_identity(test_db_session, persi test_db_session.refresh(db_user) assert db_user.account_type == BiocommonsUserAccountType.AUTH0 assert db_user.other_user_id is None + + +def _action_token_payload(user_id: str, email: str, purpose: str = "aaf_link", client_id: str = "test-client") -> dict: + return {"user_id": user_id, "email": email, "client_id": client_id, "purpose": purpose} + + +def test_check_link_no_existing_account(test_client, mocker): + aaf_user_id = random_auth0_id() + email = "new-aaf-user@example.com" + mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[]) + link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") + update_user = mocker.patch("routers.aaf.Auth0Client.update_user") + + response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + + assert response.status_code == 200 + assert response.json() == {"link": False, "aaf_only": True, "primary_id": aaf_user_id} + link_identity.assert_not_called() + update_user.assert_not_called() + + +def test_check_link_existing_account_blocked(test_client, mocker): + aaf_user_id = random_auth0_id() + email = "blocked-user@example.com" + mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + existing_account = Auth0UserDataFactory.build(email=email, blocked=True) + mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) + + response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + + assert response.status_code == 403 + + +def test_check_link_existing_account_links(test_client, test_db_session, persistent_factories, mocker): + email = "existing-user@example.com" + aaf_user_id = random_auth0_id() + db_user = BiocommonsUserFactory.create_sync( + email=email, account_type=BiocommonsUserAccountType.AUTH0, other_user_id=None + ) + test_db_session.commit() + + mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + existing_account = Auth0UserDataFactory.build(user_id=db_user.id, email=email, blocked=False) + mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) + aaf_user_data = Auth0UserDataFactory.build( + identities=[Auth0Identity(connection="AAF", provider="aaf-connection", user_id=aaf_user_id, isSocial=False)] + ) + mocker.patch("routers.aaf.Auth0Client.get_user", return_value=aaf_user_data) + link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") + update_user = mocker.patch("routers.aaf.Auth0Client.update_user") + + response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + + assert response.status_code == 200 + assert response.json() == {"link": True, "aaf_only": False, "primary_id": db_user.id} + link_identity.assert_called_once_with( + primary_user_id=db_user.id, + secondary_user_id=aaf_user_id, + secondary_provider="aaf-connection", + ) + update_user.assert_called_once() + + test_db_session.refresh(db_user) + assert db_user.account_type == BiocommonsUserAccountType.AAF + assert db_user.other_user_id == aaf_user_id From a7f67c643f2a467b1af710e75fceec4afc9cbdbb Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 14:38:26 +1000 Subject: [PATCH 15/32] fix: get connection correctly when linking accounts --- auth0/client.py | 30 +++++++++++++++++++--------- routers/aaf.py | 3 ++- tests/test_aaf.py | 10 ++++++---- tests/test_auth0_client.py | 41 +++++++++++++++++++++++++------------- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/auth0/client.py b/auth0/client.py index b35db74a..1f82a819 100644 --- a/auth0/client.py +++ b/auth0/client.py @@ -411,6 +411,7 @@ def search_users_by_email(self, email: str, connection: str | None = None) -> li """ url = f"{self.api_base}/users-by-email" resp = self._client.get(url, params={"email": email}) + resp.raise_for_status() users = self._convert_users(resp) if connection is not None: filtered_users = [] @@ -454,12 +455,17 @@ def get_connections(self) -> list[Auth0Connection]: return converted.connections def get_connection_by_name(self, name: str) -> Auth0Connection | None: - connections = self.get_connections() - result = None - for connection in connections: - if connection.name == name: - result = connection - return result + # Filter server-side rather than paginating get_connections(), which caps at 10. + url = f"{self.api_base}/connections" + resp = self._client.get(url, params={"name": name}) + resp.raise_for_status() + connections = self._convert_list(resp, Auth0Connection) + # Ensure an exact match + if connections: + for connection in connections: + if connection.name == name: + return connection + return None def get_roles(self, name_filter: Optional[str] = None, @@ -606,12 +612,18 @@ def trigger_password_change(self, user_email: str, client_id: str, settings: Set resp.raise_for_status() return True - def link_identity(self, primary_user_id: str, secondary_user_id: str, secondary_provider: str) -> bool: + def link_identity(self, primary_user_id: str, secondary_user_id: str, secondary_provider: str, + secondary_connection_name: str) -> bool: """ - Link an identity to a primary account + Link an identity to a primary account. + + secondary_provider is the identity's strategy (e.g. "samlp"), secondary_connection_name + is the connection's name (e.g. "AAF") - these can differ, so both are needed. """ url = f"{self.api_base}/users/{primary_user_id}/identities" - secondary_connection = self.get_connection_by_name(secondary_provider) + secondary_connection = self.get_connection_by_name(secondary_connection_name) + if secondary_connection is None: + raise ValueError(f"Could not find Auth0 connection named {secondary_connection_name!r}") payload = LinkIdentityRequest( provider=secondary_provider, connection_id=secondary_connection.id, diff --git a/routers/aaf.py b/routers/aaf.py index 15f37e30..e8d69c31 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -45,7 +45,8 @@ def _get_aaf_identity(aaf_user: Auth0UserData): logger.info("Linking AAF account to existing database account") now = datetime.now(tz=timezone.utc) auth0_client.link_identity(primary_user_id=db_user_id, secondary_user_id=aaf_user_id, - secondary_provider=aaf_identity.provider) + secondary_provider=aaf_identity.provider, + secondary_connection_name=aaf_identity.connection) auth0_client.update_user( db_user_id, update_data=UpdateUserData( diff --git a/tests/test_aaf.py b/tests/test_aaf.py index bbbdca8f..6bee9e01 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -17,7 +17,7 @@ def test_link_aaf_account_links_identity_updates_metadata_and_db(test_db_session aaf_user_id = random_auth0_id() aaf_user_data = Auth0UserDataFactory.build( - identities=[Auth0Identity(connection="AAF", provider="aaf-connection", user_id=aaf_user_id, isSocial=False)] + identities=[Auth0Identity(connection="AAF", provider="samlp", user_id=aaf_user_id, isSocial=False)] ) auth0_client = MagicMock(spec=Auth0Client) auth0_client.get_user.return_value = aaf_user_data @@ -33,7 +33,8 @@ def test_link_aaf_account_links_identity_updates_metadata_and_db(test_db_session auth0_client.link_identity.assert_called_once_with( primary_user_id=db_user.id, secondary_user_id=aaf_user_id, - secondary_provider="aaf-connection", + secondary_provider="samlp", + secondary_connection_name="AAF", ) auth0_client.update_user.assert_called_once() @@ -132,7 +133,7 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist existing_account = Auth0UserDataFactory.build(user_id=db_user.id, email=email, blocked=False) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) aaf_user_data = Auth0UserDataFactory.build( - identities=[Auth0Identity(connection="AAF", provider="aaf-connection", user_id=aaf_user_id, isSocial=False)] + identities=[Auth0Identity(connection="AAF", provider="samlp", user_id=aaf_user_id, isSocial=False)] ) mocker.patch("routers.aaf.Auth0Client.get_user", return_value=aaf_user_data) link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") @@ -145,7 +146,8 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist link_identity.assert_called_once_with( primary_user_id=db_user.id, secondary_user_id=aaf_user_id, - secondary_provider="aaf-connection", + secondary_provider="samlp", + secondary_connection_name="AAF", ) update_user.assert_called_once() diff --git a/tests/test_auth0_client.py b/tests/test_auth0_client.py index 5b3677cc..cb18fa63 100644 --- a/tests/test_auth0_client.py +++ b/tests/test_auth0_client.py @@ -82,25 +82,24 @@ def test_get_connections(test_auth0_client): @respx.mock def test_get_connection_by_name(test_auth0_client): db_connection = Auth0ConnectionFactory.build(name="db_connection") - other = Auth0ConnectionFactory.build(name="other") - resp = ConnectionsWithCheckpointFactory.build(connections=[db_connection, other]) - route = respx.get("https://auth0.example.com/api/v2/connections").respond(200, json=resp.model_dump(mode="json")) + route = respx.get("https://auth0.example.com/api/v2/connections").respond( + 200, json=[db_connection.model_dump(mode="json")] + ) result = test_auth0_client.get_connection_by_name(db_connection.name) assert route.called + assert route.calls.last.request.url.params["name"] == db_connection.name assert result == db_connection - @respx.mock def test_get_connection_by_name_not_found(test_auth0_client): - other = Auth0ConnectionFactory.build(name="other") - resp = ConnectionsWithCheckpointFactory.build(connections=[other]) - respx.get("https://auth0.example.com/api/v2/connections").respond(200, json=resp.model_dump(mode="json")) + route = respx.get("https://auth0.example.com/api/v2/connections").respond(200, json=[]) result = test_auth0_client.get_connection_by_name("db_connection") + assert route.called assert result is None @@ -112,38 +111,52 @@ def test_link_identity(test_auth0_client): Per https://auth0.com/docs/api/management/v2/users/post-identities, when linking with an API v2 token the request body has provider/user_id/ connection_id, and the response is a list of identity objects. + + Uses a connection name ("AAF") that differs from its provider/strategy + ("samlp") to make sure the two aren't confused with each other. """ primary_user_id = random_auth0_id() secondary_user_id = random_auth0_id() - connection = Auth0ConnectionFactory.build(name="google-oauth2") - connections_resp = ConnectionsWithCheckpointFactory.build(connections=[connection]) + connection = Auth0ConnectionFactory.build(name="AAF", strategy="samlp") respx.get("https://auth0.example.com/api/v2/connections").respond( - 200, json=connections_resp.model_dump(mode="json") + 200, json=[connection.model_dump(mode="json")] ) route = respx.post(f"https://auth0.example.com/api/v2/users/{primary_user_id}/identities").respond( 201, json=[ { "connection": connection.name, - "provider": "google-oauth2", + "provider": "samlp", "user_id": secondary_user_id, - "isSocial": True, + "isSocial": False, } ], ) - result = test_auth0_client.link_identity(primary_user_id, secondary_user_id, "google-oauth2") + result = test_auth0_client.link_identity( + primary_user_id, secondary_user_id, secondary_provider="samlp", secondary_connection_name="AAF" + ) assert route.called assert result is True call_data = json.loads(route.calls.last.request.content) assert call_data == { - "provider": "google-oauth2", + "provider": "samlp", "connection_id": connection.id, "user_id": secondary_user_id, } +@respx.mock +def test_link_identity_connection_not_found(test_auth0_client): + respx.get("https://auth0.example.com/api/v2/connections").respond(200, json=[]) + + with pytest.raises(ValueError): + test_auth0_client.link_identity( + random_auth0_id(), random_auth0_id(), secondary_provider="samlp", secondary_connection_name="AAF" + ) + + @respx.mock def test_get_role_users(test_auth0_client): """ From 40d8a0057ac6f8135af8d03adbefe69ab7859bc6 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 14:50:25 +1000 Subject: [PATCH 16/32] fix: skip linking if already done --- routers/aaf.py | 6 +++++- tests/test_aaf.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/routers/aaf.py b/routers/aaf.py index e8d69c31..cc3e01ef 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -37,6 +37,11 @@ def _get_aaf_identity(aaf_user: Auth0UserData): if identity.connection == "AAF": return identity return None + db_user = BiocommonsUser.get_by_id_or_404(db_user_id, session=session) + if db_user.account_type == BiocommonsUserAccountType.AAF and db_user.other_user_id == aaf_user_id: + logger.info("AAF account already linked, skipping re-link") + return db_user + aaf_user_info = auth0_client.get_user(user_id=aaf_user_id) aaf_identity = _get_aaf_identity(aaf_user_info) if aaf_identity is None: @@ -58,7 +63,6 @@ def _get_aaf_identity(aaf_user: Auth0UserData): ) ) logger.info("Updating DB record") - db_user = BiocommonsUser.get_by_id_or_404(db_user_id, session=session) db_user.link_aaf_account(aaf_user_id=aaf_user_id, session=session, updated_by=db_user, commit=True) return db_user diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 6bee9e01..bdb45544 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -89,6 +89,28 @@ def test_link_aaf_account_raises_404_when_no_aaf_identity(test_db_session, persi assert db_user.other_user_id is None +def test_link_aaf_account_idempotent_when_already_linked(test_db_session, persistent_factories): + aaf_user_id = random_auth0_id() + db_user = BiocommonsUserFactory.create_sync( + account_type=BiocommonsUserAccountType.AAF, other_user_id=aaf_user_id + ) + test_db_session.commit() + + auth0_client = MagicMock(spec=Auth0Client) + + result = link_aaf_account( + db_user_id=db_user.id, + aaf_user_id=aaf_user_id, + auth0_client=auth0_client, + session=test_db_session, + ) + + auth0_client.get_user.assert_not_called() + auth0_client.link_identity.assert_not_called() + auth0_client.update_user.assert_not_called() + assert result.id == db_user.id + + def _action_token_payload(user_id: str, email: str, purpose: str = "aaf_link", client_id: str = "test-client") -> dict: return {"user_id": user_id, "email": email, "client_id": client_id, "purpose": purpose} @@ -154,3 +176,27 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist test_db_session.refresh(db_user) assert db_user.account_type == BiocommonsUserAccountType.AAF assert db_user.other_user_id == aaf_user_id + + +def test_check_link_already_linked_is_idempotent(test_client, test_db_session, persistent_factories, mocker): + email = "already-linked@example.com" + aaf_user_id = random_auth0_id() + db_user = BiocommonsUserFactory.create_sync( + email=email, account_type=BiocommonsUserAccountType.AAF, other_user_id=aaf_user_id + ) + test_db_session.commit() + + mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + existing_account = Auth0UserDataFactory.build(user_id=db_user.id, email=email, blocked=False) + mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) + get_user = mocker.patch("routers.aaf.Auth0Client.get_user") + link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") + update_user = mocker.patch("routers.aaf.Auth0Client.update_user") + + response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + + assert response.status_code == 200 + assert response.json() == {"link": True, "aaf_only": False, "primary_id": db_user.id} + get_user.assert_not_called() + link_identity.assert_not_called() + update_user.assert_not_called() From 63a59a1b49b8eb899b6aef9b4ee85e224df75407 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 15:10:26 +1000 Subject: [PATCH 17/32] fix: update app_metadata to mark user AAF only if no existing account --- routers/aaf.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/routers/aaf.py b/routers/aaf.py index cc3e01ef..16541ecb 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -67,7 +67,16 @@ def _get_aaf_identity(aaf_user: Auth0UserData): return db_user - +def mark_user_aaf_only(user_email: str, aaf_user_id: str, auth0_client: Auth0Client): + update_data = UpdateUserData( + app_metadata=BiocommonsAppMetadataUpdate( + aaf_only=True, + checked_email=user_email, + linking_completed=True, + linking_completed_at=datetime.now(tz=timezone.utc), + ) + ) + auth0_client.update_user(user_id=aaf_user_id, update_data=update_data) @@ -86,7 +95,7 @@ def check_aaf_account_link( ) # No existing account: no need to link if not auth0_matches: - # TODO: update app_metadata + mark_user_aaf_only(email, aaf_user_id, auth0_client) return AccountLinkResponse(link=False, aaf_only=True, primary_id=token.user_id) existing_account = None @@ -94,8 +103,9 @@ def check_aaf_account_link( if user.email.lower() == email.lower(): existing_account = user break - # No exact match: are we safe to say there's no existing account here? + # No exact match: no existing account if not existing_account: + mark_user_aaf_only(email, aaf_user_id, auth0_client) return AccountLinkResponse(link=False, aaf_only=True, primary_id=email) if existing_account.blocked: From 10fd2e3a38913bf1a81189b8c4fd3a0dda0a1bd4 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 15:16:30 +1000 Subject: [PATCH 18/32] test: unit tests of app_metadata for AAF-only accounts --- tests/test_aaf.py | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/tests/test_aaf.py b/tests/test_aaf.py index bdb45544..7e2b2186 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -5,7 +5,7 @@ from fastapi import HTTPException from auth0.client import Auth0Client -from routers.aaf import link_aaf_account +from routers.aaf import link_aaf_account, mark_user_aaf_only from schemas.biocommons import Auth0Identity, BiocommonsUserAccountType from tests.datagen import Auth0UserDataFactory, random_auth0_id from tests.db.datagen import BiocommonsUserFactory @@ -115,6 +115,27 @@ def _action_token_payload(user_id: str, email: str, purpose: str = "aaf_link", c return {"user_id": user_id, "email": email, "client_id": client_id, "purpose": purpose} +def _assert_marked_aaf_only(update_user_mock, aaf_user_id: str, email: str): + update_user_mock.assert_called_once() + call_args, call_kwargs = update_user_mock.call_args + assert call_kwargs["user_id"] == aaf_user_id + app_metadata = call_kwargs["update_data"].app_metadata + assert app_metadata.aaf_only is True + assert app_metadata.checked_email == email + assert app_metadata.linking_completed is True + assert app_metadata.linking_completed_at is not None + + +def test_mark_user_aaf_only(): + aaf_user_id = random_auth0_id() + email = "aaf-user@example.com" + auth0_client = MagicMock(spec=Auth0Client) + + mark_user_aaf_only(email, aaf_user_id, auth0_client) + + _assert_marked_aaf_only(auth0_client.update_user, aaf_user_id, email) + + def test_check_link_no_existing_account(test_client, mocker): aaf_user_id = random_auth0_id() email = "new-aaf-user@example.com" @@ -128,7 +149,24 @@ def test_check_link_no_existing_account(test_client, mocker): assert response.status_code == 200 assert response.json() == {"link": False, "aaf_only": True, "primary_id": aaf_user_id} link_identity.assert_not_called() - update_user.assert_not_called() + _assert_marked_aaf_only(update_user, aaf_user_id, email) + + +def test_check_link_no_exact_email_match(test_client, mocker): + aaf_user_id = random_auth0_id() + email = "aaf-user@example.com" + mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + near_miss_account = Auth0UserDataFactory.build(email="different-user@example.com") + mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[near_miss_account]) + link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") + update_user = mocker.patch("routers.aaf.Auth0Client.update_user") + + response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + + assert response.status_code == 200 + assert response.json() == {"link": False, "aaf_only": True, "primary_id": email} + link_identity.assert_not_called() + _assert_marked_aaf_only(update_user, aaf_user_id, email) def test_check_link_existing_account_blocked(test_client, mocker): From 531039c82b09cfeae7bb4d9838e115a1057dc80a Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 25 Aug 2026 16:32:46 +1000 Subject: [PATCH 19/32] fix: handle errors when Auth0 already linked --- auth0/client.py | 7 +++++-- routers/aaf.py | 26 +++++++++++++++----------- tests/test_aaf.py | 28 ++++++++++++++++++++++++++++ tests/test_auth0_client.py | 22 ++++++++++++++++++++++ 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/auth0/client.py b/auth0/client.py index 1f82a819..87827725 100644 --- a/auth0/client.py +++ b/auth0/client.py @@ -629,8 +629,11 @@ def link_identity(self, primary_user_id: str, secondary_user_id: str, secondary_ connection_id=secondary_connection.id, user_id=secondary_user_id, ) - resp = self._client.post(url, json=payload.model_dump(mode="json", exclude_none=True)) - resp.raise_for_status() + try: + resp = self._client.post(url, json=payload.model_dump(mode="json", exclude_none=True)) + resp.raise_for_status() + except HTTPStatusError as exc: + raise ValueError(f"Failed to link identity for user {primary_user_id}: {exc.response.json()}") from exc return True diff --git a/routers/aaf.py b/routers/aaf.py index 16541ecb..83d13140 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -49,19 +49,23 @@ def _get_aaf_identity(aaf_user: Auth0UserData): logger.info("Linking AAF account to existing database account") now = datetime.now(tz=timezone.utc) - auth0_client.link_identity(primary_user_id=db_user_id, secondary_user_id=aaf_user_id, - secondary_provider=aaf_identity.provider, - secondary_connection_name=aaf_identity.connection) - auth0_client.update_user( - db_user_id, - update_data=UpdateUserData( - app_metadata=BiocommonsAppMetadataUpdate( - account_type=BiocommonsUserAccountType.AAF, - linking_completed=True, - linking_completed_at=now + try: + auth0_client.link_identity(primary_user_id=db_user_id, secondary_user_id=aaf_user_id, + secondary_provider=aaf_identity.provider, + secondary_connection_name=aaf_identity.connection) + auth0_client.update_user( + db_user_id, + update_data=UpdateUserData( + app_metadata=BiocommonsAppMetadataUpdate( + account_type=BiocommonsUserAccountType.AAF, + linking_completed=True, + linking_completed_at=now + ) ) ) - ) + except ValueError as exc: + logger.error(f"Failed to link AAF account in Auth0: {exc}") + raise HTTPException(status_code=HTTPStatus.BAD_GATEWAY, detail="Failed to link account with Auth0") from exc logger.info("Updating DB record") db_user.link_aaf_account(aaf_user_id=aaf_user_id, session=session, updated_by=db_user, commit=True) return db_user diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 7e2b2186..489ee26a 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -89,6 +89,34 @@ def test_link_aaf_account_raises_404_when_no_aaf_identity(test_db_session, persi assert db_user.other_user_id is None +def test_link_aaf_account_raises_502_when_auth0_linking_fails(test_db_session, persistent_factories): + db_user = BiocommonsUserFactory.create_sync(account_type=BiocommonsUserAccountType.AUTH0, other_user_id=None) + test_db_session.commit() + + aaf_user_id = random_auth0_id() + aaf_user_data = Auth0UserDataFactory.build( + identities=[Auth0Identity(connection="AAF", provider="samlp", user_id=aaf_user_id, isSocial=False)] + ) + auth0_client = MagicMock(spec=Auth0Client) + auth0_client.get_user.return_value = aaf_user_data + auth0_client.link_identity.side_effect = ValueError("Identity already linked") + + with pytest.raises(HTTPException) as exc_info: + link_aaf_account( + db_user_id=db_user.id, + aaf_user_id=aaf_user_id, + auth0_client=auth0_client, + session=test_db_session, + ) + + assert exc_info.value.status_code == HTTPStatus.BAD_GATEWAY + auth0_client.update_user.assert_not_called() + + test_db_session.refresh(db_user) + assert db_user.account_type == BiocommonsUserAccountType.AUTH0 + assert db_user.other_user_id is None + + def test_link_aaf_account_idempotent_when_already_linked(test_db_session, persistent_factories): aaf_user_id = random_auth0_id() db_user = BiocommonsUserFactory.create_sync( diff --git a/tests/test_auth0_client.py b/tests/test_auth0_client.py index cb18fa63..cb262c80 100644 --- a/tests/test_auth0_client.py +++ b/tests/test_auth0_client.py @@ -157,6 +157,28 @@ def test_link_identity_connection_not_found(test_auth0_client): ) +@respx.mock +def test_link_identity_already_linked(test_auth0_client): + """ + Auth0 rejects linking an identity that's already linked (e.g. a retried + request) - this should surface as a clean ValueError, not a raw HTTPStatusError. + """ + primary_user_id = random_auth0_id() + secondary_user_id = random_auth0_id() + connection = Auth0ConnectionFactory.build(name="AAF", strategy="samlp") + respx.get("https://auth0.example.com/api/v2/connections").respond( + 200, json=[connection.model_dump(mode="json")] + ) + respx.post(f"https://auth0.example.com/api/v2/users/{primary_user_id}/identities").respond( + 400, json={"statusCode": 400, "error": "Bad Request", "message": "Identity already linked"} + ) + + with pytest.raises(ValueError): + test_auth0_client.link_identity( + primary_user_id, secondary_user_id, secondary_provider="samlp", secondary_connection_name="AAF" + ) + + @respx.mock def test_get_role_users(test_auth0_client): """ From 7362c98460378a2435b0cb8fd8e95acf771fd829 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 1 Sep 2026 13:50:42 +1000 Subject: [PATCH 20/32] feat: create_action_token function to sign tokens for Auth0 actions --- auth/validator.py | 16 +++++++++++ tests/auth/test_auth_validator.py | 48 ++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/auth/validator.py b/auth/validator.py index a0491fe4..ff785cb5 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -2,6 +2,7 @@ import json import logging import weakref +from datetime import UTC, datetime, timedelta import httpx import jwt @@ -168,3 +169,18 @@ def verify_action_token(token: str, settings: Settings) -> dict: except InvalidTokenError: raise HTTPException(status_code=401, detail="invalid session_token") return payload + + +def create_action_token(payload: dict, settings: Settings, expires_in_seconds: int = 300) -> dict: + """ + Create a signed JWT that can be passed back to Auth0 actions + """ + exp = (datetime.now(tz=UTC) + timedelta(seconds=expires_in_seconds)).timestamp() + payload = {**payload, "exp": exp} + secret = settings.auth0_management_secret + signed_payload = jwt.encode( + payload, + key=secret, + algorithm="HS256", + ) + return signed_payload diff --git a/tests/auth/test_auth_validator.py b/tests/auth/test_auth_validator.py index 302f0bc5..e2b42446 100644 --- a/tests/auth/test_auth_validator.py +++ b/tests/auth/test_auth_validator.py @@ -2,7 +2,7 @@ import json import uuid from dataclasses import dataclass -from datetime import datetime, timedelta +from datetime import UTC, datetime, timedelta from typing import Optional from unittest.mock import AsyncMock, patch @@ -19,7 +19,9 @@ from fastapi import Depends, FastAPI, HTTPException from fastapi.security import HTTPAuthorizationCredentials from fastapi.testclient import TestClient +from freezegun import freeze_time from httpx import Request, Response +from jwt import InvalidSignatureError from jwt.algorithms import RSAAlgorithm from auth import auth0_security, get_auth0_token @@ -27,6 +29,7 @@ from auth.validator import ( KEY_CACHE, _fetch_rsa_keys, + create_action_token, get_rsa_key, verify_action_token, verify_jwt, @@ -39,6 +42,16 @@ TEST_MANAGEMENT_SECRET = "test-management-secret-key-with-32b" TEST_WRONG_MANAGEMENT_SECRET = "wrong-management-secret-key-32b!" TEST_CORRECT_MANAGEMENT_SECRET = "correct-management-secret-key-32" +FROZEN_TIME = datetime(2025, 1, 1, 12, 0, 0, tzinfo=UTC) + + +@pytest.fixture +def frozen_time(): + """ + Freeze time so datetime.now() returns FROZEN_TIME. + """ + with freeze_time("2025-01-01 12:00:00") as time: + yield time def generate_public_private_key_pair(): @@ -586,6 +599,39 @@ def test_verify_action_token_missing_exp(mock_settings: Settings): assert excinfo.value.detail == "invalid session_token" +def test_create_action_token_success(mock_settings: Settings, frozen_time): + secret = TEST_MANAGEMENT_SECRET + mock_settings.auth0_management_secret = secret + payload = {"data": "test-payload"} + token = create_action_token(payload, mock_settings) + decoded = jwt.decode(token, secret, algorithms=["HS256"]) + assert decoded["data"] == "test-payload" + assert decoded["exp"] == (FROZEN_TIME + timedelta(minutes=5)).timestamp() + + +def test_create_action_token_expiry(mock_settings: Settings, frozen_time): + """ + Test we can change expiry time of an action token. + """ + secret = TEST_MANAGEMENT_SECRET + mock_settings.auth0_management_secret = secret + payload = {"data": "test-payload"} + token = create_action_token(payload, mock_settings, expires_in_seconds=10 * 60) + decoded = jwt.decode(token, secret, algorithms=["HS256"]) + assert decoded["data"] == "test-payload" + assert decoded["exp"] == (FROZEN_TIME + timedelta(minutes=10)).timestamp() + + +def test_create_action_token_invalid(mock_settings: Settings, frozen_time): + secret = TEST_MANAGEMENT_SECRET + # Use invalid secret to sign + mock_settings.auth0_management_secret = "invalid-secret" + payload = {"data": "test-payload"} + token = create_action_token(payload, mock_settings, expires_in_seconds=10 * 60) + # Use expected secret to decode + with pytest.raises(InvalidSignatureError, match="Signature verification failed"): + jwt.decode(token, secret, algorithms=["HS256"]) + @pytest.mark.asyncio @respx.mock async def test_fetch_rsa_keys_only_refreshes_once_when_cache_is_expired(mock_settings: Settings): From 539cbc67791c782879c5a2ef9629afb868dfca01 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 1 Sep 2026 15:14:10 +1000 Subject: [PATCH 21/32] fix: need to return a redirect response to Auth0 in check-link --- routers/aaf.py | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/routers/aaf.py b/routers/aaf.py index 83d13140..daf82a4c 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -3,10 +3,13 @@ from http import HTTPStatus from typing import Annotated +import httpx2 from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel from sqlmodel import Session +from starlette.responses import RedirectResponse +from auth.validator import create_action_token from auth0.client import Auth0Client, UpdateUserData, get_auth0_client from config import Settings, get_settings from db.models import BiocommonsUser @@ -83,9 +86,29 @@ def mark_user_aaf_only(user_email: str, aaf_user_id: str, auth0_client: Auth0Cli auth0_client.update_user(user_id=aaf_user_id, update_data=update_data) +def return_signed_response( + state: str, + response: AccountLinkResponse, + settings: Settings, +): + """ + Auth0 Actions need to receive the response as a signed JWT + token, sign the AccountLinkResponse we want to return + and redirect to the continue endpoint + """ + token = create_action_token( + payload=response.model_dump(mode="json"), + settings=settings, + ) + redirect_url = httpx2.URL(f"{settings.auth0_custom_domain}/continue", + params={"state": state, "session_token": token}) + return RedirectResponse(url=redirect_url) + + @router.get("/check-link", response_model=AccountLinkResponse) def check_aaf_account_link( + state: str, token: Annotated[Auth0ActionToken, Depends(require_action_token(purpose="aaf_link"))], session: Annotated[Session, Depends(get_db_session)], auth0_client: Annotated[Auth0Client, Depends(get_auth0_client)], @@ -100,7 +123,8 @@ def check_aaf_account_link( # No existing account: no need to link if not auth0_matches: mark_user_aaf_only(email, aaf_user_id, auth0_client) - return AccountLinkResponse(link=False, aaf_only=True, primary_id=token.user_id) + resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=token.user_id) + return return_signed_response(state=state, response=resp, settings=settings) existing_account = None for user in auth0_matches: @@ -110,7 +134,8 @@ def check_aaf_account_link( # No exact match: no existing account if not existing_account: mark_user_aaf_only(email, aaf_user_id, auth0_client) - return AccountLinkResponse(link=False, aaf_only=True, primary_id=email) + resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=email) + return return_signed_response(state=state, response=resp, settings=settings) if existing_account.blocked: raise HTTPException(status_code=403, detail="Existing account is blocked.") @@ -121,4 +146,5 @@ def check_aaf_account_link( auth0_client=auth0_client, session=session, ) - return AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id) + resp = AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id) + return return_signed_response(state=state, response=resp, settings=settings) From 99057e631cd131efcea191e1cd326f6a423602d7 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 1 Sep 2026 15:14:33 +1000 Subject: [PATCH 22/32] test: update tests of check-link --- tests/test_aaf.py | 98 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 81 insertions(+), 17 deletions(-) diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 489ee26a..21fe1fe7 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -1,6 +1,8 @@ from http import HTTPStatus from unittest.mock import MagicMock +from urllib.parse import parse_qs, urlparse +import jwt import pytest from fastapi import HTTPException @@ -154,6 +156,20 @@ def _assert_marked_aaf_only(update_user_mock, aaf_user_id: str, email: str): assert app_metadata.linking_completed_at is not None +def _decode_check_link_redirect(response, state: str, secret: str) -> dict: + assert response.status_code == HTTPStatus.TEMPORARY_REDIRECT + assert response.is_redirect + redirect = urlparse(response.headers["location"]) + query_params = parse_qs(redirect.query) + assert query_params["state"] == [state] + assert "session_token" in query_params + return jwt.decode( + query_params["session_token"][0], + key=secret, + algorithms=["HS256"], + ) + + def test_mark_user_aaf_only(): aaf_user_id = random_auth0_id() email = "aaf-user@example.com" @@ -164,7 +180,7 @@ def test_mark_user_aaf_only(): _assert_marked_aaf_only(auth0_client.update_user, aaf_user_id, email) -def test_check_link_no_existing_account(test_client, mocker): +def test_check_link_no_existing_account(test_client, mocker, mock_settings): aaf_user_id = random_auth0_id() email = "new-aaf-user@example.com" mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) @@ -172,15 +188,27 @@ def test_check_link_no_existing_account(test_client, mocker): link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") update_user = mocker.patch("routers.aaf.Auth0Client.update_user") - response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + state = "dummy" + response = test_client.get( + "/aaf/check-link", + params={"session_token": "valid_token", "state": state}, + follow_redirects=False, + ) + + decoded_token = _decode_check_link_redirect( + response, + state=state, + secret=mock_settings.auth0_management_secret, + ) + assert decoded_token["link"] is False + assert decoded_token["aaf_only"] is True + assert decoded_token["primary_id"] == aaf_user_id - assert response.status_code == 200 - assert response.json() == {"link": False, "aaf_only": True, "primary_id": aaf_user_id} link_identity.assert_not_called() _assert_marked_aaf_only(update_user, aaf_user_id, email) -def test_check_link_no_exact_email_match(test_client, mocker): +def test_check_link_no_exact_email_match(test_client, mocker, mock_settings): aaf_user_id = random_auth0_id() email = "aaf-user@example.com" mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) @@ -189,10 +217,21 @@ def test_check_link_no_exact_email_match(test_client, mocker): link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") update_user = mocker.patch("routers.aaf.Auth0Client.update_user") - response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + state = "dummy" + response = test_client.get( + "/aaf/check-link", + params={"session_token": "valid_token", "state": state}, + follow_redirects=False, + ) - assert response.status_code == 200 - assert response.json() == {"link": False, "aaf_only": True, "primary_id": email} + decoded_token = _decode_check_link_redirect( + response, + state=state, + secret=mock_settings.auth0_management_secret, + ) + assert decoded_token["link"] is False + assert decoded_token["aaf_only"] is True + assert decoded_token["primary_id"] == email link_identity.assert_not_called() _assert_marked_aaf_only(update_user, aaf_user_id, email) @@ -204,12 +243,15 @@ def test_check_link_existing_account_blocked(test_client, mocker): existing_account = Auth0UserDataFactory.build(email=email, blocked=True) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) - response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + response = test_client.get( + "/aaf/check-link", + params={"session_token": "valid_token", "state": "dummy"}, + ) assert response.status_code == 403 -def test_check_link_existing_account_links(test_client, test_db_session, persistent_factories, mocker): +def test_check_link_existing_account_links(test_client, test_db_session, persistent_factories, mocker, mock_settings): email = "existing-user@example.com" aaf_user_id = random_auth0_id() db_user = BiocommonsUserFactory.create_sync( @@ -227,10 +269,21 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") update_user = mocker.patch("routers.aaf.Auth0Client.update_user") - response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + state = "dummy" + response = test_client.get( + "/aaf/check-link", + params={"session_token": "valid_token", "state": state}, + follow_redirects=False, + ) - assert response.status_code == 200 - assert response.json() == {"link": True, "aaf_only": False, "primary_id": db_user.id} + decoded_token = _decode_check_link_redirect( + response, + state=state, + secret=mock_settings.auth0_management_secret, + ) + assert decoded_token["link"] is True + assert decoded_token["aaf_only"] is False + assert decoded_token["primary_id"] == db_user.id link_identity.assert_called_once_with( primary_user_id=db_user.id, secondary_user_id=aaf_user_id, @@ -244,7 +297,7 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist assert db_user.other_user_id == aaf_user_id -def test_check_link_already_linked_is_idempotent(test_client, test_db_session, persistent_factories, mocker): +def test_check_link_already_linked_is_idempotent(test_client, test_db_session, persistent_factories, mocker, mock_settings): email = "already-linked@example.com" aaf_user_id = random_auth0_id() db_user = BiocommonsUserFactory.create_sync( @@ -259,10 +312,21 @@ def test_check_link_already_linked_is_idempotent(test_client, test_db_session, p link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") update_user = mocker.patch("routers.aaf.Auth0Client.update_user") - response = test_client.get("/aaf/check-link", params={"session_token": "valid_token"}) + state = "dummy" + response = test_client.get( + "/aaf/check-link", + params={"session_token": "valid_token", "state": state}, + follow_redirects=False, + ) - assert response.status_code == 200 - assert response.json() == {"link": True, "aaf_only": False, "primary_id": db_user.id} + decoded_token = _decode_check_link_redirect( + response, + state=state, + secret=mock_settings.auth0_management_secret, + ) + assert decoded_token["link"] is True + assert decoded_token["aaf_only"] is False + assert decoded_token["primary_id"] == db_user.id get_user.assert_not_called() link_identity.assert_not_called() update_user.assert_not_called() From 83bfd6f76d55ad04ab7ccfd27b9fb542a257d33b Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 1 Sep 2026 16:05:09 +1000 Subject: [PATCH 23/32] fix: fall back to Auth0 domain if custom domain not defined --- routers/aaf.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/routers/aaf.py b/routers/aaf.py index daf82a4c..828dfee3 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -100,8 +100,11 @@ def return_signed_response( payload=response.model_dump(mode="json"), settings=settings, ) - redirect_url = httpx2.URL(f"{settings.auth0_custom_domain}/continue", - params={"state": state, "session_token": token}) + auth0_base_url = settings.auth0_custom_domain or f"https://{settings.auth0_domain}" + redirect_url = httpx2.URL( + f"{auth0_base_url}/continue", + params={"state": state, "session_token": token}, + ) return RedirectResponse(url=redirect_url) From c6a5aa75675c8bf278b0ff4bba2d7072ff3d01bc Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Tue, 1 Sep 2026 16:06:37 +1000 Subject: [PATCH 24/32] test: make sure we test redirect location --- tests/test_aaf.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 21fe1fe7..50e18338 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -156,16 +156,21 @@ def _assert_marked_aaf_only(update_user_mock, aaf_user_id: str, email: str): assert app_metadata.linking_completed_at is not None -def _decode_check_link_redirect(response, state: str, secret: str) -> dict: +def _decode_check_link_redirect(response, state: str, settings) -> dict: assert response.status_code == HTTPStatus.TEMPORARY_REDIRECT assert response.is_redirect redirect = urlparse(response.headers["location"]) + expected_base_url = settings.auth0_custom_domain or f"https://{settings.auth0_domain}" + expected_continue_url = urlparse(f"{expected_base_url}/continue") + assert redirect.scheme == expected_continue_url.scheme + assert redirect.netloc == expected_continue_url.netloc + assert redirect.path == expected_continue_url.path query_params = parse_qs(redirect.query) assert query_params["state"] == [state] assert "session_token" in query_params return jwt.decode( query_params["session_token"][0], - key=secret, + key=settings.auth0_management_secret, algorithms=["HS256"], ) @@ -198,7 +203,7 @@ def test_check_link_no_existing_account(test_client, mocker, mock_settings): decoded_token = _decode_check_link_redirect( response, state=state, - secret=mock_settings.auth0_management_secret, + settings=mock_settings, ) assert decoded_token["link"] is False assert decoded_token["aaf_only"] is True @@ -227,7 +232,7 @@ def test_check_link_no_exact_email_match(test_client, mocker, mock_settings): decoded_token = _decode_check_link_redirect( response, state=state, - secret=mock_settings.auth0_management_secret, + settings=mock_settings, ) assert decoded_token["link"] is False assert decoded_token["aaf_only"] is True @@ -279,7 +284,7 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist decoded_token = _decode_check_link_redirect( response, state=state, - secret=mock_settings.auth0_management_secret, + settings=mock_settings, ) assert decoded_token["link"] is True assert decoded_token["aaf_only"] is False @@ -322,7 +327,7 @@ def test_check_link_already_linked_is_idempotent(test_client, test_db_session, p decoded_token = _decode_check_link_redirect( response, state=state, - secret=mock_settings.auth0_management_secret, + settings=mock_settings, ) assert decoded_token["link"] is True assert decoded_token["aaf_only"] is False From f9486f657324e6e26a8ea9037aa3eb1dd0350a08 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Thu, 3 Sep 2026 11:11:41 +1000 Subject: [PATCH 25/32] fix: set required fields on action token payload --- auth/validator.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/auth/validator.py b/auth/validator.py index ff785cb5..0d281154 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -175,8 +175,12 @@ def create_action_token(payload: dict, settings: Settings, expires_in_seconds: i """ Create a signed JWT that can be passed back to Auth0 actions """ + required_fields = ["sub", "iss", "state"] + for field in required_fields: + if field not in payload: + raise ValueError( f"Missing required field {field} in action token") exp = (datetime.now(tz=UTC) + timedelta(seconds=expires_in_seconds)).timestamp() - payload = {**payload, "exp": exp} + payload = {**payload, "exp": int(exp)} secret = settings.auth0_management_secret signed_payload = jwt.encode( payload, From 8fb9eec4ef5d4fd863f20b0c44d91b49d82819f2 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Thu, 3 Sep 2026 11:14:57 +1000 Subject: [PATCH 26/32] fix: make sure we're including all the required fields for a token returned to Auth0 --- routers/aaf.py | 44 ++++++++++++++++++++++++++++++++++++++------ schemas/auth0.py | 2 ++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/routers/aaf.py b/routers/aaf.py index 828dfee3..3b86b214 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -34,6 +34,16 @@ class AccountLinkResponse(BaseModel): primary_id: str +class SignedAccountLinkResponse(AccountLinkResponse): + """ + Set the fields we need a signed action token to return + to Auth0 + """ + sub: str + iss: str + state: str + + def link_aaf_account(db_user_id: str, aaf_user_id: str, auth0_client: Auth0Client, session: Session): def _get_aaf_identity(aaf_user: Auth0UserData): for identity in aaf_user.identities: @@ -89,6 +99,7 @@ def mark_user_aaf_only(user_email: str, aaf_user_id: str, auth0_client: Auth0Cli def return_signed_response( state: str, response: AccountLinkResponse, + action_token: Auth0ActionToken, settings: Settings, ): """ @@ -96,14 +107,20 @@ def return_signed_response( token, sign the AccountLinkResponse we want to return and redirect to the continue endpoint """ - token = create_action_token( - payload=response.model_dump(mode="json"), + signed_payload = SignedAccountLinkResponse( + **response.model_dump(), + sub=action_token.sub or action_token.user_id, + iss=action_token.iss or settings.auth0_domain, + state=state, + ) + signed_token = create_action_token( + payload=signed_payload.model_dump(mode="json", exclude_none=True), settings=settings, ) auth0_base_url = settings.auth0_custom_domain or f"https://{settings.auth0_domain}" redirect_url = httpx2.URL( f"{auth0_base_url}/continue", - params={"state": state, "session_token": token}, + params={"state": state, "session_token": signed_token}, ) return RedirectResponse(url=redirect_url) @@ -127,7 +144,12 @@ def check_aaf_account_link( if not auth0_matches: mark_user_aaf_only(email, aaf_user_id, auth0_client) resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=token.user_id) - return return_signed_response(state=state, response=resp, settings=settings) + return return_signed_response( + state=state, + response=resp, + action_token=token, + settings=settings, + ) existing_account = None for user in auth0_matches: @@ -138,7 +160,12 @@ def check_aaf_account_link( if not existing_account: mark_user_aaf_only(email, aaf_user_id, auth0_client) resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=email) - return return_signed_response(state=state, response=resp, settings=settings) + return return_signed_response( + state=state, + response=resp, + action_token=token, + settings=settings, + ) if existing_account.blocked: raise HTTPException(status_code=403, detail="Existing account is blocked.") @@ -150,4 +177,9 @@ def check_aaf_account_link( session=session, ) resp = AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id) - return return_signed_response(state=state, response=resp, settings=settings) + return return_signed_response( + state=state, + response=resp, + action_token=token, + settings=settings, + ) diff --git a/schemas/auth0.py b/schemas/auth0.py index 57070774..c44f7414 100644 --- a/schemas/auth0.py +++ b/schemas/auth0.py @@ -30,3 +30,5 @@ class Auth0ActionToken(BaseModel): email: EmailStr client_id: str purpose: str + sub: str | None = None + iss: str | None = None From 5d2b2ad60c8407fbc8b0298181d94eba9eea53f9 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Thu, 3 Sep 2026 11:18:39 +1000 Subject: [PATCH 27/32] test: update tests --- tests/auth/test_auth_validator.py | 47 +++++++++++++++++++++-- tests/test_aaf.py | 64 ++++++++++++++++++++++++++----- tests/test_dependencies.py | 14 +++++-- 3 files changed, 109 insertions(+), 16 deletions(-) diff --git a/tests/auth/test_auth_validator.py b/tests/auth/test_auth_validator.py index e2b42446..093d0c85 100644 --- a/tests/auth/test_auth_validator.py +++ b/tests/auth/test_auth_validator.py @@ -602,9 +602,17 @@ def test_verify_action_token_missing_exp(mock_settings: Settings): def test_create_action_token_success(mock_settings: Settings, frozen_time): secret = TEST_MANAGEMENT_SECRET mock_settings.auth0_management_secret = secret - payload = {"data": "test-payload"} + payload = { + "sub": "auth0|123", + "iss": "https://issuer.example.com", + "state": "test-state", + "data": "test-payload", + } token = create_action_token(payload, mock_settings) decoded = jwt.decode(token, secret, algorithms=["HS256"]) + assert decoded["sub"] == "auth0|123" + assert decoded["iss"] == "https://issuer.example.com" + assert decoded["state"] == "test-state" assert decoded["data"] == "test-payload" assert decoded["exp"] == (FROZEN_TIME + timedelta(minutes=5)).timestamp() @@ -615,23 +623,56 @@ def test_create_action_token_expiry(mock_settings: Settings, frozen_time): """ secret = TEST_MANAGEMENT_SECRET mock_settings.auth0_management_secret = secret - payload = {"data": "test-payload"} + payload = { + "sub": "auth0|123", + "iss": "https://issuer.example.com", + "state": "test-state", + "data": "test-payload", + } token = create_action_token(payload, mock_settings, expires_in_seconds=10 * 60) decoded = jwt.decode(token, secret, algorithms=["HS256"]) assert decoded["data"] == "test-payload" assert decoded["exp"] == (FROZEN_TIME + timedelta(minutes=10)).timestamp() +@pytest.mark.parametrize("missing_field", ["sub", "iss", "state"]) +def test_create_action_token_missing_required_field( + mock_settings: Settings, + frozen_time, + missing_field: str, +): + secret = TEST_MANAGEMENT_SECRET + mock_settings.auth0_management_secret = secret + payload = { + "sub": "auth0|123", + "iss": "https://issuer.example.com", + "state": "test-state", + } + del payload[missing_field] + + with pytest.raises( + ValueError, + match=f"Missing required field {missing_field} in action token", + ): + create_action_token(payload, mock_settings) + + def test_create_action_token_invalid(mock_settings: Settings, frozen_time): secret = TEST_MANAGEMENT_SECRET # Use invalid secret to sign mock_settings.auth0_management_secret = "invalid-secret" - payload = {"data": "test-payload"} + payload = { + "sub": "auth0|123", + "iss": "https://issuer.example.com", + "state": "test-state", + "data": "test-payload", + } token = create_action_token(payload, mock_settings, expires_in_seconds=10 * 60) # Use expected secret to decode with pytest.raises(InvalidSignatureError, match="Signature verification failed"): jwt.decode(token, secret, algorithms=["HS256"]) + @pytest.mark.asyncio @respx.mock async def test_fetch_rsa_keys_only_refreshes_once_when_cache_is_expired(mock_settings: Settings): diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 50e18338..68c7a9fb 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -141,8 +141,25 @@ def test_link_aaf_account_idempotent_when_already_linked(test_db_session, persis assert result.id == db_user.id -def _action_token_payload(user_id: str, email: str, purpose: str = "aaf_link", client_id: str = "test-client") -> dict: - return {"user_id": user_id, "email": email, "client_id": client_id, "purpose": purpose} +def _action_token_payload( + user_id: str, + email: str, + purpose: str = "aaf_link", + client_id: str = "test-client", + sub: str | None = None, + iss: str | None = "mock-domain", +) -> dict: + payload = { + "user_id": user_id, + "email": email, + "client_id": client_id, + "purpose": purpose, + } + if sub is not None: + payload["sub"] = sub + if iss is not None: + payload["iss"] = iss + return payload def _assert_marked_aaf_only(update_user_mock, aaf_user_id: str, email: str): @@ -156,7 +173,12 @@ def _assert_marked_aaf_only(update_user_mock, aaf_user_id: str, email: str): assert app_metadata.linking_completed_at is not None -def _decode_check_link_redirect(response, state: str, settings) -> dict: +def _decode_check_link_redirect( + response, + state: str, + settings, + incoming_token: dict, +) -> dict: assert response.status_code == HTTPStatus.TEMPORARY_REDIRECT assert response.is_redirect redirect = urlparse(response.headers["location"]) @@ -168,11 +190,15 @@ def _decode_check_link_redirect(response, state: str, settings) -> dict: query_params = parse_qs(redirect.query) assert query_params["state"] == [state] assert "session_token" in query_params - return jwt.decode( + decoded_token = jwt.decode( query_params["session_token"][0], key=settings.auth0_management_secret, algorithms=["HS256"], ) + assert decoded_token["state"] == state + assert decoded_token["sub"] == incoming_token.get("sub", incoming_token["user_id"]) + assert decoded_token["iss"] == incoming_token.get("iss", settings.auth0_domain) + return decoded_token def test_mark_user_aaf_only(): @@ -188,7 +214,8 @@ def test_mark_user_aaf_only(): def test_check_link_no_existing_account(test_client, mocker, mock_settings): aaf_user_id = random_auth0_id() email = "new-aaf-user@example.com" - mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + action_token_payload = _action_token_payload(aaf_user_id, email, sub=aaf_user_id) + mocker.patch("dependencies.auth.verify_action_token", return_value=action_token_payload) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[]) link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") update_user = mocker.patch("routers.aaf.Auth0Client.update_user") @@ -204,6 +231,7 @@ def test_check_link_no_existing_account(test_client, mocker, mock_settings): response, state=state, settings=mock_settings, + incoming_token=action_token_payload, ) assert decoded_token["link"] is False assert decoded_token["aaf_only"] is True @@ -216,7 +244,12 @@ def test_check_link_no_existing_account(test_client, mocker, mock_settings): def test_check_link_no_exact_email_match(test_client, mocker, mock_settings): aaf_user_id = random_auth0_id() email = "aaf-user@example.com" - mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + action_token_payload = _action_token_payload( + aaf_user_id, + email, + sub="auth0|original-sub", + ) + mocker.patch("dependencies.auth.verify_action_token", return_value=action_token_payload) near_miss_account = Auth0UserDataFactory.build(email="different-user@example.com") mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[near_miss_account]) link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") @@ -233,6 +266,7 @@ def test_check_link_no_exact_email_match(test_client, mocker, mock_settings): response, state=state, settings=mock_settings, + incoming_token=action_token_payload, ) assert decoded_token["link"] is False assert decoded_token["aaf_only"] is True @@ -244,7 +278,10 @@ def test_check_link_no_exact_email_match(test_client, mocker, mock_settings): def test_check_link_existing_account_blocked(test_client, mocker): aaf_user_id = random_auth0_id() email = "blocked-user@example.com" - mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + mocker.patch( + "dependencies.auth.verify_action_token", + return_value=_action_token_payload(aaf_user_id, email), + ) existing_account = Auth0UserDataFactory.build(email=email, blocked=True) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) @@ -264,7 +301,8 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist ) test_db_session.commit() - mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + action_token_payload = _action_token_payload(aaf_user_id, email, sub=aaf_user_id) + mocker.patch("dependencies.auth.verify_action_token", return_value=action_token_payload) existing_account = Auth0UserDataFactory.build(user_id=db_user.id, email=email, blocked=False) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) aaf_user_data = Auth0UserDataFactory.build( @@ -285,6 +323,7 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist response, state=state, settings=mock_settings, + incoming_token=action_token_payload, ) assert decoded_token["link"] is True assert decoded_token["aaf_only"] is False @@ -310,7 +349,13 @@ def test_check_link_already_linked_is_idempotent(test_client, test_db_session, p ) test_db_session.commit() - mocker.patch("dependencies.auth.verify_action_token", return_value=_action_token_payload(aaf_user_id, email)) + action_token_payload = _action_token_payload( + aaf_user_id, + email, + sub=None, + iss=None, + ) + mocker.patch("dependencies.auth.verify_action_token", return_value=action_token_payload) existing_account = Auth0UserDataFactory.build(user_id=db_user.id, email=email, blocked=False) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) get_user = mocker.patch("routers.aaf.Auth0Client.get_user") @@ -328,6 +373,7 @@ def test_check_link_already_linked_is_idempotent(test_client, test_db_session, p response, state=state, settings=mock_settings, + incoming_token=action_token_payload, ) assert decoded_token["link"] is True assert decoded_token["aaf_only"] is False diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py index 6ee38290..2f417f42 100644 --- a/tests/test_dependencies.py +++ b/tests/test_dependencies.py @@ -16,10 +16,13 @@ def create_signed_action_token(payload: Auth0ActionToken, secret: str) -> str: def test_require_action_token(mock_settings): - payload = Auth0ActionToken(user_id=random_auth0_id(), + user_id = random_auth0_id() + payload = Auth0ActionToken(user_id=user_id, email="test@example.com", client_id="abc123", - purpose="action_purpose") + purpose="action_purpose", + sub=user_id, + iss="mock-domain") signed_token = create_signed_action_token(payload, secret=mock_settings.auth0_management_secret) checker = require_action_token(purpose="action_purpose") checked_payload = checker(signed_token, mock_settings) @@ -27,10 +30,13 @@ def test_require_action_token(mock_settings): def test_require_action_token_wrong_purpose(mock_settings): - payload = Auth0ActionToken(user_id=random_auth0_id(), + user_id = random_auth0_id() + payload = Auth0ActionToken(user_id=user_id, email="test@example.com", client_id="abc123", - purpose="wrong_purpose") + purpose="wrong_purpose", + sub=user_id, + iss="mock-domain") signed_token = create_signed_action_token(payload, secret=mock_settings.auth0_management_secret) checker = require_action_token(purpose="action_purpose") with pytest.raises(HTTPException): From a9b94682d482ff0e5ea0ab35dbe08aaf5c198633 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Thu, 3 Sep 2026 11:27:54 +1000 Subject: [PATCH 28/32] fix: action token should also contain iat field --- auth/validator.py | 5 +++-- tests/auth/test_auth_validator.py | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/auth/validator.py b/auth/validator.py index 0d281154..8bb80025 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -179,8 +179,9 @@ def create_action_token(payload: dict, settings: Settings, expires_in_seconds: i for field in required_fields: if field not in payload: raise ValueError( f"Missing required field {field} in action token") - exp = (datetime.now(tz=UTC) + timedelta(seconds=expires_in_seconds)).timestamp() - payload = {**payload, "exp": int(exp)} + now = datetime.now(tz=UTC) + exp = (now + timedelta(seconds=expires_in_seconds)).timestamp() + payload = {**payload, "exp": int(exp), "iat": int(now.timestamp())} secret = settings.auth0_management_secret signed_payload = jwt.encode( payload, diff --git a/tests/auth/test_auth_validator.py b/tests/auth/test_auth_validator.py index 093d0c85..4e8f2d45 100644 --- a/tests/auth/test_auth_validator.py +++ b/tests/auth/test_auth_validator.py @@ -615,6 +615,7 @@ def test_create_action_token_success(mock_settings: Settings, frozen_time): assert decoded["state"] == "test-state" assert decoded["data"] == "test-payload" assert decoded["exp"] == (FROZEN_TIME + timedelta(minutes=5)).timestamp() + assert decoded["iat"] == FROZEN_TIME.timestamp() def test_create_action_token_expiry(mock_settings: Settings, frozen_time): From e1cf483464898f0db393418fd664b86a71bc467d Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Thu, 3 Sep 2026 15:31:44 +1000 Subject: [PATCH 29/32] fix: don't return 403 response for blocked accounts, return a token to Auth0 --- routers/aaf.py | 13 ++++++++++--- tests/test_aaf.py | 29 ++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/routers/aaf.py b/routers/aaf.py index 3b86b214..5f5cee4e 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -31,6 +31,7 @@ class AccountLinkResponse(BaseModel): link: bool aaf_only: bool = False + blocked: bool = False primary_id: str @@ -143,7 +144,7 @@ def check_aaf_account_link( # No existing account: no need to link if not auth0_matches: mark_user_aaf_only(email, aaf_user_id, auth0_client) - resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=token.user_id) + resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=aaf_user_id) return return_signed_response( state=state, response=resp, @@ -159,7 +160,7 @@ def check_aaf_account_link( # No exact match: no existing account if not existing_account: mark_user_aaf_only(email, aaf_user_id, auth0_client) - resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=email) + resp = AccountLinkResponse(link=False, aaf_only=True, primary_id=aaf_user_id) return return_signed_response( state=state, response=resp, @@ -168,7 +169,13 @@ def check_aaf_account_link( ) if existing_account.blocked: - raise HTTPException(status_code=403, detail="Existing account is blocked.") + resp = AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id, blocked=True) + return return_signed_response( + state=state, + response=resp, + action_token=token, + settings=settings, + ) link_aaf_account( db_user_id=existing_account.user_id, diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 68c7a9fb..5461ae2a 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -270,27 +270,46 @@ def test_check_link_no_exact_email_match(test_client, mocker, mock_settings): ) assert decoded_token["link"] is False assert decoded_token["aaf_only"] is True - assert decoded_token["primary_id"] == email + assert decoded_token["primary_id"] == aaf_user_id link_identity.assert_not_called() _assert_marked_aaf_only(update_user, aaf_user_id, email) -def test_check_link_existing_account_blocked(test_client, mocker): +def test_check_link_existing_account_blocked(test_client, mocker, mock_settings): aaf_user_id = random_auth0_id() email = "blocked-user@example.com" + action_token_payload = _action_token_payload(aaf_user_id, email) mocker.patch( "dependencies.auth.verify_action_token", - return_value=_action_token_payload(aaf_user_id, email), + return_value=action_token_payload, ) existing_account = Auth0UserDataFactory.build(email=email, blocked=True) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) + get_user = mocker.patch("routers.aaf.Auth0Client.get_user") + link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") + update_user = mocker.patch("routers.aaf.Auth0Client.update_user") + state = "dummy" response = test_client.get( "/aaf/check-link", - params={"session_token": "valid_token", "state": "dummy"}, + params={"session_token": "valid_token", "state": state}, + follow_redirects=False, ) - assert response.status_code == 403 + decoded_token = _decode_check_link_redirect( + response, + state=state, + settings=mock_settings, + incoming_token=action_token_payload, + ) + assert decoded_token["link"] is True + assert decoded_token["aaf_only"] is False + assert decoded_token["blocked"] is True + assert decoded_token["primary_id"] == existing_account.user_id + + get_user.assert_not_called() + link_identity.assert_not_called() + update_user.assert_not_called() def test_check_link_existing_account_links(test_client, test_db_session, persistent_factories, mocker, mock_settings): From 0b38c1e42695bfd54d28a7876b55480d8c2987cf Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Fri, 4 Sep 2026 11:20:22 +1000 Subject: [PATCH 30/32] fix: can't link on this side, return identity to Auth0 for linking --- routers/aaf.py | 50 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/routers/aaf.py b/routers/aaf.py index 5f5cee4e..06bcfb3f 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -2,6 +2,7 @@ from datetime import datetime, timezone from http import HTTPStatus from typing import Annotated +from urllib.parse import urlparse import httpx2 from fastapi import APIRouter, Depends, HTTPException @@ -17,6 +18,7 @@ from dependencies.auth import require_action_token from schemas.auth0 import Auth0ActionToken from schemas.biocommons import ( + Auth0Identity, Auth0UserData, BiocommonsAppMetadataUpdate, BiocommonsUserAccountType, @@ -33,6 +35,7 @@ class AccountLinkResponse(BaseModel): aaf_only: bool = False blocked: bool = False primary_id: str + aaf_identity: Auth0Identity | None = None class SignedAccountLinkResponse(AccountLinkResponse): @@ -45,28 +48,32 @@ class SignedAccountLinkResponse(AccountLinkResponse): state: str -def link_aaf_account(db_user_id: str, aaf_user_id: str, auth0_client: Auth0Client, session: Session): - def _get_aaf_identity(aaf_user: Auth0UserData): +def link_aaf_account(db_user_id: str, aaf_user_id: str, auth0_client: Auth0Client, session: Session) -> Auth0Identity: + """ + Update app_metadata and DB record for AAF account linking. + + NOTE: we can't do the actual linking here, it needs to be done + on the Auth0 side. See: https://support.auth0.com/center/s/article/Unable-to-process-redirect-callback + """ + def _get_aaf_identity(aaf_user: Auth0UserData) -> Auth0Identity | None: for identity in aaf_user.identities: if identity.connection == "AAF": return identity return None - db_user = BiocommonsUser.get_by_id_or_404(db_user_id, session=session) - if db_user.account_type == BiocommonsUserAccountType.AAF and db_user.other_user_id == aaf_user_id: - logger.info("AAF account already linked, skipping re-link") - return db_user aaf_user_info = auth0_client.get_user(user_id=aaf_user_id) aaf_identity = _get_aaf_identity(aaf_user_info) if aaf_identity is None: raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Couldn't get AAF provider information") - logger.info("Linking AAF account to existing database account") + db_user = BiocommonsUser.get_by_id_or_404(db_user_id, session=session) + if db_user.account_type == BiocommonsUserAccountType.AAF and db_user.other_user_id == aaf_user_id: + logger.info("AAF account already linked, skipping re-link") + return aaf_identity + + logger.info("Updating DB record and account metadata") now = datetime.now(tz=timezone.utc) try: - auth0_client.link_identity(primary_user_id=db_user_id, secondary_user_id=aaf_user_id, - secondary_provider=aaf_identity.provider, - secondary_connection_name=aaf_identity.connection) auth0_client.update_user( db_user_id, update_data=UpdateUserData( @@ -78,11 +85,11 @@ def _get_aaf_identity(aaf_user: Auth0UserData): ) ) except ValueError as exc: - logger.error(f"Failed to link AAF account in Auth0: {exc}") + logger.error(f"Failed to update account metadata in Auth0: {exc}") raise HTTPException(status_code=HTTPStatus.BAD_GATEWAY, detail="Failed to link account with Auth0") from exc logger.info("Updating DB record") db_user.link_aaf_account(aaf_user_id=aaf_user_id, session=session, updated_by=db_user, commit=True) - return db_user + return aaf_identity def mark_user_aaf_only(user_email: str, aaf_user_id: str, auth0_client: Auth0Client): @@ -118,7 +125,7 @@ def return_signed_response( payload=signed_payload.model_dump(mode="json", exclude_none=True), settings=settings, ) - auth0_base_url = settings.auth0_custom_domain or f"https://{settings.auth0_domain}" + auth0_base_url = get_auth0_continue_base_url(action_token, settings) redirect_url = httpx2.URL( f"{auth0_base_url}/continue", params={"state": state, "session_token": signed_token}, @@ -126,6 +133,19 @@ def return_signed_response( return RedirectResponse(url=redirect_url) +def get_auth0_continue_base_url(action_token: Auth0ActionToken, settings: Settings) -> str: + """ + Get domain to continue action from action token's issuer, if possible - want + to ensure we continue on the same domain + """ + if action_token.iss: + issuer = action_token.iss.rstrip("/") + parsed_issuer = urlparse(issuer) + if parsed_issuer.scheme and parsed_issuer.netloc: + return f"{parsed_issuer.scheme}://{parsed_issuer.netloc}" + return f"https://{issuer}" + return settings.auth0_custom_domain or f"https://{settings.auth0_domain}" + @router.get("/check-link", response_model=AccountLinkResponse) def check_aaf_account_link( @@ -177,13 +197,13 @@ def check_aaf_account_link( settings=settings, ) - link_aaf_account( + aaf_identity = link_aaf_account( db_user_id=existing_account.user_id, aaf_user_id=aaf_user_id, auth0_client=auth0_client, session=session, ) - resp = AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id) + resp = AccountLinkResponse(link=True, aaf_only=False, primary_id=existing_account.user_id, aaf_identity=aaf_identity) return return_signed_response( state=state, response=resp, From 59cc94c71c063d9669ae55f49123abf7a283ce50 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Fri, 4 Sep 2026 11:31:15 +1000 Subject: [PATCH 31/32] test: update tests --- tests/test_aaf.py | 73 +++++++++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/tests/test_aaf.py b/tests/test_aaf.py index 5461ae2a..42478e48 100644 --- a/tests/test_aaf.py +++ b/tests/test_aaf.py @@ -32,13 +32,7 @@ def test_link_aaf_account_links_identity_updates_metadata_and_db(test_db_session ) auth0_client.get_user.assert_called_once_with(user_id=aaf_user_id) - auth0_client.link_identity.assert_called_once_with( - primary_user_id=db_user.id, - secondary_user_id=aaf_user_id, - secondary_provider="samlp", - secondary_connection_name="AAF", - ) - + auth0_client.link_identity.assert_not_called() auth0_client.update_user.assert_called_once() call_args, call_kwargs = auth0_client.update_user.call_args assert call_args[0] == db_user.id @@ -47,9 +41,7 @@ def test_link_aaf_account_links_identity_updates_metadata_and_db(test_db_session assert app_metadata.linking_completed is True assert app_metadata.linking_completed_at is not None - assert result.id == db_user.id - assert result.account_type == BiocommonsUserAccountType.AAF - assert result.other_user_id == aaf_user_id + assert result == aaf_user_data.identities[0] test_db_session.refresh(db_user) assert db_user.account_type == BiocommonsUserAccountType.AAF @@ -91,7 +83,7 @@ def test_link_aaf_account_raises_404_when_no_aaf_identity(test_db_session, persi assert db_user.other_user_id is None -def test_link_aaf_account_raises_502_when_auth0_linking_fails(test_db_session, persistent_factories): +def test_link_aaf_account_raises_502_when_auth0_update_fails(test_db_session, persistent_factories): db_user = BiocommonsUserFactory.create_sync(account_type=BiocommonsUserAccountType.AUTH0, other_user_id=None) test_db_session.commit() @@ -101,7 +93,7 @@ def test_link_aaf_account_raises_502_when_auth0_linking_fails(test_db_session, p ) auth0_client = MagicMock(spec=Auth0Client) auth0_client.get_user.return_value = aaf_user_data - auth0_client.link_identity.side_effect = ValueError("Identity already linked") + auth0_client.update_user.side_effect = ValueError("Auth0 update failed") with pytest.raises(HTTPException) as exc_info: link_aaf_account( @@ -112,7 +104,8 @@ def test_link_aaf_account_raises_502_when_auth0_linking_fails(test_db_session, p ) assert exc_info.value.status_code == HTTPStatus.BAD_GATEWAY - auth0_client.update_user.assert_not_called() + auth0_client.link_identity.assert_not_called() + auth0_client.update_user.assert_called_once() test_db_session.refresh(db_user) assert db_user.account_type == BiocommonsUserAccountType.AUTH0 @@ -126,7 +119,11 @@ def test_link_aaf_account_idempotent_when_already_linked(test_db_session, persis ) test_db_session.commit() + aaf_user_data = Auth0UserDataFactory.build( + identities=[Auth0Identity(connection="AAF", provider="samlp", user_id=aaf_user_id, isSocial=False)] + ) auth0_client = MagicMock(spec=Auth0Client) + auth0_client.get_user.return_value = aaf_user_data result = link_aaf_account( db_user_id=db_user.id, @@ -135,10 +132,10 @@ def test_link_aaf_account_idempotent_when_already_linked(test_db_session, persis session=test_db_session, ) - auth0_client.get_user.assert_not_called() + auth0_client.get_user.assert_called_once_with(user_id=aaf_user_id) auth0_client.link_identity.assert_not_called() auth0_client.update_user.assert_not_called() - assert result.id == db_user.id + assert result == aaf_user_data.identities[0] def _action_token_payload( @@ -173,6 +170,17 @@ def _assert_marked_aaf_only(update_user_mock, aaf_user_id: str, email: str): assert app_metadata.linking_completed_at is not None +def _expected_auth0_continue_base_url(incoming_token: dict, settings) -> str: + issuer = incoming_token.get("iss") + if issuer is None: + return settings.auth0_custom_domain or f"https://{settings.auth0_domain}" + issuer = issuer.rstrip("/") + parsed_issuer = urlparse(issuer) + if parsed_issuer.scheme and parsed_issuer.netloc: + return f"{parsed_issuer.scheme}://{parsed_issuer.netloc}" + return f"https://{issuer}" + + def _decode_check_link_redirect( response, state: str, @@ -182,7 +190,7 @@ def _decode_check_link_redirect( assert response.status_code == HTTPStatus.TEMPORARY_REDIRECT assert response.is_redirect redirect = urlparse(response.headers["location"]) - expected_base_url = settings.auth0_custom_domain or f"https://{settings.auth0_domain}" + expected_base_url = _expected_auth0_continue_base_url(incoming_token, settings) expected_continue_url = urlparse(f"{expected_base_url}/continue") assert redirect.scheme == expected_continue_url.scheme assert redirect.netloc == expected_continue_url.netloc @@ -214,7 +222,12 @@ def test_mark_user_aaf_only(): def test_check_link_no_existing_account(test_client, mocker, mock_settings): aaf_user_id = random_auth0_id() email = "new-aaf-user@example.com" - action_token_payload = _action_token_payload(aaf_user_id, email, sub=aaf_user_id) + action_token_payload = _action_token_payload( + aaf_user_id, + email, + sub=aaf_user_id, + iss="https://login.example.com/", + ) mocker.patch("dependencies.auth.verify_action_token", return_value=action_token_payload) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[]) link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") @@ -347,12 +360,13 @@ def test_check_link_existing_account_links(test_client, test_db_session, persist assert decoded_token["link"] is True assert decoded_token["aaf_only"] is False assert decoded_token["primary_id"] == db_user.id - link_identity.assert_called_once_with( - primary_user_id=db_user.id, - secondary_user_id=aaf_user_id, - secondary_provider="samlp", - secondary_connection_name="AAF", - ) + assert decoded_token["aaf_identity"] == { + "connection": "AAF", + "provider": "samlp", + "user_id": aaf_user_id, + "isSocial": False, + } + link_identity.assert_not_called() update_user.assert_called_once() test_db_session.refresh(db_user) @@ -377,7 +391,10 @@ def test_check_link_already_linked_is_idempotent(test_client, test_db_session, p mocker.patch("dependencies.auth.verify_action_token", return_value=action_token_payload) existing_account = Auth0UserDataFactory.build(user_id=db_user.id, email=email, blocked=False) mocker.patch("routers.aaf.Auth0Client.search_users_by_email", return_value=[existing_account]) - get_user = mocker.patch("routers.aaf.Auth0Client.get_user") + aaf_user_data = Auth0UserDataFactory.build( + identities=[Auth0Identity(connection="AAF", provider="samlp", user_id=aaf_user_id, isSocial=False)] + ) + get_user = mocker.patch("routers.aaf.Auth0Client.get_user", return_value=aaf_user_data) link_identity = mocker.patch("routers.aaf.Auth0Client.link_identity") update_user = mocker.patch("routers.aaf.Auth0Client.update_user") @@ -397,6 +414,12 @@ def test_check_link_already_linked_is_idempotent(test_client, test_db_session, p assert decoded_token["link"] is True assert decoded_token["aaf_only"] is False assert decoded_token["primary_id"] == db_user.id - get_user.assert_not_called() + assert decoded_token["aaf_identity"] == { + "connection": "AAF", + "provider": "samlp", + "user_id": aaf_user_id, + "isSocial": False, + } + get_user.assert_called_once_with(user_id=aaf_user_id) link_identity.assert_not_called() update_user.assert_not_called() From cfa75c69dde92113db1e9c43b9e34ef1d379f199 Mon Sep 17 00:00:00 2001 From: Marius Mather Date: Fri, 4 Sep 2026 11:55:21 +1000 Subject: [PATCH 32/32] fix: remove response_model - response is a redirect --- routers/aaf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/aaf.py b/routers/aaf.py index 06bcfb3f..430b9ba3 100644 --- a/routers/aaf.py +++ b/routers/aaf.py @@ -147,7 +147,7 @@ def get_auth0_continue_base_url(action_token: Auth0ActionToken, settings: Settin return settings.auth0_custom_domain or f"https://{settings.auth0_domain}" -@router.get("/check-link", response_model=AccountLinkResponse) +@router.get("/check-link") def check_aaf_account_link( state: str, token: Annotated[Auth0ActionToken, Depends(require_action_token(purpose="aaf_link"))],