From 880f7c2788a81d6003a9b0d4e7d46d8dcf31a12a Mon Sep 17 00:00:00 2001 From: Matthew Fortunka Date: Thu, 6 Feb 2025 15:01:50 +0000 Subject: [PATCH 01/48] Added User Management (WIP) --- api_app/api/dependencies/database.py | 6 +- api_app/api/routes/workspaces.py | 53 ++++- api_app/core/config.py | 1 - api_app/models/domain/authentication.py | 15 ++ api_app/models/schemas/roles.py | 7 + api_app/models/schemas/users.py | 5 +- api_app/resources/strings.py | 4 + api_app/services/aad_authentication.py | 185 +++++++++++++++++- api_app/services/access_service.py | 2 + .../components/workspaces/WorkspaceUsers.tsx | 177 +++++++++++++---- .../workspaces/WorkspaceUsersAssignNew.tsx | 183 +++++++++++++++++ ui/app/src/models/apiEndpoints.ts | 4 +- 12 files changed, 592 insertions(+), 50 deletions(-) create mode 100644 api_app/models/schemas/roles.py create mode 100644 ui/app/src/components/workspaces/WorkspaceUsersAssignNew.tsx diff --git a/api_app/api/dependencies/database.py b/api_app/api/dependencies/database.py index 7bfc89ff22..d80e0e5faf 100644 --- a/api_app/api/dependencies/database.py +++ b/api_app/api/dependencies/database.py @@ -1,7 +1,7 @@ from azure.cosmos.aio import CosmosClient, DatabaseProxy, ContainerProxy from azure.mgmt.cosmosdb.aio import CosmosDBManagementClient -from core.config import MANAGED_IDENTITY_CLIENT_ID, STATE_STORE_ENDPOINT, STATE_STORE_KEY, STATE_STORE_SSL_VERIFY, SUBSCRIPTION_ID, RESOURCE_MANAGER_ENDPOINT, CREDENTIAL_SCOPES, RESOURCE_GROUP_NAME, COSMOSDB_ACCOUNT_NAME, STATE_STORE_DATABASE +from core.config import MANAGED_IDENTITY_CLIENT_ID, STATE_STORE_ENDPOINT, STATE_STORE_KEY, STATE_STORE_SSL_VERIFY, SUBSCRIPTION_ID, RESOURCE_MANAGER_ENDPOINT, CREDENTIAL_SCOPES, RESOURCE_GROUP_NAME, COSMOSDB_ACCOUNT_NAME, STATE_STORE_DATABASE,ENABLE_LOCAL_DEBUGGING from core.credentials import get_credential_async from services.logging import logger @@ -28,8 +28,8 @@ async def _connect_to_db(cls) -> CosmosClient: logger.debug(f"Connecting to {STATE_STORE_ENDPOINT}") credential = await get_credential_async() - if MANAGED_IDENTITY_CLIENT_ID: - logger.debug("Connecting with managed identity") + if MANAGED_IDENTITY_CLIENT_ID or ENABLE_LOCAL_DEBUGGING: + logger.debug("Connecting with AAD") cosmos_client = CosmosClient( url=STATE_STORE_ENDPOINT, credential=credential diff --git a/api_app/api/routes/workspaces.py b/api_app/api/routes/workspaces.py index e1d049c3f0..8db18f42a8 100644 --- a/api_app/api/routes/workspaces.py +++ b/api_app/api/routes/workspaces.py @@ -21,7 +21,8 @@ from models.schemas.workspace_service import WorkspaceServiceInCreate, WorkspaceServicesInList, WorkspaceServiceInResponse from models.schemas.resource import ResourceHistoryInList, ResourcePatch from models.schemas.resource_template import ResourceTemplateInformationInList -from models.schemas.users import UsersInResponse +from models.schemas.users import UsersInResponse, AssignableUsersInResponse +from models.schemas.roles import RolesInResponse from resources import strings from services.access_service import AuthConfigValidationError from services.authentication import get_current_admin_user, \ @@ -543,3 +544,53 @@ async def retrieve_user_resource_operations_by_user_resource_id_and_operation_id async def retrieve_user_resource_history_by_user_resource_id(user_resource=Depends(get_user_resource_by_id_from_path), user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager), resource_history_repo=Depends(get_repository(ResourceHistoryRepository))) -> ResourceHistoryInList: validate_user_has_valid_role_for_user_resource(user, user_resource) return ResourceHistoryInList(resource_history=await resource_history_repo.get_resource_history_by_resource_id(resource_id=user_resource.id)) + +@workspaces_shared_router.get("/workspaces/{workspace_id}/assignable-users", response_model=AssignableUsersInResponse, name=strings.API_GET_ASSIGNABLE_USERS) +async def get_assignable_users(workspace=Depends(get_workspace_by_id_from_path)) -> AssignableUsersInResponse: + access_service = get_access_service() + assignable_users = access_service.get_assignable_users() + return AssignableUsersInResponse(assignable_users=assignable_users) + + +@workspaces_shared_router.get("/workspaces/{workspace_id}/roles", response_model=RolesInResponse, name=strings.API_GET_WORKSPACE_ROLES) +async def get_workspace_roles(workspace=Depends(get_workspace_by_id_from_path)) -> RolesInResponse: + access_service = get_access_service() + roles = access_service.get_workspace_roles(workspace) + return RolesInResponse(roles=roles) + +@workspaces_shared_router.post("/workspaces/{workspace_id}/users/assign", status_code=status.HTTP_202_ACCEPTED, name=strings.API_ASSIGN_WORKSPACE_USER) +async def assign_workspace_user(response: Response, user_email: str, role_name: str, workspace=Depends(get_workspace_by_id_from_path)) -> UsersInResponse: + access_service = get_access_service() + + user = access_service.get_user_by_email(user_email) + role = access_service.get_workspace_role_by_name(role_name, workspace) + + access_service.assign_workspace_user( + user, + workspace, + role, + ) + + users = access_service.get_workspace_users(workspace) + return UsersInResponse(users=users) + +@workspaces_shared_router.delete("/workspaces/{workspace_id}/users/assign", + status_code=status.HTTP_202_ACCEPTED, + name=strings.API_REMOVE_WORKSPACE_USER_ASSIGNMENT) +async def remove_workspace_user_assignment(user_email: str, + role_name: str, + workspace=Depends(get_workspace_by_id_from_path)) -> UsersInResponse: + + access_service = get_access_service() + + user = access_service.get_user_by_email(user_email) + role = access_service.get_workspace_role_by_name(role_name, workspace) + + access_service.remove_workspace_role_user_assignment( + user, + role, + workspace + ) + + users = access_service.get_workspace_users(workspace) + return UsersInResponse(users=users) diff --git a/api_app/core/config.py b/api_app/core/config.py index e34608f1c6..34f753beb5 100644 --- a/api_app/core/config.py +++ b/api_app/core/config.py @@ -36,7 +36,6 @@ SUBSCRIPTION_ID: str = config("SUBSCRIPTION_ID", default="") RESOURCE_GROUP_NAME: str = config("RESOURCE_GROUP_NAME", default="") - # Service bus configuration SERVICE_BUS_FULLY_QUALIFIED_NAMESPACE: str = config("SERVICE_BUS_FULLY_QUALIFIED_NAMESPACE", default="") SERVICE_BUS_RESOURCE_REQUEST_QUEUE: str = config("SERVICE_BUS_RESOURCE_REQUEST_QUEUE", default="") diff --git a/api_app/models/domain/authentication.py b/api_app/models/domain/authentication.py index 3011440c7f..143d3dbf1f 100644 --- a/api_app/models/domain/authentication.py +++ b/api_app/models/domain/authentication.py @@ -13,3 +13,18 @@ class User(BaseModel): email: str = Field(None) roles: List[str] = Field([]) roleAssignments: List[RoleAssignment] = Field([]) + +class AssignableUser(BaseModel): + name: str + email: str + +class Role(BaseModel): + id: str + value: str + isEnabled: bool + email: str = Field(None) + allowedMemberTypes: List[str] = Field([]) + description: str + displayName: str + origin: str + roleAssignments: List[RoleAssignment] = Field([]) diff --git a/api_app/models/schemas/roles.py b/api_app/models/schemas/roles.py new file mode 100644 index 0000000000..ede73c1a34 --- /dev/null +++ b/api_app/models/schemas/roles.py @@ -0,0 +1,7 @@ +from pydantic import BaseModel, Field +from typing import List + +from models.domain.authentication import Role + +class RolesInResponse(BaseModel): + roles: List[Role] = Field(..., title="Roles", description="List of roles in a workspace") diff --git a/api_app/models/schemas/users.py b/api_app/models/schemas/users.py index 56c8025e56..ae6108b690 100644 --- a/api_app/models/schemas/users.py +++ b/api_app/models/schemas/users.py @@ -1,7 +1,7 @@ from pydantic import BaseModel, Field from typing import List -from models.domain.authentication import User +from models.domain.authentication import User, AssignableUser class UsersInResponse(BaseModel): @@ -26,3 +26,6 @@ class Config: ] } } + +class AssignableUsersInResponse(BaseModel): + assignable_users: List[AssignableUser] = Field(..., title="Assignable Users", description="List of users assignable to a workspace") diff --git a/api_app/resources/strings.py b/api_app/resources/strings.py index a9184cf289..7431da86a7 100644 --- a/api_app/resources/strings.py +++ b/api_app/resources/strings.py @@ -16,6 +16,10 @@ API_INVOKE_ACTION_ON_WORKSPACE = "Invoke action on a workspace" API_GET_WORKSPACE_USERS = "Get all users for a workspace" +API_GET_ASSIGNABLE_USERS = "Get all users assignable to a workspace" +API_GET_WORKSPACE_ROLES = "Get all the roles belonging to a workspace" +API_ASSIGN_WORKSPACE_USER = "Assign a user to a workspace role" +API_REMOVE_WORKSPACE_USER_ASSIGNMENT = "Remove a user from a workspace role" API_GET_ALL_WORKSPACE_SERVICES = "Get all workspace services for workspace" API_GET_WORKSPACE_SERVICE_BY_ID = "Get workspace service by Id" diff --git a/api_app/services/aad_authentication.py b/api_app/services/aad_authentication.py index d99e194fdf..47004e1485 100644 --- a/api_app/services/aad_authentication.py +++ b/api_app/services/aad_authentication.py @@ -1,17 +1,19 @@ import base64 +import json from collections import defaultdict from enum import Enum from typing import List, Optional import jwt import requests +import urllib.parse from fastapi import Request, HTTPException, status from msal import ConfidentialClientApplication -from services.access_service import AccessService, AuthConfigValidationError +from services.access_service import AccessService, AuthConfigValidationError, UserRoleAssignmentError from core import config -from db.errors import EntityDoesNotExist -from models.domain.authentication import User, RoleAssignment +from db.errors import DuplicateEntity, EntityDoesNotExist +from models.domain.authentication import User, AssignableUser, Role, RoleAssignment from models.domain.workspace import Workspace, WorkspaceRole from resources import strings from db.repositories.workspaces import WorkspaceRepository @@ -37,7 +39,7 @@ class AzureADAuthorization(AccessService): require_one_of_roles = None aad_instance = config.AAD_AUTHORITY_URL - TRE_CORE_ROLES = ['TREAdmin', 'TREUser'] + TRE_CORE_ROLES = ['TREAdmin', 'TREUser', 'TREAirlockAutomation'] WORKSPACE_ROLES_DICT = {'WorkspaceOwner': 'app_role_id_workspace_owner', 'WorkspaceResearcher': 'app_role_id_workspace_researcher', 'AirlockManager': 'app_role_id_workspace_airlock_manager'} def __init__(self, auto_error: bool = True, require_one_of_roles: Optional[list] = None): @@ -227,11 +229,11 @@ def _get_batch_endpoint() -> str: @staticmethod def _get_users_endpoint(user_object_id) -> str: - return "/users/" + user_object_id + "?$select=displayName,mail,id" + return "/users/" + user_object_id + "?$select=displayName,mail,id,userPrincipalName" @staticmethod def _get_group_members_endpoint(group_object_id) -> str: - return "/groups/" + group_object_id + "/transitiveMembers?$select=displayName,mail,id" + return "/groups/" + group_object_id + "/transitiveMembers?$select=displayName,mail,id,userPrincipalName" def _get_app_sp_graph_data(self, client_id: str) -> dict: msgraph_token = self._get_msgraph_token() @@ -278,10 +280,15 @@ def _get_users_inc_groups_from_response(self, users_graph_data, roles_graph_data user_name = user_data["body"]["displayName"] if "users" in user_data["body"]["@odata.context"]: - user_email = user_data["body"]["mail"] + user_email = user_data["body"]["userPrincipalName"] # if user with id does not already exist in users + user_roles=self._get_roles_for_principal(user_id, roles_graph_data, app_id_to_role_name) if not any(user.id == user_id for user in users): - users.append(User(id=user_id, name=user_name, email=user_email, roles=self._get_roles_for_principal(user_id, roles_graph_data, app_id_to_role_name))) + users.append(User(id=user_id, name=user_name, email=user_email, roles=user_roles)) + else: + user = next((user for user in users if user.id == user_id), None) + user.roles = list(set(user.roles + user_roles)) + # Handle group endpoint response elif "directoryObjects" in user_data["body"]["@odata.context"]: @@ -289,10 +296,14 @@ def _get_users_inc_groups_from_response(self, users_graph_data, roles_graph_data for group_member in user_data["body"]["value"]: user_id = group_member["id"] user_name = group_member["displayName"] - user_email = group_member["mail"] + user_email = group_member["userPrincipalName"] + group_roles=self._get_roles_for_principal(group_id, roles_graph_data, app_id_to_role_name) if not any(user.id == user_id for user in users): - users.append(User(id=user_id, name=user_name, email=user_email, roles=self._get_roles_for_principal(group_id, roles_graph_data, app_id_to_role_name))) + users.append(User(id=user_id, name=user_name, email=user_email, roles=group_roles)) + else: + user = next((user for user in users if user.id == user_id), None) + user.roles = list(set(user.roles + group_roles)) return users @@ -306,6 +317,36 @@ def get_workspace_users(self, workspace: Workspace) -> List[User]: return users_inc_groups + def get_assignable_users(self) -> List[AssignableUser]: + msgraph_token = self._get_msgraph_token() + users_endpoint = f"{MICROSOFT_GRAPH_URL}/v1.0/users/" + graph_data = requests.get(users_endpoint, headers=self._get_auth_header(msgraph_token)).json() + + result = [] + + for user_data in graph_data["value"]: + result.append( + AssignableUser(name=user_data["displayName"], email=user_data["userPrincipalName"]) + ) + + return result + + def get_workspace_roles(self, workspace: Workspace) -> List[Role]: + app_roles_endpoint = f"{MICROSOFT_GRAPH_URL}/v1.0/servicePrincipals/{workspace.properties['sp_id']}/appRoles" + graph_data = self._ms_graph_query(app_roles_endpoint, "GET") + + roles = [] + + for role in graph_data["value"]: + roles.append(Role(id=role["id"], value=role["value"], + isEnabled=role["isEnabled"], + description=role["description"], + displayName=role["displayName"], + origin=role["origin"], + allowedMemberTypes=role["allowedMemberTypes"])) + + return roles + def get_workspace_user_emails_by_role_assignment(self, workspace: Workspace): users = self.get_workspace_users(workspace) workspace_role_assignments_details = {} @@ -317,6 +358,130 @@ def get_workspace_user_emails_by_role_assignment(self, workspace: Workspace): workspace_role_assignments_details[role].append(user.email) return workspace_role_assignments_details + def get_workspace_role_by_name(self, name: str, workspace: Workspace) -> Role: + app_roles_endpoint = f"{MICROSOFT_GRAPH_URL}/v1.0/servicePrincipals/{workspace.properties['sp_id']}/appRoles" + graph_data = self._ms_graph_query(app_roles_endpoint, "GET") + + for role in graph_data["value"]: + if role["value"] == name: + return Role(id=role["id"], value=role["value"], + isEnabled=role["isEnabled"], + description=role["description"], + displayName=role["displayName"], + origin=role["origin"], + allowedMemberTypes=role["allowedMemberTypes"]) + + return None + + def get_user_by_email(self, user_email: str) -> User: + encoded_email = urllib.parse.quote(user_email) + user_by_email_endpoint = f"{MICROSOFT_GRAPH_URL}/v1.0/users/{encoded_email}" + user_data = self._ms_graph_query(user_by_email_endpoint, "GET") + + return User(id=user_data["id"][0], name=user_data["displayName"][0], email=user_email, roles=[]) + + def assign_workspace_user(self, user: User, workspace: Workspace, role: Role) -> None: + # User already has the role, do nothing + if self._is_user_in_role(user, role): + return + if self._is_workspace_role_group_in_use(workspace): + return self._assign_workspace_user_to_application_group(user, workspace, role) + else: + return self._assign_workspace_user_to_application(user, workspace, role) + + def _is_user_in_role(self, user: User, role: Role) -> bool: + return any(r for r in user.roles if r == role.value) + + def _is_workspace_role_group_in_use(self, workspace: Workspace) -> bool: + msgraph_token = self._get_msgraph_token() + application_id = workspace.properties["sp_id"] + url = self._get_service_principal_assigned_roles_endpoint(application_id) + response = requests.get(url, headers=self._get_auth_header(msgraph_token)).json() + + return any (item for item in response["value"] if item["principalType"] == PrincipalType.Group.value) + + def _assign_workspace_user_to_application_group(self, user: User, workspace: Workspace, role: Role): + msgraph_token = self._get_msgraph_token() + roles_graph_data = self._get_user_role_assignments(workspace.properties["sp_id"], msgraph_token) + sp_graph_data = self._get_app_sp_graph_data(workspace.properties["client_id"]) + app_id_to_role_name = {app_role["id"]: app_role["value"] for app_role in sp_graph_data["value"][0]["appRoles"]} + + for group in (item for item in roles_graph_data["value"] if item["principalType"] == PrincipalType.Group.value): + group_id = group["principalId"] + group_roles=self._get_roles_for_principal(group_id, roles_graph_data, app_id_to_role_name) + if role.value in group_roles: + # Assign user to this group + self._add_user_to_group(user.id, group_id) + return + raise UserRoleAssignmentError(f"Unable to assign user to group with role: {role.value}") + + def _remove_workspace_user_from_application_group(self, user: User, workspace: Workspace, role: Role): + msgraph_token = self._get_msgraph_token() + roles_graph_data = self._get_user_role_assignments(workspace.properties["sp_id"], msgraph_token) + sp_graph_data = self._get_app_sp_graph_data(workspace.properties["client_id"]) + app_id_to_role_name = {app_role["id"]: app_role["value"] for app_role in sp_graph_data["value"][0]["appRoles"]} + + for group in (item for item in roles_graph_data["value"] if item["principalType"] == PrincipalType.Group.value): + group_id = group["principalId"] + group_roles=self._get_roles_for_principal(group_id, roles_graph_data, app_id_to_role_name) + if role.value in group_roles: + # Assign user to this group + self._remove_user_from_group(user.id, group_id) + return + raise UserRoleAssignmentError(f"Unable to assign user to group with role: {role.value}") + + def _add_user_to_group(self, user_id: str, group_id: str): + msgraph_token = self._get_msgraph_token() + url = f"{MICROSOFT_GRAPH_URL}/v1.0/groups/{group_id}/members/$ref" + body = { + "@odata.id": f"{MICROSOFT_GRAPH_URL}/v1.0/users/{user_id}" + } + response = requests.post(url, json=body, headers=self._get_auth_header(msgraph_token)) + return response + + def _remove_user_from_group(self, user_id: str, group_id: str): + msgraph_token = self._get_msgraph_token() + url = f"{MICROSOFT_GRAPH_URL}/v1.0/groups/{group_id}/members/{user_id}/$ref" + + response = requests.delete(url, headers=self._get_auth_header(msgraph_token)) + return response + + def _assign_workspace_user_to_application(self, user: User, workspace: Workspace, role: Role): + msgraph_token = self._get_msgraph_token() + url = f"{MICROSOFT_GRAPH_URL}/v1.0/users/{user.id}/appRoleAssignments" + application_id = workspace.properties["sp_id"] + + body = { + "principalId": user.id, + "resourceId": application_id, + "appRoleId": role.id, + } + + response = requests.post(url, json=body, headers=self._get_auth_header(msgraph_token)) + return response + + def _get_role_assignment_for_user(self, user_id: str, role_id: str) -> dict: + user_role_assignments = self._get_role_assignment_graph_data_for_user(user_id) + for role in user_role_assignments["value"]: + if role["appRoleId"] == role_id: + return role + + def remove_workspace_role_user_assignment(self, user: User, + role: Role, + workspace: Workspace) -> None: + msgraph_token = self._get_msgraph_token() + + # Get the role assignment id for the role we want to remove + role_assignment = self._get_role_assignment_for_user(user.id, role.id) + + if self._is_workspace_role_group_in_use(workspace): + self._remove_workspace_user_from_application_group(user, workspace, role) + else: + # Remove the role assignment directly + url = f"{MICROSOFT_GRAPH_URL}/v1.0/users/{user.id}/appRoleAssignments/{role_assignment['id']}" + response = requests.delete(url, headers=self._get_auth_header(msgraph_token)) + return response + def _get_batch_users_by_role_assignments_body(self, roles_graph_data): request_body = {"requests": []} met_principal_ids = set() diff --git a/api_app/services/access_service.py b/api_app/services/access_service.py index 5c19126177..070996e337 100644 --- a/api_app/services/access_service.py +++ b/api_app/services/access_service.py @@ -9,6 +9,8 @@ class AuthConfigValidationError(Exception): """Raised when the input auth information is invalid""" +class UserRoleAssignmentError(Exception): + """Raised when a user role assignment fails""" class AccessService(OAuth2AuthorizationCodeBearer): @abstractmethod diff --git a/ui/app/src/components/workspaces/WorkspaceUsers.tsx b/ui/app/src/components/workspaces/WorkspaceUsers.tsx index 9bdde2f745..b01498a0ae 100644 --- a/ui/app/src/components/workspaces/WorkspaceUsers.tsx +++ b/ui/app/src/components/workspaces/WorkspaceUsers.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { useState, useCallback, useEffect, useMemo, useContext } from 'react'; import { GroupedList, IGroup } from '@fluentui/react/lib/GroupedList'; import { IColumn, DetailsRow } from '@fluentui/react/lib/DetailsList'; -import { SelectionMode } from '@fluentui/react/lib/Selection'; +import { SelectionMode, Selection, SelectionZone } from '@fluentui/react/lib/Selection'; import { Persona, PersonaSize } from '@fluentui/react/lib/Persona'; import { HttpMethod, useAuthApiCall } from '../../hooks/useAuthApiCall'; import { APIError } from '../../models/exceptions'; @@ -11,10 +11,16 @@ import { ApiEndpoint } from '../../models/apiEndpoints'; import { LoadingState } from '../../models/loadingState'; import { ExceptionLayout } from '../shared/ExceptionLayout'; import { User } from '../../models/user'; -import { Stack } from '@fluentui/react'; +import { AppRolesContext } from '../../contexts/AppRolesContext'; + +import { CommandBarButton, DefaultButton, Dialog, DialogFooter, getTheme, Spinner, SpinnerSize, Stack } from '@fluentui/react'; +import { useNavigate, Route, Routes } from 'react-router-dom'; +import { destructiveButtonStyles } from '../../styles'; +import { WorkSpaceUsersAssignNew } from './WorkspaceUsersAssignNew'; interface IUser { id: string; + key: string; name: string; email: string; role: string; @@ -28,6 +34,21 @@ export const WorkspaceUsers: React.FunctionComponent = () => { loadingState: LoadingState.Loading, }); + const [selectedUserRole, setSelectedUserRole] = useState(undefined); + const [hideCancelDialog, setHideCancelDialog] = useState(true); + const [deassigning, setDeassigning] = useState(false); + const [deassignmentError, setDeassignmentError] = useState(false); + const [apiError, setApiError] = useState({} as APIError); + + const appRolesCtx = useContext(AppRolesContext); + const [isTreAdmin, setIsTreAdmin] = useState(false); + + useEffect(() => { + setIsTreAdmin(appRolesCtx.roles.includes('TREAdmin')); + }, [appRolesCtx.roles]); + + const navigate = useNavigate(); + const theme = getTheme(); const apiCall = useAuthApiCall(); const { workspace, roles, workspaceApplicationIdURI } = useContext(WorkspaceContext); @@ -55,32 +76,75 @@ export const WorkspaceUsers: React.FunctionComponent = () => { } }, [apiCall, workspace.id, roles.length, workspaceApplicationIdURI]); + const addedAssignment = async () => { + navigate(-1); + await getUsers(); + } + useEffect(() => { getUsers(); }, [getUsers]); - const groupedUsers = useMemo(() => { - const groups: { [key: string]: IUser[] } = {}; + // De-assign user from role + const deassignUser = useCallback(async () => { + try { + setDeassigning(true); + + const encodedUser=selectedUserRole?.email?.replaceAll('#', '%23'); + const selectedRole = selectedUserRole?.role; + await apiCall(`${ApiEndpoint.Workspaces}/${workspace.id}/${ApiEndpoint.Users}/assign?user_email=${encodedUser}&role_name=${selectedRole}`, + HttpMethod.Delete, workspaceApplicationIdURI); + + await getUsers(); + + + setSelectedUserRole(undefined); + setHideCancelDialog(true); + + } catch (err: any) { + err.userMessage = 'Error deassigning user'; + setApiError(err); + setDeassignmentError(true); + } + }, [apiCall, workspaceApplicationIdURI,selectedUserRole, workspace.id, getUsers]); + + const groups: IGroup[] = useMemo(() => { + const groupMap: any = {}; + const groups: any = []; + let currentIndex = 0; + state.users.forEach(user => { - if (!groups[user.role]) { - groups[user.role] = []; - } - groups[user.role].push(user); + if (!groupMap[user.role]) { + groupMap[user.role] = { + count: 0, + key: user.role.replace(/\s+/g, '').toLowerCase(), + name: user.role, + startIndex: currentIndex, + level: 0 + }; + + groups.push(groupMap[user.role]); + } + + groupMap[user.role].count += 1; + currentIndex += 1; }); + + console.log("new groups", groups) return groups; }, [state.users]); - const groups: IGroup[] = useMemo(() => { - return Object.keys(groupedUsers).map((role, index) => ({ - key: role, - name: role, - startIndex: index, - count: groupedUsers[role].length, - })); - }, [groupedUsers]); - + const selection = useMemo(() => { + const s = new Selection({ + onSelectionChanged: () => { + setSelectedUserRole(s.getSelection()[0] as IUser); + } + }); + s.setItems(state.users, true); + return s; + }, [state.users]); - const columns: IColumn[] = [ + const columns: IColumn[] = useMemo(() => [ { key: 'name', name: 'Name', @@ -93,28 +157,24 @@ export const WorkspaceUsers: React.FunctionComponent = () => { size={PersonaSize.size40} imageAlt={item.name} /> - ), + ) } - ]; - - const onRenderCell = ( - nestingDepth?: number, - item?: IUser, - itemIndex?: number, - group?: IGroup, - ): React.ReactNode => { - return item && typeof itemIndex === 'number' && itemIndex > -1 ? ( + ], []); + + const onRenderCell = React.useCallback( + (nestingDepth?: number, item?: User, itemIndex?: number, group?: IGroup): React.ReactNode => ( - ) : null; - }; + ), + [columns, selection, isTreAdmin], + ); return ( <> @@ -122,19 +182,70 @@ export const WorkspaceUsers: React.FunctionComponent = () => {

