From 87c4077f849e3e63704d687bc698e34966811255 Mon Sep 17 00:00:00 2001 From: marrobi Date: Sat, 3 Sep 2022 13:03:17 +0000 Subject: [PATCH 1/6] AML roles --- .../workspace_services/azureml/porter.yaml | 17 ++++++++++++ .../azureml/terraform/get_app_role_members.sh | 12 +++++++++ .../azureml/terraform/roles.tf | 27 +++++++++++++++++++ .../azureml/terraform/variables.tf | 13 +++++++++ 4 files changed, 69 insertions(+) create mode 100644 templates/workspace_services/azureml/terraform/get_app_role_members.sh create mode 100644 templates/workspace_services/azureml/terraform/roles.tf diff --git a/templates/workspace_services/azureml/porter.yaml b/templates/workspace_services/azureml/porter.yaml index 65312294c3..d34921cf61 100644 --- a/templates/workspace_services/azureml/porter.yaml +++ b/templates/workspace_services/azureml/porter.yaml @@ -6,6 +6,14 @@ registry: azuretre dockerfile: Dockerfile.tmpl credentials: + # Credentials for interacting with the AAD Auth tenant + - name: auth_client_id + env: AUTH_CLIENT_ID + - name: auth_client_secret + env: AUTH_CLIENT_SECRET + - name: auth_tenant_id + env: AUTH_TENANT_ID + # Credentials for interacting with Azure - name: azure_tenant_id env: ARM_TENANT_ID - name: azure_subscription_id @@ -92,6 +100,9 @@ install: arm_client_id: "{{ bundle.credentials.azure_client_id }}" arm_client_secret: "{{ bundle.credentials.azure_client_secret }}" arm_use_msi: "{{ bundle.parameters.arm_use_msi }}" + auth_client_id: "{{ bundle.credentials.auth_client_id }}" + auth_client_secret: "{{ bundle.credentials.auth_client_secret }}" + auth_tenant_id: "{{ bundle.credentials.auth_tenant_id }}" backendConfig: resource_group_name: "{{ bundle.parameters.tfstate_resource_group_name }}" storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" @@ -118,6 +129,9 @@ upgrade: arm_client_id: "{{ bundle.credentials.azure_client_id }}" arm_client_secret: "{{ bundle.credentials.azure_client_secret }}" arm_use_msi: "{{ bundle.parameters.arm_use_msi }}" + auth_client_id: "{{ bundle.credentials.auth_client_id }}" + auth_client_secret: "{{ bundle.credentials.auth_client_secret }}" + auth_tenant_id: "{{ bundle.credentials.auth_tenant_id }}" backendConfig: resource_group_name: "{{ bundle.parameters.tfstate_resource_group_name }}" storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" @@ -144,6 +158,9 @@ uninstall: arm_tenant_id: "{{ bundle.credentials.azure_tenant_id }}" arm_client_id: "{{ bundle.credentials.azure_client_id }}" arm_client_secret: "{{ bundle.credentials.azure_client_secret }}" + auth_client_id: "{{ bundle.credentials.auth_client_id }}" + auth_client_secret: "{{ bundle.credentials.auth_client_secret }}" + auth_tenant_id: "{{ bundle.credentials.auth_tenant_id }}" backendConfig: resource_group_name: "{{ bundle.parameters.tfstate_resource_group_name }}" storage_account_name: "{{ bundle.parameters.tfstate_storage_account_name }}" diff --git a/templates/workspace_services/azureml/terraform/get_app_role_members.sh b/templates/workspace_services/azureml/terraform/get_app_role_members.sh new file mode 100644 index 0000000000..7e456b5e9b --- /dev/null +++ b/templates/workspace_services/azureml/terraform/get_app_role_members.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +set -euo pipefail + +eval "$(jq -r '@sh "AUTH_CLIENT_ID=\(.auth_client_id) AUTH_CLIENT_SECRET=\(.auth_client_secret) AUTH_TENANT_ID=\(.auth_tenant_id)" WORSKPACE_CLIENT_ID=\(.workspace_client_id)"')" + +az login --allow-no-subscriptions --service-principal --username "$AUTH_CLIENT_ID" --password "$AUTH_CLIENT_SECRET" --tenant "$AUTH_TENANT_ID" + +spId=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals?\$filter=appid eq '${WORSKPACE_CLIENT_ID}'" -o json | jq -r '.value[0].id') +principals=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals/${spId}/appRoleAssignedTo?\$select=principalId" -o json | jq -r '.value[].principalId') + +jq -n --arg principals "$principals" '{"principals":$principals}' diff --git a/templates/workspace_services/azureml/terraform/roles.tf b/templates/workspace_services/azureml/terraform/roles.tf new file mode 100644 index 0000000000..7bb740a848 --- /dev/null +++ b/templates/workspace_services/azureml/terraform/roles.tf @@ -0,0 +1,27 @@ + +data "azurerm_key_vault_secret" "workspace_client_id" { + name = "workspace-client-id" + key_vault_id = data.azurerm_key_vault.ws.id +} + +data "external" "app_role_members" { + program = ["bash", "${path.module}/get_app_role_members.sh"] + + query = { + auth_client_id = var.auth_client_id + auth_client_secret = var.auth_client_secret + auth_tenant_id = var.auth_tenant_id + workspace_client_id = data.azurerm_key_vault_secret.workspace_client_id.id + } +} + +data "azurerm_role_definition" "azure_ml_data_scientist" { + name = "AzureML Data Scientist" +} + +resource "azurerm_role_assignment" "app_role_members_aml_data_sceintist" { + for_each = split("\n",data.external.app_role_members.result) + scope = data.external.app_role_members.id + role_definition_id = data.azurerm_role_definition.azure_ml_data_scientist.id + principal_id = each.value +} diff --git a/templates/workspace_services/azureml/terraform/variables.tf b/templates/workspace_services/azureml/terraform/variables.tf index 042f237a72..2269674339 100644 --- a/templates/workspace_services/azureml/terraform/variables.tf +++ b/templates/workspace_services/azureml/terraform/variables.tf @@ -9,3 +9,16 @@ variable "arm_client_id" {} variable "arm_client_secret" {} variable "display_name" {} variable "description" {} + +variable "auth_tenant_id" { + type = string + description = "Used to authenticate into the AAD Tenant to get app role members" +} +variable "auth_client_id" { + type = string + description = "Used to authenticate into the AAD Tenant to get app role members" +} +variable "auth_client_secret" { + type = string + description = "Used to authenticate into the AAD Tenant to get app role members" +} From 919bcee623f8cf1bdb46d3ec450579f1160cfb67 Mon Sep 17 00:00:00 2001 From: marrobi Date: Sat, 3 Sep 2022 13:31:30 +0000 Subject: [PATCH 2/6] Filter to just WorkspaceResearcher role --- .../azureml/terraform/get_app_role_members.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/templates/workspace_services/azureml/terraform/get_app_role_members.sh b/templates/workspace_services/azureml/terraform/get_app_role_members.sh index 7e456b5e9b..c178212596 100644 --- a/templates/workspace_services/azureml/terraform/get_app_role_members.sh +++ b/templates/workspace_services/azureml/terraform/get_app_role_members.sh @@ -6,7 +6,12 @@ eval "$(jq -r '@sh "AUTH_CLIENT_ID=\(.auth_client_id) AUTH_CLIENT_SECRET=\(.auth az login --allow-no-subscriptions --service-principal --username "$AUTH_CLIENT_ID" --password "$AUTH_CLIENT_SECRET" --tenant "$AUTH_TENANT_ID" -spId=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals?\$filter=appid eq '${WORSKPACE_CLIENT_ID}'" -o json | jq -r '.value[0].id') -principals=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals/${spId}/appRoleAssignedTo?\$select=principalId" -o json | jq -r '.value[].principalId') +# get the service principal object id +sp=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals?\$filter=appid eq '${WORSKPACE_CLIENT_ID}'" -o json) +spId=$(echo "$sp" | jq -r '.value[0].id') + +# filter to the Workspace Researcher Role +workspaceResearcherRoleId=$(echo "$sp" | jq -r '.value[0].appRoles[] | select(.value == "WorkspaceResearcher") | .id') +principals=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals/${spId}/appRoleAssignedTo" -o json | jq -r --arg workspaceResearcherRoleId "${workspaceResearcherRoleId}" '.value[] | select(.appRoleId == $workspaceResearcherRoleId) | .principalId') jq -n --arg principals "$principals" '{"principals":$principals}' From e2340d68d83175c569c87f92df6d4630358aed6f Mon Sep 17 00:00:00 2001 From: marrobi Date: Sat, 3 Sep 2022 13:33:22 +0000 Subject: [PATCH 3/6] add principals key --- templates/workspace_services/azureml/terraform/roles.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/workspace_services/azureml/terraform/roles.tf b/templates/workspace_services/azureml/terraform/roles.tf index 7bb740a848..859dd16167 100644 --- a/templates/workspace_services/azureml/terraform/roles.tf +++ b/templates/workspace_services/azureml/terraform/roles.tf @@ -20,7 +20,7 @@ data "azurerm_role_definition" "azure_ml_data_scientist" { } resource "azurerm_role_assignment" "app_role_members_aml_data_sceintist" { - for_each = split("\n",data.external.app_role_members.result) + for_each = split("\n",data.external.app_role_members.result.principals) scope = data.external.app_role_members.id role_definition_id = data.azurerm_role_definition.azure_ml_data_scientist.id principal_id = each.value From e127319185089d7159085842ec3b215d640df919 Mon Sep 17 00:00:00 2001 From: marrobi Date: Sat, 3 Sep 2022 13:41:58 +0000 Subject: [PATCH 4/6] fix linting --- .../workspace_services/azureml/terraform/roles.tf | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/workspace_services/azureml/terraform/roles.tf b/templates/workspace_services/azureml/terraform/roles.tf index 859dd16167..9962d68e21 100644 --- a/templates/workspace_services/azureml/terraform/roles.tf +++ b/templates/workspace_services/azureml/terraform/roles.tf @@ -8,9 +8,9 @@ data "external" "app_role_members" { program = ["bash", "${path.module}/get_app_role_members.sh"] query = { - auth_client_id = var.auth_client_id - auth_client_secret = var.auth_client_secret - auth_tenant_id = var.auth_tenant_id + auth_client_id = var.auth_client_id + auth_client_secret = var.auth_client_secret + auth_tenant_id = var.auth_tenant_id workspace_client_id = data.azurerm_key_vault_secret.workspace_client_id.id } } @@ -20,8 +20,8 @@ data "azurerm_role_definition" "azure_ml_data_scientist" { } resource "azurerm_role_assignment" "app_role_members_aml_data_sceintist" { - for_each = split("\n",data.external.app_role_members.result.principals) - scope = data.external.app_role_members.id + for_each = split("\n", data.external.app_role_members.result.principals) + scope = data.external.app_role_members.id role_definition_id = data.azurerm_role_definition.azure_ml_data_scientist.id - principal_id = each.value + principal_id = each.value } From 6861e7ce87fd8b9ae2d9473b4ca3f856c7ff472e Mon Sep 17 00:00:00 2001 From: marrobi Date: Mon, 5 Sep 2022 14:29:18 +0000 Subject: [PATCH 5/6] Fix issues, succesfully deploys. --- templates/workspace_services/azureml/porter.yaml | 2 +- .../azureml/terraform/get_app_role_members.sh | 4 ++-- templates/workspace_services/azureml/terraform/roles.tf | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) mode change 100644 => 100755 templates/workspace_services/azureml/terraform/get_app_role_members.sh diff --git a/templates/workspace_services/azureml/porter.yaml b/templates/workspace_services/azureml/porter.yaml index d34921cf61..3d9f9fdb3c 100644 --- a/templates/workspace_services/azureml/porter.yaml +++ b/templates/workspace_services/azureml/porter.yaml @@ -1,6 +1,6 @@ --- name: tre-service-azureml -version: 0.4.4 +version: 0.4.5 description: "An Azure TRE service for Azure Machine Learning" registry: azuretre dockerfile: Dockerfile.tmpl diff --git a/templates/workspace_services/azureml/terraform/get_app_role_members.sh b/templates/workspace_services/azureml/terraform/get_app_role_members.sh old mode 100644 new mode 100755 index c178212596..9de90ca735 --- a/templates/workspace_services/azureml/terraform/get_app_role_members.sh +++ b/templates/workspace_services/azureml/terraform/get_app_role_members.sh @@ -2,9 +2,9 @@ set -euo pipefail -eval "$(jq -r '@sh "AUTH_CLIENT_ID=\(.auth_client_id) AUTH_CLIENT_SECRET=\(.auth_client_secret) AUTH_TENANT_ID=\(.auth_tenant_id)" WORSKPACE_CLIENT_ID=\(.workspace_client_id)"')" +eval "$(jq -r '@sh "AUTH_CLIENT_ID=\(.auth_client_id) AUTH_CLIENT_SECRET=\(.auth_client_secret) AUTH_TENANT_ID=\(.auth_tenant_id) WORSKPACE_CLIENT_ID=\(.workspace_client_id)"')" -az login --allow-no-subscriptions --service-principal --username "$AUTH_CLIENT_ID" --password "$AUTH_CLIENT_SECRET" --tenant "$AUTH_TENANT_ID" +az login --allow-no-subscriptions --service-principal --username "$AUTH_CLIENT_ID" --password "$AUTH_CLIENT_SECRET" --tenant "$AUTH_TENANT_ID" > /dev/null # get the service principal object id sp=$(az rest --method GET --uri "https://graph.microsoft.com/v1.0/serviceprincipals?\$filter=appid eq '${WORSKPACE_CLIENT_ID}'" -o json) diff --git a/templates/workspace_services/azureml/terraform/roles.tf b/templates/workspace_services/azureml/terraform/roles.tf index 9962d68e21..6c32067682 100644 --- a/templates/workspace_services/azureml/terraform/roles.tf +++ b/templates/workspace_services/azureml/terraform/roles.tf @@ -11,7 +11,7 @@ data "external" "app_role_members" { auth_client_id = var.auth_client_id auth_client_secret = var.auth_client_secret auth_tenant_id = var.auth_tenant_id - workspace_client_id = data.azurerm_key_vault_secret.workspace_client_id.id + workspace_client_id = data.azurerm_key_vault_secret.workspace_client_id.value } } @@ -19,9 +19,9 @@ data "azurerm_role_definition" "azure_ml_data_scientist" { name = "AzureML Data Scientist" } -resource "azurerm_role_assignment" "app_role_members_aml_data_sceintist" { - for_each = split("\n", data.external.app_role_members.result.principals) - scope = data.external.app_role_members.id +resource "azurerm_role_assignment" "app_role_members_aml_data_scientist" { + for_each = toset(split("\n", data.external.app_role_members.result.principals)) + scope = azapi_resource.aml_workspace.id role_definition_id = data.azurerm_role_definition.azure_ml_data_scientist.id principal_id = each.value } From 5476b7ded8ed6ce43f60d4082675e0d7202076dd Mon Sep 17 00:00:00 2001 From: marrobi Date: Mon, 5 Sep 2022 15:11:17 +0000 Subject: [PATCH 6/6] update docs and changelog --- CHANGELOG.md | 1 + docs/tre-templates/workspace-services/azure-ml.md | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 612e30cb65..0de9eb0767 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ ENHANCEMENTS: * Airlock requests contain a field with information about the files that were submitted ([#2504](https://github.com/microsoft/AzureTRE/pull/2504)) * UI - Operations and notifications stability improvements ([[#2530](https://github.com/microsoft/AzureTRE/pull/2530)) * UI - Initial implemetation of Workspace Airlock Request View ([#2512](https://github.com/microsoft/AzureTRE/pull/2512)) +* Azure ML workspace service assigns Azure ML Data Scientist role to Workspace Researchers ([#2539](https://github.com/microsoft/AzureTRE/pull/2539)) BUG FIXES: diff --git a/docs/tre-templates/workspace-services/azure-ml.md b/docs/tre-templates/workspace-services/azure-ml.md index d8836ec802..540ed6cfa3 100644 --- a/docs/tre-templates/workspace-services/azure-ml.md +++ b/docs/tre-templates/workspace-services/azure-ml.md @@ -6,6 +6,8 @@ This service installs the following resources into an existing virtual network w ![Azure Machine Learning Service](images/aml_service.png) +Any users with the role of `Workspace Researcher` will be assigned the `AzureML Data Scientist` role within the AML workspace. + ## Firewall Rules Please be aware that the following outbound Firewall rules are opened for the workspace when this service is deployed, including to Azure Storage. This does open the possibility to extract data from a workspace if the user is determined to do so. Work is ongoing to remove some of these requirements: