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

Create groups for AAD app reg roles #2532

Merged
merged 18 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -38,6 +38,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))
* Add ability to automatically create Azure AD groups for each application role. Requires API version 0.4.30 or later ([#2532](https://github.com/microsoft/AzureTRE/pull/2532))
* Add `is_expsed_externally` option to Azure ML Workspace Service ([#2548](https://github.com/microsoft/AzureTRE/pull2548))
* Azure ML workspace service assigns Azure ML Data Scientist role to Workspace Researchers ([#2539](https://github.com/microsoft/AzureTRE/pull/2539))
* UI is deployed by default ([#2554](https://github.com/microsoft/AzureTRE/pull/2554))
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.32"
__version__ = "0.4.33"
19 changes: 15 additions & 4 deletions devops/scripts/aad/create_application_administrator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,17 @@ fi
msGraphAppId="00000003-0000-0000-c000-000000000000"
msGraphObjectId=$(az ad sp show --id ${msGraphAppId} --query "id" --output tsv --only-show-errors)

applicationPermissionId=$(az ad sp show --id ${msGraphAppId} --query "appRoles[?value=='${applicationPermission}'].id" --output tsv --only-show-errors)
roleApplicationPermission=$(get_msgraph_role "${applicationPermission}")
# split permissions into array
IFS=',' read -ra applicationPermissions <<< "${applicationPermission}"

applicationPermissionIds=()
roleApplicationPermissions=()
for permission in "${applicationPermissions[@]}"; do # access each element of array
applicationPermissionIds+=("$(az ad sp show --id "${msGraphAppId}" --query "appRoles[?value=='${permission}'].id" --output tsv --only-show-errors)")
roleApplicationPermissions+=("$(get_msgraph_role "${permission}")")
done

printf -v roleApplicationPermissionsJson '%s,' "${roleApplicationPermissions[@]}"

appDefinition=$(jq -c . << JSON
{
Expand All @@ -121,7 +130,7 @@ appDefinition=$(jq -c . << JSON
{
"resourceAppId": "${msGraphAppId}",
"resourceAccess": [
${roleApplicationPermission}
${roleApplicationPermissionsJson%,}
]
}]
}
Expand Down Expand Up @@ -153,7 +162,9 @@ spId=$(az ad sp list --filter "appId eq '${appId}'" --query '[0].id' --output ts
if [[ $grantAdminConsent -eq 1 ]]; then
echo "Granting admin consent for '${appName}' app (service principal ID ${spId}) - NOTE: Directory admin privileges required for this step"
wait_for_new_service_principal "${spId}"
grant_admin_consent "${spId}" "$msGraphObjectId" "${applicationPermissionId}"
for applicationPermissionId in "${applicationPermissionIds[@]}"; do # access each element of array
grant_admin_consent "${spId}" "$msGraphObjectId" "${applicationPermissionId}"
done
fi

echo "APPLICATION_ADMIN_CLIENT_ID=\"${appId}\"" > "devops/auth.env"
Expand Down
4 changes: 4 additions & 0 deletions devops/scripts/create_aad_assets.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ if [ "${AUTO_WORKSPACE_APP_REGISTRATION:-}" == true ]; then
APPLICATION_PERMISSION="Application.ReadWrite.All"
fi

if [ "${AUTO_WORKSPACE_GROUP_CREATION:-}" == true ]; then
APPLICATION_PERMISSION="Application.ReadWrite.All,Directory.Read.All,Group.ReadWrite.All"
fi

# Create the identity that is able to administer other applications
"$DIR/aad/create_application_administrator.sh" \
--name "${TRE_ID}" \
Expand Down
1 change: 1 addition & 0 deletions docs/tre-admins/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The automation utilises a `make` command, which reads a few environment variable
|AAD_TENANT_ID|The tenant id of where your AAD identities will be placed. This can be different to the tenant where your Azure resources are created.|
| LOCATION | Where your Azure assets will be provisioned (eg. westeurope). This is used to add a redirect URI from the Swagger UI to the API Application.
|AUTO_WORKSPACE_APP_REGISTRATION| Default of `false`. Setting this to true grants the `Application.ReadWrite.All` permission to the *Application Admin* identity. This identity is used to manage other AAD applications that it owns, e.g. Workspaces. If you do not set this, the identity will have `Application.ReadWrite.OwnedBy`. Further information can be found [here](./identities/application_admin.md).
|AUTO_WORKSPACE_GROUP_CREATION| Default of `false`. Setting this to true grants the `Directory.Read.All` and `Group.ReadWrite.All` permission to the *Application Admin* identity. This identity can then create security groups aligned to each applciation role. Active Directory licencing implications need to be considered as Group assignment is a [premium feature](https://docs.microsoft.com/en-us/azure/architecture/multitenant-identity/app-roles#roles-using-azure-ad-app-roles).

## Create Authentication assets
You can build all of the Identity assets by running the following at the command line
Expand Down
4 changes: 3 additions & 1 deletion docs/tre-admins/identities/application_admin.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ This application does not have any roles defined.
| --- | -- | -----| --------- |
| Application.ReadWrite.OwnedBy | Application | Yes | This user has `Application.ReadWrite.OwnedBy` as a minimum permission for it to function. If the tenant is managed by a customer administrator, then this user must be added to the **Owners** of every workspace that is created. This will allow TRE to manage the AAD Application. This will be a manual process for the Tenant Admin. |
| Application.ReadWrite.All | Application | Yes | If the AAD Administrator has delegated AAD administrative operations to the TRE, then this user should be granted `Application.ReadWrite.All`. This will allow the user to create workspace applications and administer any applications in the tenant. There will be no need for the Tenant Admin to oversee the Tenant. |
| Directory.Read.All | Application | Yes | This permission is required to read User details from Azure Active Directory. This is requried if Azure AD groups are to be created automatically by the TRE. |
| Group.ReadWrite.All | Application | Yes | This permission is required to create and update Azure AD groups. This is requried if Azure AD groups are to be created automatically by the TRE. |

'*' See the difference between [delegated and application permission](https://docs.microsoft.com/graph/auth/auth-concepts#delegated-and-application-permissions) types. See [Microsoft Graph permissions reference](https://docs.microsoft.com/graph/permissions-reference) for more details.

Expand All @@ -26,7 +28,7 @@ This user is currently only used from the Porter bundles hosted on the Resource
| -------- | ----------- |
| `--name` | This is used to put a friendly name to the Application that can be seen in the portal. It is typical to use the name of your TRE instance. |
| `--admin-consent` | If you have the appropriate permission to grant admin consent, then pass in this argument. If you do not, you will have to ask an AAD Admin to consent after you have created the identity. Consent is required for this permission.
| `--application-permission` | This should be either `Application.ReadWrite.All` or `Application.ReadWrite.OwnedBy` |
| `--application-permission` | This is a comma seperated list of the permissions that need to be assigned. For exampler `Application.ReadWrite.All,Directory.Read.All,Group.ReadWrite.All` |
| `--reset-password` | Optional, default is 0. When run in a headless fashion, 1 is passed in to always reset the password. |

## Environment Variables
Expand Down
3 changes: 3 additions & 0 deletions templates/core/.env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ AAD_TENANT_ID=__CHANGE_ME__
# When this is false, the AAD Application will need creating manually.
AUTO_WORKSPACE_APP_REGISTRATION=true

# Setting AUTO_WORKSPACE_GROUP_CREATION to true will create an identity with `Directory.Read.All` and `Group.ReadWrite.All`
AUTO_WORKSPACE_GROUP_CREATION=false

# If your local machine/build agent cannot get the public IP
# address from https://ipecho.net/plain, then you can circumvent
# this by setting this Environment variable. This blockage can
Expand Down
2 changes: 1 addition & 1 deletion templates/core/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.4.24"
__version__ = "0.4.25"
1 change: 1 addition & 0 deletions templates/workspaces/base/.env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ AUTH_TENANT_ID="__CHANGE_ME__"

# These are passed in if Terraform will create the Workspace AAD Application
REGISTER_AAD_APPLICATION=true
CREATE_AAD_GROUPS=true
AUTH_CLIENT_ID="__CHANGE_ME__"
AUTH_CLIENT_SECRET="__CHANGE_ME__"
WORKSPACE_OWNER_OBJECT_ID="__CHANGE_ME__"
Expand Down
6 changes: 6 additions & 0 deletions templates/workspaces/base/parameters.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@
"env": "REGISTER_AAD_APPLICATION"
}
},
{
"name": "create_aad_groups",
"source": {
"env": "CREATE_AAD_GROUPS"
}
},
{
"name": "client_id",
"source": {
Expand Down
8 changes: 7 additions & 1 deletion templates/workspaces/base/porter.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: tre-workspace-base
version: 0.3.28
version: 0.3.30
marrobi marked this conversation as resolved.
Show resolved Hide resolved
description: "A base Azure TRE workspace"
dockerfile: Dockerfile.tmpl
registry: azuretre
Expand Down Expand Up @@ -59,6 +59,10 @@ parameters:
type: boolean
default: false
description: "Whether this bundle should register the workspace in AAD"
- name: create_aad_groups
type: boolean
default: false
description: "Whether this bundle should create AAD groups for the workspace app roles"
- name: workspace_owner_object_id
type: string
description: "The object id of the user that will be granted WorkspaceOwner after it is created."
Expand Down Expand Up @@ -148,6 +152,7 @@ install:
shared_storage_quota: "{{ bundle.parameters.shared_storage_quota }}"
enable_local_debugging: "{{ bundle.parameters.enable_local_debugging }}"
register_aad_application: "{{ bundle.parameters.register_aad_application }}"
create_aad_groups: "{{ bundle.parameters.create_aad_groups }}"
auth_client_id: "{{ bundle.credentials.auth_client_id }}"
auth_client_secret: "{{ bundle.credentials.auth_client_secret }}"
auth_tenant_id: "{{ bundle.credentials.auth_tenant_id }}"
Expand Down Expand Up @@ -241,6 +246,7 @@ uninstall:
shared_storage_quota: "{{ bundle.parameters.shared_storage_quota }}"
enable_local_debugging: "{{ bundle.parameters.enable_local_debugging }}"
register_aad_application: "{{ bundle.parameters.register_aad_application }}"
create_aad_groups: "{{ bundle.parameters.create_aad_groups }}"
auth_client_id: "{{ bundle.credentials.auth_client_id }}"
auth_client_secret: "{{ bundle.credentials.auth_client_secret }}"
auth_tenant_id: "{{ bundle.credentials.auth_tenant_id }}"
Expand Down
6 changes: 6 additions & 0 deletions templates/workspaces/base/template_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@
"default": true,
"title": "Enable Airlock",
"description": "If enabled, allows imports and exports for the workspace."
},
"create_aad_groups": {
"type": "boolean",
"title": "Create AAD Groups for each workspace role",
"description": "Create AAD Groups for the workspace roles, requires `auto_create`. If this is set to true, the workspace will create new AAD Groups.",
"default": false
}
},
"uiSchema": {
Expand Down
48 changes: 48 additions & 0 deletions templates/workspaces/base/terraform/aad/aad.tf
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,51 @@ resource "azuread_app_role_assignment" "workspace_owner" {
principal_object_id = var.workspace_owner_object_id
resource_object_id = azuread_service_principal.workspace.object_id
}

resource "azuread_group" "workspace_owners" {
count = var.create_aad_groups ? 1 : 0
display_name = "${var.workspace_resource_name_suffix} Workspace Owners"
owners = [var.workspace_owner_object_id]
security_enabled = true
}

resource "azuread_group" "workspace_researchers" {
count = var.create_aad_groups ? 1 : 0
display_name = "${var.workspace_resource_name_suffix} Workspace Researchers"
owners = [var.workspace_owner_object_id]
security_enabled = true
}

resource "azuread_group" "workspace_airlock_managers" {
count = var.create_aad_groups ? 1 : 0
display_name = "${var.workspace_resource_name_suffix} Airlock Managers"
owners = [var.workspace_owner_object_id]
security_enabled = true
}

resource "azuread_group_member" "workspace_owner" {
count = var.create_aad_groups ? 1 : 0
group_object_id = azuread_group.workspace_owners[count.index].id
member_object_id = var.workspace_owner_object_id
}

resource "azuread_app_role_assignment" "workspace_owners_group" {
count = var.create_aad_groups ? 1 : 0
app_role_id = azuread_service_principal.workspace.app_role_ids["WorkspaceOwner"]
principal_object_id = azuread_group.workspace_owners[count.index].id
resource_object_id = azuread_service_principal.workspace.object_id
}

resource "azuread_app_role_assignment" "workspace_researchers_group" {
count = var.create_aad_groups ? 1 : 0
app_role_id = azuread_service_principal.workspace.app_role_ids["WorkspaceResearcher"]
principal_object_id = azuread_group.workspace_researchers[count.index].id
resource_object_id = azuread_service_principal.workspace.object_id
}

resource "azuread_app_role_assignment" "workspace_airlock_managers_group" {
count = var.create_aad_groups ? 1 : 0
app_role_id = azuread_service_principal.workspace.app_role_ids["AirlockManager"]
principal_object_id = azuread_group.workspace_airlock_managers[count.index].id
resource_object_id = azuread_service_principal.workspace.object_id
}
1 change: 1 addition & 0 deletions templates/workspaces/base/terraform/aad/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ variable "tre_workspace_tags" {}
variable "aad_redirect_uris_b64" {
type = string # list of objects like [{"name": "my uri 1", "value": "https://..."}, {}]
}
variable "create_aad_groups" {}
6 changes: 6 additions & 0 deletions templates/workspaces/base/terraform/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ variable "register_aad_application" {
description = "Create an AAD application automatically for the Workspace."
}

variable "create_aad_groups" {
type = bool
default = false
description = "Create AAD groups automatically for the Workspace Application Roles."
}

variable "enable_airlock" {
type = bool
description = "Controls the deployment of Airlock resources in the workspace."
Expand Down
2 changes: 2 additions & 0 deletions templates/workspaces/base/terraform/workspace.tf
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ module "aad" {
workspace_resource_name_suffix = local.workspace_resource_name_suffix
workspace_owner_object_id = var.workspace_owner_object_id
aad_redirect_uris_b64 = var.aad_redirect_uris_b64
create_aad_groups = var.create_aad_groups

depends_on = [
azurerm_key_vault_access_policy.deployer,
azurerm_key_vault_access_policy.resource_processor,
Expand Down