diff --git a/CHANGELOG.md b/CHANGELOG.md index 35214cdd0f..f63f3b5d30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ FEATURES: ENHANCEMENTS: +- Make user details avialble to resource processor, and update Windows and Linux VMs to use them. ([#4905](https://github.com/microsoft/AzureTRE/pull/3770)) * Disable storage account cross tenant replication ([#4116](https://github.com/microsoft/AzureTRE/pull/4116)) * Key Vaults should use RBAC instead of access policies for access control ([#4000](https://github.com/microsoft/AzureTRE/issues/4000)) * Split log entries with [Log chunk X of Y] for better readability. ([#3992](https://github.com/microsoft/AzureTRE/issues/3992)) diff --git a/api_app/_version.py b/api_app/_version.py index be6f6da961..6a726d853b 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.20.2" +__version__ = "0.21.0" diff --git a/api_app/models/domain/authentication.py b/api_app/models/domain/authentication.py index 3011440c7f..c52a123c91 100644 --- a/api_app/models/domain/authentication.py +++ b/api_app/models/domain/authentication.py @@ -10,6 +10,7 @@ class User(BaseModel): id: str name: str + username: str email: str = Field(None) roles: List[str] = Field([]) roleAssignments: List[RoleAssignment] = Field([]) diff --git a/api_app/models/domain/resource.py b/api_app/models/domain/resource.py index 1e660059ba..973d9e3b59 100644 --- a/api_app/models/domain/resource.py +++ b/api_app/models/domain/resource.py @@ -1,6 +1,7 @@ from enum import StrEnum from typing import Optional, Union, List from pydantic import BaseModel, Field, validator +from models.domain.authentication import User from models.domain.azuretremodel import AzureTREModel from models.domain.request_action import RequestAction from resources import strings @@ -50,7 +51,7 @@ class Resource(AzureTREModel): etag: str = Field(title="_etag", description="eTag of the document", alias="_etag") resourcePath: str = "" resourceVersion: int = 0 - user: dict = {} + user: Optional[User] updatedWhen: float = 0 def get_resource_request_message_payload(self, operation_id: str, step_id: str, action: RequestAction) -> dict: @@ -58,6 +59,12 @@ def get_resource_request_message_payload(self, operation_id: str, step_id: str, "operationId": operation_id, "stepId": step_id, "action": action, + "user": { + "id": self.user.id, + "name": self.user.name, + "email": self.user.email, + "username": self.user.username, + }, "id": self.id, "name": self.templateName, "version": self.templateVersion, diff --git a/api_app/services/aad_authentication.py b/api_app/services/aad_authentication.py index d99e194fdf..cd6ff840b1 100644 --- a/api_app/services/aad_authentication.py +++ b/api_app/services/aad_authentication.py @@ -141,6 +141,7 @@ def _get_user_from_token(decoded_token: dict) -> User: return User(id=user_id, name=decoded_token.get('name', ''), email=decoded_token.get('email', ''), + username=decoded_token.get('preferred_username', ''), roles=decoded_token.get('roles', [])) def _decode_token(self, token: str, ws_app_reg_id: str) -> dict: @@ -227,11 +228,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() @@ -276,23 +277,26 @@ def _get_users_inc_groups_from_response(self, users_graph_data, roles_graph_data # Handle user endpoint response user_id = user_data["body"]["id"] user_name = user_data["body"]["displayName"] + user_username = user_data["body"]["userPrincipalName"] if "users" in user_data["body"]["@odata.context"]: user_email = user_data["body"]["mail"] # if user with id does not already exist in users 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, username=user_username, roles=self._get_roles_for_principal(user_id, roles_graph_data, app_id_to_role_name))) # Handle group endpoint response elif "directoryObjects" in user_data["body"]["@odata.context"]: group_id = user_data["id"] for group_member in user_data["body"]["value"]: - user_id = group_member["id"] - user_name = group_member["displayName"] - user_email = group_member["mail"] - - 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))) + if group_member["@odata.type"] == "#microsoft.graph.user": + user_id = group_member["id"] + user_name = group_member["displayName"] + user_email = group_member["mail"] + user_username = group_member["userPrincipalName"] + + if not any(user.id == user_id for user in users): + users.append(User(id=user_id, name=user_name, email=user_email, username=user_username, roles=self._get_roles_for_principal(group_id, roles_graph_data, app_id_to_role_name))) return users diff --git a/api_app/tests_ma/conftest.py b/api_app/tests_ma/conftest.py index 0bd06e076d..929f8b01fe 100644 --- a/api_app/tests_ma/conftest.py +++ b/api_app/tests_ma/conftest.py @@ -321,7 +321,7 @@ def multi_step_resource_template(basic_shared_service_template) -> ResourceTempl @pytest.fixture def test_user(): - return User(id="user-id", name="test user", email="test@user.com") + return User(id="user-id", name="test user", email="test@user.com", username="testuser") @pytest.fixture diff --git a/api_app/tests_ma/test_api/conftest.py b/api_app/tests_ma/test_api/conftest.py index e781cf854e..f3589d3bd8 100644 --- a/api_app/tests_ma/test_api/conftest.py +++ b/api_app/tests_ma/test_api/conftest.py @@ -26,6 +26,7 @@ def create_test_user() -> User: return User( id="user-guid-here", name="Test User", + username="testuser", email="test@user.com", roles=[], roleAssignments=[] 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..5d9920098f 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 @@ -1661,14 +1661,16 @@ async def test_get_workspace_users_returns_users(self, _, auth_class, app, clien "name": "John Doe", "email": "john.doe@example.com", "roles": ["WorkspaceOwner", "WorkspaceResearcher"], - 'roleAssignments': [] + "roleAssignments": [], + "username": "johndoe" }, { "id": "456", "name": "Jane Smith", "email": "jane.smith@example.com", "roles": ["WorkspaceResearcher"], - 'roleAssignments': [] + "roleAssignments": [], + "username": "janesmith" } ] get_workspace_users_mock.return_value = users diff --git a/api_app/tests_ma/test_models/test_resource.py b/api_app/tests_ma/test_models/test_resource.py index cde5b7f764..58eef09e69 100644 --- a/api_app/tests_ma/test_models/test_resource.py +++ b/api_app/tests_ma/test_models/test_resource.py @@ -22,12 +22,12 @@ def test_resource_is_enabled_returns_correct_value(resource, expected): assert resource.isEnabled == expected -def test_user_resource_get_resource_request_message_payload_augments_payload_with_extra_params(): +def test_user_resource_get_resource_request_message_payload_augments_payload_with_extra_params(test_user): owner_id = "abc" workspace_id = "123" parent_service_id = "abcdef" - user_resource = UserResource(id="123", templateName="user-template", templateVersion="1.0", etag="", ownerId=owner_id, workspaceId=workspace_id, parentWorkspaceServiceId=parent_service_id, resourcePath="test") + user_resource = UserResource(id="123", templateName="user-template", templateVersion="1.0", etag="", ownerId=owner_id, workspaceId=workspace_id, parentWorkspaceServiceId=parent_service_id, resourcePath="test", user=test_user) message_payload = user_resource.get_resource_request_message_payload(OPERATION_ID, STEP_ID, RequestAction.Install) @@ -36,9 +36,9 @@ def test_user_resource_get_resource_request_message_payload_augments_payload_wit assert message_payload["parentWorkspaceServiceId"] == parent_service_id -def test_workspace_service_get_resource_request_message_payload_augments_payload_with_extra_params(): +def test_workspace_service_get_resource_request_message_payload_augments_payload_with_extra_params(test_user): workspace_id = "123" - workspace_service = WorkspaceService(id="123", templateName="service-template", templateVersion="1.0", etag="", workspaceId=workspace_id, resourcePath="test") + workspace_service = WorkspaceService(id="123", templateName="service-template", templateVersion="1.0", etag="", workspaceId=workspace_id, resourcePath="test", user=test_user) message_payload = workspace_service.get_resource_request_message_payload(OPERATION_ID, STEP_ID, RequestAction.Install) diff --git a/api_app/tests_ma/test_service_bus/test_resource_request_sender.py b/api_app/tests_ma/test_service_bus/test_resource_request_sender.py index 71e3785191..c992e20b1d 100644 --- a/api_app/tests_ma/test_service_bus/test_resource_request_sender.py +++ b/api_app/tests_ma/test_service_bus/test_resource_request_sender.py @@ -10,7 +10,6 @@ try_update_with_retries, update_resource_for_step, ) -from tests_ma.test_api.conftest import create_test_user from tests_ma.test_service_bus.test_deployment_status_update import ( create_sample_operation, ) @@ -25,7 +24,8 @@ pytestmark = pytest.mark.asyncio -def create_test_resource(): +@pytest.fixture +def test_resource(test_user): return Resource( id=str(uuid.uuid4()), resourceType=ResourceType.Workspace, @@ -34,6 +34,7 @@ def create_test_resource(): etag="", properties={"testParameter": "testValue"}, resourcePath="test", + user=test_user ) @@ -52,22 +53,23 @@ async def test_resource_request_message_generated_correctly( operations_repo_mock, resource_history_repo_mock, request_action, - multi_step_resource_template + multi_step_resource_template, + test_resource, + test_user ): service_bus_client_mock().get_queue_sender().send_messages = AsyncMock() - resource = create_test_resource() - operation = create_sample_operation(resource.id, request_action) + operation = create_sample_operation(test_resource.id, request_action) operations_repo_mock.create_operation_item.return_value = operation - resource_repo.get_resource_by_id.return_value = resource + resource_repo.get_resource_by_id.return_value = test_resource resource_template_repo.get_template_by_name_and_version.return_value = multi_step_resource_template - resource_repo.patch_resource.return_value = (resource, multi_step_resource_template) + resource_repo.patch_resource.return_value = (test_resource, multi_step_resource_template) await send_resource_request_message( - resource=resource, + resource=test_resource, operations_repo=operations_repo_mock, resource_repo=resource_repo, - user=create_test_user(), + user=test_user, resource_template_repo=resource_template_repo, resource_history_repo=resource_history_repo_mock, action=request_action @@ -80,7 +82,7 @@ async def test_resource_request_message_generated_correctly( sent_message = args[0] assert sent_message.correlation_id == operation.id sent_message_as_json = json.loads(str(sent_message)) - assert sent_message_as_json["id"] == resource.id + assert sent_message_as_json["id"] == test_resource.id assert sent_message_as_json["action"] == request_action 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..3943ffa461 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 @@ -21,6 +21,7 @@ def __init__(self, principal_id, mail, name): self.principal_id = principal_id self.mail = mail self.display_name = name + self.userPrincipalName = mail class GroupPrincipal: @@ -166,6 +167,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock): roleAssignments=[RoleAssignment(resource_id="ab123", role_id="ab124")], id="123", name="test", + username="test", email="t@t.com", ), Workspace( @@ -190,6 +192,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock): roleAssignments=[RoleAssignment(resource_id="ab127", role_id="ab124")], id="123", name="test", + username="test", email="t@t.com", ), Workspace( @@ -216,6 +219,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock): ], id="123", name="test", + username="test", email="t@t.com", ), Workspace( @@ -242,6 +246,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock): ], id="123", name="test", + username="test", email="t@t.com", ), Workspace( @@ -268,6 +273,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock): ], id="123", name="test", + username="test", email="t@t.com", ), Workspace( @@ -309,10 +315,9 @@ def test_get_workspace_role_returns_correct_owner( "services.aad_authentication.AzureADAuthorization.get_identity_role_assignments", return_value=[("ab123", "ab124")], ) -def test_raises_auth_config_error_if_workspace_auth_config_is_not_set(_): +def test_raises_auth_config_error_if_workspace_auth_config_is_not_set(_, test_user): access_service = AzureADAuthorization() - user = User(id="123", name="test", email="t@t.com") workspace_with_no_auth_config = Workspace( id="abc", etag="", @@ -323,9 +328,9 @@ def test_raises_auth_config_error_if_workspace_auth_config_is_not_set(_): with pytest.raises(AuthConfigValidationError): _ = access_service.get_workspace_role( - user, + test_user, workspace_with_no_auth_config, - access_service.get_identity_role_assignments(user.id), + access_service.get_identity_role_assignments(test_user.id), ) @@ -333,10 +338,9 @@ def test_raises_auth_config_error_if_workspace_auth_config_is_not_set(_): "services.aad_authentication.AzureADAuthorization.get_identity_role_assignments", return_value=[("ab123", "ab124")], ) -def test_raises_auth_config_error_if_auth_info_has_incorrect_roles(_): +def test_raises_auth_config_error_if_auth_info_has_incorrect_roles(_, test_user): access_service = AzureADAuthorization() - user = User(id="123", name="test", email="t@t.com") workspace_with_auth_info_but_no_roles = Workspace( id="abc", templateName="template-name", @@ -348,7 +352,7 @@ def test_raises_auth_config_error_if_auth_info_has_incorrect_roles(_): with pytest.raises(AuthConfigValidationError): _ = access_service.get_workspace_role( - user, + test_user, workspace_with_auth_info_but_no_roles, access_service.get_identity_role_assignments(), ) @@ -362,7 +366,7 @@ def test_raises_auth_config_error_if_auth_info_has_incorrect_roles(_): return_value="token", ) def test_get_workspace_user_emails_by_role_assignment_with_single_user_returns_user_mail_and_role_assignment( - _, users, roles, app_sp_graph_data_mock, user_response, roles_response, get_app_sp_graph_data_mock + _, users, roles, app_sp_graph_data_mock, user_response, roles_response, get_app_sp_graph_data_mock, test_user ): access_service = AzureADAuthorization() @@ -385,6 +389,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_single_user_returns_u "app_role_id_workspace_researcher": "ab125", "app_role_id_workspace_airlock_manager": "ab130", }, + user=test_user ) ) @@ -399,7 +404,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_single_user_returns_u return_value="token", ) def test_get_workspace_user_emails_by_role_assignment_with_single_user_with_no_mail_is_not_returned( - _, users, roles, app_sp_graph_data_mock, user_response, roles_response, get_app_sp_graph_data_mock + _, users, roles, app_sp_graph_data_mock, user_response, roles_response, get_app_sp_graph_data_mock, test_user ): access_service = AzureADAuthorization() @@ -425,6 +430,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_single_user_with_no_m "app_role_id_workspace_researcher": "ab125", "app_role_id_workspace_airlock_manager": "ab130", }, + user=test_user ) ) @@ -439,7 +445,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_single_user_with_no_m return_value="token", ) def test_get_workspace_user_emails_by_role_assignment_with_only_groups_assigned_returns_group_members( - _, users_and_groups, roles, app_sp_graph_data_mock, group_response, roles_response, get_app_sp_graph_data_mock + _, users_and_groups, roles, app_sp_graph_data_mock, group_response, roles_response, get_app_sp_graph_data_mock, test_user ): access_service = AzureADAuthorization() @@ -461,6 +467,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_only_groups_assigned_ "app_role_id_workspace_researcher": "ab125", "app_role_id_workspace_airlock_manager": "ab130", }, + user=test_user ) ) @@ -477,7 +484,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_only_groups_assigned_ return_value="token", ) def test_get_workspace_user_emails_by_role_assignment_with_groups_and_users_assigned_returned_as_expected( - _, users_and_groups, roles, app_sp_graph_data_mock, roles_response, get_app_sp_graph_data_mock, users_and_group_response + _, users_and_groups, roles, app_sp_graph_data_mock, roles_response, get_app_sp_graph_data_mock, users_and_group_response, test_user ): access_service = AzureADAuthorization() @@ -500,6 +507,7 @@ def test_get_workspace_user_emails_by_role_assignment_with_groups_and_users_assi "app_role_id_workspace_researcher": "ab125", "app_role_id_workspace_airlock_manager": "ab130", }, + user=test_user ) ) @@ -587,7 +595,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, "mail": mail, "id": principal_id, "displayName": name, "userPrincipalName": mail}, } return user_response_body @@ -603,6 +611,7 @@ def get_mock_group_response(group): "mail": member.mail, "id": member.principal_id, "displayName": member.display_name, + "userPrincipalName": member.mail } ) group_response_body = { diff --git a/resource_processor/_version.py b/resource_processor/_version.py index 17c1a6260b..e07010c831 100644 --- a/resource_processor/_version.py +++ b/resource_processor/_version.py @@ -1 +1 @@ -__version__ = "0.10.2" +__version__ = "0.11.0 diff --git a/resource_processor/resources/commands.py b/resource_processor/resources/commands.py index 0111b52358..4f5d392901 100644 --- a/resource_processor/resources/commands.py +++ b/resource_processor/resources/commands.py @@ -89,7 +89,7 @@ async def build_porter_command(config, msg_body, custom_action=False): f"{' invoke --action' if custom_action else ''}" f" {msg_body['action']} \"{installation_id}\"" f" --reference {config['registry_server']}/{msg_body['name']}:v{msg_body['version']}" - f" {porter_parameters} --force" + f"{porter_parameters} --force" f" --credential-set arm_auth" f" --credential-set aad_auth" ] diff --git a/resource_processor/tests_rp/test_commands.py b/resource_processor/tests_rp/test_commands.py new file mode 100644 index 0000000000..0b3b45b67c --- /dev/null +++ b/resource_processor/tests_rp/test_commands.py @@ -0,0 +1,97 @@ +import pytest +from mock import patch +from resources.commands import build_porter_command + +pytestmark = pytest.mark.asyncio + + +@pytest.fixture +def payload(): + return { + "operationId": "op123", + "stepId": "step123", + "action": "some_action", + "user": { + "id": "user123", + "name": "John Doe", + "email": "john.doe@example.com", + "username": "johndoe", + }, + "id": "resource123", + "name": "Resource Name", + "version": "1.0", + "parameters": { + "param1": {"key": "value"}, + "param2": ["item1", "item2"] + } + } + + +@pytest.fixture +def config(): + return { + "registry_server": "myregistry.azurecr.io", + "porter_env": {}, + "bundle_params": {} + } + + +@patch('resources.commands.get_porter_parameter_keys') +async def test_build_porter_command_custom_action(mock_get_porter_parameter_keys, payload, config): + mock_get_porter_parameter_keys.return_value = ["param1", "param2"] + + msg_body = payload + msg_body["action"] = "custom_action" + + command = await build_porter_command(config, msg_body, custom_action=True) + + expected_command = [ + 'porter invoke --action custom_action "resource123" --reference myregistry.azurecr.io/Resource Name:v1.0 --param param1="eyJrZXkiOiAidmFsdWUifQ==" --param param2="WyJpdGVtMSIsICJpdGVtMiJd" --force --credential-set arm_auth --credential-set aad_auth' + ] + + assert command == expected_command + + +@patch('resources.commands.get_porter_parameter_keys') +async def test_build_porter_command_no_parameters(mock_get_porter_parameter_keys, payload, config): + mock_get_porter_parameter_keys.return_value = None + + msg_body = payload + + command = await build_porter_command(config, msg_body) + + expected_command = [ + 'porter some_action "resource123" --reference myregistry.azurecr.io/Resource Name:v1.0 --force --credential-set arm_auth --credential-set aad_auth' + ] + + assert command == expected_command + + +@patch('resources.commands.get_porter_parameter_keys') +async def test_build_porter_command_complex_parameters(mock_get_porter_parameter_keys, payload, config): + mock_get_porter_parameter_keys.return_value = ["param1", "param2"] + + msg_body = payload + + command = await build_porter_command(config, msg_body) + + expected_command = [ + 'porter some_action "resource123" --reference myregistry.azurecr.io/Resource Name:v1.0 --param param1="eyJrZXkiOiAidmFsdWUifQ==" --param param2="WyJpdGVtMSIsICJpdGVtMiJd" --force --credential-set arm_auth --credential-set aad_auth' + ] + + assert command == expected_command + + +@patch('resources.commands.get_porter_parameter_keys') +async def test_build_porter_command_user_parameters(mock_get_porter_parameter_keys, payload, config): + mock_get_porter_parameter_keys.return_value = ["user_email", "user_name"] + + msg_body = payload + + command = await build_porter_command(config, msg_body) + + expected_command = [ + 'porter some_action "resource123" --reference myregistry.azurecr.io/Resource Name:v1.0 --param user_email="john.doe@example.com" --param user_name="John Doe" --force --credential-set arm_auth --credential-set aad_auth' + ] + + assert command == expected_command diff --git a/resource_processor/tests_rp/test_logging.py b/resource_processor/tests_rp/test_logging.py index 52c4c53c5c..144bdd010b 100644 --- a/resource_processor/tests_rp/test_logging.py +++ b/resource_processor/tests_rp/test_logging.py @@ -1,16 +1,19 @@ +import pytest from mock import patch import logging from shared.logging import shell_output_logger, chunk_log_output +pytestmark = pytest.mark.asyncio + @patch("shared.logging.logger") -def test_shell_output_logger_empty_console_output(mock_logger): +async def test_shell_output_logger_empty_console_output(mock_logger): shell_output_logger("", "prefix", logging.DEBUG) mock_logger.debug.assert_called_once_with("shell console output is empty.") @patch("shared.logging.logger") -def test_shell_output_logger_image_not_present_locally(mock_logger): +async def test_shell_output_logger_image_not_present_locally(mock_logger): console_output = "Unable to find image 'test_image' locally\nexecution completed successfully!" shell_output_logger(console_output, "prefix", logging.DEBUG) mock_logger.debug.assert_called_with("Image not present locally, removing text from console output.") @@ -18,14 +21,14 @@ def test_shell_output_logger_image_not_present_locally(mock_logger): @patch("shared.logging.logger") -def test_shell_output_logger_execution_completed_successfully(mock_logger): +async def test_shell_output_logger_execution_completed_successfully(mock_logger): console_output = "execution completed successfully!" shell_output_logger(console_output, "prefix", logging.DEBUG) mock_logger.log.assert_called_with(logging.INFO, "prefix execution completed successfully!") @patch("shared.logging.logger") -def test_shell_output_logger_normal_case(mock_logger): +async def test_shell_output_logger_normal_case(mock_logger): console_output = "Some logs" shell_output_logger(console_output, "prefix", logging.DEBUG) mock_logger.log.assert_called_with(logging.DEBUG, "prefix Some logs") diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/porter.yaml b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/porter.yaml index 98b827932f..160978ee28 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/porter.yaml +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/porter.yaml @@ -106,6 +106,11 @@ parameters: - name: shared_storage_name type: string default: "vm-shared-storage" + - name: user_id + type: string + - name: user_username + type: string + outputs: - name: ip @@ -151,6 +156,8 @@ install: shared_storage_access: ${ bundle.parameters.shared_storage_access } shared_storage_name: ${ bundle.parameters.shared_storage_name } image_gallery_id: ${ bundle.parameters.image_gallery_id } + user_id: ${ bundle.parameters.user_id } + user_username: ${ bundle.parameters.user_username } enable_shutdown_schedule: ${ bundle.parameters.enable_shutdown_schedule } shutdown_time: ${ bundle.parameters.shutdown_time } shutdown_timezone: ${ bundle.parameters.shutdown_timezone } @@ -180,6 +187,8 @@ upgrade: shared_storage_access: ${ bundle.parameters.shared_storage_access } shared_storage_name: ${ bundle.parameters.shared_storage_name } image_gallery_id: ${ bundle.parameters.image_gallery_id } + user_id: ${ bundle.parameters.user_id } + user_username: ${ bundle.parameters.user_username } enable_shutdown_schedule: ${ bundle.parameters.enable_shutdown_schedule } shutdown_time: ${ bundle.parameters.shutdown_time } shutdown_timezone: ${ bundle.parameters.shutdown_timezone } @@ -221,6 +230,8 @@ uninstall: shared_storage_access: ${ bundle.parameters.shared_storage_access } shared_storage_name: ${ bundle.parameters.shared_storage_name } image_gallery_id: ${ bundle.parameters.image_gallery_id } + user_id: ${ bundle.parameters.user_id } + user_username: ${ bundle.parameters.user_username } enable_shutdown_schedule: ${ bundle.parameters.enable_shutdown_schedule } shutdown_time: ${ bundle.parameters.shutdown_time } shutdown_timezone: ${ bundle.parameters.shutdown_timezone } diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/linuxvm.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/linuxvm.tf index 68b9d36773..dc88149d5d 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/linuxvm.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/linuxvm.tf @@ -12,16 +12,6 @@ resource "azurerm_network_interface" "internal" { lifecycle { ignore_changes = [tags] } } -resource "random_string" "username" { - length = 4 - upper = true - lower = true - numeric = true - min_numeric = 1 - min_lower = 1 - special = false -} - resource "random_password" "password" { length = 16 lower = true @@ -42,7 +32,7 @@ resource "azurerm_linux_virtual_machine" "linuxvm" { network_interface_ids = [azurerm_network_interface.internal.id] size = local.vm_sizes[var.vm_size] disable_password_authentication = false - admin_username = random_string.username.result + admin_username = local.admin_username admin_password = random_password.password.result custom_data = data.template_cloudinit_config.config.rendered @@ -110,7 +100,7 @@ data "template_file" "vm_config" { FILESHARE_NAME = var.shared_storage_access ? data.azurerm_storage_share.shared_storage[0].name : "" NEXUS_PROXY_URL = local.nexus_proxy_url CONDA_CONFIG = local.selected_image.conda_config ? 1 : 0 - VM_USER = random_string.username.result + VM_USER = local.admin_username APT_SKU = replace(local.apt_sku, ".", "") } } @@ -139,7 +129,7 @@ data "template_file" "apt_sources_config" { resource "azurerm_key_vault_secret" "linuxvm_password" { name = local.vm_password_secret_name - value = "${random_string.username.result}\n${random_password.password.result}" + value = "${local.admin_username}\n${random_password.password.result}" key_vault_id = data.azurerm_key_vault.ws.id tags = local.tre_user_resources_tags diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf index e0281269fd..0d7d66dc53 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf @@ -7,12 +7,15 @@ locals { vm_name = "linuxvm${local.short_service_id}" keyvault_name = lower("kv-${substr(local.workspace_resource_name_suffix, -20, -1)}") storage_name = lower(replace("stg${substr(local.workspace_resource_name_suffix, -8, -1)}", "-", "")) + admin_username = element(split("@", var.user_username), 0) vm_password_secret_name = "${local.vm_name}-admin-credentials" tre_user_resources_tags = { tre_id = var.tre_id tre_workspace_id = var.workspace_id tre_workspace_service_id = var.parent_service_id tre_user_resource_id = var.tre_resource_id + tre_user_id = var.user_id + tre_user_username = var.user_username } nexus_proxy_url = "https://nexus-${data.azurerm_public_ip.app_gateway_ip.fqdn}" # Load VM SKU/image details from porter.yaml diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/outputs.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/outputs.tf index 30a5a90a39..8b3a56618f 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/outputs.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/outputs.tf @@ -15,7 +15,7 @@ output "connection_uri" { } output "vm_username" { - value = random_string.username.result + value = local.admin_username } output "vm_password_secret_name" { diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/variables.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/variables.tf index a515e46e30..bbdf2501a4 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/variables.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/variables.tf @@ -26,6 +26,12 @@ variable "image_gallery_id" { type = string default = "" } +variable "user_id" { + type = string +} +variable "user_username" { + type = string +} variable "enable_shutdown_schedule" { type = bool default = false diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/porter.yaml b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/porter.yaml index 7a1c1d1cf4..36367268c3 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/porter.yaml +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-guacamole-windowsvm -version: 1.0.7 +version: 1.1.0 description: "An Azure TRE User Resource Template for Guacamole (Windows 10)" dockerfile: Dockerfile.tmpl registry: azuretre @@ -98,6 +98,14 @@ parameters: - name: shared_storage_name type: string default: "vm-shared-storage" + - name: user_id + type: string + - name: user_name + type: string + - name: user_email + type: string + - name: user_username + type: string - name: arm_environment type: string @@ -145,6 +153,10 @@ install: shared_storage_access: ${ bundle.parameters.shared_storage_access } shared_storage_name: ${ bundle.parameters.shared_storage_name } image_gallery_id: ${ bundle.parameters.image_gallery_id } + user_id: ${ bundle.parameters.user_id } + user_name: ${ bundle.parameters.user_name } + user_email: ${ bundle.parameters.user_email } + user_username: ${ bundle.parameters.user_username } backendConfig: use_azuread_auth: "true" use_oidc: "true" @@ -171,6 +183,10 @@ upgrade: shared_storage_access: ${ bundle.parameters.shared_storage_access } shared_storage_name: ${ bundle.parameters.shared_storage_name } image_gallery_id: ${ bundle.parameters.image_gallery_id } + user_id: ${ bundle.parameters.user_id } + user_name: ${ bundle.parameters.user_name } + user_email: ${ bundle.parameters.user_email } + user_username: ${ bundle.parameters.user_username } backendConfig: use_azuread_auth: "true" use_oidc: "true" @@ -209,6 +225,10 @@ uninstall: shared_storage_access: ${ bundle.parameters.shared_storage_access } shared_storage_name: ${ bundle.parameters.shared_storage_name } image_gallery_id: ${ bundle.parameters.image_gallery_id } + user_id: ${ bundle.parameters.user_id } + user_name: ${ bundle.parameters.user_name } + user_email: ${ bundle.parameters.user_email } + user_username: ${ bundle.parameters.user_username } backendConfig: use_azuread_auth: "true" use_oidc: "true" diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf index e5137d1967..bc11e529a0 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf @@ -7,12 +7,15 @@ locals { vm_name = "windowsvm${local.short_service_id}" keyvault_name = lower("kv-${substr(local.workspace_resource_name_suffix, -20, -1)}") storage_name = lower(replace("stg${substr(local.workspace_resource_name_suffix, -8, -1)}", "-", "")) + admin_username = element(split("@", var.user_username), 0) vm_password_secret_name = "${local.vm_name}-admin-credentials" tre_user_resources_tags = { tre_id = var.tre_id tre_workspace_id = var.workspace_id tre_workspace_service_id = var.parent_service_id tre_user_resource_id = var.tre_resource_id + tre_user_id = var.user_id + tre_user_username = var.user_username } nexus_proxy_url = "https://nexus-${data.azurerm_public_ip.app_gateway_ip.fqdn}" diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/outputs.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/outputs.tf index 25737aa472..9d085b0ccd 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/outputs.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/outputs.tf @@ -15,7 +15,7 @@ output "connection_uri" { } output "vm_username" { - value = random_string.username.result + value = local.admin_username } output "vm_password_secret_name" { diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/variables.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/variables.tf index 4908ae52a2..6529b07c84 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/variables.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/variables.tf @@ -26,3 +26,11 @@ variable "image_gallery_id" { type = string default = "" } + +variable "user_id" { + type = string +} + +variable "user_username" { + type = string +} diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf index 575f8a7efd..68c08e7151 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf @@ -13,16 +13,6 @@ resource "azurerm_network_interface" "internal" { lifecycle { ignore_changes = [tags] } } -resource "random_string" "username" { - length = 4 - upper = true - lower = true - numeric = true - min_numeric = 1 - min_lower = 1 - special = false -} - resource "random_password" "password" { length = 16 lower = true @@ -43,7 +33,7 @@ resource "azurerm_windows_virtual_machine" "windowsvm" { network_interface_ids = [azurerm_network_interface.internal.id] size = local.vm_sizes[var.vm_size] allow_extension_operations = true - admin_username = random_string.username.result + admin_username = local.admin_username admin_password = random_password.password.result custom_data = base64encode(templatefile( @@ -104,7 +94,7 @@ PROT resource "azurerm_key_vault_secret" "windowsvm_password" { name = "${local.vm_name}-admin-credentials" - value = "${random_string.username.result}\n${random_password.password.result}" + value = "${local.admin_username}\n${random_password.password.result}" key_vault_id = data.azurerm_key_vault.ws.id tags = local.tre_user_resources_tags