From 48dc5557f8aa12a5e0bda7fa4ef3b705df4f7ee3 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 14:51:56 +1000 Subject: [PATCH 01/22] user endpoints init --- auth/validator.py | 9 +++- main.py | 10 ++-- routers/admin.py | 0 routers/user.py | 99 ++++++++++++++++++++++++++++++++++++++ tests/test_main.py | 7 ++- tests/test_user.py | 116 +++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 229 insertions(+), 12 deletions(-) create mode 100644 routers/admin.py create mode 100644 routers/user.py create mode 100644 tests/test_user.py diff --git a/auth/validator.py b/auth/validator.py index 89c0aaf0..8041aaac 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -1,11 +1,13 @@ from jose import jwt from jose.exceptions import JWTError -from fastapi import HTTPException +from fastapi import HTTPException, Depends +from fastapi.security import OAuth2PasswordBearer from typing import Dict import httpx from auth.config import get_settings +oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") def verify_jwt(token: str) -> Dict: settings = get_settings() @@ -48,4 +50,7 @@ def verify_jwt(token: str) -> Dict: except JWTError as e: raise HTTPException(status_code=401, detail=f"Invalid token: {e}") - raise HTTPException(status_code=401, detail="Unable to verify token") \ No newline at end of file + raise HTTPException(status_code=401, detail="Unable to verify token") + +def get_current_user(token: str = Depends(oauth2_scheme)): + return verify_jwt(token) \ No newline at end of file diff --git a/main.py b/main.py index 86f8a856..2d4ce904 100644 --- a/main.py +++ b/main.py @@ -1,13 +1,9 @@ from fastapi import Depends, FastAPI, HTTPException -from fastapi.security import OAuth2PasswordBearer from auth.management import get_management_token -from auth.validator import verify_jwt +from auth.validator import get_current_user +from routers import user app = FastAPI() -oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") - -def get_current_user(token: str = Depends(oauth2_scheme)): - return verify_jwt(token) @app.get("/") def public_route(): @@ -16,3 +12,5 @@ def public_route(): @app.get("/private") def private_route(user=Depends(get_current_user)): return {"message": "Private route", "user_claims": user, "management_token": get_management_token()} + +app.include_router(user.router) diff --git a/routers/admin.py b/routers/admin.py new file mode 100644 index 00000000..e69de29b diff --git a/routers/user.py b/routers/user.py new file mode 100644 index 00000000..eacba190 --- /dev/null +++ b/routers/user.py @@ -0,0 +1,99 @@ +from fastapi import APIRouter, Depends, HTTPException +from auth.management import get_management_token +from auth.config import get_settings +from auth.validator import get_current_user +import httpx + +router = APIRouter() +settings = get_settings() + +async def fetch_user_data(user_id: str, token: str): + url = f"https://{settings.auth0_domain}/api/v2/users/{user_id}" + headers = {"Authorization": f"Bearer {token}"} + async with httpx.AsyncClient() as client: + response = await client.get(url, headers=headers) + if response.status_code != 200: + raise HTTPException(status_code=response.status_code, detail="Failed to fetch user data") + return response.json() + +@router.get("/me/services") +async def get_all_services(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + return {"services": services} + +@router.get("/me/services/approved") +async def get_approved_services(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + approved_services = [service for service in services if service.get("status") == "approved"] + return {"approved_services": approved_services} + +@router.get("/me/services/pending") +async def get_pending_services(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + pending_services = [service for service in services if service.get("status") == "pending"] + return {"pending_services": pending_services} + +@router.get("/me/resources") +async def get_all_resources(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + resources = [ + resource + for service in services + for resource in service.get("resources", []) + ] + return {"resources": resources} + +@router.get("/me/resources/approved") +async def get_approved_resources(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + approved_resources = [ + resource + for service in services + for resource in service.get("resources", []) + if resource.get("status") == "approved" + ] + return {"approved_resources": approved_resources} + +@router.get("/me/resources/pending") +async def get_pending_resources(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + pending_resources = [ + resource + for service in services + for resource in service.get("resources", []) + if resource.get("status") == "pending" + ] + return {"pending_resources": pending_resources} + +@router.get("/me/all/pending") +async def get_all_pending(user: dict = Depends(get_current_user)): + user_id = user["sub"] + token = get_management_token() + user_data = await fetch_user_data(user_id, token) + services = user_data.get("app_metadata", {}).get("services", []) + pending_services = [service for service in services if service.get("status") == "pending"] + pending_resources = [ + resource + for service in services + for resource in service.get("resources", []) + if resource.get("status") == "pending" + ] + return {"pending_services": pending_services, "pending_resources": pending_resources} diff --git a/tests/test_main.py b/tests/test_main.py index ab9fa5be..893cb1b7 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -19,11 +19,11 @@ def test_private_valid_token(mocker): auth0_algorithms=["RS256"] )) - mocker.patch("main.verify_jwt", return_value={ + mocker.patch("auth.validator.verify_jwt", return_value={ "sub": "auth0|123456789", "biocommons.org.au/roles": ["acdc/indexd_admin"] }) - mocker.patch("main.get_management_token", return_value="mock_management_token") + mocker.patch("auth.management.get_management_token", return_value="mock_management_token") headers = {"Authorization": "Bearer valid_token"} response = client.get("/private", headers=headers) assert response.status_code == 200 @@ -42,7 +42,6 @@ def test_private_missing_token(): assert response.json() == {"detail": "Not authenticated"} def test_private_invalid_token(mocker): - mocker.patch("httpx.get", return_value=mocker.Mock(json=lambda: {"keys": []})) mocker.patch("auth.validator.verify_jwt", side_effect=Exception("Invalid token: Error decoding token headers.")) headers = {"Authorization": "Bearer invalid_token"} @@ -51,7 +50,7 @@ def test_private_invalid_token(mocker): assert response.json() == {"detail": "Invalid token: Error decoding token headers."} def test_private_insufficient_permissions(mocker): - mocker.patch("main.verify_jwt", side_effect=HTTPException( + mocker.patch("auth.validator.verify_jwt", side_effect=HTTPException( status_code=403, detail="Access denied: Insufficient permissions" )) headers = {"Authorization": "Bearer insufficient_permissions_token"} diff --git a/tests/test_user.py b/tests/test_user.py new file mode 100644 index 00000000..2e3e6de9 --- /dev/null +++ b/tests/test_user.py @@ -0,0 +1,116 @@ +import pytest +from fastapi.testclient import TestClient +from fastapi import HTTPException +from routers.user import router +from main import app + +client = TestClient(app) + +@pytest.fixture +def mock_user(): + return {"sub": "auth0|123456789"} + +@pytest.fixture +def mock_user_data(): + return { + "app_metadata": { + "services": [ + { + "name": "Service 1", + "status": "approved", + "resources": [{"name": "Resource 1", "status": "approved", "id": "res1"}] + }, + { + "name": "Service 2", + "status": "pending", + "resources": [{"name": "Resource 2", "status": "pending", "id": "res2"}] + } + ] + } + } + +@pytest.fixture(autouse=True) +def mock_dependencies(mocker, mock_user): + mocker.patch("auth.validator.get_current_user", return_value=mock_user) + mocker.patch("auth.management.get_management_token", return_value="mock_token") + mocker.patch("auth.validator.verify_jwt", return_value={ + "sub": "auth0|123456789", + "biocommons.org.au/roles": ["user"] + }) + +def test_get_all_services(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/services", headers=headers) + assert response.status_code == 200 + assert response.json() == {"services": mock_user_data["app_metadata"]["services"]} + +def test_get_approved_services(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/services/approved", headers=headers) + assert response.status_code == 200 + assert response.json() == { + "approved_services": [ + {"name": "Service 1", "status": "approved", "resources": [{"name": "Resource 1", "status": "approved", "id": "res1"}]} + ] + } + +def test_get_pending_services(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/services/pending", headers=headers) + assert response.status_code == 200 + assert response.json() == { + "pending_services": [ + {"name": "Service 2", "status": "pending", "resources": [{"name": "Resource 2", "status": "pending", "id": "res2"}]} + ] + } + +def test_get_all_resources(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/resources", headers=headers) + assert response.status_code == 200 + assert response.json() == { + "resources": [ + {"name": "Resource 1", "status": "approved", "id": "res1"}, + {"name": "Resource 2", "status": "pending", "id": "res2"} + ] + } + +def test_get_approved_resources(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/resources/approved", headers=headers) + assert response.status_code == 200 + assert response.json() == { + "approved_resources": [ + {"name": "Resource 1", "status": "approved", "id": "res1"} + ] + } + +def test_get_pending_resources(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/resources/pending", headers=headers) + assert response.status_code == 200 + assert response.json() == { + "pending_resources": [ + {"name": "Resource 2", "status": "pending", "id": "res2"} + ] + } + +def test_get_all_pending(mocker, mock_user_data): + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/all/pending", headers=headers) + assert response.status_code == 200 + assert response.json() == { + "pending_services": [ + {"name": "Service 2", "status": "pending", "resources": [{"name": "Resource 2", "status": "pending", "id": "res2"}]} + ], + "pending_resources": [ + {"name": "Resource 2", "status": "pending", "id": "res2"} + ] + } \ No newline at end of file From f5cad695d76fb58fdd292d1c31333c831acb232d Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 15:36:16 +1000 Subject: [PATCH 02/22] Refactor user router to improve code organization and maintainability --- auth/validator.py | 16 +++++++++++++--- main.py | 13 +++++++++++-- routers/user.py | 14 +++++++++++--- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/auth/validator.py b/auth/validator.py index 19d4aee2..1e46687c 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -1,14 +1,17 @@ from typing import Dict import httpx -from fastapi import HTTPException +from fastapi import Depends, HTTPException +from fastapi.security import OAuth2PasswordBearer from jose import jwt from jose.exceptions import JWTError from auth.config import get_settings + oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") + def verify_jwt(token: str) -> Dict: settings = get_settings() try: @@ -39,11 +42,17 @@ def verify_jwt(token: str) -> Dict: roles_claim = "biocommons.org.au/roles" if roles_claim not in payload: - raise HTTPException(status_code=403, detail=f"Missing required claim: {roles_claim}") + raise HTTPException( + status_code=403, + detail=f"Missing required claim: {roles_claim}" + ) roles = payload[roles_claim] if not isinstance(roles, list) or not any("admin" in role.lower() for role in roles): - raise HTTPException(status_code=403, detail=f"Access denied: Insufficient permissions") + raise HTTPException( + status_code=403, + detail=f"Access denied: Insufficient permissions" + ) return payload @@ -52,5 +61,6 @@ def verify_jwt(token: str) -> Dict: raise HTTPException(status_code=401, detail="Unable to verify token") + def get_current_user(token: str = Depends(oauth2_scheme)): return verify_jwt(token) \ No newline at end of file diff --git a/main.py b/main.py index 2d4ce904..f9ca588c 100644 --- a/main.py +++ b/main.py @@ -1,16 +1,25 @@ -from fastapi import Depends, FastAPI, HTTPException +from fastapi import Depends, FastAPI + from auth.management import get_management_token from auth.validator import get_current_user from routers import user + app = FastAPI() + @app.get("/") def public_route(): return {"message": "Public route"} + @app.get("/private") def private_route(user=Depends(get_current_user)): - return {"message": "Private route", "user_claims": user, "management_token": get_management_token()} + return { + "message": "Private route", + "user_claims": user, + "management_token": get_management_token() + } + app.include_router(user.router) diff --git a/routers/user.py b/routers/user.py index eacba190..bb6a385a 100644 --- a/routers/user.py +++ b/routers/user.py @@ -1,19 +1,27 @@ +from typing import Dict, List + +import httpx from fastapi import APIRouter, Depends, HTTPException -from auth.management import get_management_token + from auth.config import get_settings +from auth.management import get_management_token from auth.validator import get_current_user -import httpx + router = APIRouter() settings = get_settings() + async def fetch_user_data(user_id: str, token: str): url = f"https://{settings.auth0_domain}/api/v2/users/{user_id}" headers = {"Authorization": f"Bearer {token}"} async with httpx.AsyncClient() as client: response = await client.get(url, headers=headers) if response.status_code != 200: - raise HTTPException(status_code=response.status_code, detail="Failed to fetch user data") + raise HTTPException( + status_code=response.status_code, + detail="Failed to fetch user data" + ) return response.json() @router.get("/me/services") From 67c3c84630f3e8f1346697798c9b6375c4ea59c1 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 15:37:44 +1000 Subject: [PATCH 03/22] Refactor user router to improve code organization and maintainability --- routers/user.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/routers/user.py b/routers/user.py index bb6a385a..1cc90371 100644 --- a/routers/user.py +++ b/routers/user.py @@ -1,5 +1,3 @@ -from typing import Dict, List - import httpx from fastapi import APIRouter, Depends, HTTPException From efe89650e927cb6f7ed400d20389a57e0897cf49 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 15:38:58 +1000 Subject: [PATCH 04/22] Fix indentation in main.py for consistency --- main.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/main.py b/main.py index f9ca588c..94c39656 100644 --- a/main.py +++ b/main.py @@ -10,16 +10,16 @@ @app.get("/") def public_route(): - return {"message": "Public route"} + return {"message": "Public route"} @app.get("/private") def private_route(user=Depends(get_current_user)): - return { - "message": "Private route", - "user_claims": user, - "management_token": get_management_token() - } + return { + "message": "Private route", + "user_claims": user, + "management_token": get_management_token() + } app.include_router(user.router) From 064194b35bde0a4a875f0e167fdd06f245ddde71 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 15:45:24 +1000 Subject: [PATCH 05/22] Refactor test files for improved readability and consistency --- tests/test_main.py | 59 ++++++++++++++++++++++++++++++++-------------- tests/test_user.py | 21 ++++++++++++----- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 893cb1b7..c0d83b2e 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,29 +1,42 @@ from fastapi import HTTPException from fastapi.testclient import TestClient -from main import app + from auth.config import Settings +from main import app + client = TestClient(app) + def test_public(): response = client.get("/") assert response.status_code == 200 assert response.json() == {"message": "Public route"} + def test_private_valid_token(mocker): - mocker.patch("auth.config.get_settings", return_value=Settings( - auth0_domain="mock-domain", - auth0_audience="mock-audience", - auth0_management_id="mock-id", - auth0_management_secret="mock-secret", - auth0_algorithms=["RS256"] - )) - - mocker.patch("auth.validator.verify_jwt", return_value={ - "sub": "auth0|123456789", - "biocommons.org.au/roles": ["acdc/indexd_admin"] - }) - mocker.patch("auth.management.get_management_token", return_value="mock_management_token") + mocker.patch( + "auth.config.get_settings", + return_value=Settings( + auth0_domain="mock-domain", + auth0_audience="mock-audience", + auth0_management_id="mock-id", + auth0_management_secret="mock-secret", + auth0_algorithms=["RS256"] + ) + ) + + mocker.patch( + "auth.validator.verify_jwt", + return_value={ + "sub": "auth0|123456789", + "biocommons.org.au/roles": ["acdc/indexd_admin"] + } + ) + mocker.patch( + "auth.management.get_management_token", + return_value="mock_management_token" + ) headers = {"Authorization": "Bearer valid_token"} response = client.get("/private", headers=headers) assert response.status_code == 200 @@ -36,23 +49,33 @@ def test_private_valid_token(mocker): "management_token": "mock_management_token" } + def test_private_missing_token(): response = client.get("/private") assert response.status_code == 401 assert response.json() == {"detail": "Not authenticated"} + def test_private_invalid_token(mocker): - mocker.patch("auth.validator.verify_jwt", side_effect=Exception("Invalid token: Error decoding token headers.")) + mocker.patch( + "auth.validator.verify_jwt", + side_effect=Exception("Invalid token: Error decoding token headers.") + ) headers = {"Authorization": "Bearer invalid_token"} response = client.get("/private", headers=headers) assert response.status_code == 401 assert response.json() == {"detail": "Invalid token: Error decoding token headers."} + def test_private_insufficient_permissions(mocker): - mocker.patch("auth.validator.verify_jwt", side_effect=HTTPException( - status_code=403, detail="Access denied: Insufficient permissions" - )) + mocker.patch( + "auth.validator.verify_jwt", + side_effect=HTTPException( + status_code=403, + detail="Access denied: Insufficient permissions" + ) + ) headers = {"Authorization": "Bearer insufficient_permissions_token"} response = client.get("/private", headers=headers) assert response.status_code == 403 diff --git a/tests/test_user.py b/tests/test_user.py index 2e3e6de9..7ceab0dc 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -1,15 +1,19 @@ import pytest -from fastapi.testclient import TestClient from fastapi import HTTPException -from routers.user import router +from fastapi.testclient import TestClient + from main import app +from routers.user import router + client = TestClient(app) + @pytest.fixture def mock_user(): return {"sub": "auth0|123456789"} + @pytest.fixture def mock_user_data(): return { @@ -29,14 +33,19 @@ def mock_user_data(): } } + @pytest.fixture(autouse=True) def mock_dependencies(mocker, mock_user): mocker.patch("auth.validator.get_current_user", return_value=mock_user) mocker.patch("auth.management.get_management_token", return_value="mock_token") - mocker.patch("auth.validator.verify_jwt", return_value={ - "sub": "auth0|123456789", - "biocommons.org.au/roles": ["user"] - }) + mocker.patch( + "auth.validator.verify_jwt", + return_value={ + "sub": "auth0|123456789", + "biocommons.org.au/roles": ["user"] + } + ) + def test_get_all_services(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) From 9231d1804b165564c9c19d93384755b56147a605 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 16:48:43 +1000 Subject: [PATCH 06/22] Enhance JWT verification and admin access control; update tests for clarity and coverage --- auth/validator.py | 21 +++++++++------ main.py | 14 +++++----- schemas/__init__.py | 3 ++- schemas/service.py | 6 +++-- tests/test_main.py | 63 +++++++++++++++++++++++++++++++-------------- tests/test_user.py | 10 +++---- 6 files changed, 74 insertions(+), 43 deletions(-) diff --git a/auth/validator.py b/auth/validator.py index 1e46687c..d308d353 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -12,7 +12,7 @@ oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") -def verify_jwt(token: str) -> Dict: +def verify_jwt(token: str, require_admin: bool = False) -> Dict: settings = get_settings() try: jwks_url = f"https://{settings.auth0_domain}/.well-known/jwks.json" @@ -47,12 +47,13 @@ def verify_jwt(token: str) -> Dict: detail=f"Missing required claim: {roles_claim}" ) - roles = payload[roles_claim] - if not isinstance(roles, list) or not any("admin" in role.lower() for role in roles): - raise HTTPException( - status_code=403, - detail=f"Access denied: Insufficient permissions" - ) + if require_admin: + roles = payload[roles_claim] + if not isinstance(roles, list) or not any("admin" in role.lower() for role in roles): + raise HTTPException( + status_code=403, + detail=f"Access denied: Admin privileges required" + ) return payload @@ -63,4 +64,8 @@ def verify_jwt(token: str) -> Dict: def get_current_user(token: str = Depends(oauth2_scheme)): - return verify_jwt(token) \ No newline at end of file + return verify_jwt(token, require_admin=False) + + +def get_current_admin(token: str = Depends(oauth2_scheme)): + return verify_jwt(token, require_admin=True) \ No newline at end of file diff --git a/main.py b/main.py index 94c39656..a89c9b94 100644 --- a/main.py +++ b/main.py @@ -1,7 +1,7 @@ from fastapi import Depends, FastAPI from auth.management import get_management_token -from auth.validator import get_current_user +from auth.validator import get_current_admin from routers import user @@ -14,12 +14,12 @@ def public_route(): @app.get("/private") -def private_route(user=Depends(get_current_user)): - return { - "message": "Private route", - "user_claims": user, - "management_token": get_management_token() - } +def private_route(user=Depends(get_current_admin)): + return { + "message": "Private route", + "user_claims": user, + "management_token": get_management_token() + } app.include_router(user.router) diff --git a/schemas/__init__.py b/schemas/__init__.py index 68bf508c..201f81b1 100644 --- a/schemas/__init__.py +++ b/schemas/__init__.py @@ -1,4 +1,5 @@ -from .service import Service, Resource from .group import Group +from .service import Resource, Service + __all__ = ["Service", "Resource", "Group"] \ No newline at end of file diff --git a/schemas/service.py b/schemas/service.py index b9378416..f6649bba 100644 --- a/schemas/service.py +++ b/schemas/service.py @@ -1,6 +1,7 @@ -from pydantic import BaseModel, Field -from typing import Literal from datetime import datetime +from typing import Literal + +from pydantic import BaseModel, Field class Resource(BaseModel): @@ -8,6 +9,7 @@ class Resource(BaseModel): status: Literal["approved", "revoked", "pending"] id: str + class Service(BaseModel): name: str id: str diff --git a/tests/test_main.py b/tests/test_main.py index c0d83b2e..e1a60f1c 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -14,7 +14,7 @@ def test_public(): assert response.json() == {"message": "Public route"} -def test_private_valid_token(mocker): +def test_private_valid_admin_token(mocker): mocker.patch( "auth.config.get_settings", return_value=Settings( @@ -26,28 +26,37 @@ def test_private_valid_token(mocker): ) ) + mock_user_claims = { + "sub": "auth0|123456789", + "biocommons.org.au/roles": ["admin"] + } mocker.patch( "auth.validator.verify_jwt", - return_value={ - "sub": "auth0|123456789", - "biocommons.org.au/roles": ["acdc/indexd_admin"] - } + return_value=mock_user_claims ) + + mock_token = "mock_management_token" mocker.patch( - "auth.management.get_management_token", - return_value="mock_management_token" + "main.get_management_token", + return_value=mock_token ) + headers = {"Authorization": "Bearer valid_token"} response = client.get("/private", headers=headers) + assert response.status_code == 200 - assert response.json() == { - "message": "Private route", - "user_claims": { - "sub": "auth0|123456789", - "biocommons.org.au/roles": ["acdc/indexd_admin"] - }, - "management_token": "mock_management_token" - } + + response_data = response.json() + assert "message" in response_data + assert "user_claims" in response_data + assert "management_token" in response_data + + assert response_data["message"] == "Private route" + assert response_data["user_claims"] == mock_user_claims + + + assert isinstance(response_data["management_token"], str) + assert response_data["management_token"] == mock_token def test_private_missing_token(): @@ -59,7 +68,7 @@ def test_private_missing_token(): def test_private_invalid_token(mocker): mocker.patch( "auth.validator.verify_jwt", - side_effect=Exception("Invalid token: Error decoding token headers.") + side_effect=HTTPException(status_code=401, detail="Invalid token: Error decoding token headers.") ) headers = {"Authorization": "Bearer invalid_token"} @@ -68,15 +77,29 @@ def test_private_invalid_token(mocker): assert response.json() == {"detail": "Invalid token: Error decoding token headers."} -def test_private_insufficient_permissions(mocker): +def test_private_non_admin_token(mocker): + mocker.patch( + "auth.validator.verify_jwt", + side_effect=HTTPException( + status_code=403, + detail="Access denied: Admin privileges required" + ) + ) + headers = {"Authorization": "Bearer non_admin_token"} + response = client.get("/private", headers=headers) + assert response.status_code == 403 + assert response.json() == {"detail": "Access denied: Admin privileges required"} + + +def test_private_missing_roles(mocker): mocker.patch( "auth.validator.verify_jwt", side_effect=HTTPException( status_code=403, - detail="Access denied: Insufficient permissions" + detail="Missing required claim: biocommons.org.au/roles" ) ) - headers = {"Authorization": "Bearer insufficient_permissions_token"} + headers = {"Authorization": "Bearer token_without_roles"} response = client.get("/private", headers=headers) assert response.status_code == 403 - assert response.json() == {"detail": "Access denied: Insufficient permissions"} + assert response.json() == {"detail": "Missing required claim: biocommons.org.au/roles"} diff --git a/tests/test_user.py b/tests/test_user.py index 7ceab0dc..40430fde 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -1,9 +1,7 @@ import pytest -from fastapi import HTTPException from fastapi.testclient import TestClient from main import app -from routers.user import router client = TestClient(app) @@ -11,7 +9,10 @@ @pytest.fixture def mock_user(): - return {"sub": "auth0|123456789"} + return { + "sub": "auth0|123456789", + "biocommons.org.au/roles": ["user"] + } @pytest.fixture @@ -36,8 +37,6 @@ def mock_user_data(): @pytest.fixture(autouse=True) def mock_dependencies(mocker, mock_user): - mocker.patch("auth.validator.get_current_user", return_value=mock_user) - mocker.patch("auth.management.get_management_token", return_value="mock_token") mocker.patch( "auth.validator.verify_jwt", return_value={ @@ -45,6 +44,7 @@ def mock_dependencies(mocker, mock_user): "biocommons.org.au/roles": ["user"] } ) + mocker.patch("auth.management.get_management_token", return_value="mock_token") def test_get_all_services(mocker, mock_user_data): From a37f55d8479cd6621daee339556bad318d6e2fc2 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 17:16:55 +1000 Subject: [PATCH 07/22] Add authentication tests and error handling for service endpoints --- tests/test_user.py | 69 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/tests/test_user.py b/tests/test_user.py index 40430fde..60f836f0 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -1,5 +1,6 @@ import pytest from fastapi.testclient import TestClient +from fastapi import HTTPException from main import app @@ -47,6 +48,24 @@ def mock_dependencies(mocker, mock_user): mocker.patch("auth.management.get_management_token", return_value="mock_token") +# Authentication tests +@pytest.mark.parametrize("endpoint", [ + "/me/services", + "/me/services/approved", + "/me/services/pending", + "/me/resources", + "/me/resources/approved", + "/me/resources/pending", + "/me/all/pending" +]) +def test_endpoints_require_auth(endpoint): + """Test that all endpoints require authentication.""" + response = client.get(endpoint) + assert response.status_code == 401 + assert response.json() == {"detail": "Not authenticated"} + + +# Service endpoint tests def test_get_all_services(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -54,6 +73,7 @@ def test_get_all_services(mocker, mock_user_data): assert response.status_code == 200 assert response.json() == {"services": mock_user_data["app_metadata"]["services"]} + def test_get_approved_services(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -65,6 +85,7 @@ def test_get_approved_services(mocker, mock_user_data): ] } + def test_get_pending_services(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -76,6 +97,8 @@ def test_get_pending_services(mocker, mock_user_data): ] } + +# Resource endpoint tests def test_get_all_resources(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -88,6 +111,7 @@ def test_get_all_resources(mocker, mock_user_data): ] } + def test_get_approved_resources(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -99,6 +123,7 @@ def test_get_approved_resources(mocker, mock_user_data): ] } + def test_get_pending_resources(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -110,6 +135,8 @@ def test_get_pending_resources(mocker, mock_user_data): ] } + +# Combined endpoint tests def test_get_all_pending(mocker, mock_user_data): mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) headers = {"Authorization": "Bearer mock_token"} @@ -122,4 +149,44 @@ def test_get_all_pending(mocker, mock_user_data): "pending_resources": [ {"name": "Resource 2", "status": "pending", "id": "res2"} ] - } \ No newline at end of file + } + + +# Error handling tests +def test_get_services_unauthorized(): + response = client.get("/me/services") + assert response.status_code == 401 + assert response.json() == {"detail": "Not authenticated"} + + +def test_get_services_failed_fetch(mocker): + mocker.patch( + "routers.user.fetch_user_data", + side_effect=HTTPException(status_code=500, detail="Failed to fetch user data") + ) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/services", headers=headers) + assert response.status_code == 500 + assert response.json() == {"detail": "Failed to fetch user data"} + + +def test_get_services_empty_metadata(mocker): + mocker.patch( + "routers.user.fetch_user_data", + return_value={"app_metadata": {}} + ) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/services", headers=headers) + assert response.status_code == 200 + assert response.json() == {"services": []} + + +def test_get_services_no_metadata(mocker): + mocker.patch( + "routers.user.fetch_user_data", + return_value={} + ) + headers = {"Authorization": "Bearer mock_token"} + response = client.get("/me/services", headers=headers) + assert response.status_code == 200 + assert response.json() == {"services": []} \ No newline at end of file From 60951991ab71831e1ebebdaf4356209a7abf58cb Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 17:30:23 +1000 Subject: [PATCH 08/22] Add mock for HTTP client in authentication tests to simulate API responses --- tests/test_user.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_user.py b/tests/test_user.py index 60f836f0..5de7f56f 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -45,7 +45,22 @@ def mock_dependencies(mocker, mock_user): "biocommons.org.au/roles": ["user"] } ) + mocker.patch("auth.management.get_management_token", return_value="mock_token") + + async def mock_get(*args, **kwargs): + class MockResponse: + status_code = 200 + + def json(self): + return mock_user_data() + + def raise_for_status(self): + pass + + return MockResponse() + + mocker.patch("httpx.AsyncClient.get", side_effect=mock_get) # Authentication tests From 7ab5a7c59caf997ae8b01876d75fdc67bbe2d3a9 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 17:38:39 +1000 Subject: [PATCH 09/22] Refactor mock user data and response fixtures for consistency; update dependencies in tests --- tests/test_user.py | 62 ++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/tests/test_user.py b/tests/test_user.py index 5de7f56f..862bc4b5 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -11,56 +11,58 @@ @pytest.fixture def mock_user(): return { - "sub": "auth0|123456789", - "biocommons.org.au/roles": ["user"] + 'sub': 'auth0|123456789', + 'biocommons.org.au/roles': ['user'] } @pytest.fixture def mock_user_data(): return { - "app_metadata": { - "services": [ + 'app_metadata': { + 'services': [ { - "name": "Service 1", - "status": "approved", - "resources": [{"name": "Resource 1", "status": "approved", "id": "res1"}] + 'name': 'Service 1', + 'status': 'approved', + 'resources': [{'name': 'Resource 1', 'status': 'approved', 'id': 'res1'}] }, { - "name": "Service 2", - "status": "pending", - "resources": [{"name": "Resource 2", "status": "pending", "id": "res2"}] + 'name': 'Service 2', + 'status': 'pending', + 'resources': [{'name': 'Resource 2', 'status': 'pending', 'id': 'res2'}] } ] } } +@pytest.fixture +def mock_response(mock_user_data): + class MockResponse: + status_code = 200 + + def json(self): + return mock_user_data + + return MockResponse() + + @pytest.fixture(autouse=True) -def mock_dependencies(mocker, mock_user): +def mock_dependencies(mocker, mock_user, mock_response): mocker.patch( - "auth.validator.verify_jwt", - return_value={ - "sub": "auth0|123456789", - "biocommons.org.au/roles": ["user"] - } + 'auth.validator.verify_jwt', + return_value=mock_user ) - mocker.patch("auth.management.get_management_token", return_value="mock_token") - - async def mock_get(*args, **kwargs): - class MockResponse: - status_code = 200 - - def json(self): - return mock_user_data() - - def raise_for_status(self): - pass - - return MockResponse() + mocker.patch( + 'auth.management.get_management_token', + return_value='mock_token' + ) - mocker.patch("httpx.AsyncClient.get", side_effect=mock_get) + mocker.patch( + 'routers.user.fetch_user_data', + return_value=mock_response.json() + ) # Authentication tests From abc72a8069c4b83cf978526bc7283016ddcc682c Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 19:15:53 +1000 Subject: [PATCH 10/22] Update environment variables in GitHub Actions workflow for authentication and disable network calls in tests --- .github/workflows/python-tests.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 4b599662..f5f9dfa9 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -13,10 +13,11 @@ jobs: runs-on: ubuntu-latest env: - AUTH0_DOMAIN: "mock-domain" - AUTH0_AUDIENCE: "mock-audience" - AUTH0_MANAGEMENT_ID: "mock-id" - AUTH0_MANAGEMENT_SECRET: "mock-secret" + AUTH0_DOMAIN: mock-domain.auth0.com + AUTH0_AUDIENCE: https://mock-domain.auth0.com/api/v2/ + AUTH0_MANAGEMENT_ID: mock-management-id + AUTH0_MANAGEMENT_SECRET: mock-management-secret + PYTEST_DISABLE_NETWORK_CALLS: 1 steps: - name: Checkout code @@ -39,4 +40,4 @@ jobs: - name: Run tests run: | - uv run pytest + uv run pytest --disable-socket From cb1710edd3dc65d082b164ff23f6095a372547e3 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 24 Apr 2025 19:19:55 +1000 Subject: [PATCH 11/22] Refactor GitHub Actions workflow: update environment variables for clarity and remove network call disabling in tests --- .github/workflows/python-tests.yml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index f5f9dfa9..4b599662 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -13,11 +13,10 @@ jobs: runs-on: ubuntu-latest env: - AUTH0_DOMAIN: mock-domain.auth0.com - AUTH0_AUDIENCE: https://mock-domain.auth0.com/api/v2/ - AUTH0_MANAGEMENT_ID: mock-management-id - AUTH0_MANAGEMENT_SECRET: mock-management-secret - PYTEST_DISABLE_NETWORK_CALLS: 1 + AUTH0_DOMAIN: "mock-domain" + AUTH0_AUDIENCE: "mock-audience" + AUTH0_MANAGEMENT_ID: "mock-id" + AUTH0_MANAGEMENT_SECRET: "mock-secret" steps: - name: Checkout code @@ -40,4 +39,4 @@ jobs: - name: Run tests run: | - uv run pytest --disable-socket + uv run pytest From 3acad74da8d0be3870d764a9c86e0fcc9bbff5d3 Mon Sep 17 00:00:00 2001 From: Amanda Zhu Date: Fri, 9 May 2025 14:57:13 +1000 Subject: [PATCH 12/22] compily with PEP8 --- auth/validator.py | 22 ++++++++++--- routers/admin.py | 0 routers/user.py | 79 +++++++++++++++++++++------------------------ schemas/__init__.py | 2 +- 4 files changed, 54 insertions(+), 49 deletions(-) delete mode 100644 routers/admin.py diff --git a/auth/validator.py b/auth/validator.py index 46af7112..0647d9d6 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -1,4 +1,5 @@ import httpx + from fastapi import Depends, HTTPException from fastapi.security import OAuth2PasswordBearer from jose import jwt, jwk @@ -16,10 +17,15 @@ def verify_jwt(token: str) -> AccessTokenPayload: try: rsa_key = get_rsa_key(token) except JWTError as e: - raise HTTPException(status_code=401, detail=f"Invalid token: {e}") + raise HTTPException( + status_code=401, + detail=f"Invalid token: {e}" + ) + if rsa_key is None: raise HTTPException( - status_code=401, detail="Couldn't find a matching signing key." + status_code=401, + detail="Couldn't find a matching signing key." ) try: @@ -31,12 +37,16 @@ def verify_jwt(token: str) -> AccessTokenPayload: issuer=f"https://{settings.auth0_domain}/", ) except JWTError as e: - raise HTTPException(status_code=401, detail=f"Invalid token: {e}") + raise HTTPException( + status_code=401, + detail=f"Invalid token: {e}" + ) roles_claim = "biocommons.org.au/roles" if roles_claim not in payload: raise HTTPException( - status_code=403, detail=f"Missing required claim: {roles_claim}" + status_code=403, + detail=f"Missing required claim: {roles_claim}" ) roles = payload[roles_claim] @@ -44,7 +54,8 @@ def verify_jwt(token: str) -> AccessTokenPayload: "admin" in role.lower() for role in roles ): raise HTTPException( - status_code=403, detail="Access denied: Insufficient permissions" + status_code=403, + detail="Access denied: Insufficient permissions" ) return AccessTokenPayload(**payload) @@ -60,6 +71,7 @@ def get_rsa_key(token: str) -> jwk.RSAKey | None: for key in jwks["keys"]: if key["kid"] == unverified_header["kid"]: return jwk.construct(key) + return None diff --git a/routers/admin.py b/routers/admin.py deleted file mode 100644 index e69de29b..00000000 diff --git a/routers/user.py b/routers/user.py index 54103418..01638339 100644 --- a/routers/user.py +++ b/routers/user.py @@ -1,15 +1,15 @@ import httpx -from fastapi import APIRouter, Depends, HTTPException + from datetime import datetime, timezone +from fastapi import APIRouter, Depends, HTTPException from auth.config import get_settings from auth.management import get_management_token from auth.validator import get_current_user +from schemas.requests import ResourceRequest, ServiceRequest from schemas.user import User -from schemas.requests import ServiceRequest, ResourceRequest router = APIRouter() - settings = get_settings() @@ -20,13 +20,14 @@ async def fetch_user_data(user_id: str, token: str): response = await client.get(url, headers=headers) if response.status_code != 200: raise HTTPException( - status_code=response.status_code, detail="Failed to fetch user data" + status_code=response.status_code, + detail="Failed to fetch user data" ) return response.json() async def update_user_metadata(user_id: str, token: str, metadata: dict): - """Utility function to update user metadata in Auth0""" + """Utility function to update user metadata in Auth0.""" url = f"https://{settings.auth0_domain}/api/v2/users/{user_id}" headers = { "Authorization": f"Bearer {token}", @@ -34,12 +35,14 @@ async def update_user_metadata(user_id: str, token: str, metadata: dict): } async with httpx.AsyncClient() as client: response = await client.patch( - url, headers=headers, json={"app_metadata": metadata} + url, + headers=headers, + json={"app_metadata": metadata} ) if response.status_code != 200: raise HTTPException( status_code=response.status_code, - detail="Failed to update user metadata", + detail="Failed to update user metadata" ) return response.json() @@ -60,7 +63,8 @@ async def get_approved_services(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) approved_services = [ - service for service in services if service.get("status") == "approved" + service for service in services + if service.get("status") == "approved" ] return {"approved_services": approved_services} @@ -72,7 +76,8 @@ async def get_pending_services(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) pending_services = [ - service for service in services if service.get("status") == "pending" + service for service in services + if service.get("status") == "pending" ] return {"pending_services": pending_services} @@ -84,7 +89,9 @@ async def get_all_resources(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) resources = [ - resource for service in services for resource in service.get("resources", []) + resource + for service in services + for resource in service.get("resources", []) ] return {"resources": resources} @@ -126,7 +133,8 @@ async def get_all_pending(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) pending_services = [ - service for service in services if service.get("status") == "pending" + service for service in services + if service.get("status") == "pending" ] pending_resources = [ resource @@ -142,31 +150,27 @@ async def get_all_pending(user: User = Depends(get_current_user)): @router.post("/request/service") async def request_service( - service_request: ServiceRequest, user: User = Depends(get_current_user) + service_request: ServiceRequest, + user: User = Depends(get_current_user) ): - """Submit a request for a service""" - # Validate that the user_id matches the authenticated user + """Submit a request for a service.""" if user.access_token.sub != service_request.user_id: raise HTTPException( status_code=403, - detail="User ID in request does not match authenticated user", + detail="User ID in request does not match authenticated user" ) user_id = user.access_token.sub token = get_management_token() - - # Fetch current user data user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) - # Check if service request already exists if any(s.get("id") == service_request.id for s in services): raise HTTPException( status_code=400, - detail=f"Service request with ID {service_request.id} already exists", + detail=f"Service request with ID {service_request.id} already exists" ) - # Create new service request new_service = { "name": service_request.name, "id": service_request.id, @@ -176,15 +180,15 @@ async def request_service( "resources": [], } - # Append new service to existing services services.append(new_service) - - # Update user metadata updated_metadata = user_data.get("app_metadata", {}) updated_metadata["services"] = services await update_user_metadata(user_id, token, updated_metadata) - return {"message": "Service request submitted successfully", "service": new_service} + return { + "message": "Service request submitted successfully", + "service": new_service + } @router.post("/request/{service_id}/{resource_id}") @@ -192,69 +196,58 @@ async def request_resource( service_id: str, resource_id: str, resource_request: ResourceRequest, - user: User = Depends(get_current_user), + user: User = Depends(get_current_user) ): - """Submit a request for a resource within a service""" - # Validate that the user_id matches the authenticated user + """Submit a request for a resource within a service.""" if user.access_token.sub != resource_request.user_id: raise HTTPException( status_code=403, - detail="User ID in request does not match authenticated user", + detail="User ID in request does not match authenticated user" ) - # Validate service_id in path matches request body if service_id != resource_request.service_id: raise HTTPException( status_code=400, - detail="Service ID in path does not match request body", + detail="Service ID in path does not match request body" ) user_id = user.access_token.sub token = get_management_token() - - # Fetch current user data user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) - # Find the service service = next((s for s in services if s.get("id") == service_id), None) - if not service: raise HTTPException( - status_code=404, detail=f"Service with ID {service_id} not found" + status_code=404, + detail=f"Service with ID {service_id} not found" ) - # Check if service status is approved if service.get("status") != "approved": raise HTTPException( status_code=400, - detail="Cannot request resources for a service that is not approved", + detail="Cannot request resources for a service that is not approved" ) - # Check if resource request already exists if any(r.get("id") == resource_id for r in service.get("resources", [])): raise HTTPException( status_code=400, - detail=f"Resource request with ID {resource_id} already exists", + detail=f"Resource request with ID {resource_id} already exists" ) - # Create new resource request new_resource = { "name": resource_request.name, "id": resource_id, "status": "pending", } - # Initialize resources list if it doesn't exist if "resources" not in service: service["resources"] = [] - # Append new resource and update timestamp service["resources"].append(new_resource) service["last_updated"] = datetime.now(timezone.utc).isoformat() service["updated_by"] = user_id - # Update user metadata updated_metadata = user_data.get("app_metadata", {}) updated_metadata["services"] = services diff --git a/schemas/__init__.py b/schemas/__init__.py index 201f81b1..f6e9fa5a 100644 --- a/schemas/__init__.py +++ b/schemas/__init__.py @@ -2,4 +2,4 @@ from .service import Resource, Service -__all__ = ["Service", "Resource", "Group"] \ No newline at end of file +__all__ = ["Service", "Resource", "Group"] From 3531aa445f5b7ff9fde5f80eb350b9e36630ee5e Mon Sep 17 00:00:00 2001 From: Amanda Zhu Date: Fri, 9 May 2025 16:20:38 +1000 Subject: [PATCH 13/22] fix current tests and add more tests --- routers/user.py | 7 +- tests/test_user.py | 188 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 170 insertions(+), 25 deletions(-) diff --git a/routers/user.py b/routers/user.py index 01638339..1e9713e7 100644 --- a/routers/user.py +++ b/routers/user.py @@ -10,11 +10,12 @@ from schemas.user import User router = APIRouter() -settings = get_settings() +def get_settings_instance(): + return get_settings() async def fetch_user_data(user_id: str, token: str): - url = f"https://{settings.auth0_domain}/api/v2/users/{user_id}" + url = f"https://{get_settings_instance().auth0_domain}/api/v2/users/{user_id}" headers = {"Authorization": f"Bearer {token}"} async with httpx.AsyncClient() as client: response = await client.get(url, headers=headers) @@ -28,7 +29,7 @@ async def fetch_user_data(user_id: str, token: str): async def update_user_metadata(user_id: str, token: str, metadata: dict): """Utility function to update user metadata in Auth0.""" - url = f"https://{settings.auth0_domain}/api/v2/users/{user_id}" + url = f"https://{ get_settings_instance().auth0_domain}/api/v2/users/{user_id}" headers = { "Authorization": f"Bearer {token}", "Content-Type": "application/json", diff --git a/tests/test_user.py b/tests/test_user.py index 50b8162b..0456fe04 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -8,20 +8,19 @@ client = TestClient(app) -@pytest.fixture +@pytest.fixture(autouse=True) def mock_auth_settings(mocker): - """Fixture to mock auth settings""" - return mocker.patch( - "auth.config.get_settings", - return_value=Settings( - auth0_domain="mock-domain", - auth0_audience="mock-audience", - auth0_management_id="mock-id", - auth0_management_secret="mock-secret", - auth0_algorithms=["RS256"], - ), + """Fixture to mock auth settings globally""" + mock_settings = Settings( + auth0_domain="mock-domain.com", + auth0_audience="mock-audience", + auth0_management_id="mock-id", + auth0_management_secret="mock-secret", + auth0_algorithms=["RS256"], ) - + mocker.patch("auth.config.get_settings", return_value=mock_settings) + mocker.patch("auth.management.get_settings", return_value=mock_settings) + return mock_settings @pytest.fixture def mock_auth_token(mocker): @@ -81,6 +80,7 @@ def mock_user_data(): "/me/all/pending", ], ) + def test_endpoints_require_auth(endpoint): """Test that all endpoints require authentication""" response = client.get(endpoint) @@ -97,6 +97,11 @@ def test_get_all_services( return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), ) + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + response = client.get("/me/services", headers=auth_headers) assert response.status_code == 200 assert response.json() == {"services": mock_user_data["app_metadata"]["services"]} @@ -106,6 +111,11 @@ def test_get_approved_services( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test getting approved services""" + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + mocker.patch( "httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), @@ -114,28 +124,32 @@ def test_get_approved_services( response = client.get("/me/services/approved", headers=auth_headers) assert response.status_code == 200 approved_services = [ - s - for s in mock_user_data["app_metadata"]["services"] - if s["status"] == "approved" + s for s in mock_user_data["app_metadata"]["services"] if s["status"] == "approved" ] assert response.json() == {"approved_services": approved_services} - def test_get_pending_services( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test getting pending services""" + + # Patch the Auth0 token request + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + + # Patch the user metadata fetch mocker.patch( "httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), ) response = client.get("/me/services/pending", headers=auth_headers) + assert response.status_code == 200 pending_services = [ - s - for s in mock_user_data["app_metadata"]["services"] - if s["status"] == "pending" + s for s in mock_user_data["app_metadata"]["services"] if s["status"] == "pending" ] assert response.json() == {"pending_services": pending_services} @@ -144,6 +158,14 @@ def test_get_all_resources( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test getting all resources""" + + # Patch token fetch + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + + # Patch user metadata fetch mocker.patch( "httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), @@ -163,11 +185,17 @@ def test_get_services_failed_fetch( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of failed API calls""" + + # Patch token acquisition + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + + # Patch failed metadata fetch mocker.patch( "httpx.AsyncClient.get", - return_value=mocker.Mock( - status_code=403, json=lambda: {"error": "Unauthorized"} - ), + return_value=mocker.Mock(status_code=403, json=lambda: {"error": "Unauthorized"}), ) response = client.get("/me/services", headers=auth_headers) @@ -179,6 +207,12 @@ def test_get_services_empty_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of empty metadata""" + # Patch token acquisition + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + mocker.patch( "httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: {"app_metadata": {}}), @@ -193,6 +227,11 @@ def test_get_services_no_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of missing metadata""" + mocker.patch( + "httpx.post", + return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + ) + mocker.patch( "httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: {}), @@ -201,3 +240,108 @@ def test_get_services_no_metadata( response = client.get("/me/services", headers=auth_headers) assert response.status_code == 200 assert response.json() == {"services": []} + +def test_request_service_success( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + from datetime import datetime + from schemas.requests import ServiceRequest + + mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) + mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) + mocker.patch("httpx.AsyncClient.patch", return_value=mocker.Mock(status_code=200, json=lambda: {})) + + new_service = { + "name": "New Service", + "id": "service3", + "user_id": mock_auth_token.sub, + } + + response = client.post("/request/service", json=new_service, headers=auth_headers) + assert response.status_code == 200 + assert response.json()["message"] == "Service request submitted successfully" + assert response.json()["service"]["id"] == "service3" + +def test_request_service_duplicate( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) + mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) + + existing_service = { + "name": "Service 1", + "id": "service1", + "user_id": mock_auth_token.sub, + } + + response = client.post("/request/service", json=existing_service, headers=auth_headers) + assert response.status_code == 400 + assert response.json()["detail"] == "Service request with ID service1 already exists" + +def test_request_service_user_mismatch( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + request_payload = { + "name": "Service Mismatch", + "id": "svc-mismatch", + "user_id": "auth0|WRONG_USER", + } + + response = client.post("/request/service", json=request_payload, headers=auth_headers) + assert response.status_code == 403 + assert response.json()["detail"] == "User ID in request does not match authenticated user" + +def test_request_resource_success( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + approved_service = mock_user_data["app_metadata"]["services"][0] + approved_service["resources"] = [] + + mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) + mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) + mocker.patch("httpx.AsyncClient.patch", return_value=mocker.Mock(status_code=200, json=lambda: {})) + + request_payload = { + "name": "New Resource", + "id": "resource-new", + "user_id": mock_auth_token.sub, + "service_id": "service1", + } + + response = client.post("/request/service1/resource-new", json=request_payload, headers=auth_headers) + assert response.status_code == 200 + assert response.json()["resource"]["id"] == "resource-new" + +def test_request_resource_user_mismatch( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + request_payload = { + "name": "Invalid Resource", + "id": "res-invalid", + "user_id": "wrong-user", + "service_id": "service1", + } + + response = client.post("/request/service1/res-invalid", json=request_payload, headers=auth_headers) + assert response.status_code == 403 + assert response.json()["detail"] == "User ID in request does not match authenticated user" + +def test_request_resource_non_approved_service( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + # Set service2 to be requested + service = mock_user_data["app_metadata"]["services"][1] + + mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) + mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) + + request_payload = { + "name": "Blocked Resource", + "id": "blocked-resource", + "user_id": mock_auth_token.sub, + "service_id": "service2", + } + + response = client.post("/request/service2/blocked-resource", json=request_payload, headers=auth_headers) + assert response.status_code == 400 + assert response.json()["detail"] == "Cannot request resources for a service that is not approved" From 384203f5e0eeddb3480357a15e17644f62a9bf59 Mon Sep 17 00:00:00 2001 From: Amanda Zhu Date: Fri, 9 May 2025 16:26:44 +1000 Subject: [PATCH 14/22] add nit fixes --- auth/validator.py | 3 +-- main.py | 5 ++--- routers/user.py | 5 ++--- schemas/service.py | 3 +-- tests/test_main.py | 3 +-- tests/test_user.py | 2 +- 6 files changed, 8 insertions(+), 13 deletions(-) diff --git a/auth/validator.py b/auth/validator.py index 0647d9d6..89f8aee0 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -1,11 +1,10 @@ import httpx +from auth.config import get_settings from fastapi import Depends, HTTPException from fastapi.security import OAuth2PasswordBearer from jose import jwt, jwk from jose.exceptions import JWTError - -from auth.config import get_settings from schemas.tokens import AccessTokenPayload from schemas.user import User diff --git a/main.py b/main.py index 51405440..7becfafe 100644 --- a/main.py +++ b/main.py @@ -1,7 +1,6 @@ -from fastapi import Depends, FastAPI - from auth.management import get_management_token from auth.validator import get_current_user +from fastapi import Depends, FastAPI from routers import user app = FastAPI() @@ -21,4 +20,4 @@ def private_route(user=Depends(get_current_user)): } -app.include_router(user.router) \ No newline at end of file +app.include_router(user.router) diff --git a/routers/user.py b/routers/user.py index 1e9713e7..2c2969f4 100644 --- a/routers/user.py +++ b/routers/user.py @@ -1,11 +1,10 @@ import httpx -from datetime import datetime, timezone -from fastapi import APIRouter, Depends, HTTPException - from auth.config import get_settings from auth.management import get_management_token from auth.validator import get_current_user +from datetime import datetime, timezone +from fastapi import APIRouter, Depends, HTTPException from schemas.requests import ResourceRequest, ServiceRequest from schemas.user import User diff --git a/schemas/service.py b/schemas/service.py index f6649bba..4e699eaf 100644 --- a/schemas/service.py +++ b/schemas/service.py @@ -1,7 +1,6 @@ from datetime import datetime -from typing import Literal - from pydantic import BaseModel, Field +from typing import Literal class Resource(BaseModel): diff --git a/tests/test_main.py b/tests/test_main.py index ebb8e044..6491a4c1 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,9 +1,8 @@ import pytest +from auth.config import Settings from fastapi import HTTPException from fastapi.testclient import TestClient - -from auth.config import Settings from main import app from tests.datagen import AccessTokenPayloadFactory diff --git a/tests/test_user.py b/tests/test_user.py index 0456fe04..429490a6 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -1,7 +1,7 @@ import pytest -from fastapi.testclient import TestClient from auth.config import Settings +from fastapi.testclient import TestClient from main import app from tests.datagen import AccessTokenPayloadFactory From 1c742210cb1c2fefdc536225d9363b3848e61833 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 12 May 2025 14:51:31 +1000 Subject: [PATCH 15/22] refactor: clean up code formatting and improve readability across multiple files --- .github/workflows/build-ecr.yml | 2 +- .github/workflows/deploy.yml | 2 +- README.md | 4 +- auth/config.py | 2 +- auth/management.py | 12 +-- auth/validator.py | 21 ++--- main.py | 15 +--- routers/user.py | 57 +++++-------- schemas/group.py | 2 +- schemas/tokens.py | 3 +- schemas/user.py | 2 +- tests/datagen.py | 6 +- tests/test_main.py | 74 +--------------- tests/test_user.py | 146 +++++++++++++++++++++++++------- 14 files changed, 164 insertions(+), 184 deletions(-) diff --git a/.github/workflows/build-ecr.yml b/.github/workflows/build-ecr.yml index ce78c081..ecf78dc5 100644 --- a/.github/workflows/build-ecr.yml +++ b/.github/workflows/build-ecr.yml @@ -7,7 +7,7 @@ on: permissions: contents: read - id-token: write # Required for OIDC + id-token: write # Required for OIDC actions: read env: diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index c1f27b05..b3e72c7b 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -58,4 +58,4 @@ jobs: EOF - name: CDK Deploy - run: cdk deploy --require-approval never \ No newline at end of file + run: cdk deploy --require-approval never diff --git a/README.md b/README.md index 20a22204..0da9856e 100644 --- a/README.md +++ b/README.md @@ -53,8 +53,8 @@ uv run pytest # Deployment -Currently the service is deployed to AWS via the CDK scripts in `deploy/`, +Currently the service is deployed to AWS via the CDK scripts in `deploy/`, and updated on each commit to `main`. Secrets/configuration variables for the deployment are stored in the -GitHub Secrets for the repository. \ No newline at end of file +GitHub Secrets for the repository. diff --git a/auth/config.py b/auth/config.py index e8e92374..85b47881 100644 --- a/auth/config.py +++ b/auth/config.py @@ -14,4 +14,4 @@ class Settings(BaseSettings): @lru_cache() def get_settings(): - return Settings() \ No newline at end of file + return Settings() diff --git a/auth/management.py b/auth/management.py index 1527ce6e..0584ed75 100644 --- a/auth/management.py +++ b/auth/management.py @@ -5,13 +5,13 @@ def get_management_token(): settings = get_settings() - url = f'https://{settings.auth0_domain}/oauth/token' + url = f"https://{settings.auth0_domain}/oauth/token" payload = { - 'grant_type': 'client_credentials', - 'client_id': settings.auth0_management_id, - 'client_secret': settings.auth0_management_secret, - 'audience': f'https://{settings.auth0_domain}/api/v2/' + "grant_type": "client_credentials", + "client_id": settings.auth0_management_id, + "client_secret": settings.auth0_management_secret, + "audience": f"https://{settings.auth0_domain}/api/v2/", } response = httpx.post(url, json=payload) response.raise_for_status() - return response.json()['access_token'] + return response.json()["access_token"] diff --git a/auth/validator.py b/auth/validator.py index 89f8aee0..b625a719 100644 --- a/auth/validator.py +++ b/auth/validator.py @@ -16,15 +16,11 @@ def verify_jwt(token: str) -> AccessTokenPayload: try: rsa_key = get_rsa_key(token) except JWTError as e: - raise HTTPException( - status_code=401, - detail=f"Invalid token: {e}" - ) + raise HTTPException(status_code=401, detail=f"Invalid token: {e}") if rsa_key is None: raise HTTPException( - status_code=401, - detail="Couldn't find a matching signing key." + status_code=401, detail="Couldn't find a matching signing key." ) try: @@ -36,16 +32,12 @@ def verify_jwt(token: str) -> AccessTokenPayload: issuer=f"https://{settings.auth0_domain}/", ) except JWTError as e: - raise HTTPException( - status_code=401, - detail=f"Invalid token: {e}" - ) + raise HTTPException(status_code=401, detail=f"Invalid token: {e}") roles_claim = "biocommons.org.au/roles" if roles_claim not in payload: raise HTTPException( - status_code=403, - detail=f"Missing required claim: {roles_claim}" + status_code=403, detail=f"Missing required claim: {roles_claim}" ) roles = payload[roles_claim] @@ -53,14 +45,13 @@ def verify_jwt(token: str) -> AccessTokenPayload: "admin" in role.lower() for role in roles ): raise HTTPException( - status_code=403, - detail="Access denied: Insufficient permissions" + status_code=403, detail="Access denied: Insufficient permissions" ) return AccessTokenPayload(**payload) -def get_rsa_key(token: str) -> jwk.RSAKey | None: +def get_rsa_key(token: str) -> jwk.RSAKey | None: # type: ignore settings = get_settings() jwks_url = f"https://{settings.auth0_domain}/.well-known/jwks.json" response = httpx.get(jwks_url) diff --git a/main.py b/main.py index 7becfafe..d8e102f9 100644 --- a/main.py +++ b/main.py @@ -1,6 +1,4 @@ -from auth.management import get_management_token -from auth.validator import get_current_user -from fastapi import Depends, FastAPI +from fastapi import FastAPI from routers import user app = FastAPI() @@ -8,16 +6,7 @@ @app.get("/") def public_route(): - return {"message": "Public route"} - - -@app.get("/private") -def private_route(user=Depends(get_current_user)): - return { - "message": "Private route", - "user_claims": user, - "management_token": get_management_token() - } + return {"message": "AAI Backend API"} app.include_router(user.router) diff --git a/routers/user.py b/routers/user.py index 2c2969f4..83f8b3e6 100644 --- a/routers/user.py +++ b/routers/user.py @@ -6,43 +6,39 @@ from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException from schemas.requests import ResourceRequest, ServiceRequest +from schemas.service import Service from schemas.user import User router = APIRouter() -def get_settings_instance(): - return get_settings() async def fetch_user_data(user_id: str, token: str): - url = f"https://{get_settings_instance().auth0_domain}/api/v2/users/{user_id}" + url = f"https://{get_settings().auth0_domain}/api/v2/users/{user_id}" headers = {"Authorization": f"Bearer {token}"} async with httpx.AsyncClient() as client: response = await client.get(url, headers=headers) if response.status_code != 200: raise HTTPException( - status_code=response.status_code, - detail="Failed to fetch user data" + status_code=response.status_code, detail="Failed to fetch user data" ) return response.json() async def update_user_metadata(user_id: str, token: str, metadata: dict): """Utility function to update user metadata in Auth0.""" - url = f"https://{ get_settings_instance().auth0_domain}/api/v2/users/{user_id}" + url = f"https://{get_settings().auth0_domain}/api/v2/users/{user_id}" headers = { "Authorization": f"Bearer {token}", "Content-Type": "application/json", } async with httpx.AsyncClient() as client: response = await client.patch( - url, - headers=headers, - json={"app_metadata": metadata} + url, headers=headers, json={"app_metadata": metadata} ) if response.status_code != 200: raise HTTPException( status_code=response.status_code, - detail="Failed to update user metadata" + detail="Failed to update user metadata", ) return response.json() @@ -63,8 +59,7 @@ async def get_approved_services(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) approved_services = [ - service for service in services - if service.get("status") == "approved" + service for service in services if service.get("status") == "approved" ] return {"approved_services": approved_services} @@ -76,8 +71,7 @@ async def get_pending_services(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) pending_services = [ - service for service in services - if service.get("status") == "pending" + service for service in services if service.get("status") == "pending" ] return {"pending_services": pending_services} @@ -89,9 +83,7 @@ async def get_all_resources(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) resources = [ - resource - for service in services - for resource in service.get("resources", []) + resource for service in services for resource in service.get("resources", []) ] return {"resources": resources} @@ -133,8 +125,7 @@ async def get_all_pending(user: User = Depends(get_current_user)): user_data = await fetch_user_data(user_id, token) services = user_data.get("app_metadata", {}).get("services", []) pending_services = [ - service for service in services - if service.get("status") == "pending" + service for service in services if service.get("status") == "pending" ] pending_resources = [ resource @@ -150,14 +141,13 @@ async def get_all_pending(user: User = Depends(get_current_user)): @router.post("/request/service") async def request_service( - service_request: ServiceRequest, - user: User = Depends(get_current_user) + service_request: ServiceRequest, user: User = Depends(get_current_user) ): """Submit a request for a service.""" if user.access_token.sub != service_request.user_id: raise HTTPException( status_code=403, - detail="User ID in request does not match authenticated user" + detail="User ID in request does not match authenticated user", ) user_id = user.access_token.sub @@ -168,10 +158,10 @@ async def request_service( if any(s.get("id") == service_request.id for s in services): raise HTTPException( status_code=400, - detail=f"Service request with ID {service_request.id} already exists" + detail=f"Service request with ID {service_request.id} already exists", ) - new_service = { + new_service: Service = { "name": service_request.name, "id": service_request.id, "status": "pending", @@ -185,10 +175,7 @@ async def request_service( updated_metadata["services"] = services await update_user_metadata(user_id, token, updated_metadata) - return { - "message": "Service request submitted successfully", - "service": new_service - } + return {"message": "Service request submitted successfully", "service": new_service} @router.post("/request/{service_id}/{resource_id}") @@ -196,19 +183,18 @@ async def request_resource( service_id: str, resource_id: str, resource_request: ResourceRequest, - user: User = Depends(get_current_user) + user: User = Depends(get_current_user), ): """Submit a request for a resource within a service.""" if user.access_token.sub != resource_request.user_id: raise HTTPException( status_code=403, - detail="User ID in request does not match authenticated user" + detail="User ID in request does not match authenticated user", ) if service_id != resource_request.service_id: raise HTTPException( - status_code=400, - detail="Service ID in path does not match request body" + status_code=400, detail="Service ID in path does not match request body" ) user_id = user.access_token.sub @@ -219,20 +205,19 @@ async def request_resource( service = next((s for s in services if s.get("id") == service_id), None) if not service: raise HTTPException( - status_code=404, - detail=f"Service with ID {service_id} not found" + status_code=404, detail=f"Service with ID {service_id} not found" ) if service.get("status") != "approved": raise HTTPException( status_code=400, - detail="Cannot request resources for a service that is not approved" + detail="Cannot request resources for a service that is not approved", ) if any(r.get("id") == resource_id for r in service.get("resources", [])): raise HTTPException( status_code=400, - detail=f"Resource request with ID {resource_id} already exists" + detail=f"Resource request with ID {resource_id} already exists", ) new_resource = { diff --git a/schemas/group.py b/schemas/group.py index 0085fdb3..b82385c0 100644 --- a/schemas/group.py +++ b/schemas/group.py @@ -3,4 +3,4 @@ class Group(BaseModel): name: str - id: str \ No newline at end of file + id: str diff --git a/schemas/tokens.py b/schemas/tokens.py index ea88d00b..a6ba2246 100644 --- a/schemas/tokens.py +++ b/schemas/tokens.py @@ -6,9 +6,10 @@ class AccessTokenPayload(BaseModel): """ Schema for the access token payload. """ + biocommons_roles: list[str] = Field( alias="biocommons.org.au/roles", - description="BioCommons-specific roles assigned to the user" + description="BioCommons-specific roles assigned to the user", ) email: Optional[str] = Field(None, description="Email address") iss: str = Field(description="Issuer identifier") diff --git a/schemas/user.py b/schemas/user.py index 7aef6284..b9f7b8fb 100644 --- a/schemas/user.py +++ b/schemas/user.py @@ -8,6 +8,7 @@ class User(BaseModel): permissions checks here, instead of doing individual checks in different places. """ + access_token: AccessTokenPayload def is_admin(self) -> bool: @@ -20,4 +21,3 @@ def is_admin(self) -> bool: if "admin" in role.lower(): return True return False - diff --git a/tests/datagen.py b/tests/datagen.py index 52af0de9..3925087b 100644 --- a/tests/datagen.py +++ b/tests/datagen.py @@ -4,9 +4,7 @@ from schemas.user import User -class AccessTokenPayloadFactory(ModelFactory[AccessTokenPayload]): - ... +class AccessTokenPayloadFactory(ModelFactory[AccessTokenPayload]): ... -class UserFactory(ModelFactory[User]): - ... \ No newline at end of file +class UserFactory(ModelFactory[User]): ... diff --git a/tests/test_main.py b/tests/test_main.py index 6491a4c1..a5918cd3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,80 +1,10 @@ -import pytest - -from auth.config import Settings -from fastapi import HTTPException from fastapi.testclient import TestClient from main import app -from tests.datagen import AccessTokenPayloadFactory client = TestClient(app) -def test_public(): +def test_root(): response = client.get("/") assert response.status_code == 200 - assert response.json() == {"message": "Public route"} - - -def test_private_valid_token(mocker): - mocker.patch( - "auth.config.get_settings", - return_value=Settings( - auth0_domain="mock-domain", - auth0_audience="mock-audience", - auth0_management_id="mock-id", - auth0_management_secret="mock-secret", - auth0_algorithms=["RS256"], - ), - ) - token = AccessTokenPayloadFactory.build( - sub="auth0|123456789", - biocommons_roles=["acdc/indexd_admin"], - ) - mocker.patch( - "auth.validator.verify_jwt", - return_value=token, - ) - mocker.patch("main.get_management_token", return_value="mock_management_token") - headers = {"Authorization": "Bearer valid_token"} - response = client.get("/private", headers=headers) - assert response.status_code == 200 - assert response.json() == { - "message": "Private route", - "user_claims": {"access_token": token.model_dump(by_alias=True)}, - "management_token": "mock_management_token", - } - - -def test_private_missing_token(): - response = client.get("/private") - assert response.status_code == 401 - assert response.json() == {"detail": "Not authenticated"} - - -def test_private_invalid_token(mocker): - mocker.patch("httpx.get", return_value=mocker.Mock(json=lambda: {"keys": []})) - mocker.patch( - "auth.validator.verify_jwt", - side_effect=HTTPException( - status_code=401, detail="Invalid token: Error decoding token headers." - ), - ) - - headers = {"Authorization": "Bearer invalid_token"} - response = client.get("/private", headers=headers) - assert response.status_code == 401 - assert response.json() == {"detail": "Invalid token: Error decoding token headers."} - - -@pytest.mark.parametrize("roles", [[], ["user_only"]]) -def test_private_insufficient_permissions(roles, mocker): - """ - Test that we return an error when a user has no admin roles - """ - # Bypass finding the signing key - mocker.patch("auth.validator.get_rsa_key", return_value={"kid": "dummy"}) - mocker.patch("jose.jwt.decode", return_value={"biocommons.org.au/roles": roles}) - headers = {"Authorization": "Bearer insufficient_permissions_token"} - response = client.get("/private", headers=headers) - assert response.status_code == 403 - assert response.json() == {"detail": "Access denied: Insufficient permissions"} + assert response.json() == {"message": "AAI Backend API"} diff --git a/tests/test_user.py b/tests/test_user.py index 429490a6..173791c8 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -22,6 +22,7 @@ def mock_auth_settings(mocker): mocker.patch("auth.management.get_settings", return_value=mock_settings) return mock_settings + @pytest.fixture def mock_auth_token(mocker): """Fixture to mock authentication token""" @@ -80,7 +81,6 @@ def mock_user_data(): "/me/all/pending", ], ) - def test_endpoints_require_auth(endpoint): """Test that all endpoints require authentication""" response = client.get(endpoint) @@ -99,7 +99,9 @@ def test_get_all_services( mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) response = client.get("/me/services", headers=auth_headers) @@ -113,7 +115,9 @@ def test_get_approved_services( """Test getting approved services""" mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) mocker.patch( @@ -124,10 +128,13 @@ def test_get_approved_services( response = client.get("/me/services/approved", headers=auth_headers) assert response.status_code == 200 approved_services = [ - s for s in mock_user_data["app_metadata"]["services"] if s["status"] == "approved" + s + for s in mock_user_data["app_metadata"]["services"] + if s["status"] == "approved" ] assert response.json() == {"approved_services": approved_services} + def test_get_pending_services( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -136,7 +143,9 @@ def test_get_pending_services( # Patch the Auth0 token request mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) # Patch the user metadata fetch @@ -149,7 +158,9 @@ def test_get_pending_services( assert response.status_code == 200 pending_services = [ - s for s in mock_user_data["app_metadata"]["services"] if s["status"] == "pending" + s + for s in mock_user_data["app_metadata"]["services"] + if s["status"] == "pending" ] assert response.json() == {"pending_services": pending_services} @@ -162,7 +173,9 @@ def test_get_all_resources( # Patch token fetch mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) # Patch user metadata fetch @@ -189,13 +202,17 @@ def test_get_services_failed_fetch( # Patch token acquisition mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) # Patch failed metadata fetch mocker.patch( "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=403, json=lambda: {"error": "Unauthorized"}), + return_value=mocker.Mock( + status_code=403, json=lambda: {"error": "Unauthorized"} + ), ) response = client.get("/me/services", headers=auth_headers) @@ -210,7 +227,9 @@ def test_get_services_empty_metadata( # Patch token acquisition mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) mocker.patch( @@ -229,7 +248,9 @@ def test_get_services_no_metadata( """Test handling of missing metadata""" mocker.patch( "httpx.post", - return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"}), + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), ) mocker.patch( @@ -241,15 +262,27 @@ def test_get_services_no_metadata( assert response.status_code == 200 assert response.json() == {"services": []} + def test_request_service_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): from datetime import datetime from schemas.requests import ServiceRequest - mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) - mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) - mocker.patch("httpx.AsyncClient.patch", return_value=mocker.Mock(status_code=200, json=lambda: {})) + mocker.patch( + "httpx.post", + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), + ) + mocker.patch( + "httpx.AsyncClient.get", + return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + ) + mocker.patch( + "httpx.AsyncClient.patch", + return_value=mocker.Mock(status_code=200, json=lambda: {}), + ) new_service = { "name": "New Service", @@ -262,11 +295,20 @@ def test_request_service_success( assert response.json()["message"] == "Service request submitted successfully" assert response.json()["service"]["id"] == "service3" + def test_request_service_duplicate( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): - mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) - mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) + mocker.patch( + "httpx.post", + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), + ) + mocker.patch( + "httpx.AsyncClient.get", + return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + ) existing_service = { "name": "Service 1", @@ -274,9 +316,14 @@ def test_request_service_duplicate( "user_id": mock_auth_token.sub, } - response = client.post("/request/service", json=existing_service, headers=auth_headers) + response = client.post( + "/request/service", json=existing_service, headers=auth_headers + ) assert response.status_code == 400 - assert response.json()["detail"] == "Service request with ID service1 already exists" + assert ( + response.json()["detail"] == "Service request with ID service1 already exists" + ) + def test_request_service_user_mismatch( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker @@ -287,9 +334,15 @@ def test_request_service_user_mismatch( "user_id": "auth0|WRONG_USER", } - response = client.post("/request/service", json=request_payload, headers=auth_headers) + response = client.post( + "/request/service", json=request_payload, headers=auth_headers + ) assert response.status_code == 403 - assert response.json()["detail"] == "User ID in request does not match authenticated user" + assert ( + response.json()["detail"] + == "User ID in request does not match authenticated user" + ) + def test_request_resource_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker @@ -297,9 +350,20 @@ def test_request_resource_success( approved_service = mock_user_data["app_metadata"]["services"][0] approved_service["resources"] = [] - mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) - mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) - mocker.patch("httpx.AsyncClient.patch", return_value=mocker.Mock(status_code=200, json=lambda: {})) + mocker.patch( + "httpx.post", + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), + ) + mocker.patch( + "httpx.AsyncClient.get", + return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + ) + mocker.patch( + "httpx.AsyncClient.patch", + return_value=mocker.Mock(status_code=200, json=lambda: {}), + ) request_payload = { "name": "New Resource", @@ -308,10 +372,13 @@ def test_request_resource_success( "service_id": "service1", } - response = client.post("/request/service1/resource-new", json=request_payload, headers=auth_headers) + response = client.post( + "/request/service1/resource-new", json=request_payload, headers=auth_headers + ) assert response.status_code == 200 assert response.json()["resource"]["id"] == "resource-new" + def test_request_resource_user_mismatch( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -322,9 +389,15 @@ def test_request_resource_user_mismatch( "service_id": "service1", } - response = client.post("/request/service1/res-invalid", json=request_payload, headers=auth_headers) + response = client.post( + "/request/service1/res-invalid", json=request_payload, headers=auth_headers + ) assert response.status_code == 403 - assert response.json()["detail"] == "User ID in request does not match authenticated user" + assert ( + response.json()["detail"] + == "User ID in request does not match authenticated user" + ) + def test_request_resource_non_approved_service( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker @@ -332,8 +405,16 @@ def test_request_resource_non_approved_service( # Set service2 to be requested service = mock_user_data["app_metadata"]["services"][1] - mocker.patch("httpx.post", return_value=mocker.Mock(status_code=200, json=lambda: {"access_token": "test-token"})) - mocker.patch("httpx.AsyncClient.get", return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data)) + mocker.patch( + "httpx.post", + return_value=mocker.Mock( + status_code=200, json=lambda: {"access_token": "test-token"} + ), + ) + mocker.patch( + "httpx.AsyncClient.get", + return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + ) request_payload = { "name": "Blocked Resource", @@ -342,6 +423,11 @@ def test_request_resource_non_approved_service( "service_id": "service2", } - response = client.post("/request/service2/blocked-resource", json=request_payload, headers=auth_headers) + response = client.post( + "/request/service2/blocked-resource", json=request_payload, headers=auth_headers + ) assert response.status_code == 400 - assert response.json()["detail"] == "Cannot request resources for a service that is not approved" + assert ( + response.json()["detail"] + == "Cannot request resources for a service that is not approved" + ) From 07a09873f0974992334f7d4a1346bde848017801 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 12 May 2025 15:28:56 +1000 Subject: [PATCH 16/22] fix(tests): update service and resource tests to use new mock functions --- tests/test_user.py | 116 +++++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 57 deletions(-) diff --git a/tests/test_user.py b/tests/test_user.py index 173791c8..d0e85188 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -93,15 +93,11 @@ def test_get_all_services( ): """Test getting all services""" mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.get_management_token", return_value="mock_management_token" ) response = client.get("/me/services", headers=auth_headers) @@ -114,15 +110,11 @@ def test_get_approved_services( ): """Test getting approved services""" mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.get_management_token", return_value="mock_management_token" ) response = client.get("/me/services/approved", headers=auth_headers) @@ -194,73 +186,83 @@ def test_get_all_resources( assert response.json() == {"resources": all_resources} -def test_get_services_failed_fetch( - mock_auth_settings, mock_auth_token, auth_headers, mocker +def test_get_approved_resources( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): - """Test handling of failed API calls""" - - # Patch token acquisition + """Test getting approved resources""" mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - - # Patch failed metadata fetch mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock( - status_code=403, json=lambda: {"error": "Unauthorized"} - ), + "routers.user.get_management_token", return_value="mock_management_token" ) - response = client.get("/me/services", headers=auth_headers) - assert response.status_code == 403 - assert response.json() == {"detail": "Failed to fetch user data"} + response = client.get("/me/resources/approved", headers=auth_headers) + assert response.status_code == 200 + approved_resources = [ + resource + for service in mock_user_data["app_metadata"]["services"] + for resource in service["resources"] + if resource["status"] == "approved" + ] + assert response.json() == {"approved_resources": approved_resources} -def test_get_services_empty_metadata( - mock_auth_settings, mock_auth_token, auth_headers, mocker +def test_get_pending_resources( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): - """Test handling of empty metadata""" - # Patch token acquisition + """Test getting pending resources""" mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: {"app_metadata": {}}), + "routers.user.get_management_token", return_value="mock_management_token" ) - response = client.get("/me/services", headers=auth_headers) + response = client.get("/me/resources/pending", headers=auth_headers) assert response.status_code == 200 - assert response.json() == {"services": []} + pending_resources = [ + resource + for service in mock_user_data["app_metadata"]["services"] + for resource in service["resources"] + if resource["status"] == "pending" + ] + assert response.json() == {"pending_resources": pending_resources} -def test_get_services_no_metadata( - mock_auth_settings, mock_auth_token, auth_headers, mocker +def test_get_all_pending( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): - """Test handling of missing metadata""" + """Test getting all pending items""" mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: {}), + "routers.user.get_management_token", return_value="mock_management_token" ) - response = client.get("/me/services", headers=auth_headers) + response = client.get("/me/all/pending", headers=auth_headers) assert response.status_code == 200 - assert response.json() == {"services": []} + + pending_services = [ + s + for s in mock_user_data["app_metadata"]["services"] + if s["status"] == "pending" + ] + pending_resources = [ + resource + for service in mock_user_data["app_metadata"]["services"] + for resource in service["resources"] + if resource["status"] == "pending" + ] + + assert response.json() == { + "pending_services": pending_services, + "pending_resources": pending_resources, + } def test_request_service_success( From 86e71d51e31943c14fa846b5703939d62d483553 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 12 May 2025 15:50:47 +1000 Subject: [PATCH 17/22] fix(tests): enhance service and resource tests with additional scenarios and error handling --- tests/test_user.py | 152 +++++++++++++++++++++++++++------------------ 1 file changed, 90 insertions(+), 62 deletions(-) diff --git a/tests/test_user.py b/tests/test_user.py index d0e85188..0f7575b9 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -1,3 +1,4 @@ +from fastapi import HTTPException import pytest from auth.config import Settings @@ -69,6 +70,7 @@ def mock_user_data(): } +# Authentication Tests (GET) @pytest.mark.parametrize( "endpoint", [ @@ -88,6 +90,7 @@ def test_endpoints_require_auth(endpoint): assert response.json() == {"detail": "Not authenticated"} +# Service Tests (GET) def test_get_all_services( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -131,23 +134,15 @@ def test_get_pending_services( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test getting pending services""" - - # Patch the Auth0 token request mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - - # Patch the user metadata fetch mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.get_management_token", return_value="mock_management_token" ) response = client.get("/me/services/pending", headers=auth_headers) - assert response.status_code == 200 pending_services = [ s @@ -157,6 +152,55 @@ def test_get_pending_services( assert response.json() == {"pending_services": pending_services} +def test_get_services_failed_fetch( + mock_auth_settings, mock_auth_token, auth_headers, mocker +): + """Test handling of failed API calls""" + mocker.patch( + "routers.user.fetch_user_data", + side_effect=HTTPException(status_code=403, detail="Failed to fetch user data"), + ) + mocker.patch( + "routers.user.get_management_token", return_value="mock_management_token" + ) + + response = client.get("/me/services", headers=auth_headers) + assert response.status_code == 403 + assert response.json() == {"detail": "Failed to fetch user data"} + + +def test_get_services_empty_metadata( + mock_auth_settings, mock_auth_token, auth_headers, mocker +): + """Test handling of empty metadata""" + mocker.patch("routers.user.fetch_user_data", return_value={"app_metadata": {}}) + mocker.patch( + "routers.user.get_management_token", return_value="mock_management_token" + ) + + response = client.get("/me/services", headers=auth_headers) + assert response.status_code == 200 + assert response.json() == {"services": []} + + +def test_get_services_no_metadata( + mock_auth_settings, mock_auth_token, auth_headers, mocker +): + """Test handling of missing metadata""" + mocker.patch( + "routers.user.fetch_user_data", + return_value={}, + ) + mocker.patch( + "routers.user.get_management_token", return_value="mock_management_token" + ) + + response = client.get("/me/services", headers=auth_headers) + assert response.status_code == 200 + assert response.json() == {"services": []} + + +# Resource Tests (GET) def test_get_all_resources( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -232,59 +276,49 @@ def test_get_pending_resources( assert response.json() == {"pending_resources": pending_resources} -def test_get_all_pending( - mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +def test_get_resources_failed_fetch( + mock_auth_settings, mock_auth_token, auth_headers, mocker ): - """Test getting all pending items""" + """Test handling of failed resource API calls""" mocker.patch( "routers.user.fetch_user_data", - return_value=mock_user_data, + side_effect=HTTPException(status_code=403, detail="Failed to fetch user data"), ) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) - response = client.get("/me/all/pending", headers=auth_headers) - assert response.status_code == 200 - - pending_services = [ - s - for s in mock_user_data["app_metadata"]["services"] - if s["status"] == "pending" - ] - pending_resources = [ - resource - for service in mock_user_data["app_metadata"]["services"] - for resource in service["resources"] - if resource["status"] == "pending" - ] - - assert response.json() == { - "pending_services": pending_services, - "pending_resources": pending_resources, - } + response = client.get("/me/resources", headers=auth_headers) + assert response.status_code == 403 + assert response.json() == {"detail": "Failed to fetch user data"} -def test_request_service_success( - mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +def test_get_resources_empty_metadata( + mock_auth_settings, mock_auth_token, auth_headers, mocker ): - from datetime import datetime - from schemas.requests import ServiceRequest - + """Test handling of empty resource metadata""" mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", return_value={"app_metadata": {"services": []}} ) mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.get_management_token", return_value="mock_management_token" ) + + response = client.get("/me/resources", headers=auth_headers) + assert response.status_code == 200 + assert response.json() == {"resources": []} + + +# Service Request Tests (POST) +def test_request_service_success( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + """Test successful service request""" + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) mocker.patch( - "httpx.AsyncClient.patch", - return_value=mocker.Mock(status_code=200, json=lambda: {}), + "routers.user.get_management_token", return_value="mock_management_token" ) + mocker.patch("routers.user.update_user_metadata", return_value={}) new_service = { "name": "New Service", @@ -301,6 +335,7 @@ def test_request_service_success( def test_request_service_duplicate( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): + """Test duplicate service request""" mocker.patch( "httpx.post", return_value=mocker.Mock( @@ -330,6 +365,7 @@ def test_request_service_duplicate( def test_request_service_user_mismatch( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): + """Test service request with mismatched user""" request_payload = { "name": "Service Mismatch", "id": "svc-mismatch", @@ -346,26 +382,16 @@ def test_request_service_user_mismatch( ) +# Resource Request Tests (POST) def test_request_resource_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): - approved_service = mock_user_data["app_metadata"]["services"][0] - approved_service["resources"] = [] - + """Test successful resource request""" + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), - ) - mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), - ) - mocker.patch( - "httpx.AsyncClient.patch", - return_value=mocker.Mock(status_code=200, json=lambda: {}), + "routers.user.get_management_token", return_value="mock_management_token" ) + mocker.patch("routers.user.update_user_metadata", return_value={}) request_payload = { "name": "New Resource", @@ -384,6 +410,7 @@ def test_request_resource_success( def test_request_resource_user_mismatch( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): + """Test resource request with mismatched user""" request_payload = { "name": "Invalid Resource", "id": "res-invalid", @@ -404,6 +431,7 @@ def test_request_resource_user_mismatch( def test_request_resource_non_approved_service( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): + """Test resource request for non-approved service""" # Set service2 to be requested service = mock_user_data["app_metadata"]["services"][1] From ace44c5d8d43c5362184c5122888c5fc8b488f81 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 12 May 2025 15:58:27 +1000 Subject: [PATCH 18/22] fix(tests): update resource request test to use mocked user data and management token --- tests/test_user.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/test_user.py b/tests/test_user.py index 0f7575b9..a9f3b072 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -432,18 +432,9 @@ def test_request_resource_non_approved_service( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test resource request for non-approved service""" - # Set service2 to be requested - service = mock_user_data["app_metadata"]["services"][1] - - mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), - ) + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.get_management_token", return_value="mock_management_token" ) request_payload = { From 3d4223dad1d17ce2cb13dba5a3dd98cd17e6d8ee Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 12 May 2025 16:20:04 +1000 Subject: [PATCH 19/22] update tests --- tests/test_user.py | 98 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 23 deletions(-) diff --git a/tests/test_user.py b/tests/test_user.py index a9f3b072..b0d86290 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -9,6 +9,7 @@ client = TestClient(app) +# --- Test Fixtures --- @pytest.fixture(autouse=True) def mock_auth_settings(mocker): """Fixture to mock auth settings globally""" @@ -70,7 +71,7 @@ def mock_user_data(): } -# Authentication Tests (GET) +# --- Authentication Tests (GET) --- @pytest.mark.parametrize( "endpoint", [ @@ -90,7 +91,7 @@ def test_endpoints_require_auth(endpoint): assert response.json() == {"detail": "Not authenticated"} -# Service Tests (GET) +# --- Service Endpoints (GET) --- def test_get_all_services( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -200,24 +201,17 @@ def test_get_services_no_metadata( assert response.json() == {"services": []} -# Resource Tests (GET) +# --- Resource Endpoints (GET) --- def test_get_all_resources( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test getting all resources""" - - # Patch token fetch mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), + "routers.user.fetch_user_data", + return_value=mock_user_data, ) - - # Patch user metadata fetch mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.get_management_token", return_value="mock_management_token" ) response = client.get("/me/resources", headers=auth_headers) @@ -309,7 +303,21 @@ def test_get_resources_empty_metadata( assert response.json() == {"resources": []} -# Service Request Tests (POST) +def test_get_resources_no_metadata( + mock_auth_settings, mock_auth_token, auth_headers, mocker +): + """Test handling of missing resource metadata""" + mocker.patch("routers.user.fetch_user_data", return_value={}) + mocker.patch( + "routers.user.get_management_token", return_value="mock_management_token" + ) + + response = client.get("/me/resources", headers=auth_headers) + assert response.status_code == 200 + assert response.json() == {"resources": []} + + +# --- Service Request Endpoints (POST) --- def test_request_service_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -336,15 +344,9 @@ def test_request_service_duplicate( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test duplicate service request""" + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) mocker.patch( - "httpx.post", - return_value=mocker.Mock( - status_code=200, json=lambda: {"access_token": "test-token"} - ), - ) - mocker.patch( - "httpx.AsyncClient.get", - return_value=mocker.Mock(status_code=200, json=lambda: mock_user_data), + "routers.user.get_management_token", return_value="mock_management_token" ) existing_service = { @@ -382,7 +384,7 @@ def test_request_service_user_mismatch( ) -# Resource Request Tests (POST) +# --- Resource Request Endpoints (POST) --- def test_request_resource_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): @@ -452,3 +454,53 @@ def test_request_resource_non_approved_service( response.json()["detail"] == "Cannot request resources for a service that is not approved" ) + + +def test_request_resource_duplicate( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + """Test duplicate resource request""" + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch( + "routers.user.get_management_token", return_value="mock_management_token" + ) + + existing_resource = { + "name": "Resource 1", + "id": "resource1", + "user_id": mock_auth_token.sub, + "service_id": "service1", + } + + response = client.post( + "/request/service1/resource1", json=existing_resource, headers=auth_headers + ) + assert response.status_code == 400 + assert ( + response.json()["detail"] == "Resource request with ID resource1 already exists" + ) + + +def test_request_resource_invalid_service( + mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker +): + """Test resource request for non-existent service""" + mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch( + "routers.user.get_management_token", return_value="mock_management_token" + ) + + request_payload = { + "name": "Invalid Service Resource", + "id": "resource-invalid", + "user_id": mock_auth_token.sub, + "service_id": "non-existent-service", + } + + response = client.post( + "/request/non-existent-service/resource-invalid", + json=request_payload, + headers=auth_headers, + ) + assert response.status_code == 404 + assert response.json()["detail"] == "Service with ID non-existent-service not found" From 1dcebe0e57e9e2587d1a77911c5bffc00e99d900 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 13 May 2025 09:53:41 +1000 Subject: [PATCH 20/22] update models, routes & tests --- routers/user.py | 287 ++++++++++++++++++++++----------------------- schemas/service.py | 84 ++++++++++++- tests/test_user.py | 277 +++++++++++++++++++++++++++---------------- 3 files changed, 394 insertions(+), 254 deletions(-) diff --git a/routers/user.py b/routers/user.py index 83f8b3e6..0dc43e59 100644 --- a/routers/user.py +++ b/routers/user.py @@ -1,4 +1,5 @@ -import httpx +from typing import Dict, List, Any +from httpx import AsyncClient from auth.config import get_settings from auth.management import get_management_token @@ -6,143 +7,130 @@ from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException from schemas.requests import ResourceRequest, ServiceRequest -from schemas.service import Service +from schemas.service import Auth0User, Service, Resource from schemas.user import User -router = APIRouter() +router = APIRouter( + prefix="/me", tags=["user"], responses={401: {"description": "Unauthorized"}} +) -async def fetch_user_data(user_id: str, token: str): - url = f"https://{get_settings().auth0_domain}/api/v2/users/{user_id}" +async def get_user_data(user: User) -> Auth0User: + """Fetch and return user data from Auth0.""" + url = f"https://{get_settings().auth0_domain}/api/v2/users/{user.access_token.sub}" + token = get_management_token() headers = {"Authorization": f"Bearer {token}"} - async with httpx.AsyncClient() as client: - response = await client.get(url, headers=headers) - if response.status_code != 200: - raise HTTPException( - status_code=response.status_code, detail="Failed to fetch user data" - ) - return response.json() + + try: + async with AsyncClient() as client: + response = await client.get(url, headers=headers) + if response.status_code != 200: + raise HTTPException( + status_code=403, + detail="Failed to fetch user data", + ) + return Auth0User(**response.json()) + except HTTPException: + raise + except Exception as e: + raise HTTPException( + status_code=403, detail=f"Failed to fetch user data: {str(e)}" + ) -async def update_user_metadata(user_id: str, token: str, metadata: dict): +async def update_user_metadata( + user_id: str, token: str, metadata: Dict[str, Any] +) -> Dict[str, Any]: """Utility function to update user metadata in Auth0.""" url = f"https://{get_settings().auth0_domain}/api/v2/users/{user_id}" headers = { "Authorization": f"Bearer {token}", "Content-Type": "application/json", } - async with httpx.AsyncClient() as client: - response = await client.patch( - url, headers=headers, json={"app_metadata": metadata} - ) - if response.status_code != 200: - raise HTTPException( - status_code=response.status_code, - detail="Failed to update user metadata", + try: + async with AsyncClient() as client: + response = await client.patch( + url, headers=headers, json={"app_metadata": metadata} ) - return response.json() + if response.status_code != 200: + raise HTTPException( + status_code=403, + detail="Failed to update user metadata", + ) + return response.json() + except HTTPException: + raise + except Exception as e: + raise HTTPException( + status_code=403, + detail=f"Failed to update user metadata: {str(e)}", + ) -@router.get("/me/services") -async def get_all_services(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - return {"services": services} +@router.get("/services", response_model=Dict[str, List[Service]]) +async def get_services(user: User = Depends(get_current_user)): + """Get all services for the authenticated user.""" + user_data = await get_user_data(user) + return {"services": user_data.app_metadata.services} -@router.get("/me/services/approved") +@router.get("/services/approved", response_model=Dict[str, List[Service]]) async def get_approved_services(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - approved_services = [ - service for service in services if service.get("status") == "approved" - ] - return {"approved_services": approved_services} + """Get approved services for the authenticated user.""" + user_data = await get_user_data(user) + return {"approved_services": user_data.approved_services} -@router.get("/me/services/pending") +@router.get("/services/pending", response_model=Dict[str, List[Service]]) async def get_pending_services(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - pending_services = [ - service for service in services if service.get("status") == "pending" - ] - return {"pending_services": pending_services} + """Get pending services for the authenticated user.""" + user_data = await get_user_data(user) + return {"pending_services": user_data.pending_services} -@router.get("/me/resources") -async def get_all_resources(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - resources = [ - resource for service in services for resource in service.get("resources", []) - ] - return {"resources": resources} +@router.get("/resources", response_model=Dict[str, List[Resource]]) +async def get_resources(user: User = Depends(get_current_user)): + """Get all resources for the authenticated user.""" + user_data = await get_user_data(user) + return {"resources": user_data.app_metadata.get_all_resources()} -@router.get("/me/resources/approved") +@router.get("/resources/approved", response_model=Dict[str, List[Resource]]) async def get_approved_resources(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - approved_resources = [ - resource - for service in services - for resource in service.get("resources", []) - if resource.get("status") == "approved" - ] - return {"approved_resources": approved_resources} - - -@router.get("/me/resources/pending") + """Get approved resources for the authenticated user.""" + user_data = await get_user_data(user) + return {"approved_resources": user_data.approved_resources} + + +@router.get("/resources/pending", response_model=Dict[str, List[Resource]]) async def get_pending_resources(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - pending_resources = [ - resource - for service in services - for resource in service.get("resources", []) - if resource.get("status") == "pending" - ] - return {"pending_resources": pending_resources} - - -@router.get("/me/all/pending") + """Get pending resources for the authenticated user.""" + user_data = await get_user_data(user) + return {"pending_resources": user_data.pending_resources} + + +@router.get("/all/pending", response_model=Dict[str, List[Any]]) async def get_all_pending(user: User = Depends(get_current_user)): - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) - pending_services = [ - service for service in services if service.get("status") == "pending" - ] - pending_resources = [ - resource - for service in services - for resource in service.get("resources", []) - if resource.get("status") == "pending" - ] + """Get all pending services and resources.""" + user_data = await get_user_data(user) return { - "pending_services": pending_services, - "pending_resources": pending_resources, + "pending_services": user_data.pending_services, + "pending_resources": user_data.pending_resources, } -@router.post("/request/service") +@router.post( + "/request/service", + response_model=Dict[str, Any], + responses={ + 400: {"description": "Bad Request - Service already exists"}, + 403: {"description": "Forbidden - User ID mismatch"}, + 500: {"description": "Internal server error"}, + }, +) async def request_service( service_request: ServiceRequest, user: User = Depends(get_current_user) -): +) -> Dict[str, Any]: """Submit a request for a service.""" if user.access_token.sub != service_request.user_id: raise HTTPException( @@ -150,41 +138,52 @@ async def request_service( detail="User ID in request does not match authenticated user", ) - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) + user_data = await get_user_data(user) - if any(s.get("id") == service_request.id for s in services): + if any(s.id == service_request.id for s in user_data.app_metadata.services): raise HTTPException( status_code=400, detail=f"Service request with ID {service_request.id} already exists", ) - new_service: Service = { - "name": service_request.name, - "id": service_request.id, - "status": "pending", - "last_updated": datetime.now(timezone.utc).isoformat(), - "updated_by": user_id, - "resources": [], - } - - services.append(new_service) - updated_metadata = user_data.get("app_metadata", {}) - updated_metadata["services"] = services + new_service = Service( + name=service_request.name, + id=service_request.id, + status="pending", + last_updated=datetime.now(timezone.utc), + updated_by=user.access_token.sub, + resources=[], + ) + + user_data.app_metadata.services.append(new_service) + await update_user_metadata( + user.access_token.sub, + get_management_token(), + user_data.app_metadata.model_dump(), + ) - await update_user_metadata(user_id, token, updated_metadata) - return {"message": "Service request submitted successfully", "service": new_service} + return { + "message": "Service request submitted successfully", + "service": new_service.model_dump(mode="json"), + } -@router.post("/request/{service_id}/{resource_id}") +@router.post( + "/request/{service_id}/{resource_id}", + response_model=Dict[str, Any], + responses={ + 400: {"description": "Bad Request"}, + 403: {"description": "Forbidden"}, + 404: {"description": "Service not found"}, + 500: {"description": "Internal server error"}, + }, +) async def request_resource( service_id: str, resource_id: str, resource_request: ResourceRequest, user: User = Depends(get_current_user), -): +) -> Dict[str, Any]: """Submit a request for a resource within a service.""" if user.access_token.sub != resource_request.user_id: raise HTTPException( @@ -197,48 +196,42 @@ async def request_resource( status_code=400, detail="Service ID in path does not match request body" ) - user_id = user.access_token.sub - token = get_management_token() - user_data = await fetch_user_data(user_id, token) - services = user_data.get("app_metadata", {}).get("services", []) + user_data = await get_user_data(user) + service = user_data.app_metadata.get_service_by_id(service_id) - service = next((s for s in services if s.get("id") == service_id), None) if not service: raise HTTPException( status_code=404, detail=f"Service with ID {service_id} not found" ) - if service.get("status") != "approved": + if service.status != "approved": raise HTTPException( status_code=400, detail="Cannot request resources for a service that is not approved", ) - if any(r.get("id") == resource_id for r in service.get("resources", [])): + if any(r.id == resource_id for r in service.resources): raise HTTPException( status_code=400, detail=f"Resource request with ID {resource_id} already exists", ) - new_resource = { - "name": resource_request.name, - "id": resource_id, - "status": "pending", - } - - if "resources" not in service: - service["resources"] = [] + new_resource = Resource( + name=resource_request.name, id=resource_id, status="pending" + ) - service["resources"].append(new_resource) - service["last_updated"] = datetime.now(timezone.utc).isoformat() - service["updated_by"] = user_id + service.resources.append(new_resource) + service.last_updated = datetime.now(timezone.utc) + service.updated_by = user.access_token.sub - updated_metadata = user_data.get("app_metadata", {}) - updated_metadata["services"] = services + await update_user_metadata( + user.access_token.sub, + get_management_token(), + user_data.app_metadata.model_dump(), + ) - await update_user_metadata(user_id, token, updated_metadata) return { "message": "Resource request submitted successfully", - "service": service, - "resource": new_resource, + "service": service.model_dump(mode="json"), + "resource": new_resource.model_dump(mode="json"), } diff --git a/schemas/service.py b/schemas/service.py index 4e699eaf..8bfdfb5b 100644 --- a/schemas/service.py +++ b/schemas/service.py @@ -1,6 +1,6 @@ from datetime import datetime -from pydantic import BaseModel, Field -from typing import Literal +from pydantic import BaseModel, Field, HttpUrl +from typing import List, Literal, Optional class Resource(BaseModel): @@ -15,4 +15,82 @@ class Service(BaseModel): status: Literal["approved", "revoked", "pending"] last_updated: datetime updated_by: str - resources: list[Resource] = Field(default_factory=list) + resources: List[Resource] = Field(default_factory=list) + + +class Group(BaseModel): + name: str + id: str + + +class AppMetadata(BaseModel): + groups: List[Group] = Field(default_factory=list) + services: List[Service] = Field(default_factory=list) + + def get_pending_services(self) -> List[Service]: + """Get all pending services.""" + return [s for s in self.services if s.status == "pending"] + + def get_approved_services(self) -> List[Service]: + """Get all approved services.""" + return [s for s in self.services if s.status == "approved"] + + def get_all_resources(self) -> List[Resource]: + """Get all resources across services.""" + return [r for s in self.services for r in s.resources] + + def get_pending_resources(self) -> List[Resource]: + """Get all pending resources.""" + return [r for s in self.services for r in s.resources if r.status == "pending"] + + def get_approved_resources(self) -> List[Resource]: + """Get all approved resources.""" + return [r for s in self.services for r in s.resources if r.status == "approved"] + + def get_service_by_id(self, service_id: str) -> Optional[Service]: + """Get a service by its ID.""" + return next((s for s in self.services if s.id == service_id), None) + + +class Identity(BaseModel): + connection: str + provider: str + user_id: str + isSocial: bool + + +class Auth0User(BaseModel): + created_at: datetime + email: str + email_verified: bool + identities: List[Identity] + name: str + nickname: str + picture: HttpUrl + updated_at: datetime + user_id: str + user_metadata: dict = Field(default_factory=dict) + app_metadata: AppMetadata + last_ip: Optional[str] = None + last_login: Optional[datetime] = None + logins_count: Optional[int] = None + + @property + def pending_services(self) -> List[Service]: + """Get all services with pending status.""" + return self.app_metadata.get_pending_services() + + @property + def approved_services(self) -> List[Service]: + """Get all services with approved status.""" + return self.app_metadata.get_approved_services() + + @property + def pending_resources(self) -> List[Resource]: + """Get all resources with pending status across all services.""" + return self.app_metadata.get_pending_resources() + + @property + def approved_resources(self) -> List[Resource]: + """Get all resources with approved status across all services.""" + return self.app_metadata.get_approved_resources() diff --git a/tests/test_user.py b/tests/test_user.py index b0d86290..4f8ef2d5 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -1,9 +1,11 @@ from fastapi import HTTPException +from datetime import datetime import pytest from auth.config import Settings from fastapi.testclient import TestClient from main import app +from schemas.service import Auth0User, Service, Resource, Group, AppMetadata from tests.datagen import AccessTokenPayloadFactory client = TestClient(app) @@ -46,29 +48,51 @@ def auth_headers(): @pytest.fixture def mock_user_data(): """Fixture to provide mock user data""" - return { - "app_metadata": { - "services": [ - { - "id": "service1", - "name": "Service 1", - "status": "approved", - "resources": [ - {"id": "resource1", "name": "Resource 1", "status": "approved"}, - {"id": "resource2", "name": "Resource 2", "status": "pending"}, + return Auth0User( + created_at=datetime.now(), + email="test@example.com", + email_verified=True, + identities=[ + { + "connection": "Username-Password-Authentication", + "provider": "auth0", + "user_id": "67d10aa30b421a4a877cce78", + "isSocial": False, + } + ], + name="Test User", + nickname="test", + picture="https://example.com/picture.jpg", + updated_at=datetime.now(), + user_id="auth0|123456789", + user_metadata={}, + app_metadata=AppMetadata( + groups=[Group(name="Australian University", id="AU")], + services=[ + Service( + id="service1", + name="Service 1", + status="approved", + last_updated=datetime.now(), + updated_by="test@example.com", + resources=[ + Resource(id="resource1", name="Resource 1", status="approved"), + Resource(id="resource2", name="Resource 2", status="pending"), ], - }, - { - "id": "service2", - "name": "Service 2", - "status": "pending", - "resources": [ - {"id": "resource3", "name": "Resource 3", "status": "pending"}, + ), + Service( + id="service2", + name="Service 2", + status="pending", + last_updated=datetime.now(), + updated_by="test@example.com", + resources=[ + Resource(id="resource3", name="Resource 3", status="pending") ], - }, - ] - } - } + ), + ], + ), + ) # --- Authentication Tests (GET) --- @@ -97,7 +121,7 @@ def test_get_all_services( ): """Test getting all services""" mocker.patch( - "routers.user.fetch_user_data", + "routers.user.get_user_data", # Changed from fetch_user_data return_value=mock_user_data, ) mocker.patch( @@ -106,7 +130,11 @@ def test_get_all_services( response = client.get("/me/services", headers=auth_headers) assert response.status_code == 200 - assert response.json() == {"services": mock_user_data["app_metadata"]["services"]} + + expected_services = [ + s.model_dump(mode="json") for s in mock_user_data.app_metadata.services + ] + assert response.json() == {"services": expected_services} def test_get_approved_services( @@ -114,7 +142,7 @@ def test_get_approved_services( ): """Test getting approved services""" mocker.patch( - "routers.user.fetch_user_data", + "routers.user.get_user_data", # Changed from fetch_user_data return_value=mock_user_data, ) mocker.patch( @@ -123,10 +151,11 @@ def test_get_approved_services( response = client.get("/me/services/approved", headers=auth_headers) assert response.status_code == 200 + approved_services = [ - s - for s in mock_user_data["app_metadata"]["services"] - if s["status"] == "approved" + s.model_dump(mode="json") + for s in mock_user_data.app_metadata.services + if s.status == "approved" ] assert response.json() == {"approved_services": approved_services} @@ -136,7 +165,7 @@ def test_get_pending_services( ): """Test getting pending services""" mocker.patch( - "routers.user.fetch_user_data", + "routers.user.get_user_data", # Changed from fetch_user_data return_value=mock_user_data, ) mocker.patch( @@ -145,10 +174,11 @@ def test_get_pending_services( response = client.get("/me/services/pending", headers=auth_headers) assert response.status_code == 200 + pending_services = [ - s - for s in mock_user_data["app_metadata"]["services"] - if s["status"] == "pending" + s.model_dump(mode="json") + for s in mock_user_data.app_metadata.services + if s.status == "pending" ] assert response.json() == {"pending_services": pending_services} @@ -158,7 +188,7 @@ def test_get_services_failed_fetch( ): """Test handling of failed API calls""" mocker.patch( - "routers.user.fetch_user_data", + "routers.user.get_user_data", # Changed from fetch_user_data side_effect=HTTPException(status_code=403, detail="Failed to fetch user data"), ) mocker.patch( @@ -174,7 +204,27 @@ def test_get_services_empty_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of empty metadata""" - mocker.patch("routers.user.fetch_user_data", return_value={"app_metadata": {}}) + empty_user = Auth0User( + created_at=datetime.now(), + email="test@example.com", + email_verified=True, + identities=[ + { + "connection": "Username-Password-Authentication", + "provider": "auth0", + "user_id": "123", + "isSocial": False, + } + ], + name="Test User", + nickname="test", + picture="https://example.com/picture.jpg", + updated_at=datetime.now(), + user_id="auth0|123", + user_metadata={}, + app_metadata=AppMetadata(services=[], groups=[]), + ) + mocker.patch("routers.user.get_user_data", return_value=empty_user) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -188,10 +238,27 @@ def test_get_services_no_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of missing metadata""" - mocker.patch( - "routers.user.fetch_user_data", - return_value={}, - ) + no_metadata_user = Auth0User( + created_at=datetime.now(), + email="test@example.com", + email_verified=True, + identities=[ + { + "connection": "Username-Password-Authentication", + "provider": "auth0", + "user_id": "123", + "isSocial": False, + } + ], + name="Test User", + nickname="test", + picture="https://example.com/picture.jpg", + updated_at=datetime.now(), + user_id="auth0|123", + user_metadata={}, + app_metadata=AppMetadata(), + ) + mocker.patch("routers.user.get_user_data", return_value=no_metadata_user) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -207,7 +274,7 @@ def test_get_all_resources( ): """Test getting all resources""" mocker.patch( - "routers.user.fetch_user_data", + "routers.user.get_user_data", # Changed from fetch_user_data return_value=mock_user_data, ) mocker.patch( @@ -217,9 +284,9 @@ def test_get_all_resources( response = client.get("/me/resources", headers=auth_headers) assert response.status_code == 200 all_resources = [ - resource - for service in mock_user_data["app_metadata"]["services"] - for resource in service["resources"] + r.model_dump() + for s in mock_user_data.app_metadata.services + for r in s.resources ] assert response.json() == {"resources": all_resources} @@ -229,7 +296,7 @@ def test_get_approved_resources( ): """Test getting approved resources""" mocker.patch( - "routers.user.fetch_user_data", + "routers.user.get_user_data", # Changed from fetch_user_data return_value=mock_user_data, ) mocker.patch( @@ -239,61 +306,39 @@ def test_get_approved_resources( response = client.get("/me/resources/approved", headers=auth_headers) assert response.status_code == 200 approved_resources = [ - resource - for service in mock_user_data["app_metadata"]["services"] - for resource in service["resources"] - if resource["status"] == "approved" + r.model_dump() + for s in mock_user_data.app_metadata.services + for r in s.resources + if r.status == "approved" ] assert response.json() == {"approved_resources": approved_resources} -def test_get_pending_resources( - mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker -): - """Test getting pending resources""" - mocker.patch( - "routers.user.fetch_user_data", - return_value=mock_user_data, - ) - mocker.patch( - "routers.user.get_management_token", return_value="mock_management_token" - ) - - response = client.get("/me/resources/pending", headers=auth_headers) - assert response.status_code == 200 - pending_resources = [ - resource - for service in mock_user_data["app_metadata"]["services"] - for resource in service["resources"] - if resource["status"] == "pending" - ] - assert response.json() == {"pending_resources": pending_resources} - - -def test_get_resources_failed_fetch( - mock_auth_settings, mock_auth_token, auth_headers, mocker -): - """Test handling of failed resource API calls""" - mocker.patch( - "routers.user.fetch_user_data", - side_effect=HTTPException(status_code=403, detail="Failed to fetch user data"), - ) - mocker.patch( - "routers.user.get_management_token", return_value="mock_management_token" - ) - - response = client.get("/me/resources", headers=auth_headers) - assert response.status_code == 403 - assert response.json() == {"detail": "Failed to fetch user data"} - - def test_get_resources_empty_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of empty resource metadata""" - mocker.patch( - "routers.user.fetch_user_data", return_value={"app_metadata": {"services": []}} - ) + empty_user = Auth0User( + created_at=datetime.now(), + email="test@example.com", + email_verified=True, + identities=[ + { + "connection": "Username-Password-Authentication", + "provider": "auth0", + "user_id": "123", + "isSocial": False, + } + ], + name="Test User", + nickname="test", + picture="https://example.com/picture.jpg", + updated_at=datetime.now(), + user_id="auth0|123", + user_metadata={}, + app_metadata=AppMetadata(services=[], groups=[]), + ) + mocker.patch("routers.user.get_user_data", return_value=empty_user) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -307,7 +352,27 @@ def test_get_resources_no_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of missing resource metadata""" - mocker.patch("routers.user.fetch_user_data", return_value={}) + no_metadata_user = Auth0User( + created_at=datetime.now(), + email="test@example.com", + email_verified=True, + identities=[ + { + "connection": "Username-Password-Authentication", + "provider": "auth0", + "user_id": "123", + "isSocial": False, + } + ], + name="Test User", + nickname="test", + picture="https://example.com/picture.jpg", + updated_at=datetime.now(), + user_id="auth0|123", + user_metadata={}, + app_metadata=AppMetadata(), + ) + mocker.patch("routers.user.get_user_data", return_value=no_metadata_user) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -322,7 +387,7 @@ def test_request_service_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test successful service request""" - mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch("routers.user.get_user_data", return_value=mock_user_data) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -334,7 +399,9 @@ def test_request_service_success( "user_id": mock_auth_token.sub, } - response = client.post("/request/service", json=new_service, headers=auth_headers) + response = client.post( + "/me/request/service", json=new_service, headers=auth_headers + ) assert response.status_code == 200 assert response.json()["message"] == "Service request submitted successfully" assert response.json()["service"]["id"] == "service3" @@ -344,7 +411,7 @@ def test_request_service_duplicate( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test duplicate service request""" - mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch("routers.user.get_user_data", return_value=mock_user_data) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -356,7 +423,7 @@ def test_request_service_duplicate( } response = client.post( - "/request/service", json=existing_service, headers=auth_headers + "/me/request/service", json=existing_service, headers=auth_headers ) assert response.status_code == 400 assert ( @@ -375,7 +442,7 @@ def test_request_service_user_mismatch( } response = client.post( - "/request/service", json=request_payload, headers=auth_headers + "/me/request/service", json=request_payload, headers=auth_headers ) assert response.status_code == 403 assert ( @@ -389,7 +456,7 @@ def test_request_resource_success( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test successful resource request""" - mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch("routers.user.get_user_data", return_value=mock_user_data) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -403,7 +470,7 @@ def test_request_resource_success( } response = client.post( - "/request/service1/resource-new", json=request_payload, headers=auth_headers + "/me/request/service1/resource-new", json=request_payload, headers=auth_headers ) assert response.status_code == 200 assert response.json()["resource"]["id"] == "resource-new" @@ -421,7 +488,7 @@ def test_request_resource_user_mismatch( } response = client.post( - "/request/service1/res-invalid", json=request_payload, headers=auth_headers + "/me/request/service1/res-invalid", json=request_payload, headers=auth_headers ) assert response.status_code == 403 assert ( @@ -434,7 +501,7 @@ def test_request_resource_non_approved_service( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test resource request for non-approved service""" - mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch("routers.user.get_user_data", return_value=mock_user_data) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -447,7 +514,9 @@ def test_request_resource_non_approved_service( } response = client.post( - "/request/service2/blocked-resource", json=request_payload, headers=auth_headers + "/me/request/service2/blocked-resource", + json=request_payload, + headers=auth_headers, ) assert response.status_code == 400 assert ( @@ -460,7 +529,7 @@ def test_request_resource_duplicate( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test duplicate resource request""" - mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch("routers.user.get_user_data", return_value=mock_user_data) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -473,7 +542,7 @@ def test_request_resource_duplicate( } response = client.post( - "/request/service1/resource1", json=existing_resource, headers=auth_headers + "/me/request/service1/resource1", json=existing_resource, headers=auth_headers ) assert response.status_code == 400 assert ( @@ -485,7 +554,7 @@ def test_request_resource_invalid_service( mock_auth_settings, mock_auth_token, auth_headers, mock_user_data, mocker ): """Test resource request for non-existent service""" - mocker.patch("routers.user.fetch_user_data", return_value=mock_user_data) + mocker.patch("routers.user.get_user_data", return_value=mock_user_data) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" ) @@ -498,7 +567,7 @@ def test_request_resource_invalid_service( } response = client.post( - "/request/non-existent-service/resource-invalid", + "/me/request/non-existent-service/resource-invalid", json=request_payload, headers=auth_headers, ) From 93a241b7d27ce65047e796d83ec8e618095b0c34 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Tue, 13 May 2025 13:43:54 +1000 Subject: [PATCH 21/22] Update tests to use automatically generated Auth0User --- tests/datagen.py | 4 ++ tests/test_user.py | 102 +++------------------------------------------ 2 files changed, 10 insertions(+), 96 deletions(-) diff --git a/tests/datagen.py b/tests/datagen.py index 3925087b..8fae5e63 100644 --- a/tests/datagen.py +++ b/tests/datagen.py @@ -1,5 +1,6 @@ from polyfactory.factories.pydantic_factory import ModelFactory +from schemas.service import Auth0User from schemas.tokens import AccessTokenPayload from schemas.user import User @@ -8,3 +9,6 @@ class AccessTokenPayloadFactory(ModelFactory[AccessTokenPayload]): ... class UserFactory(ModelFactory[User]): ... + + +class Auth0UserFactory(ModelFactory[Auth0User]): ... diff --git a/tests/test_user.py b/tests/test_user.py index 4f8ef2d5..0416c650 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -6,7 +6,7 @@ from fastapi.testclient import TestClient from main import app from schemas.service import Auth0User, Service, Resource, Group, AppMetadata -from tests.datagen import AccessTokenPayloadFactory +from tests.datagen import AccessTokenPayloadFactory, Auth0UserFactory client = TestClient(app) @@ -48,24 +48,7 @@ def auth_headers(): @pytest.fixture def mock_user_data(): """Fixture to provide mock user data""" - return Auth0User( - created_at=datetime.now(), - email="test@example.com", - email_verified=True, - identities=[ - { - "connection": "Username-Password-Authentication", - "provider": "auth0", - "user_id": "67d10aa30b421a4a877cce78", - "isSocial": False, - } - ], - name="Test User", - nickname="test", - picture="https://example.com/picture.jpg", - updated_at=datetime.now(), - user_id="auth0|123456789", - user_metadata={}, + return Auth0UserFactory.build( app_metadata=AppMetadata( groups=[Group(name="Australian University", id="AU")], services=[ @@ -204,24 +187,7 @@ def test_get_services_empty_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of empty metadata""" - empty_user = Auth0User( - created_at=datetime.now(), - email="test@example.com", - email_verified=True, - identities=[ - { - "connection": "Username-Password-Authentication", - "provider": "auth0", - "user_id": "123", - "isSocial": False, - } - ], - name="Test User", - nickname="test", - picture="https://example.com/picture.jpg", - updated_at=datetime.now(), - user_id="auth0|123", - user_metadata={}, + empty_user = Auth0UserFactory.build( app_metadata=AppMetadata(services=[], groups=[]), ) mocker.patch("routers.user.get_user_data", return_value=empty_user) @@ -238,26 +204,7 @@ def test_get_services_no_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of missing metadata""" - no_metadata_user = Auth0User( - created_at=datetime.now(), - email="test@example.com", - email_verified=True, - identities=[ - { - "connection": "Username-Password-Authentication", - "provider": "auth0", - "user_id": "123", - "isSocial": False, - } - ], - name="Test User", - nickname="test", - picture="https://example.com/picture.jpg", - updated_at=datetime.now(), - user_id="auth0|123", - user_metadata={}, - app_metadata=AppMetadata(), - ) + no_metadata_user = Auth0UserFactory.build(app_metadata=AppMetadata()) mocker.patch("routers.user.get_user_data", return_value=no_metadata_user) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" @@ -318,25 +265,7 @@ def test_get_resources_empty_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of empty resource metadata""" - empty_user = Auth0User( - created_at=datetime.now(), - email="test@example.com", - email_verified=True, - identities=[ - { - "connection": "Username-Password-Authentication", - "provider": "auth0", - "user_id": "123", - "isSocial": False, - } - ], - name="Test User", - nickname="test", - picture="https://example.com/picture.jpg", - updated_at=datetime.now(), - user_id="auth0|123", - user_metadata={}, - app_metadata=AppMetadata(services=[], groups=[]), + empty_user = Auth0UserFactory.build(app_metadata=AppMetadata(services=[], groups=[]), ) mocker.patch("routers.user.get_user_data", return_value=empty_user) mocker.patch( @@ -352,26 +281,7 @@ def test_get_resources_no_metadata( mock_auth_settings, mock_auth_token, auth_headers, mocker ): """Test handling of missing resource metadata""" - no_metadata_user = Auth0User( - created_at=datetime.now(), - email="test@example.com", - email_verified=True, - identities=[ - { - "connection": "Username-Password-Authentication", - "provider": "auth0", - "user_id": "123", - "isSocial": False, - } - ], - name="Test User", - nickname="test", - picture="https://example.com/picture.jpg", - updated_at=datetime.now(), - user_id="auth0|123", - user_metadata={}, - app_metadata=AppMetadata(), - ) + no_metadata_user = Auth0UserFactory.build(app_metadata=AppMetadata()) mocker.patch("routers.user.get_user_data", return_value=no_metadata_user) mocker.patch( "routers.user.get_management_token", return_value="mock_management_token" From c81d439fdb17afb26013a15eccd474ff84718acb Mon Sep 17 00:00:00 2001 From: marius-mather Date: Tue, 13 May 2025 14:09:53 +1000 Subject: [PATCH 22/22] Remove unused import --- tests/test_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_user.py b/tests/test_user.py index 0416c650..cf484a1e 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -5,7 +5,7 @@ from auth.config import Settings from fastapi.testclient import TestClient from main import app -from schemas.service import Auth0User, Service, Resource, Group, AppMetadata +from schemas.service import Service, Resource, Group, AppMetadata from tests.datagen import AccessTokenPayloadFactory, Auth0UserFactory client = TestClient(app)