Merge branch 'guybartal/ci-multiple-lines' of https://github.com/micr…

…osoft/AzureTRE into guybartal/ci-multiple-lines

.devcontainer/devcontainer.json

@@ -275,6 +275,7 @@
       "extensions": [
         "ms-python.python",
         "ms-python.pylance",
+        "ms-python.flake8",
         "hashicorp.terraform",
         "github.vscode-pull-request-github",
         "gitHub.copilot",

CHANGELOG.md

@@ -5,8 +5,15 @@
 
 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))
+
+
 
 COMPONENTS:
 
@@ -74,6 +81,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 |

airlock_processor/_version.py

@@ -1 +1 @@
-__version__ = "0.8.2"
+__version__ = "0.8.3"

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.20.4"
+__version__ = "0.21.0"

api_app/api/routes/api.py

@@ -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
 
@@ -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()
@@ -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")

api_app/api/routes/requests.py

@@ -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))

api_app/db/repositories/airlock_requests.py

@@ -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, \
@@ -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)
 
@@ -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,

api_app/resources/strings.py

@@ -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"

api_app/tests_ma/test_api/test_routes/test_requests.py

@@ -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