Skip to content

Commit

Permalink
Merge branch 'tamirkamara/2490-ms-graph-paging' of https://github.com…
Browse files Browse the repository at this point in the history
…/microsoft/AzureTRE into tamirkamara/2490-ms-graph-paging
  • Loading branch information
tamirkamara committed Aug 23, 2022
2 parents 583888a + 206f206 commit ce3366d
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 49 deletions.
12 changes: 6 additions & 6 deletions api_app/api/routes/airlock_resource_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ async def save_and_publish_event_airlock_request(airlock_request: AirlockRequest
try:
logging.debug(f"Sending status changed event for airlock request item: {airlock_request.id}")
await send_status_changed_event(airlock_request)
await send_airlock_notification_event(airlock_request, role_assignment_details["researcher_emails"], role_assignment_details["owner_emails"])
await send_airlock_notification_event(airlock_request, role_assignment_details)
except Exception as e:
airlock_request_repo.delete_item(airlock_request.id)
logging.error(f"Failed sending status_changed message: {e}")
Expand All @@ -58,7 +58,7 @@ async def update_and_publish_event_airlock_request(airlock_request: AirlockReque
await send_status_changed_event(updated_airlock_request)
access_service = get_access_service()
role_assignment_details = access_service.get_workspace_role_assignment_details(workspace)
await send_airlock_notification_event(updated_airlock_request, role_assignment_details["researcher_emails"], role_assignment_details["owner_emails"])
await send_airlock_notification_event(updated_airlock_request, role_assignment_details)
return updated_airlock_request
except Exception as e:
logging.error(f"Failed sending status_changed message: {e}")
Expand All @@ -70,12 +70,12 @@ def get_timestamp() -> float:


def check_email_exists(role_assignment_details: defaultdict(list)):
if "researcher_emails" not in role_assignment_details or not role_assignment_details["researcher_emails"]:
if "WorkspaceResearcher" not in role_assignment_details or not role_assignment_details["WorkspaceResearcher"]:
logging.error('Creating an airlock request but the researcher does not have an email address.')
raise HTTPException(status_code=status.HTTP_417_EXPECTATION_FAILED, detail=strings.AIRLOCK_NO_RESEARCHER_EMAIL)
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)
if "AirlockManager" not in role_assignment_details or not role_assignment_details["AirlockManager"]:
logging.error('Creating an airlock request but the airlock manager does not have an email address.')
raise HTTPException(status_code=status.HTTP_417_EXPECTATION_FAILED, detail=strings.AIRLOCK_NO_AIRLOCK_MANAGER_EMAIL)


def get_airlock_requests_by_user_and_workspace(user: User, workspace: Workspace, airlock_request_repo: AirlockRequestRepository,
Expand Down
7 changes: 5 additions & 2 deletions api_app/event_grid/event_sender.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import logging
import re
from typing import Dict
from azure.eventgrid import EventGridEvent
from models.domain.events import StatusChangedData, AirlockNotificationData
from event_grid.helpers import publish_event
Expand All @@ -22,14 +24,15 @@ async def send_status_changed_event(airlock_request: AirlockRequest):
await publish_event(status_changed_event, config.EVENT_GRID_STATUS_CHANGED_TOPIC_ENDPOINT)


async def send_airlock_notification_event(airlock_request: AirlockRequest, researchers_emails, owners_emails):
async def send_airlock_notification_event(airlock_request: AirlockRequest, emails: Dict):
request_id = airlock_request.id
status = airlock_request.status.value
short_workspace_id = airlock_request.workspaceId[-4:]
snake_case_emails = {re.sub(r'(?<!^)(?=[A-Z])', '_', role_name).lower(): role_id for role_name, role_id in emails.items()}

airlock_notification = EventGridEvent(
event_type="airlockNotification",
data=AirlockNotificationData(request_id=request_id, event_type="status_changed", event_value=status, researchers_emails=researchers_emails, owners_emails=owners_emails, workspace_id=short_workspace_id).__dict__,
data=AirlockNotificationData(request_id=request_id, event_type="status_changed", event_value=status, emails=snake_case_emails, workspace_id=short_workspace_id).__dict__,
subject=f"{request_id}/airlockNotification",
data_version="2.0"
)
Expand Down
5 changes: 2 additions & 3 deletions api_app/models/domain/events.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
from typing import List
from typing import Dict
from models.domain.azuretremodel import AzureTREModel


class AirlockNotificationData(AzureTREModel):
request_id: str
event_type: str
event_value: str
researchers_emails: List[str]
owners_emails: List[str]
emails: Dict
workspace_id: str


Expand Down
2 changes: 1 addition & 1 deletion api_app/resources/strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
AIRLOCK_UNAUTHORIZED_TO_SA = "User is unauthorized to access airlock request files in its current status."
AIRLOCK_NOT_ENABLED_IN_WORKSPACE = "Airlock is not enabled in this workspace."
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_NO_AIRLOCK_MANAGER_EMAIL = "There are no Airlock Managers with an email address."

# Airlock Actions
AIRLOCK_ACTION_REVIEW = "review"
Expand Down
19 changes: 12 additions & 7 deletions api_app/services/aad_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,9 @@ def _get_user_emails_with_role_asssignment(self, client_id):
return roles_graph_data, users_graph_data

def get_workspace_role_assignment_details(self, workspace: Workspace):
researcher_app_role_id = workspace.properties["app_role_id_workspace_researcher"]
owner_app_role_id = workspace.properties["app_role_id_workspace_owner"]
app_role_ids = {role_name: workspace.properties[role_id] for role_name, role_id in self.WORKSPACE_ROLES_DICT.items()}
inverted_app_role_ids = {role_id: role_name for role_name, role_id in app_role_ids.items()}

sp_id = workspace.properties["sp_id"]
roles_graph_data, users_graph_data = self._get_user_emails_with_role_asssignment(sp_id)
user_emails = {}
Expand All @@ -235,11 +236,15 @@ def get_workspace_role_assignment_details(self, workspace: Workspace):

workspace_role_assignments_details = defaultdict(list)
for role_assignment in roles_graph_data["value"]:
if role_assignment["principalType"] == "User" and role_assignment["principalId"] in user_emails:
if role_assignment["appRoleId"] == researcher_app_role_id:
workspace_role_assignments_details["researcher_emails"].append(user_emails[role_assignment["principalId"]])
elif role_assignment["appRoleId"] == owner_app_role_id:
workspace_role_assignments_details["owner_emails"].append(user_emails[role_assignment["principalId"]])
principal_id = role_assignment["principalId"]
principal_type = role_assignment["principalType"]

if principal_type == "User" and principal_id in user_emails:
app_role_id = role_assignment["appRoleId"]
app_role_name = inverted_app_role_ids[app_role_id]

if app_role_name:
workspace_role_assignments_details[app_role_name].append(user_emails[principal_id])

return workspace_role_assignments_details

Expand Down
2 changes: 1 addition & 1 deletion api_app/tests_ma/test_api/test_routes/test_airlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def log_in_with_researcher_user(self, app, researcher_user):
patch("api.routes.workspaces.OperationRepository.resource_has_deployed_operation"), \
patch("api.routes.airlock.AirlockRequestRepository.save_item"), \
patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id"), \
patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]}):
patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]}):
yield
app.dependency_overrides = {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def sample_status_changed_event(status="draft"):
def sample_airlock_notification_event(status="draft"):
status_changed_event = EventGridEvent(
event_type="airlockNotification",
data=AirlockNotificationData(request_id=AIRLOCK_REQUEST_ID, event_type="status_changed", event_value=status, researchers_emails=['researcher@outlook.com'], owners_emails=['owner@outlook.com'], workspace_id=WORKSPACE_ID[-4:]).__dict__,
data=AirlockNotificationData(request_id=AIRLOCK_REQUEST_ID, event_type="status_changed", event_value=status, emails={"workspace_researcher": ["researcher@outlook.com"], "workspace_owner": ["owner@outlook.com"], "airlock_manager": ["manager@outlook.com"]}, workspace_id=WORKSPACE_ID[-4:]).__dict__,
subject=f"{AIRLOCK_REQUEST_ID}/airlockNotification",
data_version="2.0"
)
Expand All @@ -77,7 +77,7 @@ def get_required_roles(endpoint):


@patch("event_grid.helpers.EventGridPublisherClient", return_value=AsyncMock())
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]})
async def test_save_and_publish_event_airlock_request_saves_item(_, event_grid_publisher_client_mock,
airlock_request_repo_mock):
airlock_request_mock = sample_airlock_request()
Expand All @@ -103,7 +103,7 @@ async def test_save_and_publish_event_airlock_request_saves_item(_, event_grid_p
assert actual_airlock_notification_event.data == airlock_notification_event_mock.data


@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]})
async def test_save_and_publish_event_airlock_request_raises_503_if_save_to_db_fails(_, airlock_request_repo_mock):
airlock_request_mock = sample_airlock_request()
airlock_request_repo_mock.save_item = MagicMock(side_effect=Exception)
Expand All @@ -118,7 +118,7 @@ async def test_save_and_publish_event_airlock_request_raises_503_if_save_to_db_f


