improve testing and managing email sending

Author: amandazhuyilan

.env.example

@@ -15,6 +15,6 @@ GALAXY_API_KEY=api-key
 #   don't process this with pydantic-settings as it needs
 #   to be used before the FastAPI app loads
 CORS_ALLOWED_ORIGINS=http://localhost:8000
-# AWS SES configs
+# AWS SES configs - required for running tests locally. Ask amanda@biocommons.org.au for credentials values
 AWS_ACCESS_KEY_ID=<aws-access-key-id>
 AWS_SECRET_ACCESS_KEY=<aws-secret-access-key>

pyproject.toml

@@ -21,7 +21,8 @@ dev = [
     "polyfactory>=2.21.0",
     "pre-commit>=3.7.0",
     "freezegun>=1.5.2",
-    "respx>=0.22.0"
+    "respx>=0.22.0",
+    "moto>=5.0.5"
 ]
 
 [tool.pytest.ini_options]

routers/bpa_register.py

@@ -1,7 +1,7 @@
 from datetime import datetime, timezone
 from typing import Any, Dict
 
-from fastapi import APIRouter, Depends, HTTPException
+from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException
 from httpx import AsyncClient
 
 from auth.config import Settings, get_settings
@@ -14,6 +14,29 @@
 router = APIRouter(prefix="/bpa", tags=["bpa", "registration"])
 
 
+def send_approver_email(
+    email_service: EmailService,
+    approver_email: str,
+    registration: BPARegistrationRequest,
+    bpa_resources: list[Dict[str, Any]],
+) -> None:
+    subject = "New BPA User Access Request"
+
+    org_list_html = "".join(
+        f"<li>{res['name']} (ID: {res['id']})</li>" for res in bpa_resources
+    )
+
+    body_html = f"""
+        <p>A new user has requested access to one or more organizations in the BPA service.</p>
+        <p><strong>User:</strong> {registration.fullname} ({registration.email})</p>
+        <p><strong>Requested access to:</strong></p>
+        <ul>{org_list_html}</ul>
+        <p>Please log into the AAI Admin Portal to review and approve access.</p>
+    """
+
+    email_service.send(approver_email, subject, body_html)
+
+
 @router.post(
     "/register",
     response_model=Dict[str, Any],
@@ -24,7 +47,9 @@
     },
 )
 async def register_bpa_user(
-    registration: BPARegistrationRequest, settings: Settings = Depends(get_settings)
+    registration: BPARegistrationRequest,
+    background_tasks: BackgroundTasks,
+    settings: Settings = Depends(get_settings),
 ) -> Dict[str, Any]:
     """Register a new BPA user with selected organization resources."""
     url = f"https://{settings.auth0_domain}/api/v2/users"
@@ -82,25 +107,14 @@ async def register_bpa_user(
         if bpa_resources:
             email_service = EmailService()
             approver_email = "aai-dev@biocommons.org.au"  # ideally move to settings
-            subject = "New BPA User Access Request"
 
-            org_list_html = "".join(
-                f"<li>{res['name']} (ID: {res['id']})</li>" for res in bpa_resources
+            # Add email sending as a background task
+            background_tasks.add_task(
+                send_approver_email, email_service, approver_email, registration, bpa_resources
             )
 
-            body_html = f"""
-                <p>A new user has requested access to one or more organizations in the BPA service.</p>
-                <p><strong>User:</strong> {registration.fullname} ({registration.email})</p>
-                <p><strong>Requested access to:</strong></p>
-                <ul>{org_list_html}</ul>
-                <p>Please log into the AAI Admin Portal to review and approve access.</p>
-            """
-
-            email_service.send(approver_email, subject, body_html)
-
         return {"message": "User registered successfully", "user": response.json()}
 
-
     except HTTPException:
         raise
     except Exception as e:

tests/test_auth_validator.py

@@ -1,7 +1,7 @@
 from unittest.mock import patch
 
 from jose import jwt
-from jose.backends.rsa_backend import RSAKey
+from jose.backends.cryptography_backend import CryptographyRSAKey
 
 from auth.config import Settings
 from auth.validator import get_rsa_key
@@ -26,4 +26,4 @@ def test_get_rsa_key_returns_key(mock_settings: Settings):
 
         key = get_rsa_key(token, settings=mock_settings)
         assert key is not None
-        assert isinstance(key, RSAKey)
+        assert isinstance(key, CryptographyRSAKey)

tests/test_ses.py

@@ -1,57 +1,35 @@
-from unittest.mock import MagicMock, patch
-
+import boto3
 import pytest
+from moto import mock_aws
 
 from auth.ses import EmailService
 
 
 @pytest.fixture
-def mock_boto_client():
-    with patch("auth.ses.boto3.client") as mock_client:
-        yield mock_client
-
-def test_send_email_success(mock_boto_client):
-    """Test successful email sending"""
-    mock_ses = MagicMock()
-    mock_ses.send_email.return_value = {"MessageId": "test-message-id"}
-    mock_boto_client.return_value = mock_ses
-
-    service = EmailService()
+def ses_client():
+    with mock_aws():
+        client = boto3.client("ses", region_name="ap-southeast-2")
+        client.verify_email_identity(EmailAddress="sender@example.com")
+        yield client
+
+def test_send_email_success_with_moto(ses_client):
+    service = EmailService(region_name="ap-southeast-2")
     service.send(
         to_address="recipient@example.com",
-        subject="Test Subject",
-        body_html="<p>This is a test</p>",
+        subject="Test Moto Subject",
+        body_html="<p>This is a moto test email</p>",
         sender="sender@example.com",
     )
+    # If no exception, we assume success with moto
 
-    mock_ses.send_email.assert_called_once_with(
-        Source="sender@example.com",
-        Destination={"ToAddresses": ["recipient@example.com"]},
-        Message={
-            "Subject": {"Data": "Test Subject"},
-            "Body": {"Html": {"Data": "<p>This is a test</p>"}},
-        },
-    )
-
-def test_send_email_failure(mock_boto_client):
-    """Test SES failure raises exception and logs"""
-    from botocore.exceptions import ClientError
-
-    mock_ses = MagicMock()
-    mock_ses.send_email.side_effect = ClientError(
-        error_response={
-            "Error": {"Message": "Invalid email", "Code": "MessageRejected"}
-        },
-        operation_name="SendEmail",
-    )
-    mock_boto_client.return_value = mock_ses
-
-    service = EmailService()
+def test_send_email_failure_with_moto(ses_client):
+    service = EmailService(region_name="ap-southeast-2")
 
-    with pytest.raises(ClientError):
+    # Deliberately use an unverified sender to cause failure
+    with pytest.raises(Exception):
         service.send(
-            to_address="bad@example.com",
-            subject="Failing",
-            body_html="<p>Bad request</p>",
-            sender="no-reply@example.com",
+            to_address="recipient@example.com",
+            subject="Should Fail",
+            body_html="<p>Should not send</p>",
+            sender="unverified@example.com",
         )