Users

+ { isTreAdmin && + + navigate('new')} + /> + { + selectedUserRole && + { + console.log('De-assign', selectedUserRole); + setHideCancelDialog(false); + }} + /> + } + + } {state.apiError && }
+ +
+ + + + + } /> + ); } diff --git a/ui/app/src/components/workspaces/WorkspaceUsersAssignNew.tsx b/ui/app/src/components/workspaces/WorkspaceUsersAssignNew.tsx new file mode 100644 index 0000000000..4a06cbb8eb --- /dev/null +++ b/ui/app/src/components/workspaces/WorkspaceUsersAssignNew.tsx @@ -0,0 +1,183 @@ +import { ComboBox, Dropdown, IComboBoxOption, IDropdownOption, ISelectableOption, Label, Panel, PanelType, PrimaryButton, Spinner, Stack } from "@fluentui/react"; +import { useCallback, useContext, useEffect, useState } from "react"; +import { useNavigate } from "react-router-dom"; +import { WorkspaceContext } from "../../contexts/WorkspaceContext"; +import { HttpMethod, useAuthApiCall } from "../../hooks/useAuthApiCall"; +import { ApiEndpoint } from "../../models/apiEndpoints"; +import { APIError } from "../../models/exceptions"; +import { ExceptionLayout } from "../shared/ExceptionLayout"; + +interface WorkspaceUsersAssignProps { + onAssignUser: (request: any) => void; +} + +interface AssignableUser { + name: string; + email: string; +} + +interface WorkspaceRole { + value: string; + displayName: string; +} + +export const WorkSpaceUsersAssignNew: React.FunctionComponent = (props: WorkspaceUsersAssignProps) => { + const workspaceCtx = useContext(WorkspaceContext); + const { workspace, roles, workspaceApplicationIdURI } = workspaceCtx; + + const navigate = useNavigate(); + const apiCall = useAuthApiCall(); + + const [userOptions, setUserOptions] = useState([]); + const [roleOptions, setRoleOptions] = useState([]); + + const [selectedUser, setSelectedUser] = useState(null); + const [selectedRole, setSelectedRole] = useState(null); + + const [assigning, setAssigning] = useState(false); + const [hasAssignmentError, setHasAssignmentError] = useState(false); + const [assignmentError, setAssignmentError] = useState({} as APIError); + + const onUserChange = (event: any, option: any) => { + setSelectedUser(option ? option.key : null); + }; + + const onRoleChange = (event: any, option: any) => { + setSelectedRole(option ? option.key : null); + }; + + const dismissPanel = useCallback(() => navigate('../'), [navigate]); + + const getAssignableUsers = useCallback(async () => { + try { + const scopeId = roles.length > 0 ? workspaceApplicationIdURI : ""; + const response = await apiCall(`${ApiEndpoint.Workspaces}/${workspace.id}/${ApiEndpoint.AssignableUsers}`, HttpMethod.Get, scopeId); + const assignableUsers = response.assignable_users; + + const options: IComboBoxOption[] = assignableUsers.map((assignableUser: AssignableUser) => ({ + key: assignableUser.email, + text: assignableUser.email, + data: { name: assignableUser.name }, + })); + + setUserOptions(options); + } + catch (err: any) { + err.userMessage = 'Error retrieving assignable users'; + } + + }, [apiCall, roles.length, workspace.id, workspaceApplicationIdURI]); + + const getWorkspaceRoles = useCallback(async () => { + try { + const scopeId = roles.length > 0 ? workspaceApplicationIdURI : ""; + const response = await apiCall(`${ApiEndpoint.Workspaces}/${workspace.id}/${ApiEndpoint.Roles}`, HttpMethod.Get, scopeId); + + const options: IDropdownOption[] = response.roles.map((workspaceRole: WorkspaceRole) => ({ + key: workspaceRole.value, + text: workspaceRole.displayName + })); + + setRoleOptions(options); + } + catch (err: any) { + err.userMessage = 'Error retrieving assignable users'; + } + + }, [apiCall, roles.length, workspace.id, workspaceApplicationIdURI]); + + useEffect(() => { + getAssignableUsers(); + getWorkspaceRoles(); + }, [getAssignableUsers, getWorkspaceRoles]); + + const assign = useCallback(async () => { + setAssigning(true); + + const encodedUser=selectedUser?.replaceAll('#', '%23'); + + const scopeId = roles.length > 0 ? workspaceApplicationIdURI : ""; + try { + const response = await apiCall(`${ApiEndpoint.Workspaces}/${workspace.id}/${ApiEndpoint.Users}/assign?user_email=${encodedUser}&role_name=${selectedRole}`, HttpMethod.Post, scopeId); + props.onAssignUser(response); + } + catch (err: any) { + err.userMessage = 'Error assigning workspace user'; + setHasAssignmentError(true); + setAssignmentError(err); + } + setAssigning(false); + + }, [selectedUser, roles.length, workspaceApplicationIdURI, apiCall, workspace.id, selectedRole, props]); + + const renderFooter = useCallback(() => { + let footer = <> + footer = <> +
+ assign()} disabled={assigning || (!selectedUser || !selectedRole)}>Assign +
+ + return footer; + }, [selectedUser, selectedRole, assign, assigning]); + + const onRenderOption = (option?: ISelectableOption): JSX.Element | null => { + if (!option) { + return null; + } + return ( +
+
{option.data?.name}
+
{option.text}
+
+ ); + }; + + return ( + + + + + + + + + + + { + assigning && + + + + + } + { + hasAssignmentError && + } + + + + ) +} diff --git a/ui/app/src/models/apiEndpoints.ts b/ui/app/src/models/apiEndpoints.ts index f5c09c50d0..86b732492a 100644 --- a/ui/app/src/models/apiEndpoints.ts +++ b/ui/app/src/models/apiEndpoints.ts @@ -19,5 +19,7 @@ export enum ApiEndpoint { Costs = 'costs', Metadata = ".metadata", Health = "health", - Users = 'users' + Users = 'users', + AssignableUsers = 'assignable-users', + Roles = "roles" } From b2dde320883717262d30cef547b73a8d84ed5a2f Mon Sep 17 00:00:00 2001 From: Matthew Fortunka Date: Thu, 6 Feb 2025 16:41:13 +0000 Subject: [PATCH 02/48] Added config flag to disable user management --- config.sample.yaml | 3 +++ devops/scripts/build_deploy_ui.sh | 3 ++- ui/app/src/components/workspaces/WorkspaceUsers.tsx | 5 +++-- ui/app/src/config.source.json | 3 ++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/config.sample.yaml b/config.sample.yaml index 009f017920..956cac48dc 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -50,6 +50,9 @@ tre: firewall_sku: Standard app_gateway_sku: Standard_v2 + # Set to true if TreAdmins should be able to assign and de-assign users to workspaces via the UI + user_management_enabled: true + # Uncomment to deploy to a custom domain # custom_domain: __CHANGE_ME__ authentication: diff --git a/devops/scripts/build_deploy_ui.sh b/devops/scripts/build_deploy_ui.sh index 8245291148..d449b3eb12 100755 --- a/devops/scripts/build_deploy_ui.sh +++ b/devops/scripts/build_deploy_ui.sh @@ -20,7 +20,8 @@ jq --arg rootClientId "${SWAGGER_UI_CLIENT_ID}" \ --arg treId "${TRE_ID}" \ --arg version "${ui_version}" \ --arg activeDirectoryUri "${activeDirectoryUri}" \ - '.rootClientId = $rootClientId | .rootTenantId = $rootTenantId | .treApplicationId = $treApplicationId | .treUrl = $treUrl | .treId = $treId | .version = $version | .activeDirectoryUri = $activeDirectoryUri' ./src/config.source.json > ./src/config.json + --arg userManagementEnabled "${USER_MANAGEMENT_ENABLED}" \ + '.rootClientId = $rootClientId | .rootTenantId = $rootTenantId | .treApplicationId = $treApplicationId | .treUrl = $treUrl | .treId = $treId | .version = $version | .activeDirectoryUri = $activeDirectoryUri | .userManagementEnabled = $userManagementEnabled' ./src/config.source.json > ./src/config.json # build and deploy the app yarn install diff --git a/ui/app/src/components/workspaces/WorkspaceUsers.tsx b/ui/app/src/components/workspaces/WorkspaceUsers.tsx index b01498a0ae..4065d4d6bd 100644 --- a/ui/app/src/components/workspaces/WorkspaceUsers.tsx +++ b/ui/app/src/components/workspaces/WorkspaceUsers.tsx @@ -17,6 +17,7 @@ import { CommandBarButton, DefaultButton, Dialog, DialogFooter, getTheme, Spinne import { useNavigate, Route, Routes } from 'react-router-dom'; import { destructiveButtonStyles } from '../../styles'; import { WorkSpaceUsersAssignNew } from './WorkspaceUsersAssignNew'; +import config from "../../config.json" interface IUser { id: string; @@ -169,7 +170,7 @@ export const WorkspaceUsers: React.FunctionComponent = () => { item={item} itemIndex={itemIndex!} selection={selection} - selectionMode={isTreAdmin ? SelectionMode.single : SelectionMode.none} + selectionMode={(isTreAdmin && config.userManagementEnabled) ? SelectionMode.single : SelectionMode.none} group={group} /> ), @@ -182,7 +183,7 @@ export const WorkspaceUsers: React.FunctionComponent = () => {

