Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filtering to list Airlock requests #2433

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
32bf4db
add query parameters to 'get all airlock requests' endpoint
yuvalyaron Aug 9, 2022
f879e8e
add allowed_user_actions to 'get all airlock requests' endpoint that …
yuvalyaron Aug 9, 2022
928985e
Merge branch 'main' of https://github.com/microsoft/AzureTRE into yuv…
yuvalyaron Aug 9, 2022
dcd279b
fix tests
yuvalyaron Aug 10, 2022
14bda58
move logic from endpoint to helper service and add tests
yuvalyaron Aug 10, 2022
da95172
add test to verify that any role that can access the 'review' endpoin…
yuvalyaron Aug 10, 2022
4cb8fcf
add tests for get_allowed_actions
yuvalyaron Aug 10, 2022
cde5a00
Merge branch 'main' of https://github.com/microsoft/AzureTRE into yuv…
yuvalyaron Aug 10, 2022
d1a584a
update api version
yuvalyaron Aug 10, 2022
5efcb14
fix typo and rename method
yuvalyaron Aug 10, 2022
1f7cc7b
update changelog
yuvalyaron Aug 11, 2022
cd3b67f
Merge branch 'main' of https://github.com/microsoft/AzureTRE into yuv…
yuvalyaron Aug 14, 2022
e6b987d
rename AirlockRequestInList to AirlockRequestWithAllowedUserActionsIn…
yuvalyaron Aug 14, 2022
abf1df4
rename get_airlock_requests_by_workspace_id to get_airlock_requests
yuvalyaron Aug 14, 2022
ede57f0
rename variables
yuvalyaron Aug 14, 2022
a39c191
Merge branch 'main' of https://github.com/microsoft/AzureTRE into yuv…
yuvalyaron Aug 14, 2022
1f93593
update api version
yuvalyaron Aug 14, 2022
4064243
change endpoint permissions to allow airlock manager to access it
yuvalyaron Aug 14, 2022
3a88367
add role dependency to dependencies
yuvalyaron Aug 14, 2022
2d4572a
add get_sample_airlock_request_with_allowed_user_actions method
yuvalyaron Aug 14, 2022
ecf7ae1
add 'submit' airlock actions
yuvalyaron Aug 15, 2022
118b7b2
combine airlock action tests
yuvalyaron Aug 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
anatbal marked this conversation as resolved.
Show resolved Hide resolved
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

anatbal marked this conversation as resolved.
Show resolved Hide resolved

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")
anatbal marked this conversation as resolved.
Show resolved Hide resolved
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