@patch("event_grid.helpers.EventGridPublisherClient", return_value=AsyncMock())
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]})
async def test_save_and_publish_event_airlock_request_raises_503_if_publish_event_fails(_, event_grid_publisher_client_mock,
airlock_request_repo_mock):
airlock_request_mock = sample_airlock_request()
Expand All @@ -138,10 +138,10 @@ async def test_save_and_publish_event_airlock_request_raises_503_if_publish_even


@pytest.mark.parametrize('email_mock_return', [{},
{"owner_emails": ["owner@outlook.com"]},
{"researcher_emails": [], "owner_emails": ["owner@outlook.com"]},
{"researcher_emails": ["researcher@outlook.com"], "owner_emails": []},
{"researcher_emails": ["researcher@outlook.com"]}])
{"AirlockManager": ["owner@outlook.com"]},
{"WorkspaceResearcher": [], "AirlockManager": ["owner@outlook.com"]},
{"WorkspaceResearcher": ["researcher@outlook.com"], "owner_emails": []},
{"WorkspaceResearcher": ["researcher@outlook.com"]}])
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details")
async def test_save_and_publish_event_airlock_request_raises_417_if_email_not_present(get_workspace_role_assignment_details_patched, email_mock_return):

Expand All @@ -158,7 +158,7 @@ async def test_save_and_publish_event_airlock_request_raises_417_if_email_not_pr


