Skip to content

Commit

Permalink
Merge branch 'main' into jr/upstream-main/142-certs-kv-error
Browse files Browse the repository at this point in the history
  • Loading branch information
jonnyry authored Feb 12, 2025
2 parents 5650b91 + 72e98c3 commit 0265375
Show file tree
Hide file tree
Showing 28 changed files with 961 additions and 296 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@

ENHANCEMENTS:
* Allow workspace App Service Plan SKU to be updated ([#4331](https://github.com/microsoft/AzureTRE/issues/4331))
* Add core requests endpoint and UI to enable requests to be managed TRE wide. ([[#2510](https://github.com/microsoft/AzureTRE/issues/2510)])
* Remove public IP from TRE's firewall when forced tunneling is configured ([#4346](https://github.com/microsoft/AzureTRE/pull/4346))
* Upgrade AzureRM Terraform provider from `3.117.0` to `4.14.0`. ([[#4255](https://github.com/microsoft/AzureTRE/pull/4255/)])
* Subnet definitions are now inline in the `azurerm_virtual_network` resource, and NSG associations are set using `security_group` in each subnet block (no separate `azurerm_subnet_network_security_group_association` needed). ([[#4255](https://github.com/microsoft/AzureTRE/pull/4255/)])

BUG FIXES:
* Fix upgrade when porter install has failed ([#4338](https://github.com/microsoft/AzureTRE/pull/4338))
Expand Down Expand Up @@ -77,6 +80,7 @@ BUG FIXES:
* Recreate tre_output.json if empty. ([[#4292](https://github.com/microsoft/AzureTRE/issues/4292)])
* Ensure R directory is present before attempting to update package mirror URL ([#4332](https://github.com/microsoft/AzureTRE/pull/4332))


COMPONENTS:

| name | version |
Expand Down
2 changes: 1 addition & 1 deletion api_app/_version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.20.4"
__version__ = "0.21.0"
5 changes: 3 additions & 2 deletions api_app/api/routes/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from api.helpers import get_repository
from db.repositories.workspaces import WorkspaceRepository
from api.routes import health, ping, workspaces, workspace_templates, workspace_service_templates, user_resource_templates, \
shared_services, shared_service_templates, migrations, costs, airlock, operations, metadata
shared_services, shared_service_templates, migrations, costs, airlock, operations, metadata, requests
from core import config
from resources import strings

Expand Down Expand Up @@ -49,6 +49,7 @@
core_router.include_router(migrations.migrations_core_router, tags=["migrations"])
core_router.include_router(costs.costs_core_router, tags=["costs"])
core_router.include_router(costs.costs_workspace_router, tags=["costs"])
core_router.include_router(requests.router, tags=["requests"])

core_swagger_router = APIRouter()
swagger_disabled_router = APIRouter()
Expand Down Expand Up @@ -112,7 +113,7 @@ async def get_disabled_swagger():

def get_scope(workspace) -> str:
# Cope with the fact that scope id can have api:// at the front.
return f"api://{workspace.properties['scope_id'].replace('api://','')}/user_impersonation"
return f"api://{workspace.properties['scope_id'].replace('api://', '')}/user_impersonation"


@workspace_swagger_router.get("/workspaces/{workspace_id}/openapi.json", include_in_schema=False, name="openapi_definitions")
Expand Down
38 changes: 38 additions & 0 deletions api_app/api/routes/requests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from fastapi import APIRouter, Depends, HTTPException, status as status_code
from typing import List, Optional

from api.helpers import get_repository
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

router = APIRouter(dependencies=[Depends(get_current_tre_user_or_tre_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),
airlock_request_repo: AirlockRequestRepository = Depends(get_repository(AirlockRequestRepository)),
airlock_manager: bool = False,
creator_user_id: Optional[str] = None, type: Optional[AirlockRequestType] = None, status: Optional[AirlockRequestStatus] = None,
order_by: Optional[str] = None, order_ascending: bool = True
) -> List[AirlockRequest]:
try:
if not airlock_manager:
requests = await airlock_request_repo.get_airlock_requests(
creator_user_id=creator_user_id or user.id,
type=type,
status=status,
order_by=order_by,
order_ascending=order_ascending,
)
else:
requests = await airlock_request_repo.get_airlock_requests_for_airlock_manager(user)

return requests

except ValueError as ve:
raise HTTPException(status_code=status_code.HTTP_400_BAD_REQUEST, detail=str(ve))
except Exception as e:
raise HTTPException(status_code=status_code.HTTP_500_INTERNAL_SERVER_ERROR, detail=str(e))
49 changes: 39 additions & 10 deletions api_app/db/repositories/airlock_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from azure.cosmos.exceptions import CosmosResourceNotFoundError, CosmosAccessConditionFailedError
from fastapi import HTTPException, status
from pydantic import parse_obj_as
from db.repositories.workspaces import WorkspaceRepository
from services.authentication import get_access_service
from models.domain.authentication import User
from db.errors import EntityDoesNotExist
from models.domain.airlock_request import AirlockFile, AirlockRequest, AirlockRequestStatus, \
Expand Down Expand Up @@ -107,27 +109,33 @@ def create_airlock_request_item(self, airlock_request_input: AirlockRequestInCre

return airlock_request

async def get_airlock_requests(self, workspace_id: str, creator_user_id: Optional[str] = None, type: Optional[AirlockRequestType] = None, status: Optional[AirlockRequestStatus] = None, order_by: Optional[str] = None, order_ascending=True) -> List[AirlockRequest]:
query = self.airlock_requests_query() + f' WHERE c.workspaceId = "{workspace_id}"'
async def get_airlock_requests(self, workspace_id: Optional[str] = None, creator_user_id: Optional[str] = None, type: Optional[AirlockRequestType] = None, status: Optional[AirlockRequestStatus] = None, order_by: Optional[str] = None, order_ascending=True) -> List[AirlockRequest]:
query = self.airlock_requests_query()

# optional filters
conditions = []
parameters = []
if workspace_id:
conditions.append('c.workspaceId=@workspace_id')
parameters.append({"name": "@workspace_id", "value": workspace_id})
if creator_user_id:
query += ' AND c.createdBy.id=@user_id'
conditions.append('c.createdBy.id=@user_id')
parameters.append({"name": "@user_id", "value": creator_user_id})
if status:
query += ' AND c.status=@status'
conditions.append('c.status=@status')
parameters.append({"name": "@status", "value": status})
if type:
query += ' AND c.type=@type'
conditions.append('c.type=@type')
parameters.append({"name": "@type", "value": type})

if conditions:
query += ' WHERE ' + ' AND '.join(conditions)

# optional sorting
if order_by:
query += ' ORDER BY c.' + order_by
query += ' ASC' if order_ascending else ' DESC'

parameters = [
{"name": "@user_id", "value": creator_user_id},
{"name": "@status", "value": status},
{"name": "@type", "value": type},
]
airlock_requests = await self.query(query=query, parameters=parameters)
return parse_obj_as(List[AirlockRequest], airlock_requests)

Expand All @@ -138,6 +146,27 @@ async def get_airlock_request_by_id(self, airlock_request_id: UUID4) -> AirlockR
raise EntityDoesNotExist
return parse_obj_as(AirlockRequest, airlock_requests)

async def get_airlock_requests_for_airlock_manager(self, user: User, type: Optional[AirlockRequestType] = None, status: Optional[AirlockRequestStatus] = None, order_by: Optional[str] = None, order_ascending=True) -> List[AirlockRequest]:
workspace_repo = await WorkspaceRepository.create()
access_service = get_access_service()

workspaces = await workspace_repo.get_active_workspaces()
user_role_assignments = access_service.get_identity_role_assignments(user.id)

valid_roles = {ra.role_id for ra in user_role_assignments}

workspace_ids = [
workspace.id
for workspace in workspaces
if workspace.properties["app_role_id_workspace_airlock_manager"] in valid_roles
]
requests = []

for workspace_id in workspace_ids:
requests += await self.get_airlock_requests(workspace_id=workspace_id, type=type, status=status, order_by=order_by, order_ascending=order_ascending)

return requests

async def update_airlock_request(
self,
original_request: AirlockRequest,
Expand Down
2 changes: 2 additions & 0 deletions api_app/resources/strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
API_UPDATE_USER_RESOURCE = "Update an existing user resource"
API_INVOKE_ACTION_ON_USER_RESOURCE = "Invoke action on a user resource"

API_LIST_REQUESTS = "Get requests"

API_CREATE_AIRLOCK_REQUEST = "Create an airlock request"
API_GET_AIRLOCK_REQUEST = "Get an airlock request"
API_LIST_AIRLOCK_REQUESTS = "Get all airlock requests for a workspace"
Expand Down
42 changes: 42 additions & 0 deletions api_app/tests_ma/test_api/test_routes/test_requests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import pytest
from fastapi import status
from mock import patch

from resources import strings
from services.authentication import get_current_tre_user_or_tre_admin


pytestmark = pytest.mark.asyncio


class TestRequestsThatDontRequireAdminRigths:
@pytest.fixture(autouse=True, scope='class')
def log_in_with_non_admin_user(self, app, non_admin_user):
with patch('services.aad_authentication.AzureADAuthorization._get_user_from_token', return_value=non_admin_user()):
app.dependency_overrides[get_current_tre_user_or_tre_admin] = non_admin_user
yield
app.dependency_overrides = {}

# [GET] /requests/ - get_requests
@patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests", return_value=[])
async def test_get_all_requests_returns_200(self, _, app, client):
response = await client.get(app.url_path_for(strings.API_LIST_REQUESTS))
assert response.status_code == status.HTTP_200_OK

@patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests_for_airlock_manager")
async def test_get_airlock_manager_requests_returns_200(self, mock_get_airlock_requests_for_airlock_manager, app, client):
mock_get_airlock_requests_for_airlock_manager.return_value = []
response = await client.get(app.url_path_for(strings.API_LIST_REQUESTS), params={"airlock_manager": True})

assert response.status_code == status.HTTP_200_OK
mock_get_airlock_requests_for_airlock_manager.assert_called_once()

@patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests", side_effect=Exception("Internal Server Error"))
async def test_get_all_requests_returns_500(self, _, app, client):
response = await client.get(app.url_path_for(strings.API_LIST_REQUESTS))
assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR

@patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests_for_airlock_manager", side_effect=Exception("Internal Server Error"))
async def test_get_airlock_manager_requests_returns_500(self, _, app, client):
response = await client.get(app.url_path_for(strings.API_LIST_REQUESTS), params={"airlock_manager": True})
assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
Loading

0 comments on commit 0265375

Please sign in to comment.