Strengthen TRE API authentication and clean up codebase

.flake8

@@ -1,2 +1,4 @@
 [flake8]
-ignore = E501, W503
+# E231 is ignored because pycodestyle on Python 3.12+ mis-tokenises f-string
+# contents (commas/colons inside URL, OData and JSON string literals) as code.
+ignore = E501, W503, E231

.github/linters/.flake8

@@ -1,2 +1,4 @@
 [flake8]
-ignore = E501,W503
+# E231 is ignored because pycodestyle on Python 3.12+ mis-tokenises f-string
+# contents (commas/colons inside URL, OData and JSON string literals) as code.
+ignore = E501,W503,E231

CHANGELOG.md

@@ -12,6 +12,7 @@ ENHANCEMENTS:
 * Add Windows Server 2025 image support to Guacamole. ([#4890](https://github.com/microsoft/AzureTRE/issues/4890))
 * Add support for setting resource processor VMSS SKU via environment variables ([#4936](https://github.com/microsoft/AzureTRE/issues/4936))
 * Exclude recovery service vaults from e2e tests ([#4920](https://github.com/microsoft/AzureTRE/issues/4920))
+* Strengthen TRE API authentication with a layered `auth/` package (typed exceptions, `PyJWKClient`-backed token validation with issuer checking, immutable `AuthenticatedUser` model, composable RBAC factories), removing the redundant `AccessService` abstraction, and add Event Grid publish resilience (retry on transient 429/5xx and network errors, with Graph and publish failures reported distinctly). ([#4989](https://github.com/microsoft/AzureTRE/pull/4989))
 * Add support for formatting UI code via `pre-commit` and fix existing formatting issues. ([#4955](https://github.com/microsoft/AzureTRE/issues/4955))
 * Update the version of `super-linter` used in the `build_validation_develop` workflow to 8.7.0 ([#4957](https://github.com/microsoft/AzureTRE/issues/4957))
 

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.25.29"
+__version__ = "0.26.2"

api_app/api/routes/airlock.py

@@ -19,23 +19,23 @@
 from models.schemas.airlock_request import AirlockRequestAndOperationInResponse, AirlockRequestInCreate, AirlockRequestWithAllowedUserActions, \
     AirlockRequestWithAllowedUserActionsInList, AirlockReviewInCreate, AirlockRevokeInCreate
 from resources import strings
-from services.authentication import get_current_workspace_owner_or_researcher_user_or_airlock_manager, \
-    get_current_workspace_owner_or_researcher_user, get_current_airlock_manager_user
+from auth.rbac import require_workspace_owner_or_researcher_or_airlock_manager, \
+    require_workspace_owner_or_researcher, require_airlock_manager
 
 from .resource_helpers import construct_location_header
 
 from services.airlock import create_review_vm, review_airlock_request, get_airlock_container_link, get_allowed_actions, save_and_publish_event_airlock_request, update_and_publish_event_airlock_request, \
     enrich_requests_with_allowed_actions, get_airlock_requests_by_user_and_workspace, cancel_request, revoke_request
 from services.logging import logger
 
-airlock_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)])
+airlock_workspace_router = APIRouter(dependencies=[Depends(require_workspace_owner_or_researcher_or_airlock_manager)])
 
 
 # airlock
 @airlock_workspace_router.post("/workspaces/{workspace_id}/requests", status_code=status_code.HTTP_201_CREATED,
                                response_model=AirlockRequestWithAllowedUserActions, name=strings.API_CREATE_AIRLOCK_REQUEST,
-                               dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
-async def create_draft_request(airlock_request_input: AirlockRequestInCreate, user=Depends(get_current_workspace_owner_or_researcher_user),
+                               dependencies=[Depends(require_workspace_owner_or_researcher), Depends(get_workspace_by_id_from_path)])
+async def create_draft_request(airlock_request_input: AirlockRequestInCreate, user=Depends(require_workspace_owner_or_researcher),
                                airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
                                workspace=Depends(get_deployed_workspace_by_id_from_path)) -> AirlockRequestWithAllowedUserActions:
     if workspace.properties.get("enable_airlock") is False:
@@ -54,12 +54,12 @@ async def create_draft_request(airlock_request_input: AirlockRequestInCreate, us
                               status_code=status_code.HTTP_200_OK,
                               response_model=AirlockRequestWithAllowedUserActionsInList,
                               name=strings.API_LIST_AIRLOCK_REQUESTS,
-                              dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager),
+                              dependencies=[Depends(require_workspace_owner_or_researcher_or_airlock_manager),
                                             Depends(get_workspace_by_id_from_path)])
 async def get_all_airlock_requests_by_workspace(
         airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
         workspace=Depends(get_deployed_workspace_by_id_from_path),
-        user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager),
+        user=Depends(require_workspace_owner_or_researcher_or_airlock_manager),
         creator_user_id: Optional[str] = None, type: Optional[AirlockRequestType] = None, status: Optional[AirlockRequestStatus] = None,
         order_by: Optional[str] = None, order_ascending: bool = True) -> AirlockRequestWithAllowedUserActionsInList:
     try:
@@ -75,19 +75,19 @@ async def get_all_airlock_requests_by_workspace(
 
 @airlock_workspace_router.get("/workspaces/{workspace_id}/requests/{airlock_request_id}", status_code=status_code.HTTP_200_OK,
                               response_model=AirlockRequestWithAllowedUserActions, name=strings.API_GET_AIRLOCK_REQUEST,
-                              dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager), Depends(get_workspace_by_id_from_path)])
+                              dependencies=[Depends(require_workspace_owner_or_researcher_or_airlock_manager), Depends(get_workspace_by_id_from_path)])
 async def retrieve_airlock_request_by_id(airlock_request=Depends(get_airlock_request_by_id_from_path),
                                          airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
-                                         user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)) -> AirlockRequestWithAllowedUserActions:
+                                         user=Depends(require_workspace_owner_or_researcher_or_airlock_manager)) -> AirlockRequestWithAllowedUserActions:
     allowed_actions = get_allowed_actions(airlock_request, user, airlock_request_repo)
     return AirlockRequestWithAllowedUserActions(airlockRequest=airlock_request, allowedUserActions=allowed_actions)
 
 
 @airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/submit", status_code=status_code.HTTP_200_OK,
                                response_model=AirlockRequestWithAllowedUserActions, name=strings.API_SUBMIT_AIRLOCK_REQUEST,
-                               dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
+                               dependencies=[Depends(require_workspace_owner_or_researcher), Depends(get_workspace_by_id_from_path)])
 async def create_submit_request(airlock_request=Depends(get_airlock_request_by_id_from_path),
-                                user=Depends(get_current_workspace_owner_or_researcher_user),
+                                user=Depends(require_workspace_owner_or_researcher),
                                 airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
                                 workspace=Depends(get_workspace_by_id_from_path)) -> AirlockRequestWithAllowedUserActions:
     updated_request = await update_and_publish_event_airlock_request(airlock_request, airlock_request_repo, user, workspace,
@@ -98,9 +98,9 @@ async def create_submit_request(airlock_request=Depends(get_airlock_request_by_i
 
 @airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/cancel", status_code=status_code.HTTP_200_OK,
                                response_model=AirlockRequestWithAllowedUserActions, name=strings.API_CANCEL_AIRLOCK_REQUEST,
-                               dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
+                               dependencies=[Depends(require_workspace_owner_or_researcher), Depends(get_workspace_by_id_from_path)])
 async def create_cancel_request(airlock_request=Depends(get_airlock_request_by_id_from_path),
-                                user=Depends(get_current_workspace_owner_or_researcher_user),
+                                user=Depends(require_workspace_owner_or_researcher),
                                 workspace=Depends(get_workspace_by_id_from_path),
                                 airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
                                 user_resource_repo=Depends(get_repository(UserResourceRepository)),
@@ -115,10 +115,10 @@ async def create_cancel_request(airlock_request=Depends(get_airlock_request_by_i
 
 @airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/revoke", status_code=status_code.HTTP_200_OK,
                                response_model=AirlockRequestWithAllowedUserActions, name=strings.API_REVOKE_AIRLOCK_REQUEST,
-                               dependencies=[Depends(get_current_airlock_manager_user), Depends(get_workspace_by_id_from_path)])
+                               dependencies=[Depends(require_airlock_manager), Depends(get_workspace_by_id_from_path)])
 async def create_revoke_request(revoke_input: AirlockRevokeInCreate,
                                 airlock_request=Depends(get_airlock_request_by_id_from_path),
-                                user=Depends(get_current_airlock_manager_user),
+                                user=Depends(require_airlock_manager),
                                 workspace=Depends(get_workspace_by_id_from_path),
                                 airlock_request_repo=Depends(get_repository(AirlockRequestRepository))) -> AirlockRequestWithAllowedUserActions:
     updated_request = await revoke_request(airlock_request, user, workspace, airlock_request_repo, revoke_input.reason)
@@ -129,11 +129,11 @@ async def create_revoke_request(revoke_input: AirlockRevokeInCreate,
 @airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/review-user-resource",
                                status_code=status_code.HTTP_202_ACCEPTED, response_model=AirlockRequestAndOperationInResponse,
                                name=strings.API_CREATE_AIRLOCK_REVIEW_USER_RESOURCE,
-                               dependencies=[Depends(get_current_airlock_manager_user), Depends(get_workspace_by_id_from_path)])
+                               dependencies=[Depends(require_airlock_manager), Depends(get_workspace_by_id_from_path)])
 async def create_review_user_resource(
         response: Response,
         airlock_request=Depends(get_airlock_request_by_id_from_path),
-        user=Depends(get_current_airlock_manager_user),
+        user=Depends(require_airlock_manager),
         workspace=Depends(get_deployed_workspace_by_id_from_path),
         user_resource_repo=Depends(get_repository(UserResourceRepository)),
         workspace_service_repo=Depends(get_repository(WorkspaceServiceRepository)),
@@ -166,12 +166,12 @@ async def create_review_user_resource(
 
 @airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/review",
                                status_code=status_code.HTTP_200_OK, response_model=AirlockRequestWithAllowedUserActions,
-                               name=strings.API_REVIEW_AIRLOCK_REQUEST, dependencies=[Depends(get_current_airlock_manager_user),
+                               name=strings.API_REVIEW_AIRLOCK_REQUEST, dependencies=[Depends(require_airlock_manager),
                                                                                       Depends(get_workspace_by_id_from_path)])
 async def create_airlock_review(
         airlock_review_input: AirlockReviewInCreate,
         airlock_request=Depends(get_airlock_request_by_id_from_path),
-        user=Depends(get_current_airlock_manager_user),
+        user=Depends(require_airlock_manager),
         workspace=Depends(get_deployed_workspace_by_id_from_path),
         airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
         user_resource_repo=Depends(get_repository(UserResourceRepository)),
@@ -191,9 +191,9 @@ async def create_airlock_review(
 @airlock_workspace_router.get("/workspaces/{workspace_id}/requests/{airlock_request_id}/link",
                               status_code=status_code.HTTP_200_OK, response_model=AirlockRequestTokenInResponse,
                               name=strings.API_AIRLOCK_REQUEST_LINK,
-                              dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)])
+                              dependencies=[Depends(require_workspace_owner_or_researcher_or_airlock_manager)])
 async def get_airlock_container_link_method(workspace=Depends(get_deployed_workspace_by_id_from_path),
                                             airlock_request=Depends(get_airlock_request_by_id_from_path),
-                                            user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)) -> AirlockRequestTokenInResponse:
+                                            user=Depends(require_workspace_owner_or_researcher_or_airlock_manager)) -> AirlockRequestTokenInResponse:
     container_url = get_airlock_container_link(airlock_request, user, workspace)
     return AirlockRequestTokenInResponse(containerUrl=container_url)

api_app/api/routes/costs.py

@@ -15,13 +15,13 @@
 from db.repositories.workspaces import WorkspaceRepository
 from models.domain.costs import CostReport, GranularityEnum, WorkspaceCostReport
 from resources import strings
-from services.authentication import get_current_admin_user, get_current_workspace_owner_or_tre_admin
+from auth.rbac import require_tre_admin, require_workspace_owner_or_tre_admin
 from services.cost_service import CostService, ServiceUnavailable, SubscriptionNotSupported, TooManyRequests, WorkspaceDoesNotExist, cost_service_factory
 from services.logging import logger
 
 
-costs_core_router = APIRouter(dependencies=[Depends(get_current_admin_user)])
-costs_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_tre_admin)])
+costs_core_router = APIRouter(dependencies=[Depends(require_tre_admin)])
+costs_workspace_router = APIRouter(dependencies=[Depends(require_workspace_owner_or_tre_admin)])
 
 
 def validate_report_period(from_date: Optional[datetime], to_date: Optional[datetime]):
@@ -86,7 +86,7 @@ async def costs(
 
 @costs_workspace_router.get("/workspaces/{workspace_id}/costs", response_model=WorkspaceCostReport,
                             name=strings.API_GET_WORKSPACE_COSTS,
-                            dependencies=[Depends(get_current_workspace_owner_or_tre_admin)],
+                            dependencies=[Depends(require_workspace_owner_or_tre_admin)],
                             responses=get_workspace_cost_report_responses())
 async def workspace_costs(workspace_id: UUID4, params: CostsQueryParams = Depends(),
                           cost_service: CostService = Depends(cost_service_factory),

api_app/api/routes/migrations.py

@@ -1,17 +1,17 @@
 from fastapi import APIRouter, Depends, HTTPException, status
-from services.authentication import get_current_admin_user
+from auth.rbac import require_tre_admin
 from resources import strings
 from models.schemas.migrations import MigrationOutList
 from services.logging import logger
 
-migrations_core_router = APIRouter(dependencies=[Depends(get_current_admin_user)])
+migrations_core_router = APIRouter(dependencies=[Depends(require_tre_admin)])
 
 
 @migrations_core_router.post("/migrations",
                              status_code=status.HTTP_202_ACCEPTED,
                              name=strings.API_MIGRATE_DATABASE,
                              response_model=MigrationOutList,
-                             dependencies=[Depends(get_current_admin_user)])
+                             dependencies=[Depends(require_tre_admin)])
 async def migrate_database():
     try:
         migrations = list()

api_app/api/routes/operations.py

@@ -4,13 +4,13 @@
 from db.repositories.operations import OperationRepository
 from models.schemas.operation import OperationInList
 from resources import strings
-from services.authentication import get_current_tre_user_or_tre_admin
+from auth.rbac import require_tre_user_or_admin
 
 
-operations_router = APIRouter(dependencies=[Depends(get_current_tre_user_or_tre_admin)])
+operations_router = APIRouter(dependencies=[Depends(require_tre_user_or_admin)])
 
 
 @operations_router.get("/operations", response_model=OperationInList, name=strings.API_GET_MY_OPERATIONS)
-async def get_my_operations(user=Depends(get_current_tre_user_or_tre_admin), operations_repo=Depends(get_repository(OperationRepository))) -> OperationInList:
+async def get_my_operations(user=Depends(require_tre_user_or_admin), operations_repo=Depends(get_repository(OperationRepository))) -> OperationInList:
     operations = await operations_repo.get_my_operations(user_id=user.id)
     return OperationInList(operations=operations)

api_app/api/routes/requests.py

@@ -5,14 +5,14 @@
 from resources import strings
 from db.repositories.airlock_requests import AirlockRequestRepository
 from models.domain.airlock_request import AirlockRequest, AirlockRequestStatus, AirlockRequestType
-from services.authentication import get_current_tre_user_or_tre_admin
+from auth.rbac import require_tre_user_or_admin
 
-router = APIRouter(dependencies=[Depends(get_current_tre_user_or_tre_admin)])
+router = APIRouter(dependencies=[Depends(require_tre_user_or_admin)])
 
 
 @router.get("/requests", response_model=List[AirlockRequest], name=strings.API_LIST_REQUESTS)
 async def get_requests(
-    user=Depends(get_current_tre_user_or_tre_admin),
+    user=Depends(require_tre_user_or_admin),
     airlock_request_repo: AirlockRequestRepository = Depends(get_repository(AirlockRequestRepository)),
     airlock_manager: bool = False,
     type: Optional[AirlockRequestType] = None, status: Optional[AirlockRequestStatus] = None,

api_app/api/routes/resource_helpers.py

@@ -24,7 +24,7 @@
     send_resource_request_message,
     RequestAction,
 )
-from services.authentication import get_access_service
+from services.authentication import get_aad_service
 from services.logging import logger
 
 
@@ -157,13 +157,8 @@ def construct_location_header(operation: Operation) -> str:
 
 
 def get_identity_role_assignments(user):
-    access_service = get_access_service()
-    return access_service.get_identity_role_assignments(user.id)
-
-
-def get_app_user_roles_assignments_emails(app_obj_id):
-    access_service = get_access_service()
-    return access_service.get_app_user_role_assignments_emails(app_obj_id)
+    aad_service = get_aad_service()
+    return aad_service.get_identity_role_assignments(user.id)
 
 
 async def send_uninstall_message(