@patch("event_grid.helpers.EventGridPublisherClient", return_value=AsyncMock())
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]})
async def test_update_and_publish_event_airlock_request_updates_item(_, event_grid_publisher_client_mock,
airlock_request_repo_mock):
airlock_request_mock = sample_airlock_request()
Expand Down Expand Up @@ -187,7 +187,7 @@ async def test_update_and_publish_event_airlock_request_updates_item(_, event_gr
assert actual_airlock_notification_event.data == airlock_notification_event_mock.data


@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]})
async def test_update_and_publish_event_airlock_request_raises_400_if_status_update_invalid(_, airlock_request_repo_mock):
airlock_request_mock = sample_airlock_request()

Expand All @@ -203,7 +203,7 @@ async def test_update_and_publish_event_airlock_request_raises_400_if_status_upd


@patch("event_grid.helpers.EventGridPublisherClient", return_value=AsyncMock())
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]})
async def test_update_and_publish_event_airlock_request_raises_503_if_publish_event_fails(_, event_grid_publisher_client_mock,
airlock_request_repo_mock):
airlock_request_mock = sample_airlock_request()
Expand Down
34 changes: 34 additions & 0 deletions devops/scripts/list_versions.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash
set -o errexit
set -o pipefail
set -o nounset
# set -o xtrace

function template_version () {
version=$(yq eval ".version" "$1")
name=$(yq eval ".name" "$1")
echo -e "| $name | $version |"
}

function component_version () {
version_line=$(cat "$2")

# doesn't work with quotes
# shellcheck disable=SC2206
version_array=( ${version_line//=/ } ) # split by =
version="${version_array[1]//\"}" # second element is what we want, remove " chars
echo -e "| $1 | $version |"
}

echo -e "| name | version |\n| ----- | ----- |"

component_version "devops" "devops/version.txt"
component_version "core" "templates/core/version.txt"

find . -type f -name "porter.yaml" -not -path "*/.cnab/*" -print0 | sort | while read -r -d $'\0' file
do
template_version "$file"
done



20 changes: 20 additions & 0 deletions docs/tre-developers/release.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# How to release an AzureTRE version

A release is created when enough changes have been made and the main branch is stable enough.

The process follows these steps:

1. Update `CHANGELOG.md` in a PR with the following:
1. Rename the top-most verion noted as unreleaed with the version number that makes sense. Note that you don't have to keep the one that is currently in the file as the version number chosen should reflect the changes made (major, minor, etc.)
1. Create a new section for the next-unreleaed version so that future changes will be placed there.
1. Run `devops/scripts/list_versions.sh` and include the output in the change log for the version you're about the release
1. Merge the PR
1. Create a GitHub Release
<!-- markdownlint-disable-next-line MD034 -->
1. Go to https://github.com/microsoft/AzureTRE/releases/new
1. Click on `Choose a tag` and type a new one for you version. It should be in the form of `v0.9.2` - note the "v" in the begining.
1. The release title should be just the version number "0.9.2" in the example above.
1. Copy the text from the CHANGELOG.md file and paste in the release description.
1. Include a final line with a link to the full changelog similar to this:
<!-- markdownlint-disable-next-line MD034 -->
**Full Changelog**: https://github.com/microsoft/AzureTRE/compare/v0.9.1...v0.9.2
2 changes: 2 additions & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ nav:
- GitHub Actions: tre-admins/setup-instructions/workflows.md
- Azure DevOps: coming-soon.md

- Releases: tre-developers/release.md

- Troubleshooting FAQ: # General Troubleshooting Section for Development
- troubleshooting-faq/index.md
- Enabling Debugging for the API: troubleshooting-faq/debug-api.md
Expand Down
Loading

0 comments on commit ce3366d

Please sign in to comment.