diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ad99b47a2..cb47bd6909 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ * Add data science tooling (Azure CLI, VS Code, Storage Explorer, Git, Python/JupyterLab, R/RStudio) to Guacamole Windows VMs via a shared `vm_config.ps1` bootstrap pulled through Nexus. Existing `tre-service-guacamole-windowsvm`, `tre-service-guacamole-import-reviewvm`, and `tre-service-guacamole-export-reviewvm` resources **must not be upgraded** to these new versions — redeploy instead. Upgrade the Nexus shared service to `sonatype-nexus` 3.9.0 before deploying the new Windows VM templates to ensure the required proxy repositories are available. (`tre-service-guacamole-windowsvm` 3.0.0, `tre-service-guacamole-import-reviewvm`/`tre-service-guacamole-export-reviewvm` 2.0.0, `sonatype-nexus` 3.9.0) ([#4981](https://github.com/microsoft/AzureTRE/pull/4981)) ENHANCEMENTS: +* Add secrets retrieval API endpoints for workspace services and user resources so researchers can access secrets (e.g. storage keys, database connection details) that resources store in the workspace Key Vault. Resource properties whose name contains `keyvault_secret_id` are treated as references to workspace Key Vault secrets, and the UI adds a reveal/copy affordance to display these secret values on demand. Key Vault is accessed through an On-Behalf-Of exchange authenticated by the per-workspace app registration via a federated identity credential (no stored client secret), so secrets are read as the signed-in user rather than the core API identity. (API 0.28.0, `tre-workspace-base` 2.12.0, UI 0.9.0) ([#2402](https://github.com/microsoft/AzureTRE/issues/2402)) +* Grant the owning user `Key Vault Secrets User` (secrets reader) scoped to their own VM admin password secret on the Guacamole Windows and Linux VMs, so users get least-privilege read access to their own credentials. Airlock review VMs are unaffected as they use ephemeral random credentials. (`tre-service-guacamole-windowsvm` 3.1.0, `tre-service-guacamole-linuxvm` 1.5.0, `tre-service-guacamole-import-reviewvm`/`tre-service-guacamole-export-reviewvm` 2.0.1) ([#2402](https://github.com/microsoft/AzureTRE/issues/2402)) * Enable graceful upgrading of the Nexus shared service: modified or added repository configs and the container image (now `3.94.0`) are applied to the existing VM on upgrade without recreating it. Removed the non-functional `snapcraft` proxy (dead remote URL) and skip repositories left in a failed state so they don't block upgrades. (`sonatype-nexus` 3.10.0) ([#2721](https://github.com/microsoft/AzureTRE/issues/2721)) * Specify default_outbound_access_enabled = false setting for all subnets ([#4757](https://github.com/microsoft/AzureTRE/pull/4757)) * Pin all GitHub Actions workflow steps to full commit SHAs to prevent supply chain attacks plus update to latest releases ([#4886](https://github.com/microsoft/AzureTRE/pull/4886)) diff --git a/api_app/_version.py b/api_app/_version.py index 7c4a9591e1..df7db86487 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.26.0" +__version__ = "0.28.0" diff --git a/api_app/api/routes/workspaces.py b/api_app/api/routes/workspaces.py index 04ea2d6513..f7e8dc04b2 100644 --- a/api_app/api/routes/workspaces.py +++ b/api_app/api/routes/workspaces.py @@ -22,6 +22,7 @@ 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.secret import SecretInResponse from resources import strings from services.aad_authentication import AuthConfigValidationError from auth.rbac import require_tre_admin, require_workspace_owner, \ @@ -30,6 +31,7 @@ require_workspace_owner_or_researcher_or_airlock_manager_or_tre_admin from services.authentication import get_aad_service, extract_auth_information from services.azure_resource_status import get_azure_resource_status +from services.secrets import get_secret_value, is_secret_property, SecretRetrievalError from azure.cosmos.exceptions import CosmosAccessConditionFailedError from .resource_helpers import cascaded_update_resource, delete_validation, enrich_resource_with_available_upgrades, get_identity_role_assignments, save_and_deploy_resource, construct_location_header, send_uninstall_message, \ @@ -53,6 +55,33 @@ def validate_user_has_valid_role_for_user_resource(user, user_resource): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=strings.ACCESS_USER_IS_NOT_OWNER_OR_RESEARCHER) +def _extract_bearer_token(authorization: str) -> str: + """Return the raw bearer token from an Authorization header value, or None.""" + if authorization and authorization.lower().startswith("bearer "): + return authorization[len("bearer "):].strip() + return None + + +async def retrieve_resource_secret(resource, secret_name: str, workspace, user_token: str) -> SecretInResponse: + if not is_secret_property(secret_name): + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strings.SECRET_PROPERTY_IS_NOT_A_SECRET) + + keyvault_secret_id = resource.properties.get(secret_name) + if not keyvault_secret_id: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=strings.SECRET_PROPERTY_DOES_NOT_EXIST) + + try: + secret_value = await get_secret_value( + keyvault_secret_id, + workspace.properties.get("keyvault_uri"), + workspace.properties.get("client_id"), + user_token) + except SecretRetrievalError as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) + + return SecretInResponse(key=secret_name, value=secret_value) + + # WORKSPACE ROUTES @workspaces_core_router.get("/workspaces", response_model=WorkspacesInList, name=strings.API_GET_ALL_WORKSPACES) async def retrieve_users_active_workspaces(user=Depends(require_tre_user_or_admin), workspace_repo=Depends(get_repository(WorkspaceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository))) -> WorkspacesInList: @@ -241,6 +270,11 @@ async def retrieve_workspace_service_by_id(workspace_service=Depends(get_workspa return WorkspaceServiceInResponse(workspaceService=workspace_service) +@workspace_services_workspace_router.get("/workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name}", response_model=SecretInResponse, name=strings.API_GET_WORKSPACE_SERVICE_SECRET, dependencies=[Depends(require_workspace_owner_or_researcher_or_airlock_manager)]) +async def retrieve_workspace_service_secret(secret_name: str, workspace=Depends(get_workspace_by_id_from_path), workspace_service=Depends(get_workspace_service_by_id_from_path), authorization: str = Header(None)) -> SecretInResponse: + return await retrieve_resource_secret(workspace_service, secret_name, workspace, _extract_bearer_token(authorization)) + + @workspace_services_workspace_router.post("/workspaces/{workspace_id}/workspace-services", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_CREATE_WORKSPACE_SERVICE, dependencies=[Depends(require_workspace_owner)]) async def create_workspace_service(response: Response, workspace_service_input: WorkspaceServiceInCreate, user=Depends(require_workspace_owner), workspace_service_repo=Depends(get_repository(WorkspaceServiceRepository)), workspace_repo=Depends(get_repository(WorkspaceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), resource_history_repo=Depends(get_repository(ResourceHistoryRepository)), workspace=Depends(get_deployed_workspace_by_id_from_path)) -> OperationInResponse: @@ -400,6 +434,17 @@ async def retrieve_user_resource_by_id( return UserResourceInResponse(userResource=user_resource) +@user_resources_workspace_router.get("/workspaces/{workspace_id}/workspace-services/{service_id}/user-resources/{resource_id}/secrets/{secret_name}", response_model=SecretInResponse, name=strings.API_GET_USER_RESOURCE_SECRET, dependencies=[Depends(get_workspace_by_id_from_path)]) +async def retrieve_user_resource_secret( + secret_name: str, + workspace=Depends(get_workspace_by_id_from_path), + user_resource=Depends(get_user_resource_by_id_from_path), + authorization: str = Header(None), + user=Depends(require_workspace_owner_or_researcher_or_airlock_manager)) -> SecretInResponse: + validate_user_has_valid_role_for_user_resource(user, user_resource) + return await retrieve_resource_secret(user_resource, secret_name, workspace, _extract_bearer_token(authorization)) + + @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, diff --git a/api_app/models/schemas/secret.py b/api_app/models/schemas/secret.py new file mode 100644 index 0000000000..12e97823fc --- /dev/null +++ b/api_app/models/schemas/secret.py @@ -0,0 +1,14 @@ +from pydantic import BaseModel, Field + + +class SecretInResponse(BaseModel): + key: str = Field("", title="Property name", description="Name of the resource property that references the secret") + value: str = Field("", title="Secret value", description="The secret value retrieved from the workspace Key Vault") + + class Config: + schema_extra = { + "example": { + "key": "admin_password_keyvault_secret_id", + "value": "a-very-secret-value" + } + } diff --git a/api_app/requirements.txt b/api_app/requirements.txt index be68fe5bf5..260c5c09f7 100644 --- a/api_app/requirements.txt +++ b/api_app/requirements.txt @@ -3,6 +3,7 @@ azure-core==1.38.0 azure-cosmos==4.14.3 azure-eventgrid==4.22.0 azure-identity==1.25.1 +azure-keyvault-secrets==4.11.0 azure-mgmt-compute==37.1.0 azure-mgmt-cosmosdb==9.9.0 azure-mgmt-costmanagement==4.0.1 diff --git a/api_app/resources/strings.py b/api_app/resources/strings.py index 09432e64a3..b781253663 100644 --- a/api_app/resources/strings.py +++ b/api_app/resources/strings.py @@ -23,6 +23,7 @@ API_GET_ALL_WORKSPACE_SERVICES = "Get all workspace services for workspace" API_GET_WORKSPACE_SERVICE_BY_ID = "Get workspace service by Id" +API_GET_WORKSPACE_SERVICE_SECRET = "Get a workspace service secret value from the workspace Key Vault" API_CREATE_WORKSPACE_SERVICE = "Create a workspace service" API_UPDATE_WORKSPACE_SERVICE = "Update an existing workspace service" API_DELETE_WORKSPACE_SERVICE = "Delete workspace service" @@ -34,6 +35,7 @@ API_CREATE_USER_RESOURCE = "Create a user resource" API_GET_MY_USER_RESOURCES = "Get my user resources in the workspace service" API_GET_USER_RESOURCE = "Get user resource by id" +API_GET_USER_RESOURCE_SECRET = "Get a user resource secret value from the workspace Key Vault" API_DELETE_USER_RESOURCE = "Delete user resource" API_UPDATE_USER_RESOURCE = "Update an existing user resource" API_INVOKE_ACTION_ON_USER_RESOURCE = "Invoke action on a user resource" @@ -148,6 +150,13 @@ CUSTOM_ACTION_NOT_DEFINED = "The specified custom action isn't defined in the targeted resource." CUSTOM_ACTIONS_DO_NOT_EXIST = "The resource being targeted does not implement any custom actions." +SECRET_PROPERTY_DOES_NOT_EXIST = "The resource does not have a property with the requested name" +SECRET_PROPERTY_IS_NOT_A_SECRET = "The requested property does not reference a Key Vault secret" +INVALID_KEYVAULT_SECRET_ID = "The property value is not a valid Key Vault secret identifier" +KEYVAULT_SECRET_OUTSIDE_WORKSPACE = "The referenced secret does not belong to the workspace Key Vault" +KEYVAULT_SECRET_NOT_FOUND = "The secret could not be found in the workspace Key Vault" +UNABLE_TO_RETRIEVE_KEYVAULT_SECRET = "Unable to retrieve the secret from the workspace Key Vault" + WORKSPACE_SERVICE_TEMPLATE_DOES_NOT_EXIST = "Could not retrieve the workspace service template specified" TEMPLATE_DOES_NOT_EXIST = "Could not retrieve the template with this name, or name-version pair" NO_UNIQUE_CURRENT_FOR_TEMPLATE = "The template has multiple 'current' versions" diff --git a/api_app/services/secrets.py b/api_app/services/secrets.py new file mode 100644 index 0000000000..55400cf7d8 --- /dev/null +++ b/api_app/services/secrets.py @@ -0,0 +1,110 @@ +from typing import Optional +from urllib.parse import urlparse + +from azure.core.exceptions import ResourceNotFoundError +from azure.identity import ManagedIdentityCredential +from azure.identity.aio import OnBehalfOfCredential +from azure.keyvault.secrets import KeyVaultSecretIdentifier +from azure.keyvault.secrets.aio import SecretClient + +from core.config import AAD_AUTHORITY_URL, AAD_TENANT_ID, MANAGED_IDENTITY_CLIENT_ID +from resources import strings +from services.logging import logger + + +# Convention: any resource property whose name contains this token holds a +# Key Vault secret identifier rather than the secret value itself. +KEYVAULT_SECRET_ID_TOKEN = "keyvault_secret_id" + +# Audience used when the API's managed identity requests a token to use as the +# client assertion in the On-Behalf-Of exchange. The managed identity is +# configured as a federated identity credential on the workspace app +# registration, so this token replaces a stored client secret. +TOKEN_EXCHANGE_SCOPE = "api://AzureADTokenExchange/.default" + + +def is_secret_property(property_name: str) -> bool: + """Return True if the property name follows the secret naming convention.""" + return KEYVAULT_SECRET_ID_TOKEN in property_name + + +def _get_client_assertion() -> str: + """Return a managed identity token to use as the OBO client assertion. + + The API's managed identity is registered as a federated identity credential + on the workspace app registration. A managed identity token for the token + exchange audience is therefore accepted by Entra ID in place of a workspace + client secret, so no secret needs to be stored or rotated. + """ + credential = ManagedIdentityCredential(client_id=MANAGED_IDENTITY_CLIENT_ID) + try: + return credential.get_token(TOKEN_EXCHANGE_SCOPE).token + finally: + credential.close() + + +def _get_obo_credential(workspace_client_id: str, user_token: str) -> OnBehalfOfCredential: + """Build an On-Behalf-Of credential for the workspace app registration. + + The workspace app registration acts as the confidential client, using the + federated managed identity assertion to authenticate, and exchanges the + caller's token so that downstream Key Vault access is performed on behalf of + the signed-in user. + """ + return OnBehalfOfCredential( + tenant_id=AAD_TENANT_ID, + client_id=workspace_client_id, + user_assertion=user_token, + client_assertion_func=_get_client_assertion, + authority=urlparse(AAD_AUTHORITY_URL).netloc, + ) + + +async def get_secret_value( + keyvault_secret_id: str, + expected_keyvault_uri: Optional[str] = None, + workspace_client_id: Optional[str] = None, + user_token: Optional[str] = None, +) -> str: + """Retrieve a secret value from a workspace Key Vault on behalf of the user. + + The ``keyvault_secret_id`` is a full Key Vault secret identifier (URI) as + output by a resource template. When ``expected_keyvault_uri`` is provided the + secret's vault must match it, ensuring a resource can only expose secrets that + live in its own workspace Key Vault. + + ``workspace_client_id`` (the workspace app registration client id) and + ``user_token`` (the caller's access token) are used to perform an + On-Behalf-Of exchange so Key Vault access is authorised as the signed-in + user rather than the API's own identity. + """ + if not workspace_client_id or not user_token: + raise SecretRetrievalError(strings.UNABLE_TO_RETRIEVE_KEYVAULT_SECRET) + + try: + parsed_secret_id = KeyVaultSecretIdentifier(keyvault_secret_id) + except ValueError: + raise SecretRetrievalError(strings.INVALID_KEYVAULT_SECRET_ID) + + if expected_keyvault_uri and not _same_vault(parsed_secret_id.vault_url, expected_keyvault_uri): + raise SecretRetrievalError(strings.KEYVAULT_SECRET_OUTSIDE_WORKSPACE) + + async with _get_obo_credential(workspace_client_id, user_token) as credential: + async with SecretClient(vault_url=parsed_secret_id.vault_url, credential=credential) as secret_client: + try: + secret = await secret_client.get_secret(parsed_secret_id.name) + except ResourceNotFoundError: + raise SecretRetrievalError(strings.KEYVAULT_SECRET_NOT_FOUND) + except Exception: + logger.exception("Failed to retrieve secret from Key Vault") + raise SecretRetrievalError(strings.UNABLE_TO_RETRIEVE_KEYVAULT_SECRET) + + return secret.value + + +def _same_vault(vault_url_a: str, vault_url_b: str) -> bool: + return urlparse(vault_url_a).netloc.lower() == urlparse(vault_url_b).netloc.lower() + + +class SecretRetrievalError(Exception): + """Raised when a workspace Key Vault secret cannot be retrieved.""" 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 8eb8fd8a53..1782a9973d 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 @@ -22,6 +22,7 @@ from models.domain.workspace import Workspace, WorkspaceRole from models.domain.workspace_service import WorkspaceService from resources import strings +from services.secrets import SecretRetrievalError from models.schemas.resource_template import ResourceTemplateInformation from auth.rbac import require_tre_admin, \ require_tre_user_or_admin, require_workspace_owner, \ @@ -960,6 +961,96 @@ async def test_get_user_resource_returns_a_user_resource_if_found(self, get_user assert response.status_code == status.HTTP_200_OK assert response.json()["userResource"]["id"] == user_resource.id + # [GET] /workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name} + @patch("api.routes.workspaces.get_secret_value", return_value="a-secret-value") + @patch("api.dependencies.workspaces.WorkspaceServiceRepository.get_workspace_service_by_id") + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + async def test_get_workspace_service_secret_returns_secret_value(self, get_workspace_mock, get_workspace_service_mock, get_secret_value_mock, app, client): + get_workspace_mock.return_value = sample_workspace() + workspace_service = sample_workspace_service() + workspace_service.properties["admin_password_keyvault_secret_id"] = "https://kv/secrets/admin" + get_workspace_service_mock.return_value = workspace_service + + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_SECRET, workspace_id=WORKSPACE_ID, service_id=SERVICE_ID, secret_name="admin_password_keyvault_secret_id"), headers={"Authorization": "Bearer " "test-token"}) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["key"] == "admin_password_keyvault_secret_id" + assert response.json()["value"] == "a-secret-value" + get_secret_value_mock.assert_called_once_with("https://kv/secrets/admin", None, "12345", "test-token") + + # [GET] /workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name} + @patch("api.routes.workspaces.get_secret_value", return_value="a-secret-value") + @patch("api.dependencies.workspaces.WorkspaceServiceRepository.get_workspace_service_by_id") + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + async def test_get_workspace_service_secret_returns_400_when_property_is_not_a_secret(self, get_workspace_mock, get_workspace_service_mock, get_secret_value_mock, app, client): + get_workspace_mock.return_value = sample_workspace() + get_workspace_service_mock.return_value = sample_workspace_service() + + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_SECRET, workspace_id=WORKSPACE_ID, service_id=SERVICE_ID, secret_name="display_name")) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + get_secret_value_mock.assert_not_called() + + # [GET] /workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name} + @patch("api.routes.workspaces.get_secret_value", return_value="a-secret-value") + @patch("api.dependencies.workspaces.WorkspaceServiceRepository.get_workspace_service_by_id") + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + async def test_get_workspace_service_secret_returns_404_when_property_does_not_exist(self, get_workspace_mock, get_workspace_service_mock, get_secret_value_mock, app, client): + get_workspace_mock.return_value = sample_workspace() + get_workspace_service_mock.return_value = sample_workspace_service() + + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_SECRET, workspace_id=WORKSPACE_ID, service_id=SERVICE_ID, secret_name="missing_keyvault_secret_id")) + + assert response.status_code == status.HTTP_404_NOT_FOUND + get_secret_value_mock.assert_not_called() + + # [GET] /workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name} + @patch("api.routes.workspaces.get_secret_value", side_effect=SecretRetrievalError("boom")) + @patch("api.dependencies.workspaces.WorkspaceServiceRepository.get_workspace_service_by_id") + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + async def test_get_workspace_service_secret_returns_404_when_retrieval_fails(self, get_workspace_mock, get_workspace_service_mock, _, app, client): + get_workspace_mock.return_value = sample_workspace() + workspace_service = sample_workspace_service() + workspace_service.properties["admin_password_keyvault_secret_id"] = "https://kv/secrets/admin" + get_workspace_service_mock.return_value = workspace_service + + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_SECRET, workspace_id=WORKSPACE_ID, service_id=SERVICE_ID, secret_name="admin_password_keyvault_secret_id")) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + # [GET] /workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name} + @patch("api.routes.workspaces.get_secret_value", return_value="a-secret-value") + @patch("api.dependencies.workspaces.WorkspaceServiceRepository.get_workspace_service_by_id") + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + async def test_get_workspace_service_secret_passes_workspace_keyvault_uri(self, get_workspace_mock, get_workspace_service_mock, get_secret_value_mock, app, client): + workspace = sample_workspace() + workspace.properties["keyvault_uri"] = "https://kv.vault.azure.net/" + workspace.properties["client_id"] = "workspace-client-id" + get_workspace_mock.return_value = workspace + workspace_service = sample_workspace_service() + workspace_service.properties["admin_password_keyvault_secret_id"] = "https://kv.vault.azure.net/secrets/admin" + get_workspace_service_mock.return_value = workspace_service + + response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_SECRET, workspace_id=WORKSPACE_ID, service_id=SERVICE_ID, secret_name="admin_password_keyvault_secret_id"), headers={"Authorization": "Bearer " "test-token"}) + + assert response.status_code == status.HTTP_200_OK + get_secret_value_mock.assert_called_once_with("https://kv.vault.azure.net/secrets/admin", "https://kv.vault.azure.net/", "workspace-client-id", "test-token") + + # [GET] /workspaces/{workspace_id}/workspace-services/{service_id}/user-resources/{resource_id}/secrets/{secret_name} + @patch("api.routes.workspaces.get_secret_value", return_value="a-secret-value") + @patch("api.dependencies.workspaces.UserResourceRepository.get_user_resource_by_id") + @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id") + async def test_get_user_resource_secret_returns_secret_value(self, get_workspace_mock, get_user_resource_mock, get_secret_value_mock, app, client): + get_workspace_mock.return_value = sample_workspace() + user_resource = sample_user_resource_object() + user_resource.properties["admin_password_keyvault_secret_id"] = "https://kv/secrets/admin" + get_user_resource_mock.return_value = user_resource + + response = await client.get(app.url_path_for(strings.API_GET_USER_RESOURCE_SECRET, workspace_id=WORKSPACE_ID, service_id=SERVICE_ID, resource_id=USER_RESOURCE_ID, secret_name="admin_password_keyvault_secret_id"), headers={"Authorization": "Bearer " "test-token"}) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["value"] == "a-secret-value" + # [GET] /workspaces/{workspace_id}/services/{service_id}/user-resources/{resource_id}/history @patch("api.routes.shared_services.ResourceHistoryRepository.get_resource_history_by_resource_id") @patch("api.dependencies.workspaces.UserResourceRepository.get_user_resource_by_id") diff --git a/api_app/tests_ma/test_services/test_secrets.py b/api_app/tests_ma/test_services/test_secrets.py new file mode 100644 index 0000000000..9985bb5e42 --- /dev/null +++ b/api_app/tests_ma/test_services/test_secrets.py @@ -0,0 +1,86 @@ +import pytest +from mock import AsyncMock, MagicMock, patch + +from services.secrets import get_secret_value, is_secret_property, SecretRetrievalError +from azure.core.exceptions import ResourceNotFoundError + + +SECRET_ID = "https://kv-test.vault.azure.net/secrets/admin-password" +WORKSPACE_CLIENT_ID = "workspace-client-id" +USER_TOKEN = "user-access-token" + + +@pytest.mark.parametrize("property_name,expected", [ + ("admin_password_keyvault_secret_id", True), + ("keyvault_secret_id", True), + ("connection_string_keyvault_secret_id", True), + ("display_name", False), + ("password", False), +]) +def test_is_secret_property(property_name, expected): + assert is_secret_property(property_name) is expected + + +def _mock_secret_client(secret_value=None, get_secret_side_effect=None): + secret_client = AsyncMock() + if get_secret_side_effect is not None: + secret_client.get_secret.side_effect = get_secret_side_effect + else: + secret = MagicMock() + secret.value = secret_value + secret_client.get_secret.return_value = secret + secret_client.__aenter__.return_value = secret_client + secret_client.__aexit__.return_value = None + return secret_client + + +def _mock_obo_credential(): + credential = AsyncMock() + credential.__aenter__.return_value = credential + credential.__aexit__.return_value = None + return credential + + +@patch("services.secrets._get_obo_credential") +@patch("services.secrets.SecretClient") +@pytest.mark.asyncio +async def test_get_secret_value_returns_value(secret_client_cls, get_obo_credential): + get_obo_credential.return_value = _mock_obo_credential() + secret_client_cls.return_value = _mock_secret_client(secret_value="super-secret") + + result = await get_secret_value(SECRET_ID, None, WORKSPACE_CLIENT_ID, USER_TOKEN) + + assert result == "super-secret" + secret_client_cls.assert_called_once() + assert secret_client_cls.call_args.kwargs["vault_url"] == "https://kv-test.vault.azure.net" + # The OBO exchange uses the workspace app registration and the caller's token. + get_obo_credential.assert_called_once_with(WORKSPACE_CLIENT_ID, USER_TOKEN) + + +@pytest.mark.asyncio +async def test_get_secret_value_raises_when_no_obo_context(): + with pytest.raises(SecretRetrievalError): + await get_secret_value(SECRET_ID, None, None, None) + + +@pytest.mark.asyncio +async def test_get_secret_value_raises_on_invalid_identifier(): + with pytest.raises(SecretRetrievalError): + await get_secret_value("not-a-valid-secret-id", None, WORKSPACE_CLIENT_ID, USER_TOKEN) + + +@pytest.mark.asyncio +async def test_get_secret_value_rejects_secret_outside_workspace(): + with pytest.raises(SecretRetrievalError): + await get_secret_value(SECRET_ID, "https://other-kv.vault.azure.net/", WORKSPACE_CLIENT_ID, USER_TOKEN) + + +@patch("services.secrets._get_obo_credential") +@patch("services.secrets.SecretClient") +@pytest.mark.asyncio +async def test_get_secret_value_raises_when_secret_not_found(secret_client_cls, get_obo_credential): + get_obo_credential.return_value = _mock_obo_credential() + secret_client_cls.return_value = _mock_secret_client(get_secret_side_effect=ResourceNotFoundError("missing")) + + with pytest.raises(SecretRetrievalError): + await get_secret_value(SECRET_ID, None, WORKSPACE_CLIENT_ID, USER_TOKEN) diff --git a/docs/azure-tre-overview/secret-retrieval.md b/docs/azure-tre-overview/secret-retrieval.md new file mode 100644 index 0000000000..e2b55539ea --- /dev/null +++ b/docs/azure-tre-overview/secret-retrieval.md @@ -0,0 +1,33 @@ +# Secret retrieval + +Resources (workspace services and user resources) can surface secret values — such as VM administrator passwords, storage account keys, or database connection strings — to researchers. Rather than storing secret values in the resource document, resources store the secret in the **workspace Key Vault** and output a Key Vault secret identifier (URI) in a property whose name contains `keyvault_secret_id`. The value is fetched on demand when a user chooses to reveal it and is never persisted in the Configuration Store or returned by list operations. See [Exposing secrets to researchers](../tre-workspace-authors/authoring-workspace-templates.md#exposing-secrets-to-researchers) for the authoring convention. + +Key Vault access is performed **on behalf of the signed-in user** using an On-Behalf-Of (OBO) token exchange, so a secret is read as the caller rather than as the core API's own identity. This relies on a per-workspace federated identity credential (FIC) instead of a stored workspace client secret. + +The components and trust relationships are: + +| Component | Role in the flow | +| --- | --- | +| Core API managed identity | Requests a managed-identity token for the token-exchange audience (`api://AzureADTokenExchange`) to use as the client assertion. It holds **no standing Key Vault permission**. | +| Workspace app registration | Acts as the confidential client for the OBO exchange. A federated identity credential names the core API managed identity as its subject, so the API can authenticate *as* the workspace application without a client secret. | +| Caller's access token | Supplied as the user assertion in the OBO exchange, so the resulting Key Vault data-plane token is scoped to that user. | +| Workspace Key Vault | Holds the secrets. Data-plane RBAC on the vault determines which secrets each user can read. | + +The flow when a user reveals a secret is: + +1. The user calls the resource's secrets endpoint on the TRE API with their bearer token. +1. The API validates that the property is a secret reference (name contains `keyvault_secret_id`) and that the referenced secret lives in the workspace's own Key Vault. +1. The API's managed identity obtains a token-exchange token and uses it as the client assertion to authenticate as the workspace app registration (via the FIC). +1. The API performs an OBO exchange, presenting the caller's token as the user assertion, to obtain a Key Vault data-plane token scoped to the user. +1. The API reads the secret from the workspace Key Vault with that token and returns the value in the response. The value is not stored. + +Because the OBO exchange requires the caller's own token, a caller can only ever retrieve secrets they have themselves been granted read access to on the workspace Key Vault — even though the request is proxied through the core API. + +## Compromise and blast radius + +The federated identity credential lets the core API managed identity authenticate as the workspace application. If the core API managed identity were compromised, an attacker could impersonate the workspace app registration (the client-assertion half of the exchange). To actually read a secret via OBO they would additionally need a valid user token, captured while proxying a request. The blast radius is bounded by: + +- the permissions the workspace application itself holds (so workspace applications should be granted least privilege); and +- the secrets the impersonated user can already read (the FIC grants no direct Key Vault access of its own). + +This is a deliberately smaller blast radius than the alternatives of storing a workspace client secret or granting the core API standing `Key Vault Secrets User` access to every workspace vault. The core API managed identity should be protected accordingly. diff --git a/docs/tre-workspace-authors/authoring-workspace-templates.md b/docs/tre-workspace-authors/authoring-workspace-templates.md index eba6bc875f..6af2bbf3ad 100644 --- a/docs/tre-workspace-authors/authoring-workspace-templates.md +++ b/docs/tre-workspace-authors/authoring-workspace-templates.md @@ -85,6 +85,38 @@ When authoring a `template_schema.json` file, you can reference properties from !!! todo After a workspace with virtual machines is implemented this section can be written based on that. ([Outputs in Porter documentation](https://porter.sh/author-bundles/#outputs) to be linked here too.) +#### Exposing secrets to researchers + +Resources sometimes need to surface secret values (for example a VM administrator password, a storage account key, or a database connection string) to researchers. Rather than returning the secret value directly, store the secret in the workspace Key Vault and output a **Key Vault secret identifier** (the full secret URI) in a property whose name contains `keyvault_secret_id`. + +The API treats any resource property whose name contains `keyvault_secret_id` as a reference to a workspace Key Vault secret. Researchers (and workspace owners) can then retrieve the underlying secret value on demand via the secrets endpoints: + +* `GET /workspaces/{workspace_id}/workspace-services/{service_id}/secrets/{secret_name}` +* `GET /workspaces/{workspace_id}/workspace-services/{service_id}/user-resources/{resource_id}/secrets/{secret_name}` + +where `secret_name` is the name of the property that holds the `keyvault_secret_id`. The API only retrieves secrets from the workspace's own Key Vault. + +#### How the UI decides which secrets to show + +The UI does **not** inspect property *values* to find secrets. It relies purely on the property-name convention: for a workspace service or user resource, any property whose name contains `keyvault_secret_id` is rendered as a masked secret (`••••••••`) with a reveal button, instead of as plain text. The property value stored on the resource is only the Key Vault secret identifier (URI); the underlying secret value is fetched on demand from the secrets endpoint above when the user clicks *reveal*, and is never persisted in the resource document, the Cosmos DB store, or the browser. + +This means: + +* Only **workspace services** and **user resources** can expose secrets this way. Properties on workspaces or shared services are not offered for reveal. +* Whether a secret appears is entirely determined by the template author naming an output property `*keyvault_secret_id*`. If no template outputs such a property, no secrets appear in the UI. +* Naming a property `*keyvault_secret_id*` but storing anything other than a valid Key Vault secret identifier from the workspace's own Key Vault will result in an error when the user tries to reveal it. + +#### Security model + +Key Vault access is performed **on behalf of the signed-in user** using an On-Behalf-Of (OBO) token exchange. The core API's managed identity is registered as a federated identity credential on the per-workspace app registration, so the API authenticates as the workspace application without a stored client secret and then exchanges the caller's token for a Key Vault data-plane token scoped to that user. As a result, the caller only receives secrets they have themselves been granted read access to on the workspace Key Vault; the API's own identity is not used to read the secret. + +Because the exchange requires the caller's own token as the user assertion, a caller can never retrieve a secret they do not already have Key Vault data-plane access to — even though the request is proxied through the core API. The core API therefore holds **no standing permission** to read workspace Key Vault secrets (the previous `Key Vault Secrets User` role assignment on the API managed identity has been removed). + +For the full flow, the trust relationships involved, and the compromise/blast-radius analysis, see [Secret retrieval](../azure-tre-overview/secret-retrieval.md) in the architecture documentation. + +> [!NOTE] +> The federated identity credential is only created when the workspace app registration is managed by TRE (`register_aad_application = true`). If the workspace application is registered externally, the equivalent federated credential must be configured on that application manually for secret retrieval to work. + ### Actions The required actions are the main two of CNAB spec: diff --git a/mkdocs.yml b/mkdocs.yml index f99e74c13f..410b41ae46 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -56,6 +56,7 @@ nav: - System Architecture: azure-tre-overview/architecture.md - Network Architecture: azure-tre-overview/networking.md - Azure Resources: azure-tre-overview/tre-resources-breakdown.md + - Secret Retrieval: azure-tre-overview/secret-retrieval.md - Airlock: azure-tre-overview/airlock.md - Cost Reporting: azure-tre-overview/cost-reporting.md - Terms and Definitions: using-tre/terms-definitions.md diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm/porter.yaml b/templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm/porter.yaml index ef129291f7..2907e7ec5f 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm/porter.yaml +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-export-reviewvm/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-guacamole-export-reviewvm -version: 2.0.0 +version: 2.0.1 description: "An Azure TRE User Resource Template for reviewing Airlock export requests" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm/porter.yaml b/templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm/porter.yaml index 28e5167a4b..dcfff73003 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm/porter.yaml +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-import-reviewvm/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-guacamole-import-reviewvm -version: 2.0.0 +version: 2.0.1 description: "An Azure TRE User Resource Template for reviewing Airlock import requests" dockerfile: Dockerfile.tmpl registry: azuretre 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 186da0d571..0d32eff7f9 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 @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-service-guacamole-linuxvm -version: 1.4.3 +version: 1.5.0 description: "An Azure TRE User Resource Template for Guacamole (Linux)" dockerfile: Dockerfile.tmpl registry: azuretre 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 7b3a18975b..3ebfe61596 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 @@ -120,6 +120,16 @@ resource "azurerm_key_vault_secret" "linuxvm_password" { lifecycle { ignore_changes = [tags] } } +# Grant the owning user read access (secrets reader) to their own VM password +# secret. Scoped to this single secret so users can only read their own +# credentials. +resource "azurerm_role_assignment" "vm_password_secret_reader" { + count = var.owner_id != "" ? 1 : 0 + scope = azurerm_key_vault_secret.linuxvm_password.resource_versionless_id + role_definition_name = "Key Vault Secrets User" + principal_id = var.owner_id +} + resource "azurerm_dev_test_global_vm_shutdown_schedule" "shutdown_schedule" { count = var.enable_shutdown_schedule ? 1 : 0 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 f994d0e021..b47a0e5f11 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: 3.0.0 +version: 3.1.0 description: "An Azure TRE User Resource Template for Guacamole (Windows 11 or Windows Server 2025)" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/main.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/main.tf index f0bd5e1cfe..4d343ff83a 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/main.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/main.tf @@ -124,6 +124,16 @@ resource "azurerm_key_vault_secret" "windowsvm_password" { lifecycle { ignore_changes = [tags] } } +# Grant the owning user read access (secrets reader) to their own VM password +# secret. Scoped to this single secret so users can only read their own +# credentials. Skipped when owner_id is empty (e.g. airlock review VMs). +resource "azurerm_role_assignment" "vm_password_secret_reader" { + count = var.owner_id != "" ? 1 : 0 + scope = azurerm_key_vault_secret.windowsvm_password.resource_versionless_id + role_definition_name = "Key Vault Secrets User" + principal_id = var.owner_id +} + resource "azurerm_dev_test_global_vm_shutdown_schedule" "shutdown_schedule" { count = var.enable_shutdown_schedule ? 1 : 0 diff --git a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/variables.tf b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/variables.tf index 581ebb3d57..b07944899f 100644 --- a/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/variables.tf +++ b/templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/vm/variables.tf @@ -123,3 +123,9 @@ variable "extra_tags" { default = {} description = "Additional tags merged into the standard user-resource tags" } + +variable "owner_id" { + type = string + default = "" + description = "AAD object id of the user that owns this user resource. When set, they are granted read access to the VM password secret. Left empty for ephemeral VMs (e.g. airlock review VMs) that use random credentials." +} 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 f57659422a..d8ee7f9186 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 @@ -14,6 +14,8 @@ module "windows_vm" { admin_username = local.admin_username + owner_id = var.owner_id + nexus_proxy_url = local.nexus_proxy_url shared_storage_access = var.shared_storage_access install_azure_cli = var.install_azure_cli diff --git a/templates/workspaces/base/porter.yaml b/templates/workspaces/base/porter.yaml index 5c7c9dacbb..d852c35ac1 100644 --- a/templates/workspaces/base/porter.yaml +++ b/templates/workspaces/base/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-base -version: 2.10.0 +version: 2.12.0 description: "A base Azure TRE workspace" dockerfile: Dockerfile.tmpl registry: azuretre @@ -163,6 +163,11 @@ parameters: description: "The name of the topic to publish scan results to" outputs: + - name: keyvault_uri + type: string + applyTo: + - install + - upgrade - name: app_role_id_workspace_owner type: string applyTo: diff --git a/templates/workspaces/base/terraform/aad/aad.tf b/templates/workspaces/base/terraform/aad/aad.tf index fd3acd0c3b..17e25cd439 100644 --- a/templates/workspaces/base/terraform/aad/aad.tf +++ b/templates/workspaces/base/terraform/aad/aad.tf @@ -105,6 +105,19 @@ resource "azuread_service_principal" "workspace" { } } +# Federated identity credential so the core API's managed identity can act as +# the workspace app registration (via the On-Behalf-Of flow) without a stored +# client secret. Used to retrieve workspace Key Vault secrets on behalf of the +# signed-in user. +resource "azuread_application_federated_identity_credential" "api_obo" { + application_id = azuread_application.workspace.id + display_name = "tre-api-obo" + description = "Allows the TRE API managed identity to perform On-Behalf-Of token exchange as the workspace application." + audiences = ["api://AzureADTokenExchange"] + issuer = "https://login.microsoftonline.com/${data.azuread_client_config.current.tenant_id}/v2.0" + subject = var.api_identity_principal_id +} + resource "azuread_service_principal_delegated_permission_grant" "ui" { count = var.auto_grant_workspace_consent ? 1 : 0 service_principal_object_id = data.azuread_service_principal.ui.object_id diff --git a/templates/workspaces/base/terraform/aad/variables.tf b/templates/workspaces/base/terraform/aad/variables.tf index a93df28733..99c57c0f1b 100644 --- a/templates/workspaces/base/terraform/aad/variables.tf +++ b/templates/workspaces/base/terraform/aad/variables.tf @@ -30,3 +30,8 @@ variable "core_api_client_id" { type = string } +variable "api_identity_principal_id" { + type = string + description = "Principal (object) id of the core API managed identity, used as the subject of the workspace app registration federated identity credential." +} + diff --git a/templates/workspaces/base/terraform/api-permissions.tf b/templates/workspaces/base/terraform/api-permissions.tf index 742bb48dac..6dcf3a8110 100644 --- a/templates/workspaces/base/terraform/api-permissions.tf +++ b/templates/workspaces/base/terraform/api-permissions.tf @@ -14,5 +14,3 @@ resource "azurerm_role_assignment" "api_reader" { role_definition_name = "Reader" principal_id = data.azurerm_user_assigned_identity.api_id.principal_id } - - diff --git a/templates/workspaces/base/terraform/outputs.tf b/templates/workspaces/base/terraform/outputs.tf index 2bc0c2c716..1220d9be46 100644 --- a/templates/workspaces/base/terraform/outputs.tf +++ b/templates/workspaces/base/terraform/outputs.tf @@ -2,6 +2,10 @@ output "workspace_resource_name_suffix" { value = local.workspace_resource_name_suffix } +output "keyvault_uri" { + value = azurerm_key_vault.kv.vault_uri +} + # The following outputs are dependent on an Automatic AAD Workspace Application Registration. # If we are not creating an App Reg we simple pass back the same values that were already created # This is necessary so that we don't delete workspace properties diff --git a/templates/workspaces/base/terraform/workspace.tf b/templates/workspaces/base/terraform/workspace.tf index 8008c545bd..4333cd5bc0 100644 --- a/templates/workspaces/base/terraform/workspace.tf +++ b/templates/workspaces/base/terraform/workspace.tf @@ -45,6 +45,7 @@ module "aad" { ui_client_id = var.ui_client_id auto_grant_workspace_consent = var.auto_grant_workspace_consent core_api_client_id = var.core_api_client_id + api_identity_principal_id = data.azurerm_user_assigned_identity.api_id.principal_id depends_on = [ azurerm_role_assignment.keyvault_resourceprocessor_ws_role, diff --git a/ui/app/package-lock.json b/ui/app/package-lock.json index b1f1c7a34b..23e92964cc 100644 --- a/ui/app/package-lock.json +++ b/ui/app/package-lock.json @@ -1,12 +1,12 @@ { "name": "tre-ui", - "version": "0.8.30", + "version": "0.9.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tre-ui", - "version": "0.8.30", + "version": "0.9.0", "dependencies": { "@azure/msal-browser": "^2.35.0", "@azure/msal-react": "^1.5.12", diff --git a/ui/app/package.json b/ui/app/package.json index 179695799d..04f03e5764 100644 --- a/ui/app/package.json +++ b/ui/app/package.json @@ -1,6 +1,6 @@ { "name": "tre-ui", - "version": "0.8.30", + "version": "0.9.0", "private": true, "type": "module", "dependencies": { diff --git a/ui/app/src/components/shared/ResourcePropertyPanel.tsx b/ui/app/src/components/shared/ResourcePropertyPanel.tsx index 1f04e2dbcf..18c2cda718 100644 --- a/ui/app/src/components/shared/ResourcePropertyPanel.tsx +++ b/ui/app/src/components/shared/ResourcePropertyPanel.tsx @@ -2,7 +2,10 @@ import { DefaultPalette, IStackItemStyles, IStackStyles, Stack } from "@fluentui import moment from "moment"; import React from "react"; import { Resource } from "../../models/resource"; +import { ResourceType } from "../../models/resourceType"; +import { isSecretProperty } from "../../models/secret"; import { ComplexPropertyModal } from "./ComplexItemDisplay"; +import { SecretDisplay } from "./SecretDisplay"; interface ResourcePropertyPanelProps { resource: Resource; @@ -11,8 +14,14 @@ interface ResourcePropertyPanelProps { interface ResourcePropertyPanelItemProps { header: string; val: any; + resource?: Resource; + propertyName?: string; } +// Secret retrieval is only available for workspace services and user resources. +const canRevealSecret = (resource?: Resource): boolean => + resource?.resourceType === ResourceType.WorkspaceService || resource?.resourceType === ResourceType.UserResource; + export const ResourcePropertyPanelItem: React.FunctionComponent = ( props: ResourcePropertyPanelItemProps, ) => { @@ -26,6 +35,15 @@ export const ResourcePropertyPanelItem: React.FunctionComponent; + } + if (typeof val === "string") { if (val && val.startsWith("https://")) { return ( @@ -90,7 +108,15 @@ export const ResourcePropertyPanel: React.FunctionComponent {Object.keys(props.resource.properties).map((key) => { let val = (props.resource.properties as any)[key]; - return ; + return ( + + ); })} diff --git a/ui/app/src/components/shared/SecretDisplay.test.tsx b/ui/app/src/components/shared/SecretDisplay.test.tsx new file mode 100644 index 0000000000..743f06ecfe --- /dev/null +++ b/ui/app/src/components/shared/SecretDisplay.test.tsx @@ -0,0 +1,126 @@ +import React from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent, waitFor, createPartialFluentUIMock, mockClipboardAPI } from "../../test-utils"; +import { mockUserResource, mockWorkspaceService } from "../../test-utils/mockData"; + +// Shared mock for the auth API call +const mockApiCall = vi.fn(); +vi.mock("../../hooks/useAuthApiCall", () => ({ + useAuthApiCall: () => mockApiCall, + HttpMethod: { Get: "GET", Post: "POST", Patch: "PATCH", Delete: "DELETE" }, + ResultType: { JSON: "JSON", Text: "TEXT", None: "None" }, +})); + +// Mock ExceptionLayout so we can assert on error rendering +vi.mock("./ExceptionLayout", () => ({ + ExceptionLayout: ({ e }: any) =>
{e?.userMessage || e?.message}
, +})); + +// Mock FluentUI components using the centralized mock +vi.mock("@fluentui/react", async () => { + const actual = await vi.importActual("@fluentui/react"); + return { + ...actual, + ...createPartialFluentUIMock(["Stack", "Text", "IconButton", "TooltipHost", "Spinner", "SpinnerSize"]), + }; +}); + +import { SecretDisplay } from "./SecretDisplay"; + +const secretResource = { + ...mockWorkspaceService, + properties: { + admin_password_keyvault_secret_id: "https://kv.vault.azure.net/secrets/admin-password/abc", + }, +}; + +beforeEach(() => { + mockClipboardAPI(); + mockApiCall.mockReset(); +}); + +describe("SecretDisplay Component", () => { + it("shows a masked placeholder and reveal button initially, without calling the API", () => { + render(); + + expect(screen.getByText("••••••••")).toBeInTheDocument(); + const revealButton = screen.getByLabelText("Show secret"); + expect(revealButton).toHaveAttribute("data-icon-name", "RedEye"); + expect(mockApiCall).not.toHaveBeenCalled(); + }); + + it("retrieves and displays the secret value when reveal is clicked", async () => { + mockApiCall.mockResolvedValue({ key: "admin_password_keyvault_secret_id", value: "s3cr3t-value" }); + + render(); + + fireEvent.click(screen.getByLabelText("Show secret")); + + await waitFor(() => { + expect(screen.getByText("s3cr3t-value")).toBeInTheDocument(); + }); + + expect(mockApiCall).toHaveBeenCalledWith( + `${secretResource.resourcePath}/secrets/admin_password_keyvault_secret_id`, + "GET", + expect.anything(), + ); + }); + + it("calls the user resource secret endpoint using the resource path", async () => { + mockApiCall.mockResolvedValue({ key: "vm_password_keyvault_secret_id", value: "abc" }); + const userResource = { + ...mockUserResource, + properties: { vm_password_keyvault_secret_id: "https://kv/secrets/vm/1" }, + }; + + render(); + fireEvent.click(screen.getByLabelText("Show secret")); + + await waitFor(() => expect(screen.getByText("abc")).toBeInTheDocument()); + expect(mockApiCall).toHaveBeenCalledWith( + `${userResource.resourcePath}/secrets/vm_password_keyvault_secret_id`, + "GET", + expect.anything(), + ); + }); + + it("hides the secret again when the hide button is clicked", async () => { + mockApiCall.mockResolvedValue({ key: "admin_password_keyvault_secret_id", value: "s3cr3t-value" }); + + render(); + fireEvent.click(screen.getByLabelText("Show secret")); + + await waitFor(() => expect(screen.getByText("s3cr3t-value")).toBeInTheDocument()); + + fireEvent.click(screen.getByLabelText("Hide secret")); + + expect(screen.queryByText("s3cr3t-value")).not.toBeInTheDocument(); + expect(screen.getByText("••••••••")).toBeInTheDocument(); + }); + + it("copies the revealed secret to the clipboard", async () => { + mockApiCall.mockResolvedValue({ key: "admin_password_keyvault_secret_id", value: "s3cr3t-value" }); + + render(); + fireEvent.click(screen.getByLabelText("Show secret")); + + await waitFor(() => expect(screen.getByText("s3cr3t-value")).toBeInTheDocument()); + + fireEvent.click(screen.getByLabelText("Copy secret to clipboard")); + expect(navigator.clipboard.writeText).toHaveBeenCalledWith("s3cr3t-value"); + }); + + it("renders an error when the secret cannot be retrieved", async () => { + mockApiCall.mockRejectedValue({ message: "not found" }); + + render(); + fireEvent.click(screen.getByLabelText("Show secret")); + + await waitFor(() => { + expect(screen.getByTestId("exception-layout")).toBeInTheDocument(); + }); + expect(screen.getByText("Error retrieving secret")).toBeInTheDocument(); + expect(screen.queryByText("s3cr3t-value")).not.toBeInTheDocument(); + }); +}); diff --git a/ui/app/src/components/shared/SecretDisplay.tsx b/ui/app/src/components/shared/SecretDisplay.tsx new file mode 100644 index 0000000000..ac0d9c4a63 --- /dev/null +++ b/ui/app/src/components/shared/SecretDisplay.tsx @@ -0,0 +1,93 @@ +import { IconButton, Spinner, SpinnerSize, Stack, Text, TooltipHost } from "@fluentui/react"; +import React, { useContext, useState } from "react"; +import { WorkspaceContext } from "../../contexts/WorkspaceContext"; +import { HttpMethod, useAuthApiCall } from "../../hooks/useAuthApiCall"; +import { ApiEndpoint } from "../../models/apiEndpoints"; +import { APIError } from "../../models/exceptions"; +import { Resource } from "../../models/resource"; +import { Secret } from "../../models/secret"; +import { ExceptionLayout } from "./ExceptionLayout"; + +interface SecretDisplayProps { + resource: Resource; + propertyName: string; +} + +// Displays a masked placeholder for a workspace Key Vault secret and lets the +// user reveal the value on demand. The secret value is only retrieved from the +// API when the user chooses to reveal it, and is never persisted. +export const SecretDisplay: React.FunctionComponent = (props: SecretDisplayProps) => { + const workspaceCtx = useContext(WorkspaceContext); + const apiCall = useAuthApiCall(); + + const [secretValue, setSecretValue] = useState(undefined); + const [isLoading, setIsLoading] = useState(false); + const [apiError, setApiError] = useState(undefined); + + const COPY_TOOL_TIP_DEFAULT_MESSAGE = "Copy to clipboard"; + const [copyToolTipMessage, setCopyToolTipMessage] = useState(COPY_TOOL_TIP_DEFAULT_MESSAGE); + + const revealSecret = async () => { + setIsLoading(true); + setApiError(undefined); + try { + const secret: Secret = await apiCall( + `${props.resource.resourcePath}/${ApiEndpoint.Secrets}/${props.propertyName}`, + HttpMethod.Get, + workspaceCtx.workspaceApplicationIdURI, + ); + setSecretValue(secret.value); + } catch (err: any) { + err.userMessage = "Error retrieving secret"; + setApiError(err as APIError); + } + setIsLoading(false); + }; + + const hideSecret = () => { + setSecretValue(undefined); + setApiError(undefined); + }; + + const handleCopySecret = () => { + if (secretValue === undefined) return; + navigator.clipboard.writeText(secretValue); + setCopyToolTipMessage("Copied"); + setTimeout(() => setCopyToolTipMessage(COPY_TOOL_TIP_DEFAULT_MESSAGE), 3000); + }; + + return ( + <> + + + {secretValue !== undefined ? ( + {secretValue} + ) : ( + •••••••• + )} + + {isLoading ? ( + + ) : secretValue !== undefined ? ( + <> + + + + + + + + ) : ( + + + + )} + + {apiError && } + + ); +}; diff --git a/ui/app/src/models/apiEndpoints.ts b/ui/app/src/models/apiEndpoints.ts index 9f035cf873..88680a639b 100644 --- a/ui/app/src/models/apiEndpoints.ts +++ b/ui/app/src/models/apiEndpoints.ts @@ -2,6 +2,7 @@ export enum ApiEndpoint { Workspaces = "workspaces", WorkspaceServices = "workspace-services", UserResources = "user-resources", + Secrets = "secrets", SharedServices = "shared-services", Requests = "requests", AirlockRequests = "requests", diff --git a/ui/app/src/models/secret.ts b/ui/app/src/models/secret.ts new file mode 100644 index 0000000000..f28569bef5 --- /dev/null +++ b/ui/app/src/models/secret.ts @@ -0,0 +1,11 @@ +// Convention: any resource property whose name contains this token holds a +// reference (Key Vault secret identifier) to a secret in the workspace Key Vault, +// rather than the secret value itself. +export const KEYVAULT_SECRET_ID_TOKEN = "keyvault_secret_id"; + +export const isSecretProperty = (propertyName: string): boolean => propertyName.includes(KEYVAULT_SECRET_ID_TOKEN); + +export interface Secret { + key: string; + value: string; +}