diff --git a/CHANGELOG.md b/CHANGELOG.md index abdb51d651..98346dc298 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ **BREAKING CHANGES & MIGRATIONS**: * Firewall now blocks terraform/hasicorp domains ([#2590](https://github.com/microsoft/AzureTRE/pull/2590)). **Migration** is manual - update the templateVersion of `tre-shared-service-firewall` resource in Cosmos to `0.5.0`. Check the PR for more details. +* Add Airlock Manager Workspace ([#2505](https://github.com/microsoft/AzureTRE/pull/2505)) +* Restrict resource templates to specific roles ([#2623](https://github.com/microsoft/AzureTRE/pull/2623/)) FEATURES: diff --git a/api_app/_version.py b/api_app/_version.py index 8a64aa006c..7642353705 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.4.35" +__version__ = "0.4.38" diff --git a/api_app/api/routes/shared_service_templates.py b/api_app/api/routes/shared_service_templates.py index dc12eb75ec..917f1ec88d 100644 --- a/api_app/api/routes/shared_service_templates.py +++ b/api_app/api/routes/shared_service_templates.py @@ -15,9 +15,9 @@ shared_service_templates_core_router = APIRouter(dependencies=[Depends(get_current_tre_user_or_tre_admin)]) -@shared_service_templates_core_router.get("/shared-service-templates", response_model=ResourceTemplateInformationInList, name=strings.API_GET_SHARED_SERVICE_TEMPLATES, dependencies=[Depends(get_current_tre_user_or_tre_admin)]) -async def get_shared_service_templates(template_repo=Depends(get_repository(ResourceTemplateRepository))) -> ResourceTemplateInformationInList: - templates_infos = template_repo.get_templates_information(ResourceType.SharedService) +@shared_service_templates_core_router.get("/shared-service-templates", response_model=ResourceTemplateInformationInList, name=strings.API_GET_SHARED_SERVICE_TEMPLATES) +async def get_shared_service_templates(authorized_only: bool = False, template_repo=Depends(get_repository(ResourceTemplateRepository)), user=Depends(get_current_tre_user_or_tre_admin)) -> ResourceTemplateInformationInList: + templates_infos = template_repo.get_templates_information(ResourceType.SharedService, user.roles if authorized_only else None) return ResourceTemplateInformationInList(templates=templates_infos) diff --git a/api_app/api/routes/shared_services.py b/api_app/api/routes/shared_services.py index cc5a96a32b..ac7f309f4b 100644 --- a/api_app/api/routes/shared_services.py +++ b/api_app/api/routes/shared_services.py @@ -4,7 +4,7 @@ from jsonschema.exceptions import ValidationError from db.repositories.operations import OperationRepository -from db.errors import DuplicateEntity +from db.errors import DuplicateEntity, UserNotAuthorizedToUseTemplate from api.dependencies.database import get_repository from api.dependencies.shared_services import get_shared_service_by_id_from_path, get_operation_by_id_from_path from db.repositories.resource_templates import ResourceTemplateRepository @@ -47,13 +47,16 @@ async def retrieve_shared_service_by_id(shared_service=Depends(get_shared_servic @shared_services_router.post("/shared-services", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_CREATE_SHARED_SERVICE, dependencies=[Depends(get_current_admin_user)]) async def create_shared_service(response: Response, shared_service_input: SharedServiceInCreate, user=Depends(get_current_admin_user), shared_services_repo=Depends(get_repository(SharedServiceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository))) -> OperationInResponse: try: - shared_service, resource_template = shared_services_repo.create_shared_service_item(shared_service_input) + shared_service, resource_template = shared_services_repo.create_shared_service_item(shared_service_input, user.roles) except (ValidationError, ValueError) as e: logging.error(f"Failed create shared service model instance: {e}") raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) except DuplicateEntity as e: logging.error(f"Shared service already exists: {e}") raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) + except UserNotAuthorizedToUseTemplate as e: + logging.error(f"User not authorized to use template: {e}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) operation = await save_and_deploy_resource( resource=shared_service, diff --git a/api_app/api/routes/workspace_templates.py b/api_app/api/routes/workspace_templates.py index dddb6e81d7..8879628927 100644 --- a/api_app/api/routes/workspace_templates.py +++ b/api_app/api/routes/workspace_templates.py @@ -17,8 +17,8 @@ @workspace_templates_admin_router.get("/workspace-templates", response_model=ResourceTemplateInformationInList, name=strings.API_GET_WORKSPACE_TEMPLATES) -async def get_workspace_templates(template_repo=Depends(get_repository(ResourceTemplateRepository))) -> ResourceTemplateInformationInList: - templates_infos = template_repo.get_templates_information(ResourceType.Workspace) +async def get_workspace_templates(authorized_only: bool = False, template_repo=Depends(get_repository(ResourceTemplateRepository)), user=Depends(get_current_admin_user)) -> ResourceTemplateInformationInList: + templates_infos = template_repo.get_templates_information(ResourceType.Workspace, user.roles if authorized_only else None) return ResourceTemplateInformationInList(templates=templates_infos) diff --git a/api_app/api/routes/workspaces.py b/api_app/api/routes/workspaces.py index 13cb7d5e5d..7efd3f51c1 100644 --- a/api_app/api/routes/workspaces.py +++ b/api_app/api/routes/workspaces.py @@ -7,6 +7,7 @@ from api.dependencies.database import get_repository from api.dependencies.workspaces import get_operation_by_id_from_path, get_workspace_by_id_from_path, get_deployed_workspace_by_id_from_path, get_deployed_workspace_service_by_id_from_path, get_workspace_service_by_id_from_path, get_user_resource_by_id_from_path +from db.errors import UserNotAuthorizedToUseTemplate from db.repositories.operations import OperationRepository from db.repositories.resource_templates import ResourceTemplateRepository from db.repositories.user_resources import UserResourceRepository @@ -19,6 +20,7 @@ from models.schemas.workspace import WorkspaceInCreate, WorkspacesInList, WorkspaceInResponse from models.schemas.workspace_service import WorkspaceServiceInCreate, WorkspaceServicesInList, WorkspaceServiceInResponse from models.schemas.resource import ResourcePatch +from models.schemas.resource_template import ResourceTemplateInformationInList from resources import strings from services.access_service import AuthConfigValidationError from services.authentication import get_current_admin_user, \ @@ -32,7 +34,6 @@ 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_tre_admin)]) -workspaces_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user)]) workspace_services_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user)]) user_resources_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user)]) @@ -88,10 +89,13 @@ async def create_workspace(workspace_create: WorkspaceInCreate, response: Respon try: # TODO: This requires Directory.ReadAll ( Application.Read.All ) to be enabled in the Azure AD application to enable a users workspaces to be listed. This should be made optional. auth_info = extract_auth_information(workspace_create.properties) - workspace, resource_template = workspace_repo.create_workspace_item(workspace_create, auth_info, user.id) + workspace, resource_template = workspace_repo.create_workspace_item(workspace_create, auth_info, user.id, user.roles) except (ValidationError, ValueError) as e: logging.error(f"Failed to create workspace model instance: {e}") raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + except UserNotAuthorizedToUseTemplate as e: + logging.error(f"User not authorized to use template: {e}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) operation = await save_and_deploy_resource( resource=workspace, @@ -164,6 +168,20 @@ async def invoke_action_on_workspace(response: Response, action: str, user=Depen # workspace operations +# This method only returns templates that the authenticated user is authorized to use +@workspaces_shared_router.get("/workspace/{workspace_id}/workspace-service-templates", response_model=ResourceTemplateInformationInList, name=strings.API_GET_WORKSPACE_SERVICE_TEMPLATES_IN_WORKSPACE) +async def get_workspace_service_templates(workspace=Depends(get_workspace_by_id_from_path), template_repo=Depends(get_repository(ResourceTemplateRepository)), user=Depends(get_current_workspace_owner_or_researcher_user_or_tre_admin)) -> ResourceTemplateInformationInList: + template_infos = template_repo.get_templates_information(ResourceType.WorkspaceService, user.roles) + return ResourceTemplateInformationInList(templates=template_infos) + + +# This method only returns templates that the authenticated user is authorized to use +@workspaces_shared_router.get("/workspace/{workspace_id}/workspace-service-templates/{service_template_name}/user-resource-templates", response_model=ResourceTemplateInformationInList, name=strings.API_GET_USER_RESOURCE_TEMPLATES_IN_WORKSPACE) +async def get_user_resource_templates(service_template_name: str, workspace=Depends(get_workspace_by_id_from_path), template_repo=Depends(get_repository(ResourceTemplateRepository)), user=Depends(get_current_workspace_owner_or_researcher_user_or_tre_admin)) -> ResourceTemplateInformationInList: + template_infos = template_repo.get_templates_information(ResourceType.UserResource, user.roles, service_template_name) + return ResourceTemplateInformationInList(templates=template_infos) + + @workspaces_shared_router.get("/workspaces/{workspace_id}/operations", response_model=OperationInList, name=strings.API_GET_RESOURCE_OPERATIONS, dependencies=[Depends(get_current_workspace_owner_or_tre_admin)]) async def retrieve_workspace_operations_by_workspace_id(workspace=Depends(get_workspace_by_id_from_path), operations_repo=Depends(get_repository(OperationRepository))) -> OperationInList: return OperationInList(operations=operations_repo.get_operations_by_resource_id(resource_id=workspace.id)) @@ -194,6 +212,9 @@ async def create_workspace_service(response: Response, workspace_service_input: except (ValidationError, ValueError) as e: logging.error(f"Failed create workspace service model instance: {e}") raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + except UserNotAuthorizedToUseTemplate as e: + logging.error(f"User not authorized to use template: {e}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) operation = await save_and_deploy_resource( resource=workspace_service, @@ -306,13 +327,24 @@ async def retrieve_user_resource_by_id(user_resource=Depends(get_user_resource_b @user_resources_workspace_router.post("/workspaces/{workspace_id}/workspace-services/{service_id}/user-resources", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_CREATE_USER_RESOURCE) -async def create_user_resource(response: Response, user_resource_create: UserResourceInCreate, user_resource_repo=Depends(get_repository(UserResourceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), user=Depends(get_current_workspace_owner_or_researcher_user), workspace=Depends(get_deployed_workspace_by_id_from_path), workspace_service=Depends(get_deployed_workspace_service_by_id_from_path)) -> OperationInResponse: +async def create_user_resource( + response: Response, + user_resource_create: UserResourceInCreate, + user_resource_repo=Depends(get_repository(UserResourceRepository)), + resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), + operations_repo=Depends(get_repository(OperationRepository)), + user=Depends(get_current_workspace_owner_or_researcher_user), + workspace=Depends(get_deployed_workspace_by_id_from_path), + workspace_service=Depends(get_deployed_workspace_service_by_id_from_path)) -> OperationInResponse: try: - user_resource, resource_template = user_resource_repo.create_user_resource_item(user_resource_create, workspace.id, workspace_service.id, workspace_service.templateName, user.id) + user_resource, resource_template = user_resource_repo.create_user_resource_item(user_resource_create, workspace.id, workspace_service.id, workspace_service.templateName, user.id, user.roles) except (ValidationError, ValueError) as e: logging.error(f"Failed create user resource model instance: {e}") raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + except UserNotAuthorizedToUseTemplate as e: + logging.error(f"User not authorized to use template: {e}") + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) operation = await save_and_deploy_resource( resource=user_resource, diff --git a/api_app/db/errors.py b/api_app/db/errors.py index 9ed1db5751..3f55478a23 100644 --- a/api_app/db/errors.py +++ b/api_app/db/errors.py @@ -20,3 +20,7 @@ class ResourceIsNotDeployed(Exception): class InvalidInput(Exception): """Raised when invalid input is received when creating an entity.""" + + +class UserNotAuthorizedToUseTemplate(Exception): + """Raised when user attempts to use a template they aren't authorized to use""" diff --git a/api_app/db/repositories/resource_templates.py b/api_app/db/repositories/resource_templates.py index a3fe9258ff..d395dc4b94 100644 --- a/api_app/db/repositories/resource_templates.py +++ b/api_app/db/repositories/resource_templates.py @@ -33,15 +33,23 @@ def enrich_template(template: ResourceTemplate, is_update: bool = False) -> dict else: return enrich_user_resource_template(template, is_update=is_update) - def get_templates_information(self, resource_type: ResourceType, parent_service_name: str = "") -> List[ResourceTemplateInformation]: + def get_templates_information(self, resource_type: ResourceType, user_roles: List[str] = None, parent_service_name: str = "") -> List[ResourceTemplateInformation]: """ Returns name/title/description for all current resource_type templates + + :param user_roles: If set, only return templates that the user is authorized to use. + template.authorizedRoles should contain at least one of user_roles """ - query = f'SELECT c.name, c.title, c.description FROM c WHERE c.resourceType = "{resource_type}" AND c.current = true' + query = f'SELECT c.name, c.title, c.description, c.authorizedRoles FROM c WHERE c.resourceType = "{resource_type}" AND c.current = true' if resource_type == ResourceType.UserResource: query += f' AND c.parentWorkspaceService = "{parent_service_name}"' template_infos = self.query(query=query) - return [parse_obj_as(ResourceTemplateInformation, info) for info in template_infos] + templates = [parse_obj_as(ResourceTemplateInformation, info) for info in template_infos] + + if not user_roles: + return templates + # User can view template if they have at least one of authorizedRoles + return [t for t in templates if not t.authorizedRoles or len(set(t.authorizedRoles).intersection(set(user_roles))) > 0] def get_current_template(self, template_name: str, resource_type: ResourceType, parent_service_name: str = "") -> Union[ResourceTemplate, UserResourceTemplate]: """ @@ -97,6 +105,7 @@ def create_template(self, template_input: ResourceTemplateInCreate, resource_typ "resourceType": resource_type, "current": template_input.current, "required": template_input.json_schema["required"], + "authorizedRoles": template_input.json_schema["authorizedRoles"] if "authorizedRoles" in template_input.json_schema else [], "properties": template_input.json_schema["properties"], "customActions": template_input.customActions } diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index 991c333f6e..c1e6473f4f 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -1,11 +1,11 @@ import copy from datetime import datetime -from typing import Tuple +from typing import Tuple, List from azure.cosmos import CosmosClient from azure.cosmos.exceptions import CosmosResourceNotFoundError from core import config -from db.errors import EntityDoesNotExist +from db.errors import EntityDoesNotExist, UserNotAuthorizedToUseTemplate from db.repositories.base import BaseRepository from db.repositories.resource_templates import ResourceTemplateRepository from jsonschema import validate @@ -77,7 +77,7 @@ def get_resource_by_template_name(self, template_name: str) -> Resource: raise EntityDoesNotExist return parse_obj_as(Resource, resources[0]) - def validate_input_against_template(self, template_name: str, resource_input, resource_type: ResourceType, parent_template_name: str = "") -> ResourceTemplate: + def validate_input_against_template(self, template_name: str, resource_input, resource_type: ResourceType, user_roles: List[str] = None, parent_template_name: str = "") -> ResourceTemplate: try: template = self._get_enriched_template(template_name, resource_type, parent_template_name) except EntityDoesNotExist: @@ -86,6 +86,12 @@ def validate_input_against_template(self, template_name: str, resource_input, re else: raise ValueError(f'The template "{template_name}" does not exist') + # If authorizedRoles is empty, template is available to all users + if "authorizedRoles" in template and template["authorizedRoles"]: + # If authorizedRoles is not empty, the user is required to have at least one of authorizedRoles + if len(set(template["authorizedRoles"]).intersection(set(user_roles))) == 0: + raise UserNotAuthorizedToUseTemplate(f"User not authorized to use template {template_name}") + self._validate_resource_parameters(resource_input.dict(), template) return parse_obj_as(ResourceTemplate, template) @@ -128,7 +134,7 @@ def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: update_template["required"] = [] update_template["properties"] = {} for prop_name, prop in enriched_template["properties"].items(): - if("updateable" in prop.keys() and prop["updateable"] is True): + if ("updateable" in prop.keys() and prop["updateable"] is True): update_template["properties"][prop_name] = prop self._validate_resource_parameters(resource_patch.dict(), update_template) diff --git a/api_app/db/repositories/shared_services.py b/api_app/db/repositories/shared_services.py index 31b71ed966..84da400597 100644 --- a/api_app/db/repositories/shared_services.py +++ b/api_app/db/repositories/shared_services.py @@ -57,7 +57,7 @@ def get_deployed_shared_service_by_id(self, shared_service_id: str, operations_r def get_shared_service_spec_params(self): return self.get_resource_base_spec_params() - def create_shared_service_item(self, shared_service_input: SharedServiceTemplateInCreate) -> Tuple[SharedService, ResourceTemplate]: + def create_shared_service_item(self, shared_service_input: SharedServiceTemplateInCreate, user_roles: List[str]) -> Tuple[SharedService, ResourceTemplate]: shared_service_id = str(uuid.uuid4()) existing_shared_services = self.query(self.operating_shared_service_with_template_name_query(shared_service_input.templateName)) @@ -68,7 +68,7 @@ def create_shared_service_item(self, shared_service_input: SharedServiceTemplate raise InternalError(f"More than one active shared service exists with the same id {shared_service_id}") raise DuplicateEntity - template = self.validate_input_against_template(shared_service_input.templateName, shared_service_input, ResourceType.SharedService) + template = self.validate_input_against_template(shared_service_input.templateName, shared_service_input, ResourceType.SharedService, user_roles) resource_spec_parameters = {**shared_service_input.properties, **self.get_shared_service_spec_params()} diff --git a/api_app/db/repositories/user_resources.py b/api_app/db/repositories/user_resources.py index 9def1e055b..7670314e11 100644 --- a/api_app/db/repositories/user_resources.py +++ b/api_app/db/repositories/user_resources.py @@ -27,10 +27,10 @@ def user_resources_query(workspace_id: str, service_id: str): def active_user_resources_query(workspace_id: str, service_id: str): return f'SELECT * FROM c WHERE {IS_NOT_DELETED_CLAUSE} AND c.resourceType = "{ResourceType.UserResource}" AND c.parentWorkspaceServiceId = "{service_id}" AND c.workspaceId = "{workspace_id}"' - def create_user_resource_item(self, user_resource_input: UserResourceInCreate, workspace_id: str, parent_workspace_service_id: str, parent_template_name: str, user_id: str) -> Tuple[UserResource, ResourceTemplate]: + def create_user_resource_item(self, user_resource_input: UserResourceInCreate, workspace_id: str, parent_workspace_service_id: str, parent_template_name: str, user_id: str, user_roles: List[str]) -> Tuple[UserResource, ResourceTemplate]: full_user_resource_id = str(uuid.uuid4()) - template = self.validate_input_against_template(user_resource_input.templateName, user_resource_input, ResourceType.UserResource, parent_template_name) + template = self.validate_input_against_template(user_resource_input.templateName, user_resource_input, ResourceType.UserResource, user_roles, parent_template_name) # we don't want something in the input to overwrite the system parameters, so dict.update can't work. resource_spec_parameters = {**user_resource_input.properties, **self.get_user_resource_spec_params()} diff --git a/api_app/db/repositories/workspace_services.py b/api_app/db/repositories/workspace_services.py index 33dd0ca29b..7f6c53448d 100644 --- a/api_app/db/repositories/workspace_services.py +++ b/api_app/db/repositories/workspace_services.py @@ -54,10 +54,10 @@ def get_workspace_service_by_id(self, workspace_id: str, service_id: str) -> Wor def get_workspace_service_spec_params(self): return self.get_resource_base_spec_params() - def create_workspace_service_item(self, workspace_service_input: WorkspaceServiceInCreate, workspace_id: str) -> Tuple[WorkspaceService, ResourceTemplate]: + def create_workspace_service_item(self, workspace_service_input: WorkspaceServiceInCreate, workspace_id: str, user_roles=List[str]) -> Tuple[WorkspaceService, ResourceTemplate]: full_workspace_service_id = str(uuid.uuid4()) - template = self.validate_input_against_template(workspace_service_input.templateName, workspace_service_input, ResourceType.WorkspaceService) + template = self.validate_input_against_template(workspace_service_input.templateName, workspace_service_input, ResourceType.WorkspaceService, user_roles) # we don't want something in the input to overwrite the system parameters, so dict.update can't work. resource_spec_parameters = {**workspace_service_input.properties, **self.get_workspace_service_spec_params()} diff --git a/api_app/db/repositories/workspaces.py b/api_app/db/repositories/workspaces.py index df0623479e..0763ebbb09 100644 --- a/api_app/db/repositories/workspaces.py +++ b/api_app/db/repositories/workspaces.py @@ -62,10 +62,10 @@ def get_workspace_by_id(self, workspace_id: str) -> Workspace: raise EntityDoesNotExist return parse_obj_as(Workspace, workspaces[0]) - def create_workspace_item(self, workspace_input: WorkspaceInCreate, auth_info: dict, workspace_owner_object_id: str) -> Tuple[Workspace, ResourceTemplate]: + def create_workspace_item(self, workspace_input: WorkspaceInCreate, auth_info: dict, workspace_owner_object_id: str, user_roles: List[str]) -> Tuple[Workspace, ResourceTemplate]: full_workspace_id = str(uuid.uuid4()) - template = self.validate_input_against_template(workspace_input.templateName, workspace_input, ResourceType.Workspace) + template = self.validate_input_against_template(workspace_input.templateName, workspace_input, ResourceType.Workspace, user_roles) address_space_param = {"address_space": self.get_address_space_based_on_size(workspace_input.properties)} auto_app_registration_param = {"register_aad_application": self.automatically_create_application_registration(workspace_input.properties)} diff --git a/api_app/models/domain/resource_template.py b/api_app/models/domain/resource_template.py index 2589518ab3..9d47520985 100644 --- a/api_app/models/domain/resource_template.py +++ b/api_app/models/domain/resource_template.py @@ -65,6 +65,7 @@ class ResourceTemplate(AzureTREModel): current: bool = Field(title="Is this the current version of this template") type: str = "object" required: List[str] = Field(title="List of properties which must be provided") + authorizedRoles: Optional[List[str]] = Field(default=[], title="If not empty, the user is required to have one of these roles to install the template") properties: Dict[str, Property] = Field(title="Template properties") actions: List[CustomAction] = Field(default=[], title="Template actions") customActions: List[CustomAction] = Field(default=[], title="Template custom actions") diff --git a/api_app/models/schemas/resource_template.py b/api_app/models/schemas/resource_template.py index 84697184b0..dd3b75722e 100644 --- a/api_app/models/schemas/resource_template.py +++ b/api_app/models/schemas/resource_template.py @@ -1,4 +1,4 @@ -from typing import Dict, List +from typing import Dict, List, Optional from pydantic import BaseModel, Field @@ -21,6 +21,7 @@ class ResourceTemplateInformation(BaseModel): name: str = Field(title="Template name") title: str = Field(title="Template title", default="") description: str = Field(title="Template description", default="") + authorizedRoles: Optional[List[str]] = Field(title="If not empty, the user is required to have one of these roles to install the template", default=[]) class ResourceTemplateInformationInList(BaseModel): diff --git a/api_app/models/schemas/shared_service_template.py b/api_app/models/schemas/shared_service_template.py index 5ef87c10b0..048973b374 100644 --- a/api_app/models/schemas/shared_service_template.py +++ b/api_app/models/schemas/shared_service_template.py @@ -50,6 +50,7 @@ class Config: "title": "My Shared Service Template", "description": "These is a test shared service resource template schema", "required": [], + "authorizedRoles": [], "properties": {} }, "customActions": [ diff --git a/api_app/models/schemas/user_resource_template.py b/api_app/models/schemas/user_resource_template.py index bb3ab25275..cc0013cd75 100644 --- a/api_app/models/schemas/user_resource_template.py +++ b/api_app/models/schemas/user_resource_template.py @@ -50,6 +50,7 @@ class Config: "title": "My User Resource Template", "description": "These is a test user resource template schema", "required": [], + "authorizedRoles": [], "properties": {}, }, "customActions": [ diff --git a/api_app/models/schemas/workspace_service_template.py b/api_app/models/schemas/workspace_service_template.py index 6db4edeada..c3f493169e 100644 --- a/api_app/models/schemas/workspace_service_template.py +++ b/api_app/models/schemas/workspace_service_template.py @@ -51,6 +51,7 @@ class Config: "title": "My Workspace Service Template", "description": "These is a test workspace service resource template schema", "required": [], + "authorizedRoles": [], "properties": {} }, "customActions": [ diff --git a/api_app/models/schemas/workspace_template.py b/api_app/models/schemas/workspace_template.py index 847e521e96..bc20955217 100644 --- a/api_app/models/schemas/workspace_template.py +++ b/api_app/models/schemas/workspace_template.py @@ -58,6 +58,7 @@ class Config: "vm_size", "no_of_vms" ], + "authorizedRoles": [], "properties": { "display_name": { "type": "string", diff --git a/api_app/resources/strings.py b/api_app/resources/strings.py index 0f51f37eae..1d01c32e31 100644 --- a/api_app/resources/strings.py +++ b/api_app/resources/strings.py @@ -42,6 +42,7 @@ API_CREATE_WORKSPACE_SERVICE_TEMPLATES = "Register workspace service template" API_GET_WORKSPACE_SERVICE_TEMPLATES = "Get workspace service templates" +API_GET_WORKSPACE_SERVICE_TEMPLATES_IN_WORKSPACE = "Get workspace service templates (on workspace level)" # only returns templates that the authenticated user is authorized to use API_GET_WORKSPACE_SERVICE_TEMPLATE_BY_NAME = "Get workspace service template by name" API_CREATE_SHARED_SERVICE_TEMPLATES = "Register shared service template" @@ -57,6 +58,7 @@ API_CREATE_USER_RESOURCE_TEMPLATES = "Register user resource template" API_GET_USER_RESOURCE_TEMPLATES = "Get user resource templates applicable to the workspace service template" +API_GET_USER_RESOURCE_TEMPLATES_IN_WORKSPACE = "Get user resource templates applicable to the workspace service template (on workspace level)" # only returns templates that the authenticated user is authorized to use API_GET_USER_RESOURCE_TEMPLATE_BY_NAME = "Get user resource template by name and workspace service" # cost report diff --git a/api_app/tests_ma/conftest.py b/api_app/tests_ma/conftest.py index d6f530e22b..1f1029f064 100644 --- a/api_app/tests_ma/conftest.py +++ b/api_app/tests_ma/conftest.py @@ -129,6 +129,7 @@ def basic_resource_template(input_workspace_template): resourceType=ResourceType.Workspace, current=True, required=input_workspace_template.json_schema["required"], + authorizedRoles=input_workspace_template.json_schema["authorizedRoles"] if "authorizedRoles" in input_workspace_template.json_schema else [], properties=input_workspace_template.json_schema["properties"], customActions=input_workspace_template.customActions ) @@ -144,9 +145,9 @@ def basic_workspace_service_template(input_workspace_template): resourceType=ResourceType.WorkspaceService, current=True, required=input_workspace_template.json_schema["required"], + authorizedRoles=input_workspace_template.json_schema["authorizedRoles"] if "authorizedRoles" in input_workspace_template.json_schema else [], properties=input_workspace_template.json_schema["properties"], customActions=input_workspace_template.customActions - ) @@ -161,6 +162,7 @@ def basic_user_resource_template(input_user_resource_template): resourceType=ResourceType.UserResource, current=True, required=input_user_resource_template.json_schema["required"], + authorizedRoles=input_user_resource_template.json_schema["authorizedRoles"] if "authorizedRoles" in input_user_resource_template.json_schema else [], properties=input_user_resource_template.json_schema["properties"], customActions=input_user_resource_template.customActions ) @@ -176,6 +178,7 @@ def basic_shared_service_template(input_shared_service_template): resourceType=ResourceType.SharedService, current=True, required=input_shared_service_template.json_schema["required"], + authorizedRoles=input_shared_service_template.json_schema["authorizedRoles"] if "authorizedRoles" in input_shared_service_template.json_schema else [], properties=input_shared_service_template.json_schema["properties"], actions=input_shared_service_template.customActions ) @@ -192,6 +195,7 @@ def user_resource_template_in_response(input_user_resource_template): resourceType=ResourceType.UserResource, current=True, required=input_user_resource_template.json_schema["required"], + authorizedRoles=input_user_resource_template.json_schema["authorizedRoles"] if "authorizedRoles" in input_user_resource_template.json_schema else [], properties=input_user_resource_template.json_schema["properties"], customActions=input_user_resource_template.customActions, system_properties={} 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 ab5daba4b0..ea26520d97 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 @@ -18,6 +18,7 @@ from models.domain.user_resource import UserResource from models.domain.workspace import Workspace, WorkspaceRole from models.domain.workspace_service import WorkspaceService +from models.schemas.resource_template import ResourceTemplateInformation from resources import strings from services.authentication import get_current_admin_user, get_current_tre_user_or_tre_admin, get_current_workspace_owner_user, get_current_workspace_owner_or_researcher_user, get_current_workspace_owner_or_researcher_user_or_tre_admin from azure.cosmos.exceptions import CosmosAccessConditionFailedError @@ -214,6 +215,7 @@ class TestWorkspaceRoutesThatDontRequireAdminRights: def log_in_with_non_admin_user(self, app, non_admin_user): with patch('services.aad_authentication.AzureADAuthorization._get_user_from_token', return_value=non_admin_user()): app.dependency_overrides[get_current_tre_user_or_tre_admin] = non_admin_user + app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_tre_admin] = non_admin_user yield app.dependency_overrides = {} @@ -278,6 +280,18 @@ async def test_get_workspace_by_id_get_returns_422_if_workspace_id_is_not_a_uuid response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id="not_valid")) assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + @patch("api.routes.workspaces.ResourceTemplateRepository.get_templates_information", return_value=[ResourceTemplateInformation(name="test")]) + async def test_get_workspace_service_templates_returns_templates(self, _, __, app, client): + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_TEMPLATES_IN_WORKSPACE, workspace_id=WORKSPACE_ID)) + assert response.status_code == status.HTTP_200_OK + + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + @patch("api.routes.workspaces.ResourceTemplateRepository.get_templates_information", return_value=[ResourceTemplateInformation(name="test")]) + async def test_get_user_resource_templates_returns_templates(self, _, __, app, client): + response = await client.get(app.url_path_for(strings.API_GET_USER_RESOURCE_TEMPLATES_IN_WORKSPACE, workspace_id=WORKSPACE_ID, service_template_name="guacamole")) + assert response.status_code == status.HTTP_200_OK + class TestWorkspaceRoutesThatRequireAdminRights: @pytest.fixture(autouse=True, scope='class') diff --git a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py index dd23698bce..db859f8cd3 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py @@ -7,7 +7,7 @@ from tests_ma.test_api.test_routes.test_resource_helpers import FAKE_CREATE_TIMESTAMP, FAKE_UPDATE_TIMESTAMP from tests_ma.test_api.conftest import create_test_user -from db.errors import EntityDoesNotExist +from db.errors import EntityDoesNotExist, UserNotAuthorizedToUseTemplate from db.repositories.resources import ResourceRepository from azure.cosmos.exceptions import CosmosResourceNotFoundError from models.domain.resource import Resource, ResourceHistoryItem @@ -141,7 +141,7 @@ def test_validate_input_against_template_returns_template_version_if_template_is properties={}, customActions=[]).dict() - template = resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace) + template = resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace, []) assert template.version == "0.1.0" @@ -151,7 +151,7 @@ def test_validate_input_against_template_raises_value_error_if_template_does_not enriched_template_mock.side_effect = EntityDoesNotExist with pytest.raises(ValueError): - resource_repo.validate_input_against_template("template_name", workspace_input, ResourceType.Workspace) + resource_repo.validate_input_against_template("template_name", workspace_input, ResourceType.Workspace, []) @patch("db.repositories.resources.ResourceRepository._get_enriched_template") @@ -159,11 +159,11 @@ def test_validate_input_against_template_raises_value_error_if_the_user_resource enriched_template_mock.side_effect = EntityDoesNotExist with pytest.raises(ValueError): - resource_repo.validate_input_against_template("template_name", workspace_input, ResourceType.UserResource, "parent_template_name") + resource_repo.validate_input_against_template("template_name", workspace_input, ResourceType.UserResource, [], "parent_template_name") @patch("db.repositories.resources.ResourceRepository._get_enriched_template") -def test_validate_input_against_template_raises_value_error_if_payload_is_invalid(enriched_template_mock, resource_repo): +def test_validate_input_against_template_raises_value_error_if_payload_is_invalid(enriched_template_mock, resource_repo, workspace_input): enriched_template_mock.return_value = ResourceTemplate(id="123", name="template1", description="description", @@ -177,7 +177,63 @@ def test_validate_input_against_template_raises_value_error_if_payload_is_invali workspace_input = WorkspaceInCreate(templateName="template1") with pytest.raises(ValidationError): - resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace) + resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace, []) + + +@patch("db.repositories.resources.ResourceRepository._get_enriched_template") +def test_validate_input_against_template_raises_if_user_does_not_have_required_role(enriched_template_mock, resource_repo, workspace_input): + enriched_template_mock.return_value = ResourceTemplate(id="123", + name="template1", + description="description", + version="0.1.0", + resourceType=ResourceType.Workspace, + current=True, + required=[], + authorizedRoles=["missing_role"], + properties={}, + customActions=[]).dict() + + with pytest.raises(UserNotAuthorizedToUseTemplate): + _ = resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace, ["test_role", "another_role"]) + + +@patch("db.repositories.resources.ResourceRepository._get_enriched_template") +@patch("db.repositories.resources.ResourceRepository._validate_resource_parameters", return_value=None) +def test_validate_input_against_template_valid_if_user_has_only_one_role(_, enriched_template_mock, resource_repo, workspace_input): + enriched_template_mock.return_value = ResourceTemplate(id="123", + name="template1", + description="description", + version="0.1.0", + resourceType=ResourceType.Workspace, + current=True, + required=[], + authorizedRoles=["test_role", "missing_role"], + properties={}, + customActions=[]).dict() + + template = resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace, ["test_role", "another_role"]) + + # does not throw + assert template.version == "0.1.0" + + +@patch("db.repositories.resources.ResourceRepository._get_enriched_template") +@patch("db.repositories.resources.ResourceRepository._validate_resource_parameters", return_value=None) +def test_validate_input_against_template_valid_if_required_roles_set_is_empty(_, enriched_template_mock, resource_repo, workspace_input): + enriched_template_mock.return_value = ResourceTemplate(id="123", + name="template1", + description="description", + version="0.1.0", + resourceType=ResourceType.Workspace, + current=True, + required=[], + properties={}, + customActions=[]).dict() + + template = resource_repo.validate_input_against_template("template1", workspace_input, ResourceType.Workspace, ["test_user_role"]) + + # does not throw + assert template.version == "0.1.0" @patch("db.repositories.resources.ResourceRepository._get_enriched_template") diff --git a/api_app/tests_ma/test_db/test_repositories/test_resource_templates_repository.py b/api_app/tests_ma/test_db/test_repositories/test_resource_templates_repository.py index 831eb5f6fe..2aa466e54b 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_resource_templates_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_resource_templates_repository.py @@ -103,6 +103,24 @@ def test_get_templates_information_returns_unique_template_names(query_mock, res assert result[1].name == "template2" +@patch('db.repositories.resource_templates.ResourceTemplateRepository.query') +def test_get_templates_information_returns_only_templates_user_can_access(query_mock, resource_template_repo): + query_mock.return_value = [ + # Will get filtered out as don't have admin role + {"name": "template1", "title": "title1", "description": "description1", "authorizedRoles": ["admin"]}, + # Will get included as authorizedRoles=[] means any role is accepted + {"name": "template2", "title": "title2", "description": "description2", "authorizedRoles": []}, + # Will get included as have test role + {"name": "template3", "title": "title3", "description": "description3", "authorizedRoles": ["test"]} + ] + + result = resource_template_repo.get_templates_information(ResourceType.Workspace, ["test"]) + + assert len(result) == 2 + assert result[0].name == "template2" + assert result[1].name == "template3" + + @patch('db.repositories.resource_templates.ResourceTemplateRepository.save_item') @patch('uuid.uuid4') def test_create_workspace_template_item_calls_create_item_with_the_correct_parameters(uuid_mock, save_item_mock, resource_template_repo, input_workspace_template): diff --git a/api_app/tests_ma/test_db/test_repositories/test_shared_service_repository.py b/api_app/tests_ma/test_db/test_repositories/test_shared_service_repository.py index 7e525f1210..e923188f9a 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_shared_service_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_shared_service_repository.py @@ -92,7 +92,7 @@ def test_create_shared_service_item_creates_a_shared_with_the_right_values(valid shared_service_to_create = basic_shared_service_request validate_input_mock.return_value = basic_shared_service_template - shared_service, _ = shared_service_repo.create_shared_service_item(shared_service_to_create) + shared_service, _ = shared_service_repo.create_shared_service_item(shared_service_to_create, []) assert shared_service.templateName == basic_shared_service_request.templateName assert shared_service.resourceType == ResourceType.SharedService @@ -107,14 +107,14 @@ def test_create_shared_service_item_creates_a_shared_with_the_right_values(valid def test_create_shared_service_item_with_the_same_name_twice_fails(validate_input_mock, shared_service_repo, basic_shared_service_request, basic_shared_service_template): validate_input_mock.return_value = basic_shared_service_template - shared_service, _ = shared_service_repo.create_shared_service_item(basic_shared_service_request) + shared_service, _ = shared_service_repo.create_shared_service_item(basic_shared_service_request, []) shared_service_repo.save_item(shared_service) shared_service_repo.query = MagicMock() shared_service_repo.query.return_value = [shared_service.__dict__] with pytest.raises(DuplicateEntity): - shared_service = shared_service_repo.create_shared_service_item(basic_shared_service_request) + shared_service = shared_service_repo.create_shared_service_item(basic_shared_service_request, []) @patch('db.repositories.shared_services.SharedServiceRepository.validate_input_against_template', side_effect=ValueError) @@ -122,4 +122,4 @@ def test_create_shared_item_raises_value_error_if_template_is_invalid(_, shared_ shared_service_to_create = basic_shared_service_request with pytest.raises(ValueError): - shared_service_repo.create_shared_service_item(shared_service_to_create) + shared_service_repo.create_shared_service_item(shared_service_to_create, []) diff --git a/api_app/tests_ma/test_db/test_repositories/test_shared_service_templates_repository.py b/api_app/tests_ma/test_db/test_repositories/test_shared_service_templates_repository.py index 076533f03b..3a413ed8d9 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_shared_service_templates_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_shared_service_templates_repository.py @@ -39,7 +39,7 @@ def test_create_shared_service_template_item_calls_create_item_with_the_correct_ @patch('db.repositories.resource_templates.ResourceTemplateRepository.query') def test_get_templates_for_shared_services_queries_db(query_mock, resource_template_repo): - expected_query = 'SELECT c.name, c.title, c.description FROM c WHERE c.resourceType = "shared-service" AND c.current = true' + expected_query = 'SELECT c.name, c.title, c.description, c.authorizedRoles FROM c WHERE c.resourceType = "shared-service" AND c.current = true' query_mock.return_value = [sample_resource_template_as_dict(name="test", version="1.0", resource_type=ResourceType.SharedService)] resource_template_repo.get_templates_information(ResourceType.SharedService, parent_service_name="parent_service") diff --git a/api_app/tests_ma/test_db/test_repositories/test_user_resource_repository.py b/api_app/tests_ma/test_db/test_repositories/test_user_resource_repository.py index 69feab9e78..0d890913d0 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_user_resource_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_user_resource_repository.py @@ -45,7 +45,7 @@ def test_create_user_resource_item_creates_a_user_resource_with_the_right_values user_resource_to_create = basic_user_resource_request validate_input_mock.return_value = basic_user_resource_template - user_resource, _ = user_resource_repo.create_user_resource_item(user_resource_to_create, WORKSPACE_ID, SERVICE_ID, "parent-service-type", USER_ID) + user_resource, _ = user_resource_repo.create_user_resource_item(user_resource_to_create, WORKSPACE_ID, SERVICE_ID, "parent-service-type", USER_ID, []) assert user_resource.templateName == basic_user_resource_request.templateName assert user_resource.resourceType == ResourceType.UserResource @@ -60,7 +60,7 @@ def test_create_user_resource_item_creates_a_user_resource_with_the_right_values @patch('db.repositories.user_resources.UserResourceRepository.validate_input_against_template', side_effect=ValueError) def test_create_user_resource_item_raises_value_error_if_template_is_invalid(_, user_resource_repo, basic_user_resource_request): with pytest.raises(ValueError): - user_resource_repo.create_user_resource_item(basic_user_resource_request, WORKSPACE_ID, SERVICE_ID, "parent-service-type", USER_ID) + user_resource_repo.create_user_resource_item(basic_user_resource_request, WORKSPACE_ID, SERVICE_ID, "parent-service-type", USER_ID, []) @patch('db.repositories.user_resources.UserResourceRepository.query', return_value=[]) diff --git a/api_app/tests_ma/test_db/test_repositories/test_user_resource_templates_repository.py b/api_app/tests_ma/test_db/test_repositories/test_user_resource_templates_repository.py index d658dde264..898eb1eb52 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_user_resource_templates_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_user_resource_templates_repository.py @@ -14,7 +14,17 @@ def resource_template_repo(): def sample_user_resource_template_as_dict(name: str, version: str = "1.0") -> dict: - template = UserResourceTemplate(id="123", name=name, description="", version=version, current=True, required=[], properties={}, customActions=[], parentWorkspaceService="parent_service") + template = UserResourceTemplate( + id="123", + name=name, + description="", + version=version, + current=True, + required=[], + authorizedRoles=[], + properties={}, + customActions=[], + parentWorkspaceService="parent_service") return template.dict() @@ -95,7 +105,7 @@ def test_create_user_resource_template_item_calls_create_item_with_the_correct_p @patch('db.repositories.resource_templates.ResourceTemplateRepository.query') def test_get_template_infos_for_user_resources_queries_db(query_mock, resource_template_repo): - expected_query = 'SELECT c.name, c.title, c.description FROM c WHERE c.resourceType = "user-resource" AND c.current = true AND c.parentWorkspaceService = "parent_service"' + expected_query = 'SELECT c.name, c.title, c.description, c.authorizedRoles FROM c WHERE c.resourceType = "user-resource" AND c.current = true AND c.parentWorkspaceService = "parent_service"' query_mock.return_value = [sample_user_resource_template_as_dict(name="test", version="1.0")] resource_template_repo.get_templates_information(ResourceType.UserResource, parent_service_name="parent_service") diff --git a/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py b/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py index 5ce46a7227..e4707298cc 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py @@ -96,7 +96,7 @@ def test_create_workspace_item_creates_a_workspace_with_the_right_values(validat validate_input_mock.return_value = basic_resource_template new_cidr_mock.return_value = "1.2.3.4/24" - workspace, _ = workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id") + workspace, _ = workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"]) assert workspace.templateName == workspace_to_create.templateName assert workspace.resourceType == ResourceType.Workspace @@ -155,7 +155,7 @@ def test_create_workspace_item_creates_a_workspace_with_custom_address_space(val workspace_to_create.properties["address_space"] = "10.2.4.0/24" validate_input_mock.return_value = basic_resource_template - workspace, _ = workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id") + workspace, _ = workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"]) assert workspace.properties["address_space"] == workspace_to_create.properties["address_space"] @@ -172,7 +172,7 @@ def test_create_workspace_item_throws_exception_with_bad_custom_address_space(va validate_input_mock.return_value = basic_resource_template with pytest.raises(InvalidInput): - workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id") + workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"]) def test_get_address_space_based_on_size_with_custom_address_space_and_missing_address(workspace_repo, basic_workspace_request): @@ -190,7 +190,7 @@ def test_create_workspace_item_raises_value_error_if_template_is_invalid(validat validate_input_mock.side_effect = ValueError with pytest.raises(ValueError): - workspace_repo.create_workspace_item(workspace_input, {}, "test_object_id") + workspace_repo.create_workspace_item(workspace_input, {}, "test_object_id", ["test_role"]) def test_automatically_create_application_registration_returns_true(workspace_repo):