Users

- { isTreAdmin && + { (isTreAdmin && config.userManagementEnabled) && Date: Fri, 7 Feb 2025 14:51:36 +0000 Subject: [PATCH 03/48] fixed UI and added unit tests --- api_app/api/routes/workspaces.py | 18 ++ api_app/core/config.py | 3 + api_app/resources/strings.py | 3 + .../test_api/test_routes/test_workspaces.py | 242 +++++++++++++++++- .../test_services/test_aad_access_service.py | 82 +++++- .../components/workspaces/WorkspaceUsers.tsx | 14 +- .../workspaces/WorkspaceUsersAssignNew.tsx | 51 +--- 7 files changed, 360 insertions(+), 53 deletions(-) diff --git a/api_app/api/routes/workspaces.py b/api_app/api/routes/workspaces.py index 8db18f42a8..42e15e4f53 100644 --- a/api_app/api/routes/workspaces.py +++ b/api_app/api/routes/workspaces.py @@ -39,12 +39,16 @@ from models.domain.request_action import RequestAction from services.logging import logger +from core.config import USER_MANAGEMENT_ENABLED + workspaces_core_router = APIRouter(dependencies=[Depends(get_current_tre_user_or_tre_admin)]) workspaces_shared_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager_or_tre_admin)]) workspace_services_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)]) user_resources_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)]) +def _is_user_management_enabled(): + return USER_MANAGEMENT_ENABLED def validate_user_has_valid_role_for_user_resource(user, user_resource): if "WorkspaceOwner" in user.roles: @@ -547,6 +551,11 @@ async def retrieve_user_resource_history_by_user_resource_id(user_resource=Depen @workspaces_shared_router.get("/workspaces/{workspace_id}/assignable-users", response_model=AssignableUsersInResponse, name=strings.API_GET_ASSIGNABLE_USERS) async def get_assignable_users(workspace=Depends(get_workspace_by_id_from_path)) -> AssignableUsersInResponse: + if _is_user_management_enabled() is False: + logger.exception("Getting Assignable Users failed - User management is disabled. Enable via the USER_MANAGEMENT_ENABLED environment variable.") + raise HTTPException(status_code=status.HTTP_405_METHOD_NOT_ALLOWED, detail=strings.USER_MANAGEMENT_DISABLED) + + access_service = get_access_service() assignable_users = access_service.get_assignable_users() return AssignableUsersInResponse(assignable_users=assignable_users) @@ -560,6 +569,10 @@ async def get_workspace_roles(workspace=Depends(get_workspace_by_id_from_path)) @workspaces_shared_router.post("/workspaces/{workspace_id}/users/assign", status_code=status.HTTP_202_ACCEPTED, name=strings.API_ASSIGN_WORKSPACE_USER) async def assign_workspace_user(response: Response, user_email: str, role_name: str, workspace=Depends(get_workspace_by_id_from_path)) -> UsersInResponse: + if _is_user_management_enabled() is False: + logger.exception("User assignment failed - User management is disabled. Enable via the USER_MANAGEMENT_ENABLED environment variable.") + raise HTTPException(status_code=status.HTTP_405_METHOD_NOT_ALLOWED, detail=strings.USER_MANAGEMENT_DISABLED) + access_service = get_access_service() user = access_service.get_user_by_email(user_email) @@ -581,6 +594,11 @@ async def remove_workspace_user_assignment(user_email: str, role_name: str, workspace=Depends(get_workspace_by_id_from_path)) -> UsersInResponse: + if _is_user_management_enabled() is False: + logger.exception("User de-assignment failed - User management is disabled. Enable via the USER_MANAGEMENT_ENABLED environment variable.") + raise HTTPException(status_code=status.HTTP_405_METHOD_NOT_ALLOWED, detail=strings.USER_MANAGEMENT_DISABLED) + + access_service = get_access_service() user = access_service.get_user_by_email(user_email) diff --git a/api_app/core/config.py b/api_app/core/config.py index 34f753beb5..d2f1cf1fa4 100644 --- a/api_app/core/config.py +++ b/api_app/core/config.py @@ -71,3 +71,6 @@ ENABLE_AIRLOCK_EMAIL_CHECK: bool = config("ENABLE_AIRLOCK_EMAIL_CHECK", cast=bool, default=False) API_ROOT_SCOPE: str = f"api://{API_CLIENT_ID}/user_impersonation" + +# User Management +USER_MANAGEMENT_ENABLED: bool = config("USER_MANAGEMENT_ENABLED", cast=bool, default=False) diff --git a/api_app/resources/strings.py b/api_app/resources/strings.py index 7431da86a7..50b47867f6 100644 --- a/api_app/resources/strings.py +++ b/api_app/resources/strings.py @@ -258,3 +258,6 @@ # Value that a sensitive is replaced with in Cosmos REDACTED_SENSITIVE_VALUE = "REDACTED" + +# User Management +USER_MANAGEMENT_DISABLED = "User management is disabled" diff --git a/api_app/tests_ma/test_api/test_routes/test_workspaces.py b/api_app/tests_ma/test_api/test_routes/test_workspaces.py index 58e069d852..c890449aef 100644 --- a/api_app/tests_ma/test_api/test_routes/test_workspaces.py +++ b/api_app/tests_ma/test_api/test_routes/test_workspaces.py @@ -15,7 +15,7 @@ from db.errors import EntityDoesNotExist from db.repositories.workspaces import WorkspaceRepository from db.repositories.workspace_services import WorkspaceServiceRepository -from models.domain.authentication import RoleAssignment +from models.domain.authentication import RoleAssignment, User, Role from models.domain.operation import Operation, OperationStep, Status from models.domain.resource import ResourceHistoryItem, ResourceType from models.domain.user_resource import UserResource @@ -88,7 +88,8 @@ def sample_workspace(workspace_id=WORKSPACE_ID, auth_info: dict = {}) -> Workspa etag="", properties={ "client_id": "12345", - "scope_id": "test_scope_id" + "scope_id": "test_scope_id", + "sp_id": "test_sp_id" }, resourcePath=f'/workspaces/{workspace_id}', updatedWhen=FAKE_CREATE_TIMESTAMP, @@ -1677,3 +1678,240 @@ async def test_get_workspace_users_returns_users(self, _, auth_class, app, clien assert response.status_code == status.HTTP_200_OK assert response.json()["users"] == users + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + @patch("api.routes.workspaces._is_user_management_enabled", return_value=False) + async def test_assign_workspace_user_fails_if_feature_is_disabled(self, _, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.get_user_by_email") as get_user_by_email_mock, \ + patch(f"services.{auth_class}.get_workspace_role_by_name") as get_workspace_role_by_name_mock, \ + patch(f"services.{auth_class}.assign_workspace_user") as assign_workspace_user_mock, \ + patch(f"services.{auth_class}.get_workspace_users") as get_workspace_users_mock: + + workspace = get_workspace_by_id_mock.return_value + + user = { + "id": "123", + "name": "John Doe", + "email": "john.doe@example.com", + "roles": ["WorkspaceOwner", "WorkspaceResearcher"], + "roleAssignments": [] + } + + users = [user] + + role_name_to_assign = "AirlockManager" + role = {"id": "test_role_id"} + + get_user_by_email_mock.return_value = User.parse_obj(user) + get_workspace_role_by_name_mock.return_value = role + get_workspace_users_mock.return_value = users + + response = await client.post(app.url_path_for(strings.API_ASSIGN_WORKSPACE_USER, workspace_id=WORKSPACE_ID), params={"user_email": user["email"], "role_name": role_name_to_assign}) + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + @patch("api.routes.workspaces._is_user_management_enabled", return_value=True) + async def test_assign_workspace_user_assigns_workspace_user(self, _, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.get_user_by_email") as get_user_by_email_mock, \ + patch(f"services.{auth_class}.get_workspace_role_by_name") as get_workspace_role_by_name_mock, \ + patch(f"services.{auth_class}.assign_workspace_user") as assign_workspace_user_mock, \ + patch(f"services.{auth_class}.get_workspace_users") as get_workspace_users_mock: + + workspace = get_workspace_by_id_mock.return_value + + user = { + "id": "123", + "name": "John Doe", + "email": "john.doe@example.com", + "roles": ["WorkspaceOwner", "WorkspaceResearcher"], + "roleAssignments": [] + } + + users = [user] + + role_name_to_assign = "AirlockManager" + role = {"id": "test_role_id"} + + get_user_by_email_mock.return_value = User.parse_obj(user) + get_workspace_role_by_name_mock.return_value = role + get_workspace_users_mock.return_value = users + + response = await client.post(app.url_path_for(strings.API_ASSIGN_WORKSPACE_USER, workspace_id=WORKSPACE_ID), params={"user_email": user["email"], "role_name": role_name_to_assign}) + assert response.status_code == status.HTTP_202_ACCEPTED + + get_user_by_email_mock.assert_called_once_with(user["email"]) + get_workspace_role_by_name_mock.assert_called_once_with(role_name_to_assign, workspace) + assign_workspace_user_mock.assert_called_once_with(user, workspace, role) + get_workspace_users_mock.assert_called_once() + + assert response.json()["users"] == users + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + @patch("api.routes.workspaces._is_user_management_enabled", return_value=False) + async def test_remove_workspace_user_assignment_fails_if_feature_is_disabled(self, _, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.remove_workspace_role_user_assignment") as remove_workspace_role_user_assignment_mock, \ + patch(f"services.{auth_class}.get_user_by_email") as get_user_by_email_mock, \ + patch(f"services.{auth_class}.get_workspace_role_by_name") as get_workspace_role_by_name_mock, \ + patch(f"services.{auth_class}.get_workspace_users") as get_workspace_users_mock: + + workspace = get_workspace_by_id_mock.return_value + + user = { + "id": "123", + "name": "John Doe", + "email": "john.doe@example.com", + "roles": ["WorkspaceOwner", "WorkspaceResearcher"], + "roleAssignments": [] + } + + role_name_to_deassign = "WorkspaceResearcher" + role = {"id": "test_role_id"} + + get_user_by_email_mock.return_value = User.parse_obj(user) + get_workspace_role_by_name_mock.return_value = role + + user["roles"].remove(role_name_to_deassign) + users = [user] + + get_workspace_users_mock.return_value = users + + response = await client.delete(app.url_path_for(strings.API_ASSIGN_WORKSPACE_USER, workspace_id=WORKSPACE_ID), params={"user_email": user["email"], "role_name": role_name_to_deassign}) + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + @patch("api.routes.workspaces._is_user_management_enabled", return_value=True) + async def test_remove_workspace_user_assignment_removes_workspace_user_assignment(self, _, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.remove_workspace_role_user_assignment") as remove_workspace_role_user_assignment_mock, \ + patch(f"services.{auth_class}.get_user_by_email") as get_user_by_email_mock, \ + patch(f"services.{auth_class}.get_workspace_role_by_name") as get_workspace_role_by_name_mock, \ + patch(f"services.{auth_class}.get_workspace_users") as get_workspace_users_mock: + + workspace = get_workspace_by_id_mock.return_value + + user = { + "id": "123", + "name": "John Doe", + "email": "john.doe@example.com", + "roles": ["WorkspaceOwner", "WorkspaceResearcher"], + "roleAssignments": [] + } + + role_name_to_deassign = "WorkspaceResearcher" + role = {"id": "test_role_id"} + + get_user_by_email_mock.return_value = User.parse_obj(user) + get_workspace_role_by_name_mock.return_value = role + + user["roles"].remove(role_name_to_deassign) + users = [user] + + get_workspace_users_mock.return_value = users + + response = await client.delete(app.url_path_for(strings.API_ASSIGN_WORKSPACE_USER, workspace_id=WORKSPACE_ID), params={"user_email": user["email"], "role_name": role_name_to_deassign}) + assert response.status_code == status.HTTP_202_ACCEPTED + + get_user_by_email_mock.assert_called_once_with(user["email"]) + get_workspace_role_by_name_mock.assert_called_once_with(role_name_to_deassign, workspace) + remove_workspace_role_user_assignment_mock.assert_called_once_with(get_user_by_email_mock.return_value, role, workspace) + get_workspace_users_mock.assert_called_once() + + assert response.json()["users"] == users + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + @patch("api.routes.workspaces._is_user_management_enabled", return_value=False) + async def test_get_assignable_users_fails_if_feature_is_disabled(self, _, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.get_assignable_users") as get_assignable_users_mock: + assignable_users = [ + { + "name": "John Doe", + "email": "john.doe@example.com", + }, + { + "name": "Jane Smith", + "email": "jane.smith@example.com", + } + ] + + get_assignable_users_mock.return_value = assignable_users + + response = await client.get(app.url_path_for(strings.API_GET_ASSIGNABLE_USERS, workspace_id=WORKSPACE_ID)) + + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + @patch("api.routes.workspaces._is_user_management_enabled", return_value=True) + async def test_get_assignable_users_returns_assignable_users(self, _, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.get_assignable_users") as get_assignable_users_mock: + assignable_users = [ + { + "name": "John Doe", + "email": "john.doe@example.com", + }, + { + "name": "Jane Smith", + "email": "jane.smith@example.com", + } + ] + + get_assignable_users_mock.return_value = assignable_users + + response = await client.get(app.url_path_for(strings.API_GET_ASSIGNABLE_USERS, workspace_id=WORKSPACE_ID)) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["assignable_users"] == assignable_users + + + @pytest.mark.parametrize("auth_class", ["aad_authentication.AzureADAuthorization"]) + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace()) + async def test_get_workspace_roles_returns_workspace_roles(self, get_workspace_by_id_mock, auth_class, app, client): + with patch(f"services.{auth_class}.get_workspace_roles") as get_workspace_roles_mock: + workspace_roles = [ + Role( + id="1", + value="AirlockManager", + isEnabled=True, + email=None, + allowedMemberTypes=["Application", "User"], + description="Provides airlock managers access to the Workspace and ability to review airlock requests.", + displayName="Airlock Manager", + origin="Application", + roleAssignments=[], + ), + Role( + id="2", + value="WorkspaceResearcher", + isEnabled=True, + email=None, + allowedMemberTypes=["Application", "User"], + description="Provides researchers access to the Workspace.", + displayName="Workspace Researcher", + origin="Application", + roleAssignments=[], + ), + Role( + id="3", + value="WorkspaceOwner", + isEnabled=True, + email=None, + allowedMemberTypes=["Application", "User"], + description="Provides workspace owners access to the Workspace.", + displayName="Workspace Owner", + origin="Application", + roleAssignments=[], + ), + ] + + get_workspace_roles_mock.return_value = workspace_roles + + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_ROLES, workspace_id=WORKSPACE_ID)) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["roles"] == workspace_roles diff --git a/api_app/tests_ma/test_services/test_aad_access_service.py b/api_app/tests_ma/test_services/test_aad_access_service.py index ef90d1f885..17a77b3e9a 100644 --- a/api_app/tests_ma/test_services/test_aad_access_service.py +++ b/api_app/tests_ma/test_services/test_aad_access_service.py @@ -1,7 +1,7 @@ import pytest from mock import call, patch -from models.domain.authentication import User, RoleAssignment +from models.domain.authentication import User, Role, RoleAssignment from models.domain.workspace import Workspace, WorkspaceRole from services.aad_authentication import AzureADAuthorization from services.access_service import AuthConfigValidationError @@ -405,7 +405,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_single_user_with_no_m # Build user response user_response_no_mail = user_response.copy() - user_response_no_mail["responses"][0]["body"]["mail"] = None + user_response_no_mail["responses"][0]["body"]["userPrincipalName"] = None users.return_value = user_response_no_mail roles.return_value = roles_response @@ -569,6 +569,80 @@ def test_get_user_details_with_batch_of_more_than_20_requests(mock_graph_post, m mock_graph_post.assert_has_calls(calls, any_order=True) +@patch("services.aad_authentication.AzureADAuthorization._ms_graph_query") +def test_get_workspace_role_by_name(mock_ms_graph_query): + workspace = Workspace( + id="abc", + etag="", + templateName="template-name", + templateVersion="0.1.0", + resourcePath="test", + properties={ + "client_id": "1234", + "sp_id": "abc127", + "app_role_id_workspace_owner": "abc128", + "app_role_id_workspace_researcher": "abc129", + "app_role_id_workspace_airlock_manager": "abc130", + }, + ) + + mock_ms_graph_query.return_value = { + "value": [ + Role(id=1, value="AirlockManager", isEnabled=True, description="", displayName="Airlock Manager", origin="", allowedMemberTypes=[]).dict(), + Role(id=2, value="WorkspaceResearcher", isEnabled=True, description="", displayName="Workspace Researcher", origin="", allowedMemberTypes=[]).dict(), + Role(id=3, value="WorkspaceOwner", isEnabled=True, description="", displayName="Workspace Owner", origin="", allowedMemberTypes=[]).dict(), + ] + } + + access_service = AzureADAuthorization() + role = access_service.get_workspace_role_by_name("WorkspaceOwner", workspace) + + assert role.id == "3" + + +@patch("services.aad_authentication.AzureADAuthorization.get_user_by_email") +def test_get_user_by_email(mock_get_user_by_email): + workspace = Workspace( + id="abc", + etag="", + templateName="template-name", + templateVersion="0.1.0", + resourcePath="test", + properties={ + "client_id": "1234", + "sp_id": "abc127", + "app_role_id_workspace_owner": "abc128", + "app_role_id_workspace_researcher": "abc129", + "app_role_id_workspace_airlock_manager": "abc130", + }, + ) + + mock_get_user_by_email.return_value = User(id="1", name="John Doe", email="john.doe@example.com", roles=["WorkspaceOwner"]) + + access_service = AzureADAuthorization() + user = access_service.get_user_by_email("john.doe@example.com") + + assert user == mock_get_user_by_email.return_value + + +@patch("services.aad_authentication.AzureADAuthorization._get_role_assignment_for_user") +def test_get_role_assignment_for_user(mock_get_role_assignment_for_user): + mock_get_role_assignment_for_user.return_value = Role( + id=1, + value="AirlockManager", + isEnabled=True, + description="", + displayName="Airlock Manager", + origin="", + allowedMemberTypes=[] + ).dict() + + access_service = AzureADAuthorization() + role = access_service._get_role_assignment_for_user("123", "abc", 1) + + assert role == mock_get_role_assignment_for_user.return_value + + def get_mock_batch_response(user_principals, group_principals): response_body = {"responses": []} for user_principal in user_principals: @@ -587,7 +661,7 @@ def get_mock_user_response(principal_id, mail, name): "id": "1", "status": 200, "headers": headers, - "body": {"@odata.context": user_odata, "mail": mail, "id": principal_id, "displayName": name}, + "body": {"@odata.context": user_odata, "userPrincipalName": mail, "id": principal_id, "displayName": name}, } return user_response_body @@ -600,7 +674,7 @@ def get_mock_group_response(group): group_members_body.append( { "@odata.type": "#microsoft.graph.user", - "mail": member.mail, + "userPrincipalName": member.mail, "id": member.principal_id, "displayName": member.display_name, } diff --git a/ui/app/src/components/workspaces/WorkspaceUsers.tsx b/ui/app/src/components/workspaces/WorkspaceUsers.tsx index 4065d4d6bd..0182376389 100644 --- a/ui/app/src/components/workspaces/WorkspaceUsers.tsx +++ b/ui/app/src/components/workspaces/WorkspaceUsers.tsx @@ -53,9 +53,12 @@ export const WorkspaceUsers: React.FunctionComponent = () => { const apiCall = useAuthApiCall(); const { workspace, roles, workspaceApplicationIdURI } = useContext(WorkspaceContext); + const [loadingUsers, setloadingUsers] = useState(false); + const getUsers = useCallback(async () => { setState(prevState => ({ ...prevState, apiError: undefined, loadingState: LoadingState.Loading })); + setloadingUsers(true); try { const scopeId = roles.length > 0 ? workspaceApplicationIdURI : ""; const result = await apiCall(`${ApiEndpoint.Workspaces}/${workspace.id}/${ApiEndpoint.Users}`, HttpMethod.Get, scopeId); @@ -75,6 +78,7 @@ export const WorkspaceUsers: React.FunctionComponent = () => { err.userMessage = "Error retrieving users"; setState({ users: [], apiError: err, loadingState: LoadingState.Error }); } + setloadingUsers(false); }, [apiCall, workspace.id, roles.length, workspaceApplicationIdURI]); const addedAssignment = async () => { @@ -210,7 +214,7 @@ export const WorkspaceUsers: React.FunctionComponent = () => { {state.apiError && }
- + { !loadingUsers && { compact={false} /> +}
+ { + loadingUsers && + + + + + }