Skip to content

Commit

Permalink
Add filtering to list Airlock requests (#2433)
Browse files Browse the repository at this point in the history
* add query parameters to 'get all airlock requests' endpoint

* add allowed_user_actions to 'get all airlock requests' endpoint that specify which actions the requesting user can make on each request

* fix tests

* move logic from endpoint to helper service and add tests

* add test to verify that any role that can access the 'review' endpoint can also send 'awaiting_my_review' parameter to 'get_airlock_requests_by_user_and_workspace'

* add tests for get_allowed_actions

* update api version

* fix typo and rename method

* update changelog

* rename AirlockRequestInList to AirlockRequestWithAllowedUserActionsInList

* rename get_airlock_requests_by_workspace_id to get_airlock_requests

* rename variables

* update api version

* change endpoint permissions to allow airlock manager to access it

* add role dependency to dependencies

* add get_sample_airlock_request_with_allowed_user_actions method

* add 'submit' airlock actions

* combine airlock action tests
  • Loading branch information
yuvalyaron authored Aug 15, 2022
1 parent 22545c4 commit 2eeab96
Show file tree
Hide file tree
Showing 13 changed files with 173 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ENHANCEMENTS:

* 'CreationTime' field was added to Airlock requests ([#2432](https://github.com/microsoft/AzureTRE/issues/2432))
* Bundles mirror Terraform plugins when built ([#2446](https://github.com/microsoft/AzureTRE/pull/2446))
* 'Get all Airlock requests' endpoint supports filtering ([#2433](https://github.com/microsoft/AzureTRE/pull/2433)).

BUG FIXES:

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.4.11"
__version__ = "0.4.12"
20 changes: 12 additions & 8 deletions api_app/api/routes/airlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
from api.dependencies.database import get_repository
from api.dependencies.workspaces import get_workspace_by_id_from_path, get_deployed_workspace_by_id_from_path
from api.dependencies.airlock import get_airlock_request_by_id_from_path
from models.domain.airlock_request import AirlockRequestStatus
from models.domain.airlock_request import AirlockRequestStatus, AirlockRequestType
from db.repositories.airlock_reviews import AirlockReviewRepository
from models.schemas.airlock_request_url import AirlockRequestTokenInResponse
from models.schemas.airlock_review import AirlockReviewInCreate, AirlockReviewInResponse

from db.repositories.airlock_requests import AirlockRequestRepository
from models.schemas.airlock_request import AirlockRequestInCreate, AirlockRequestInResponse, AirlockRequestInList
from models.schemas.airlock_request import AirlockRequestInCreate, AirlockRequestInResponse, AirlockRequestWithAllowedUserActionsInList
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 .airlock_resource_helpers import save_airlock_review, save_and_publish_event_airlock_request, \
update_status_and_publish_event_airlock_request, RequestAccountDetails
update_status_and_publish_event_airlock_request, RequestAccountDetails, enrich_requests_with_allowed_actions, get_airlock_requests_by_user_and_workspace

from services.airlock import get_storage_management_client, validate_user_allowed_to_access_storage_account, \
get_account_and_rg_by_request, get_airlock_request_container_sas_token, validate_request_status
Expand All @@ -43,18 +43,22 @@ async def create_draft_request(airlock_request_input: AirlockRequestInCreate, us

@airlock_workspace_router.get("/workspaces/{workspace_id}/requests",
status_code=status.HTTP_200_OK,
response_model=AirlockRequestInList,
response_model=AirlockRequestWithAllowedUserActionsInList,
name=strings.API_LIST_AIRLOCK_REQUESTS,
dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
dependencies=[Depends(get_current_workspace_owner_or_researcher_user_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)) -> AirlockRequestInList:
workspace=Depends(get_deployed_workspace_by_id_from_path),
user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager),
creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, awaiting_current_user_review: bool = None) -> AirlockRequestWithAllowedUserActionsInList:
try:
airlock_requests = airlock_request_repo.get_airlock_requests_by_workspace_id(workspace.id)
airlock_requests = get_airlock_requests_by_user_and_workspace(user=user, workspace=workspace, airlock_request_repo=airlock_request_repo,
creator_user_id=creator_user_id, type=type, status=status, awaiting_current_user_review=awaiting_current_user_review)
airlock_requests_with_allowed_user_actions = enrich_requests_with_allowed_actions(airlock_requests, user, airlock_request_repo)
except (ValidationError, ValueError) as e:
logging.error(f"Failed retrieving all the airlock requests for a workspace: {e}")
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
return AirlockRequestInList(airlockRequests=airlock_requests)
return AirlockRequestWithAllowedUserActionsInList(airlockRequests=airlock_requests_with_allowed_user_actions)


@airlock_workspace_router.get("/workspaces/{workspace_id}/requests/{airlock_request_id}", status_code=status.HTTP_200_OK, response_model=AirlockRequestInResponse, name=strings.API_GET_AIRLOCK_REQUEST, dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
Expand Down
41 changes: 40 additions & 1 deletion api_app/api/routes/airlock_resource_helpers.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
from datetime import datetime
from collections import defaultdict
import logging
from typing import List

from fastapi import HTTPException
from starlette import status
from db.repositories.airlock_reviews import AirlockReviewRepository
from models.domain.airlock_review import AirlockReview
from db.repositories.airlock_requests import AirlockRequestRepository
from models.domain.airlock_request import AirlockRequest, AirlockRequestStatus
from models.domain.airlock_request import AirlockActions, AirlockRequest, AirlockRequestStatus, AirlockRequestType
from event_grid.event_sender import send_status_changed_event, send_airlock_notification_event
from models.domain.authentication import User
from models.domain.workspace import Workspace
from models.schemas.airlock_request import AirlockRequestWithAllowedUserActions

from resources import strings
from services.authentication import get_access_service
Expand Down Expand Up @@ -96,3 +98,40 @@ def check_email_exists(role_assignment_details: defaultdict(list)):
if "owner_emails" not in role_assignment_details or not role_assignment_details["owner_emails"]:
logging.error('Creating an airlock request but the workspace owner does not have an email address.')
raise HTTPException(status_code=status.HTTP_417_EXPECTATION_FAILED, detail=strings.AIRLOCK_NO_OWNER_EMAIL)


def get_airlock_requests_by_user_and_workspace(user: User, workspace: Workspace, airlock_request_repo: AirlockRequestRepository,
creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, awaiting_current_user_review: bool = None) -> List[AirlockRequest]:
if awaiting_current_user_review:
if "AirlockManager" not in user.roles:
return []
status = AirlockRequestStatus.InReview

return airlock_request_repo.get_airlock_requests(workspace_id=workspace.id, user_id=creator_user_id, type=type, status=status)


def get_allowed_actions(request: AirlockRequest, user: User, airlock_request_repo: AirlockRequestRepository) -> AirlockRequestWithAllowedUserActions:
allowed_actions = []

can_review_request = airlock_request_repo.validate_status_update(request.status, AirlockRequestStatus.ApprovalInProgress)
can_cancel_request = airlock_request_repo.validate_status_update(request.status, AirlockRequestStatus.Cancelled)
can_submit_request = airlock_request_repo.validate_status_update(request.status, AirlockRequestStatus.Submitted)

if can_review_request and "AirlockManager" in user.roles:
allowed_actions.append(AirlockActions.Review)

if can_cancel_request and ("WorkspaceOwner" in user.roles or "WorkspaceResearcher" in user.roles):
allowed_actions.append(AirlockActions.Cancel)

if can_submit_request and ("WorkspaceOwner" in user.roles or "WorkspaceResearcher" in user.roles):
allowed_actions.append(AirlockActions.Submit)

return allowed_actions


def enrich_requests_with_allowed_actions(requests: List[AirlockRequest], user: User, airlock_request_repo: AirlockRequestRepository) -> List[AirlockRequestWithAllowedUserActions]:
enriched_requests = []
for request in requests:
allowed_actions = get_allowed_actions(request, user, airlock_request_repo)
enriched_requests.append(AirlockRequestWithAllowedUserActions(airlockRequest=request, allowed_user_actions=allowed_actions))
return enriched_requests
24 changes: 19 additions & 5 deletions api_app/db/repositories/airlock_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from models.domain.authentication import User
from db.errors import EntityDoesNotExist
from db.repositories.airlock_resources import AirlockResourceRepository
from models.domain.airlock_request import AirlockRequest, AirlockRequestStatus
from models.domain.airlock_request import AirlockRequest, AirlockRequestStatus, AirlockRequestType
from models.schemas.airlock_request import AirlockRequestInCreate
from resources import strings

Expand All @@ -25,7 +25,7 @@ def __init__(self, client: CosmosClient):
def airlock_requests_query():
return 'SELECT * FROM c WHERE c.resourceType = "airlock-request"'

def _validate_status_update(self, current_status: AirlockRequestStatus, new_status: AirlockRequestStatus):
def validate_status_update(self, current_status: AirlockRequestStatus, new_status: AirlockRequestStatus):
# Cannot change status from approved
approved_condition = current_status != AirlockRequestStatus.Approved
# Cannot change status from rejected
Expand Down Expand Up @@ -74,9 +74,23 @@ def create_airlock_request_item(self, airlock_request_input: AirlockRequestInCre

return airlock_request

def get_airlock_requests_by_workspace_id(self, workspace_id: str) -> List[AirlockRequest]:
def get_airlock_requests(self, workspace_id: str, user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None) -> List[AirlockRequest]:
query = self.airlock_requests_query() + f' AND c.workspaceId = "{workspace_id}"'
airlock_requests = self.query(query=query)

# optional filters
if user_id:
query += ' AND c.user.id=@user_id'
if status:
query += ' AND c.status=@status'
if type:
query += ' AND c.requestType=@type'

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

def get_airlock_request_by_id(self, airlock_request_id: UUID4) -> AirlockRequest:
Expand All @@ -88,7 +102,7 @@ def get_airlock_request_by_id(self, airlock_request_id: UUID4) -> AirlockRequest

def update_airlock_request_status(self, airlock_request: AirlockRequest, new_status: AirlockRequestStatus, user: User, error_message: str = None) -> AirlockRequest:
current_status = airlock_request.status
if self._validate_status_update(current_status, new_status):
if self.validate_status_update(current_status, new_status):
updated_request = copy.deepcopy(airlock_request)
updated_request.status = new_status
if new_status == AirlockRequestStatus.Failed:
Expand Down
4 changes: 2 additions & 2 deletions api_app/db/repositories/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ def _get_container(self, container_name) -> ContainerProxy:
except Exception:
raise UnableToAccessDatabase

def query(self, query: str):
return list(self.container.query_items(query=query, enable_cross_partition_query=True))
def query(self, query: str, parameters: dict = None):
return list(self.container.query_items(query=query, parameters=parameters, enable_cross_partition_query=True))

def read_item_by_id(self, item_id: str) -> dict:
return self.container.read_item(item=item_id, partition_key=item_id)
Expand Down
6 changes: 6 additions & 0 deletions api_app/models/domain/airlock_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ class AirlockRequestType(str, Enum):
Export = strings.AIRLOCK_REQUEST_TYPE_EXPORT


class AirlockActions(str, Enum):
Review = strings.AIRLOCK_ACTION_REVIEW
Cancel = strings.AIRLOCK_ACTION_CANCEL
Submit = strings.AIRLOCK_ACTION_SUBMIT


class AirlockRequest(AirlockResource):
"""
Airlock request
Expand Down
28 changes: 23 additions & 5 deletions api_app/models/schemas/airlock_request.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import uuid
from datetime import datetime
from typing import List
from pydantic import BaseModel, Field
from models.domain.airlock_resource import AirlockResourceType
from models.domain.airlock_request import AirlockRequest, AirlockRequestType
from models.domain.airlock_request import AirlockActions, AirlockRequest, AirlockRequestType


def get_sample_airlock_request(workspace_id: str, airlock_request_id: str) -> dict:
Expand All @@ -18,6 +19,13 @@ def get_sample_airlock_request(workspace_id: str, airlock_request_id: str) -> di
}


def get_sample_airlock_request_with_allowed_user_actions(workspace_id: str) -> dict:
return {
"airlockRequest": get_sample_airlock_request(workspace_id, str(uuid.uuid4())),
"allowed_user_actions": [AirlockActions.Cancel, AirlockActions.Review, AirlockActions.Submit],
}


class AirlockRequestInResponse(BaseModel):
airlockRequest: AirlockRequest

Expand All @@ -29,15 +37,25 @@ class Config:
}


class AirlockRequestInList(BaseModel):
airlockRequests: List[AirlockRequest] = Field([], title="Airlock Requests")
class AirlockRequestWithAllowedUserActions(BaseModel):
airlockRequest: AirlockRequest = Field([], title="Airlock Request")
allowed_user_actions: List[str] = Field([], title="actions that the requesting user can do on the request")

class Config:
schema_extra = {
"example": get_sample_airlock_request_with_allowed_user_actions("933ad738-7265-4b5f-9eae-a1a62928772e"),
}


class AirlockRequestWithAllowedUserActionsInList(BaseModel):
airlockRequests: List[AirlockRequestWithAllowedUserActions] = Field([], title="Airlock Requests")

class Config:
schema_extra = {
"example": {
"airlock_requests": [
get_sample_airlock_request("933ad738-7265-4b5f-9eae-a1a62928772e", "121e921f-a4aa-44b3-90a9-e8da030495ef"),
get_sample_airlock_request("123ad738-1234-4b5f-9eae-a1a62928772e", "457e921f-a4aa-44b3-90a9-e8da030412ac"),
get_sample_airlock_request_with_allowed_user_actions("933ad738-7265-4b5f-9eae-a1a62928772e"),
get_sample_airlock_request_with_allowed_user_actions("933ad738-7265-4b5f-9eae-a1a62928772e")
]
}
}
Expand Down
5 changes: 5 additions & 0 deletions api_app/resources/strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@
AIRLOCK_NO_RESEARCHER_EMAIL = "There are no Workspace Researchers with an email address."
AIRLOCK_NO_OWNER_EMAIL = "There are no Workspace Owners with an email address."

# Airlock Actions
AIRLOCK_ACTION_REVIEW = "review"
AIRLOCK_ACTION_CANCEL = "cancel"
AIRLOCK_ACTION_SUBMIT = "submit"

# Deployments
RESOURCE_STATUS_AWAITING_DEPLOYMENT_MESSAGE = "This resource is waiting to be deployed"
RESOURCE_STATUS_AWAITING_UPDATE_MESSAGE = "This resource is waiting to be updated"
Expand Down
6 changes: 3 additions & 3 deletions api_app/tests_ma/test_api/test_routes/test_airlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def log_in_with_researcher_user(self, app, researcher_user):
app.dependency_overrides = {}

# [GET] /workspaces/{workspace_id}/requests}
@patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests_by_workspace_id", return_value=[])
@patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests", return_value=[])
async def test_get_all_airlock_requests_by_workspace_returns_200(self, _, app, client):
response = await client.get(app.url_path_for(strings.API_LIST_AIRLOCK_REQUESTS, workspace_id=WORKSPACE_ID))
assert response.status_code == status.HTTP_200_OK
Expand Down Expand Up @@ -171,7 +171,7 @@ async def test_post_submit_airlock_request_with_event_grid_not_responding_return
assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE

@patch("api.routes.airlock.AirlockRequestRepository.read_item_by_id", return_value=sample_airlock_request_object())
@patch("api.routes.airlock.AirlockRequestRepository._validate_status_update", return_value=False)
@patch("api.routes.airlock.AirlockRequestRepository.validate_status_update", return_value=False)
async def test_post_submit_airlock_request_with_illegal_status_change_returns_400(self, _, __, app, client):
response = await client.post(app.url_path_for(strings.API_SUBMIT_AIRLOCK_REQUEST, workspace_id=WORKSPACE_ID, airlock_request_id=AIRLOCK_REQUEST_ID))
assert response.status_code == status.HTTP_400_BAD_REQUEST
Expand Down Expand Up @@ -280,7 +280,7 @@ async def test_post_create_airlock_review_with_event_grid_not_responding_returns
@patch("api.routes.airlock.AirlockRequestRepository.read_item_by_id", return_value=sample_airlock_request_object(status=AirlockRequestStatus.InReview))
@patch("api.routes.airlock.AirlockReviewRepository.create_airlock_review_item", return_value=sample_airlock_review_object())
@patch("api.routes.airlock.AirlockReviewRepository.save_item")
@patch("api.routes.airlock.AirlockRequestRepository._validate_status_update", return_value=False)
@patch("api.routes.airlock.AirlockRequestRepository.validate_status_update", return_value=False)
async def test_post_create_airlock_review_with_illegal_status_change_returns_400(self, _, __, ___, ____, app, client, sample_airlock_review_input_data):
response = await client.post(app.url_path_for(strings.API_REVIEW_AIRLOCK_REQUEST, workspace_id=WORKSPACE_ID, airlock_request_id=AIRLOCK_REQUEST_ID), json=sample_airlock_review_input_data)
assert response.status_code == status.HTTP_400_BAD_REQUEST
Loading

0 comments on commit 2eeab96

Please sign in to comment.