4. API works with authInformation in properties (#1705)

* Missing Api Id returns 500 * Make app_id mandatory * Moving authInformation into workspace properties * Create AAD when the private endpoint is finished * tflint * Adding outputs back in * Default outputs * Pass auth variables into TF * TF Lint * Fix Unit Tests * app_id => client_id * Linting * E2E working locally * Put tests around truthy method * Bump the API Version * Activate the DB migration * Change return type and hope porter deals with it! Co-authored-by: David Moore <35696285+damoodamoo@users.noreply.github.com>
2022-05-05 23:38:19 +01:00 · 2022-05-05 23:38:19 +01:00 · 8a2f799882
--- a/api_app/_version.py
+++ b/api_app/_version.py
@ -1 +1 @@
-__version__ = "0.2.22"
+__version__ = "0.2.23"
--- a/api_app/api/routes/api.py
+++ b/api_app/api/routes/api.py
@ -108,7 +108,7 @@ async def get_openapi_json(workspace_id: str, request: Request, workspace_repo=D
        )

        workspace = workspace_repo.get_workspace_by_id(workspace_id)
-        ws_app_reg_id = workspace.properties['app_id']
+        ws_app_reg_id = workspace.properties['client_id']
        workspace_scopes = {
            f"api://{ws_app_reg_id}/user_impersonation": "List and Get TRE Workspaces"
        }
@ -129,7 +129,7 @@ async def get_openapi_json(workspace_id: str, request: Request, workspace_repo=D
 async def get_workspace_swagger(workspace_id, request: Request, workspace_repo=Depends(get_repository(WorkspaceRepository))):

    workspace = workspace_repo.get_workspace_by_id(workspace_id)
-    ws_app_reg_id = workspace.properties['app_id']
+    ws_app_reg_id = workspace.properties['client_id']
    swagger_ui_html = get_swagger_ui_html(
        openapi_url="openapi.json",
        title=request.app.title + " - Swagger UI",
--- a/api_app/db/repositories/workspaces.py
+++ b/api_app/db/repositories/workspaces.py
@ -63,22 +63,30 @@ class WorkspaceRepository(ResourceRepository):
        template = self.validate_input_against_template(workspace_input.templateName, workspace_input, ResourceType.Workspace)

        address_space_param = {"address_space": self.get_address_space_based_on_size(workspace_input.properties)}
+        auto_app_registration_param = {"register_aad_application": self.automatically_create_application_registration(workspace_input.properties)}

-        # we don't want something in the input to overwrite the system parameters, so dict.update can't work. Priorities from right to left.
-        resource_spec_parameters = {**workspace_input.properties, **address_space_param, **self.get_workspace_spec_params(full_workspace_id)}
+        # we don't want something in the input to overwrite the system parameters,
+        # so dict.update can't work. Priorities from right to left.
+        resource_spec_parameters = {**workspace_input.properties,
+                                    **address_space_param,
+                                    **auto_app_registration_param,
+                                    **auth_info,
+                                    **self.get_workspace_spec_params(full_workspace_id)}

        workspace = Workspace(
            id=full_workspace_id,
            templateName=workspace_input.templateName,
            templateVersion=template.version,
            properties=resource_spec_parameters,
-            authInformation=auth_info,
            resourcePath=f'/workspaces/{full_workspace_id}',
            etag=''  # need to validate the model
        )

        return workspace, template

+    def automatically_create_application_registration(self, properties: dict) -> bool:
+        return True if properties["client_id"] == "auto_create" else False
+
    def get_address_space_based_on_size(self, workspace_properties: dict):
        # Default the address space to 'small' if not supplied.
        address_space_size = workspace_properties.get("address_space_size", "small").lower()
--- a/api_app/models/domain/workspace.py
+++ b/api_app/models/domain/workspace.py
@ -15,4 +15,3 @@ class Workspace(Resource):
    """
    workspaceURL: str = Field("", title="Workspace URL", description="Main endpoint for workspace users")
    resourceType = ResourceType.Workspace
-    authInformation: dict = Field({})
--- a/api_app/models/schemas/workspace.py
+++ b/api_app/models/schemas/workspace.py
@ -20,8 +20,7 @@ def get_sample_workspace(workspace_id: str, spec_workspace_id: str = "0001") ->
            "address_space_size": "small",
        },
        "resourceType": ResourceType.Workspace,
-        "workspaceURL": "",
-        "authInformation": {}
+        "workspaceURL": ""
    }


@ -73,7 +72,7 @@ class WorkspaceInCreate(BaseModel):
                "properties": {
                    "display_name": "the workspace display name",
                    "description": "workspace description",
-                    "app_id": "9d52b04f-89cf-47b4-868a-e12be7133b36",
+                    "client_id": "9d52b04f-89cf-47b4-868a-e12be7133b36",
                    "address_space_size": "small"
                }
            }
--- a/api_app/models/schemas/workspace_template.py
+++ b/api_app/models/schemas/workspace_template.py
@ -13,11 +13,11 @@ def get_sample_workspace_template_object(template_name: str = "tre-workspace-bas
        resourceType=ResourceType.Workspace,
        current=True,
        type="object",
-        required=["display_name", "description", "app_id"],
+        required=["display_name", "description", "client_id"],
        properties={
            "display_name": Property(type="string"),
            "description": Property(type="string"),
-            "app_id": Property(type="string"),
+            "client_id": Property(type="string"),
            "address_space_size": Property(
                type="string",
                default="small",
--- a/api_app/resources/strings.py
+++ b/api_app/resources/strings.py
@ -68,7 +68,7 @@ RESOURCE_PROCESSOR_HEALTHY_MESSAGE = "HealthState/healthy"

 # Error strings
 ACCESS_APP_IS_MISSING_ROLE = "The App is missing role"
-ACCESS_PLEASE_SUPPLY_APP_ID = "Please supply the app_id for the AAD application"
+ACCESS_PLEASE_SUPPLY_CLIENT_ID = "Please supply the client_id for the AAD application"
 ACCESS_UNABLE_TO_GET_INFO_FOR_APP = "Unable to get app info for app:"
 ACCESS_UNABLE_TO_GET_ROLE_ASSIGNMENTS_FOR_USER = "Unable to get role assignments for user"

--- a/api_app/schemas/azuread.json
+++ b/api_app/schemas/azuread.json
@ -5,13 +5,13 @@
    "title": "Azure AD Authorisation Schema",
    "default": {},
    "required": [
-        "app_id"
+        "client_id"
    ],
    "properties": {
-        "app_id": {
+        "client_id": {
            "type": "string",
-            "title": "App Registration ID",
+            "title": "Application (Client) ID",
            "description": "AAD App registration of the workspace"
        }
    }
-}
+}
--- a/api_app/services/aad_authentication.py
+++ b/api_app/services/aad_authentication.py
@ -24,7 +24,7 @@ class AzureADAuthorization(AccessService):
    require_one_of_roles = None

    TRE_CORE_ROLES = ['TREAdmin', 'TREUser']
-    WORKSPACE_ROLES = ['WorkspaceOwner', 'WorkspaceResearcher']
+    WORKSPACE_ROLES_DICT = {'WorkspaceOwner': 'app_role_id_workspace_owner', 'WorkspaceResearcher': 'app_role_id_workspace_researcher'}

    def __init__(self, auto_error: bool = True, require_one_of_roles: list = None):
        super(AzureADAuthorization, self).__init__(
@ -43,7 +43,7 @@ class AzureADAuthorization(AccessService):
        decoded_token = None

        # Try workspace app registration if appropriate
-        if 'workspace_id' in request.path_params and any(role in self.require_one_of_roles for role in self.WORKSPACE_ROLES):
+        if 'workspace_id' in request.path_params and any(role in self.require_one_of_roles for role in self.WORKSPACE_ROLES_DICT.keys()):
            # as we have a workspace_id not given, try decoding token
            logging.debug("Workspace ID was provided. Getting Workspace API app registration")
            try:
@ -91,7 +91,7 @@ class AzureADAuthorization(AccessService):
            workspace_id = request.path_params['workspace_id']
            ws_repo = WorkspaceRepository(get_db_client_from_request(request))
            workspace = ws_repo.get_workspace_by_id(workspace_id)
-            ws_app_reg_id = workspace.properties['app_id']
+            ws_app_reg_id = workspace.properties['client_id']

            return ws_app_reg_id
        except EntityDoesNotExist as e:
@ -177,29 +177,30 @@ class AzureADAuthorization(AccessService):
        return {'Authorization': 'Bearer ' + msgraph_token}

    @staticmethod
-    def _get_service_principal_endpoint(app_id) -> str:
-        return f"https://graph.microsoft.com/v1.0/serviceprincipals?$filter=appid eq '{app_id}'"
+    def _get_service_principal_endpoint(client_id) -> str:
+        return f"https://graph.microsoft.com/v1.0/serviceprincipals?$filter=appid eq '{client_id}'"

-    def _get_app_sp_graph_data(self, app_id: str) -> dict:
+    def _get_app_sp_graph_data(self, client_id: str) -> dict:
        msgraph_token = self._get_msgraph_token()
-        sp_endpoint = self._get_service_principal_endpoint(app_id)
+        sp_endpoint = self._get_service_principal_endpoint(client_id)
        graph_data = requests.get(sp_endpoint, headers=self._get_auth_header(msgraph_token)).json()
        return graph_data

-    def _get_app_auth_info(self, app_id: str) -> dict:
-        graph_data = self._get_app_sp_graph_data(app_id)
+    def _get_app_auth_info(self, client_id: str) -> dict:
+        graph_data = self._get_app_sp_graph_data(client_id)
        if 'value' not in graph_data or len(graph_data['value']) == 0:
            logging.debug(graph_data)
-            raise AuthConfigValidationError(f"{strings.ACCESS_UNABLE_TO_GET_INFO_FOR_APP} {app_id}")
+            raise AuthConfigValidationError(f"{strings.ACCESS_UNABLE_TO_GET_INFO_FOR_APP} {client_id}")

        app_info = graph_data['value'][0]
-        sp_id = app_info['id']
-        roles = app_info['appRoles']
+        authInfo = {'sp_id': app_info['id']}

-        return {
-            'sp_id': sp_id,
-            'roles': {role['value']: role['id'] for role in roles}
-        }
+        # Convert the roles into ids (We could have more roles defined in the app than we need.)
+        for appRole in app_info['appRoles']:
+            if appRole['value'] in self.WORKSPACE_ROLES_DICT.keys():
+                authInfo[self.WORKSPACE_ROLES_DICT[appRole['value']]] = appRole['id']
+
+        return authInfo

    def _get_role_assignment_graph_data(self, user_id: str) -> dict:
        msgraph_token = self._get_msgraph_token()
@ -208,16 +209,19 @@ class AzureADAuthorization(AccessService):
        return graph_data

    def extract_workspace_auth_information(self, data: dict) -> dict:
-        if "app_id" not in data:
-            raise AuthConfigValidationError(strings.ACCESS_PLEASE_SUPPLY_APP_ID)
+        if "client_id" not in data:
+            raise AuthConfigValidationError(strings.ACCESS_PLEASE_SUPPLY_CLIENT_ID)

-        auth_info = self._get_app_auth_info(data["app_id"])
+        auth_info = {}
+        # The user may want us to create the AAD workspace app and therefore they
+        # don't know the client_id yet.
+        if data["client_id"] != "auto_create":
+            auth_info = self._get_app_auth_info(data["client_id"])

-        for role in ['WorkspaceOwner', 'WorkspaceResearcher']:
-            if role not in auth_info['roles']:
-                raise AuthConfigValidationError(f"{strings.ACCESS_APP_IS_MISSING_ROLE} {role}")
-
-        auth_info["app_id"] = data["app_id"]
+            # Check we've get all our required roles
+            for role in self.WORKSPACE_ROLES_DICT.items():
+                if role[1] not in auth_info:
+                    raise AuthConfigValidationError(f"{strings.ACCESS_APP_IS_MISSING_ROLE} {role[0]}")

        return auth_info

@ -231,17 +235,17 @@ class AzureADAuthorization(AccessService):
        return [RoleAssignment(role_assignment['resourceId'], role_assignment['appRoleId']) for role_assignment in graph_data['value']]

    def get_workspace_role(self, user: User, workspace: Workspace, user_role_assignments: List[RoleAssignment]) -> WorkspaceRole:
-        if 'sp_id' not in workspace.authInformation or 'roles' not in workspace.authInformation:
+        if 'sp_id' not in workspace.properties:
            raise AuthConfigValidationError(strings.AUTH_CONFIGURATION_NOT_AVAILABLE_FOR_WORKSPACE)

-        workspace_sp_id = workspace.authInformation['sp_id']
-        workspace_roles = workspace.authInformation['roles']
+        workspace_sp_id = workspace.properties['sp_id']

-        if 'WorkspaceOwner' not in workspace_roles or 'WorkspaceResearcher' not in workspace_roles:
-            raise AuthConfigValidationError(strings.AUTH_CONFIGURATION_NOT_AVAILABLE_FOR_WORKSPACE)
+        for requiredRole in self.WORKSPACE_ROLES_DICT.values():
+            if requiredRole not in workspace.properties:
+                raise AuthConfigValidationError(strings.AUTH_CONFIGURATION_NOT_AVAILABLE_FOR_WORKSPACE)

-        if RoleAssignment(resource_id=workspace_sp_id, role_id=workspace_roles['WorkspaceOwner']) in user_role_assignments:
+        if RoleAssignment(resource_id=workspace_sp_id, role_id=workspace.properties['app_role_id_workspace_owner']) in user_role_assignments:
            return WorkspaceRole.Owner
-        if RoleAssignment(resource_id=workspace_sp_id, role_id=workspace_roles['WorkspaceResearcher']) in user_role_assignments:
+        if RoleAssignment(resource_id=workspace_sp_id, role_id=workspace.properties['app_role_id_workspace_researcher']) in user_role_assignments:
            return WorkspaceRole.Researcher
        return WorkspaceRole.NoRole
--- a/api_app/tests_ma/test_api/test_routes/test_api_access.py
+++ b/api_app/tests_ma/test_api/test_routes/test_api_access.py
@ -19,7 +19,7 @@ USER_RESOURCE_ID = 'abcad738-7265-4b5f-9eae-a1a62928772e'


 def sample_workspace():
-    return Workspace(id=WORKSPACE_ID, templateName='template name', templateVersion='1.0', etag='', properties={"app_id": "12345"}, resourcePath="test")
+    return Workspace(id=WORKSPACE_ID, templateName='template name', templateVersion='1.0', etag='', properties={"client_id": "12345"}, resourcePath="test")


 def sample_workspace_service():
@ -92,7 +92,7 @@ class TestWorkspaceServiceOwnerRoutesAccess:
            "templateName": "test-workspace-service",
            "properties": {
                "display_name": "display",
-                "app_id": "f0acf127-a672-a672-a672-a15e5bf9f127"
+                "client_id": "f0acf127-a672-a672-a672-a15e5bf9f127"
            }
        }
        response = await client.post(app.url_path_for(strings.API_CREATE_WORKSPACE_SERVICE, workspace_id=WORKSPACE_ID), json=workspace_service_input)
--- a/api_app/tests_ma/test_api/test_routes/test_resource_helpers.py
+++ b/api_app/tests_ma/test_api/test_routes/test_resource_helpers.py
@ -37,21 +37,19 @@ def operations_repo() -> OperationRepository:
        return OperationRepository(cosmos_client_mock)


-def sample_resource(workspace_id=WORKSPACE_ID, auth_info: dict = {}):
+def sample_resource(workspace_id=WORKSPACE_ID):
    workspace = Workspace(
        id=workspace_id,
        templateName="tre-workspace-base",
        templateVersion="0.1.0",
        etag="",
        properties={
-            "app_id": "12345"
+            "client_id": "12345"
        },
        resourcePath=f'/workspaces/{workspace_id}',
        user=create_test_user(),
        updatedWhen=FAKE_CREATE_TIMESTAMP
    )
-    if auth_info:
-        workspace.authInformation = auth_info
    return workspace


--- a/api_app/tests_ma/test_api/test_routes/test_workspaces.py
+++ b/api_app/tests_ma/test_api/test_routes/test_workspaces.py
@ -29,7 +29,7 @@ pytestmark = pytest.mark.asyncio
 WORKSPACE_ID = '933ad738-7265-4b5f-9eae-a1a62928772e'
 SERVICE_ID = 'abcad738-7265-4b5f-9eae-a1a62928772e'
 USER_RESOURCE_ID = 'a33ad738-7265-4b5f-9eae-a1a62928772a'
-APP_ID = 'f0acf127-a672-a672-a672-a15e5bf9f127'
+CLIENT_ID = 'f0acf127-a672-a672-a672-a15e5bf9f127'
 OPERATION_ID = '11111111-7265-4b5f-9eae-a1a62928772f'


@ -39,7 +39,7 @@ def workspace_input():
        "templateName": "test-workspace",
        "properties": {
            "display_name": "display",
-            "app_id": APP_ID
+            "client_id": CLIENT_ID
        }
    }

@ -78,14 +78,14 @@ def sample_workspace(workspace_id=WORKSPACE_ID, auth_info: dict = {}) -> Workspa
        templateVersion="0.1.0",
        etag="",
        properties={
-            "app_id": "12345"
+            "client_id": "12345"
        },
        resourcePath=f'/workspaces/{workspace_id}',
        updatedWhen=FAKE_CREATE_TIMESTAMP,
        user=create_admin_user()
    )
    if auth_info:
-        workspace.authInformation = auth_info
+        workspace.properties = {**auth_info}
    return workspace


@ -118,7 +118,7 @@ def sample_resource_operation_in_response(resource_id: str, operation_id: str):
    return OperationInResponse(operation=op)


-def sample_deployed_workspace(workspace_id=WORKSPACE_ID, auth_info: dict = {}):
+def sample_deployed_workspace(workspace_id=WORKSPACE_ID, authInfo={}):
    workspace = Workspace(
        id=workspace_id,
        templateName="tre-workspace-base",
@ -128,8 +128,8 @@ def sample_deployed_workspace(workspace_id=WORKSPACE_ID, auth_info: dict = {}):
        resourcePath="test",
        updatedWhen=FAKE_CREATE_TIMESTAMP
    )
-    if auth_info:
-        workspace.authInformation = auth_info
+    if authInfo:
+        workspace.properties = {**authInfo}
    return workspace


@ -231,9 +231,9 @@ class TestWorkspaceRoutesThatDontRequireAdminRights:
    @patch("api.routes.workspaces.WorkspaceRepository.get_active_workspaces")
    @patch("api.routes.workspaces.get_user_role_assignments")
    async def test_get_workspaces_returns_correct_data_when_resources_exist(self, access_service_mock, get_workspaces_mock, app, client) -> None:
-        auth_info_user_in_workspace_owner_role = {'sp_id': 'ab123', 'roles': {'WorkspaceOwner': 'ab124', 'WorkspaceResearcher': 'ab125'}}
-        auth_info_user_in_workspace_researcher_role = {'sp_id': 'ab123', 'roles': {'WorkspaceOwner': 'ab127', 'WorkspaceResearcher': 'ab126'}}
-        auth_info_user_not_in_workspace_role = {'sp_id': 'ab127', 'roles': {'WorkspaceOwner': 'ab128', 'WorkspaceResearcher': 'ab129'}}
+        auth_info_user_in_workspace_owner_role = {'sp_id': 'ab123', 'app_role_id_workspace_owner': 'ab124', 'app_role_id_workspace_researcher': 'ab125'}
+        auth_info_user_in_workspace_researcher_role = {'sp_id': 'ab123', 'app_role_id_workspace_owner': 'ab127', 'app_role_id_workspace_researcher': 'ab126'}
+        auth_info_user_not_in_workspace_role = {'sp_id': 'ab127', 'app_role_id_workspace_owner': 'ab128', 'app_role_id_workspace_researcher': 'ab129'}

        valid_ws_1 = sample_workspace(workspace_id=str(uuid.uuid4()), auth_info=auth_info_user_in_workspace_owner_role)
        valid_ws_2 = sample_workspace(workspace_id=str(uuid.uuid4()), auth_info=auth_info_user_in_workspace_researcher_role)
@ -383,7 +383,7 @@ class TestWorkspaceRoutesThatRequireAdminRights:

        modified_workspace = sample_workspace()
        modified_workspace.isEnabled = False
-        modified_workspace.history = [ResourceHistoryItem(properties={'app_id': '12345'}, isEnabled=True, resourceVersion=0, updatedWhen=FAKE_CREATE_TIMESTAMP, user=create_admin_user())]
+        modified_workspace.history = [ResourceHistoryItem(properties={'client_id': '12345'}, isEnabled=True, resourceVersion=0, updatedWhen=FAKE_CREATE_TIMESTAMP, user=create_admin_user())]
        modified_workspace.resourceVersion = 1
        modified_workspace.user = create_admin_user()
        modified_workspace.updatedWhen = FAKE_UPDATE_TIMESTAMP
@ -403,7 +403,7 @@ class TestWorkspaceRoutesThatRequireAdminRights:
        etag = "some-etag-value"
        modified_workspace = sample_workspace()
        modified_workspace.isEnabled = False
-        modified_workspace.history = [ResourceHistoryItem(properties={'app_id': '12345'}, isEnabled=True, resourceVersion=0, updatedWhen=FAKE_CREATE_TIMESTAMP, user=create_admin_user())]
+        modified_workspace.history = [ResourceHistoryItem(properties={'client_id': '12345'}, isEnabled=True, resourceVersion=0, updatedWhen=FAKE_CREATE_TIMESTAMP, user=create_admin_user())]
        modified_workspace.resourceVersion = 1
        modified_workspace.user = create_admin_user()
        modified_workspace.updatedWhen = FAKE_UPDATE_TIMESTAMP
--- a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py
+++ b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py
@ -28,7 +28,7 @@ def resource_repo():

@pytest.fixture
 def workspace_input():
-    return WorkspaceInCreate(templateName="base-tre", properties={"display_name": "test", "description": "test", "app_id": "123"})
+    return WorkspaceInCreate(templateName="base-tre", properties={"display_name": "test", "description": "test", "client_id": "123"})


 def sample_resource() -> Resource:
--- a/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py
+++ b/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py
@ -12,7 +12,7 @@ from models.schemas.workspace import WorkspaceInCreate

@pytest.fixture
 def basic_workspace_request():
-    return WorkspaceInCreate(templateName="base-tre", properties={"display_name": "test", "description": "test", "app_id": "123", "tre_id": "test"})
+    return WorkspaceInCreate(templateName="base-tre", properties={"display_name": "test", "description": "test", "client_id": "123", "tre_id": "test"})


@pytest.fixture
@ -181,3 +181,15 @@ def test_create_workspace_item_raises_value_error_if_template_is_invalid(validat

    with pytest.raises(ValueError):
        workspace_repo.create_workspace_item(workspace_input, {})
+
+
+def test_automatically_create_application_registration_returns_true(workspace_repo):
+    dictToTest = {"client_id": "auto_create"}
+
+    assert workspace_repo.automatically_create_application_registration(dictToTest) is True
+
+
+def test_automatically_create_application_registration_returns_false(workspace_repo):
+    dictToTest = {"client_id": "12345"}
+
+    assert workspace_repo.automatically_create_application_registration(dictToTest) is False
--- a/api_app/tests_ma/test_services/test_aad_access_service.py
+++ b/api_app/tests_ma/test_services/test_aad_access_service.py
@ -7,31 +7,31 @@ from services.aad_authentication import AzureADAuthorization
 from services.access_service import AuthConfigValidationError


-def test_extract_workspace__raises_error_if_app_id_not_available():
+def test_extract_workspace__raises_error_if_client_id_not_available():
    access_service = AzureADAuthorization()
    with pytest.raises(AuthConfigValidationError):
        access_service.extract_workspace_auth_information(data={})


-@patch("services.aad_authentication.AzureADAuthorization._get_app_auth_info", return_value={"roles": {"WorkspaceResearcher": "1234"}})
+@patch("services.aad_authentication.AzureADAuthorization._get_app_auth_info", return_value={"app_role_id_workspace_researcher": "1234"})
 def test_extract_workspace__raises_error_if_owner_not_in_roles(get_app_auth_info_mock):
    access_service = AzureADAuthorization()
    with pytest.raises(AuthConfigValidationError):
-        access_service.extract_workspace_auth_information(data={"app_id": "1234"})
+        access_service.extract_workspace_auth_information(data={"client_id": "1234"})


-@patch("services.aad_authentication.AzureADAuthorization._get_app_auth_info", return_value={"roles": {"WorkspaceOwner": "1234"}})
+@patch("services.aad_authentication.AzureADAuthorization._get_app_auth_info", return_value={"app_role_id_workspace_owner": "1234"})
 def test_extract_workspace__raises_error_if_researcher_not_in_roles(get_app_auth_info_mock):
    access_service = AzureADAuthorization()
    with pytest.raises(AuthConfigValidationError):
-        access_service.extract_workspace_auth_information(data={"app_id": "1234"})
+        access_service.extract_workspace_auth_information(data={"client_id": "1234"})


@patch("services.aad_authentication.AzureADAuthorization._get_app_sp_graph_data", return_value={})
 def test_extract_workspace__raises_error_if_graph_data_is_invalid(get_app_sp_graph_data_mock):
    access_service = AzureADAuthorization()
    with pytest.raises(AuthConfigValidationError):
-        access_service.extract_workspace_auth_information(data={"app_id": "1234"})
+        access_service.extract_workspace_auth_information(data={"client_id": "1234"})


@patch("services.aad_authentication.AzureADAuthorization._get_app_sp_graph_data")
@ -48,13 +48,13 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock):
        ]
    }
    expected_auth_info = {
-        "app_id": "1234",
-        'sp_id': '12345',
-        'roles': {'WorkspaceResearcher': '1abc3', 'WorkspaceOwner': '1abc4'}
+        "sp_id": "12345",
+        "app_role_id_workspace_owner": "1abc4",
+        "app_role_id_workspace_researcher": "1abc3"
    }

    access_service = AzureADAuthorization()
-    actual_auth_info = access_service.extract_workspace_auth_information(data={"app_id": "1234"})
+    actual_auth_info = access_service.extract_workspace_auth_information(data={"client_id": "1234"})

    assert actual_auth_info == expected_auth_info

@ -63,23 +63,23 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock):
                         [
                             # user not a member of the workspace app
                             (User(roleAssignments=[RoleAssignment(resource_id="ab123", role_id="ab124")], id='123', name="test", email="t@t.com"),
-                              Workspace(authInformation={'app_id': '1234', 'sp_id': 'abc127', 'roles': {'WorkspaceOwner': 'abc128', 'WorkspaceResearcher': 'abc129'}},
-                                        id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test"),
+                              Workspace(id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test",
+                                        properties={'client_id': '1234', 'sp_id': 'abc127', 'app_role_id_workspace_owner': 'abc128', 'app_role_id_workspace_researcher': 'abc129'}),
                              WorkspaceRole.NoRole),
                             # user is member of the workspace app but not in role
                             (User(roleAssignments=[RoleAssignment(resource_id="ab127", role_id="ab124")], id='123', name="test", email="t@t.com"),
-                              Workspace(authInformation={'app_id': '1234', 'sp_id': 'abc127', 'roles': {'WorkspaceOwner': 'abc128', 'WorkspaceResearcher': 'abc129'}},
-                                        id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test"),
+                              Workspace(id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test",
+                                        properties={'client_id': '1234', 'sp_id': 'abc127', 'app_role_id_workspace_owner': 'abc128', 'app_role_id_workspace_researcher': 'abc129'}),
                              WorkspaceRole.NoRole),
                             # user has owner role in workspace
                             (User(roleAssignments=[RoleAssignment(resource_id="abc127", role_id="abc128")], id='123', name="test", email="t@t.com"),
-                              Workspace(authInformation={'app_id': '1234', 'sp_id': 'abc127', 'roles': {'WorkspaceOwner': 'abc128', 'WorkspaceResearcher': 'abc129'}},
-                                        id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test"),
+                              Workspace(id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test",
+                                        properties={'client_id': '1234', 'sp_id': 'abc127', 'app_role_id_workspace_owner': 'abc128', 'app_role_id_workspace_researcher': 'abc129'}),
                              WorkspaceRole.Owner),
                             # user has researcher role in workspace
                             (User(roleAssignments=[RoleAssignment(resource_id="abc127", role_id="abc129")], id='123', name="test", email="t@t.com"),
-                              Workspace(authInformation={'app_id': '1234', 'sp_id': 'abc127', 'roles': {'WorkspaceOwner': 'abc128', 'WorkspaceResearcher': 'abc129'}},
-                                        id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test"),
+                              Workspace(id='abc', etag="", templateName='template-name', templateVersion='0.1.0', resourcePath="test",
+                                        properties={'client_id': '1234', 'sp_id': 'abc127', 'app_role_id_workspace_owner': 'abc128', 'app_role_id_workspace_researcher': 'abc129'}),
                              WorkspaceRole.Researcher)
                         ])
@patch("services.aad_authentication.AzureADAuthorization.get_user_role_assignments")
@ -114,7 +114,7 @@ def test_raises_auth_config_error_if_auth_info_has_incorrect_roles(_):
        templateName='template-name',
        templateVersion='0.1.0',
        etag='',
-        authInformation={'sp_id': '123', 'roles': {}},
+        properties={'sp_id': '123', 'roles': {}},
        resourcePath="test")

    with pytest.raises(AuthConfigValidationError):
--- a/api_app/tests_ma/test_services/test_schema_service.py
+++ b/api_app/tests_ma/test_services/test_schema_service.py
@ -10,7 +10,7 @@ def test_enrich_workspace_template_enriches_with_workspace_defaults_and_aad(enri
    workspace_template = basic_resource_template
    # read schema called twice - once for default props and once for aad
    default_props = (['description'], {'description': {'type': 'string'}})
-    aad_props = (['app_id'], {'app_id': {'type': 'string'}})
+    aad_props = (['client_id'], {'client_id': {'type': 'string'}})
    read_schema_mock.side_effect = [default_props, aad_props]

    services.schema_service.enrich_workspace_template(workspace_template)
@ -50,29 +50,29 @@ def test_enrich_user_resource_template_enriches_with_user_resource_defaults(enri
    (
        {'num_vms': {'type': 'string'}},
        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}},
-        {'app_id': {'type': 'string'}},
-        {'num_vms': {'type': 'string'}, 'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'app_id': {'type': 'string'}}
+        {'client_id': {'type': 'string'}},
+        {'num_vms': {'type': 'string'}, 'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'client_id': {'type': 'string'}}
    ),
    # empty original
    (
        {},
        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}},
-        {'app_id': {'type': 'string'}},
-        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'app_id': {'type': 'string'}}
+        {'client_id': {'type': 'string'}},
+        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'client_id': {'type': 'string'}}
    ),
    # duplicates
    (
        {'description': {'type': 'string'}},
        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}},
-        {'app_id': {'type': 'string'}},
-        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'app_id': {'type': 'string'}}
+        {'client_id': {'type': 'string'}},
+        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'client_id': {'type': 'string'}}
    ),
    # duplicate names - different types
    (
        {'description': {'type': 'bool'}},
        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}},
-        {'app_id': {'type': 'string'}},
-        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'app_id': {'type': 'string'}}
+        {'client_id': {'type': 'string'}},
+        {'description': {'type': 'string'}, 'display_name': {'type': 'string'}, 'client_id': {'type': 'string'}}
    )])
 def test_enrich_template_combines_properties(original, extra1, extra2, expected, basic_resource_template):
    original_template = basic_resource_template
@ -88,22 +88,22 @@ def test_enrich_template_combines_properties(original, extra1, extra2, expected,
    (
        ['num_vms'],
        ['description', 'display_name'],
-        ['app_id'],
-        ['num_vms', 'description', 'display_name', 'app_id']
+        ['client_id'],
+        ['num_vms', 'description', 'display_name', 'client_id']
    ),
    # empty original
    (
        [],
        ['description', 'display_name'],
-        ['app_id'],
-        ['description', 'display_name', 'app_id']
+        ['client_id'],
+        ['description', 'display_name', 'client_id']
    ),
    # duplicates
    (
        ['description'],
        ['description', 'display_name'],
-        ['app_id'],
-        ['description', 'display_name', 'app_id']
+        ['client_id'],
+        ['description', 'display_name', 'client_id']
    )])
 def test_enrich_template_combines_required(original, extra1, extra2, expected, basic_resource_template):
    original_template = basic_resource_template
--- a/api_http_requests/API
+++ b/api_http_requests/API
@ -10,7 +10,7 @@ Content-Type: {{contentType}}
  "properties": {
    "display_name": "my workspace",
    "description": "my workspace",
-    "app_id": "{{appId}}",
+    "client_id": "{{clientId}}",
    "vm_size": "Standard_A1",
    "no_of_vms": 2
  }
--- a/api_http_requests/http-client.env.json
+++ b/api_http_requests/http-client.env.json
@ -8,6 +8,6 @@
    "workspaceId": "49ab7315-49bb-48ed-b9ca-c37369f15e7a",
    "workspaceServiceId": "2a3165e7-5b5c-40e5-b3b6-94f528e9fcf0",
    "userResourceId": "726e00b5-9408-4d81-a913-d890b4851307",
-    "appId": "9d52b04f-89cf-47b4-868a-e12be7133b36"
+    "clientId": "9d52b04f-89cf-47b4-868a-e12be7133b36"
  }
 }
--- a/docs/tre-admins/setup-instructions/installing-base-workspace.md
+++ b/docs/tre-admins/setup-instructions/installing-base-workspace.md
@ -56,7 +56,7 @@ Go to `https://<azure_tre_fqdn>/api/docs` and use POST `/api/workspaces` with th
  "properties": {
    "display_name": "manual-from-swagger",
    "description": "workspace for team X",
-    "app_id": "WORKSPACE_API_CLIENT_ID",
+    "client_id": "WORKSPACE_API_CLIENT_ID",
    "address_space_size": "medium"
  }
 }
--- a/docs/tre-templates/workspace-services/inner-eye.md
+++ b/docs/tre-templates/workspace-services/inner-eye.md
@ -28,7 +28,7 @@ Provision an InnerEye workspace by invoking a POST to ```https://<treid>.<region
            "properties": {
                "display_name": "InnerEye",
                "description": "InnerEyer workspace",
-                "app_id": "<app_id>",
+                "client_id": "<WORKSPACE_API_CLIENT_ID>",
                "inference_sp_client_id": "<spn_client_id>",
                "inference_sp_client_secret": "<spn_client_secret>"
            }
--- a/e2e_tests/config.py
+++ b/e2e_tests/config.py
@ -6,6 +6,7 @@ config = Config(".env")
 # Resource Info
 RESOURCE_LOCATION: str = config("RESOURCE_LOCATION", default="")
 TRE_ID: str = config("TRE_ID", default="")
+TRE_URL: str = config("TRE_URL", default="")
 API_CLIENT_ID: str = config("API_CLIENT_ID", default="")
 TEST_USER_NAME: str = config("TEST_USER_NAME", default="")
 TEST_USER_PASSWORD: str = config("TEST_USER_PASSWORD", default="")
--- a/e2e_tests/helpers.py
+++ b/e2e_tests/helpers.py
@ -50,7 +50,10 @@ async def post_resource(payload, endpoint, resource_type, token, admin_token, ve
        else:
            auth_headers = get_auth_header(token)

-        full_endpoint = f"https://{config.TRE_ID}.{config.RESOURCE_LOCATION}.cloudapp.azure.com{endpoint}"
+        if (config.TRE_URL != ""):
+            full_endpoint = f"{config.TRE_URL}{endpoint}"
+        else:
+            full_endpoint = f"https://{config.TRE_ID}.{config.RESOURCE_LOCATION}.cloudapp.azure.com{endpoint}"
        LOGGER.info(f'POSTING RESOURCE TO: {full_endpoint}')

        if method == "POST":
--- a/e2e_tests/test.http
+++ b/e2e_tests/test.http
@ -29,7 +29,7 @@ Content-type: application/json
  "properties": {
      "display_name": "E2E test",
      "description": "workspace for E2E",
-      "app_id": "<CHANGE_ME_APP_ID>",
+      "client_id": "<CHANGE_ME_CLIENT_ID>",
      "address_space_size": "small"
  }
 }
--- a/e2e_tests/test_performance.py
+++ b/e2e_tests/test_performance.py
@ -24,7 +24,7 @@ async def test_parallel_resource_creations(admin_token, workspace_owner_token, v
                "display_name": f'Perf Test Workspace {i}',
                "description": "workspace for perf test",
                "address_space_size": "small",
-                "app_id": f"{config.TEST_WORKSPACE_APP_ID}"
+                "client_id": f"{config.TEST_WORKSPACE_APP_ID}"
            }
        }

@ -61,7 +61,7 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(admin_toke
                "display_name": "E2E test guacamole service",
                "description": "",
                "address_space_size": "small",
-                "app_id": f"{config.TEST_WORKSPACE_APP_ID}"
+                "client_id": f"{config.TEST_WORKSPACE_APP_ID}"
            }
        }

--- a/e2e_tests/test_workspace_services.py
+++ b/e2e_tests/test_workspace_services.py
@ -18,7 +18,7 @@ async def test_create_guacamole_service_into_base_workspace(admin_token, workspa
            "display_name": "E2E test guacamole service",
            "description": "workspace for E2E",
            "address_space_size": "small",
-            "app_id": f"{config.TEST_WORKSPACE_APP_ID}"
+            "client_id": f"{config.TEST_WORKSPACE_APP_ID}"
        }
    }

--- a/scripts/db_migrations.py
+++ b/scripts/db_migrations.py
@ -92,7 +92,7 @@ class TRECosmosDBMigrations:

        for item in resources_container.query_items(query='SELECT * FROM c', enable_cross_partition_query=True):
            template_version = semantic_version.Version(item["templateVersion"])
-            if (template_version > semantic_version.Version('0.3.0') and "authInformation" in item):
+            if (template_version < semantic_version.Version('2.5.0') and "authInformation" in item):
                print(f'Found workspace {item["id"]} that needs migrating')

                # Rename app_id to be client_id
--- a/templates/core/terraform/.terraform.lock.hcl
+++ b/templates/core/terraform/.terraform.lock.hcl
@ -58,21 +58,21 @@ provider "registry.terraform.io/hashicorp/local" {
 }

 provider "registry.terraform.io/hashicorp/random" {
-  version = "3.1.2"
+  version = "3.1.3"
  hashes = [
-    "h1:5A5VsY5wNmOZlupUcLnIoziMPn8htSZBXbP3lI7lBEM=",
-    "zh:0daceba867b330d3f8e2c5dc895c4291845a78f31955ce1b91ab2c4d1cd1c10b",
-    "zh:104050099efd30a630741f788f9576b19998e7a09347decbec3da0b21d64ba2d",
-    "zh:173f4ef3fdf0c7e2564a3db0fac560e9f5afdf6afd0b75d6646af6576b122b16",
-    "zh:41d50f975e535f968b3f37170fb07937c15b76d85ba947d0ce5e5ff9530eda65",
-    "zh:51a5038867e5e60757ed7f513dd6a973068241190d158a81d1b69296efb9cb8d",
-    "zh:6432a568e97a5a36cc8aebca5a7e9c879a55d3bc71d0da1ab849ad905f41c0be",
-    "zh:6bac6501394b87138a5e17c9f3a41e46ff7833ad0ba2a96197bb7787e95b641c",
-    "zh:6c0a7f5faacda644b022e7718e53f5868187435be6d000786d1ca05aa6683a25",
-    "zh:74c89de3fa6ef3027efe08f8473c2baeb41b4c6cee250ba7aeb5b64e8c79800d",
+    "h1:nLWniS8xhb32qRQy+n4bDPjQ7YWZPVMR3v1vSrx7QyY=",
+    "zh:26e07aa32e403303fc212a4367b4d67188ac965c37a9812e07acee1470687a73",
+    "zh:27386f48e9c9d849fbb5a8828d461fde35e71f6b6c9fc235bc4ae8403eb9c92d",
+    "zh:5f4edda4c94240297bbd9b83618fd362348cadf6bf24ea65ea0e1844d7ccedc0",
+    "zh:646313a907126cd5e69f6a9fafe816e9154fccdc04541e06fed02bb3a8fa2d2e",
+    "zh:7349692932a5d462f8dee1500ab60401594dddb94e9aa6bf6c4c0bd53e91bbb8",
    "zh:78d5eefdd9e494defcb3c68d282b8f96630502cac21d1ea161f53cfe9bb483b3",
-    "zh:b29eabbf0a5298f0e95a1df214c7cfe06ea9bcf362c63b3ad2f72d85da7d4685",
-    "zh:e891458c7a61e5b964e09616f1a4f87d0471feae1ec04cc51776e7dec1a3abce",
+    "zh:9034daba8d9b32b35930d168f363af04cecb153d5849a7e4a5966c97c5dc956e",
+    "zh:bb81dfca59ef5f949ef39f19ea4f4de25479907abc28cdaa36d12ecd7c0a9699",
+    "zh:bcf7806b99b4c248439ae02c8e21f77aff9fadbc019ce619b929eef09d1221bb",
+    "zh:d708e14d169e61f326535dd08eecd3811cd4942555a6f8efabc37dbff9c6fc61",
+    "zh:dc294e19a46e1cefb9e557a7b789c8dd8f319beca99b8c265181bc633dc434cc",
+    "zh:f9d758ee53c55dc016dd736427b6b0c3c8eb4d0dbbc785b6a3579b0ffedd9e42",
  ]
 }

--- a/templates/workspaces/base/.env.sample
+++ b/templates/workspaces/base/.env.sample
@ -5,6 +5,7 @@ export SHARED_STORAGE_QUOTA=50
 export KEYVAULT_PURGE_PROTECTION_ENABLED=false
 export ENABLE_LOCAL_DEBUGGING=true
 export REGISTER_AAD_APPLICATION=true
+export CLIENT_ID=$WORKSPACE_APP_CLIENT_ID

 # These are needed if you are testing at the Terraform level.
 export TRE_ID=__CHANGE_ME__
@ -15,6 +16,7 @@ export TF_VAR_address_space="10.2.6.0/24"
 export TF_VAR_enable_local_debugging="true"
 export TF_VAR_purge_protection_enabled="false"
 export TF_VAR_register_aad_application="true"
+export TF_VAR_client_id=$WORKSPACE_APP_CLIENT_ID

 export TF_VAR_mgmt_resource_group_name=__CHANGE_ME__
 export TF_VAR_mgmt_storage_account_name=__CHANGE_ME__
--- a/templates/workspaces/base/parameters.json
+++ b/templates/workspaces/base/parameters.json
@ -69,6 +69,30 @@
      "source": {
        "env": "REGISTER_AAD_APPLICATION"
      }
+    },
+    {
+      "name": "client_id",
+      "source": {
+        "env": "CLIENT_ID"
+      }
+    },
+    {
+      "name": "sp_id",
+      "source": {
+        "env": "SP_ID"
+      }
+    },
+    {
+      "name": "app_role_id_workspace_owner",
+      "source": {
+        "env": "APP_ROLE_ID_WORKSPACE_OWNER"
+      }
+    },
+    {
+      "name": "app_role_id_workspace_researcher",
+      "source": {
+        "env": "APP_ROLE_ID_WORKSPACE_RESEARCHER"
+      }
    }
  ]
 }
--- a/templates/workspaces/base/porter.yaml
+++ b/templates/workspaces/base/porter.yaml
@ -1,6 +1,6 @@
 ---
 name: tre-workspace-base
-version: 0.2.1
+version: 0.2.5
 description: "A base Azure TRE workspace"
 registry: azuretre

@ -60,24 +60,44 @@ parameters:
  - name: register_aad_application
    type: boolean
    default: false
+    description: "Whether this bundle should register the workspace in AAD"
+  - name: client_id
+    type: string
+    description: "The client id of the workspace in the identity provider."
+  - name: sp_id
+    type: string
+    default: ""
+    description: "The Service Principal in the Identity provider to be able to get claims"
+  - name: app_role_id_workspace_owner
+    type: string
+    default: ""
+    description: "The id of the application role WorkspaceOwner in the identity provider"
+  - name: app_role_id_workspace_researcher
+    type: string
+    default: ""
+    description: "The id of the application role WorkspaceResearcher in the identity provider"

-# outputs:
-#   - name: WorkspaceOwnerAppRoleId
-#     type: string
-#     applyTo:
-#       - install
-#   - name: WorkspaceResearcherAppRoleId
-#     type: string
-#     applyTo:
-#       - install
-#   - name: client_id
-#     type: string
-#     applyTo:
-#       - install
-#   - name: sp_id
-#     type: string
-#     applyTo:
-#       - install
+outputs:
+  - name: app_role_id_workspace_owner
+    type: string
+    default: "{{ bundle.parameters.app_role_id_workspace_owner }}"
+    applyTo:
+      - install
+  - name: app_role_id_workspace_researcher
+    type: string
+    default: "{{ bundle.parameters.app_role_id_workspace_researcher }}"
+    applyTo:
+      - install
+  - name: client_id
+    type: string
+    default: "{{ bundle.parameters.client_id }}"
+    applyTo:
+      - install
+  - name: sp_id
+    default: "{{ bundle.parameters.sp_id }}"
+    type: string
+    applyTo:
+      - install

 mixins:
  - exec
@ -102,6 +122,10 @@ install:
        auth_client_id: "{{ bundle.credentials.auth_client_id }}"
        auth_client_secret: "{{ bundle.credentials.auth_client_secret }}"
        auth_tenant_id: "{{ bundle.credentials.auth_tenant_id }}"
+        client_id: "{{ bundle.parameters.client_id }}"
+        sp_id: "{{ bundle.parameters.sp_id }}"
+        app_role_id_workspace_owner: "{{ bundle.parameters.app_role_id_workspace_owner }}"
+        app_role_id_workspace_researcher: "{{ bundle.parameters.app_role_id_workspace_researcher }}"
      backendConfig:
        resource_group_name:
          "{{ bundle.parameters.tfstate_resource_group_name }}"
@ -109,11 +133,11 @@ install:
          "{{ bundle.parameters.tfstate_storage_account_name }}"
        container_name: "{{ bundle.parameters.tfstate_container_name }}"
        key: "{{ bundle.parameters.tre_id }}-ws-{{ bundle.parameters.id }}"
-      # outputs:
-      #   - name: WorkspaceOwnerAppRoleId
-      #   - name: WorkspaceResearcherAppRoleId
-      #   - name: client_id
-      #   - name: sp_id
+      outputs:
+        - name: app_role_id_workspace_owner
+        - name: app_role_id_workspace_researcher
+        - name: client_id
+        - name: sp_id

 upgrade:
  - exec:
--- a/templates/workspaces/base/terraform/outputs.tf
+++ b/templates/workspaces/base/terraform/outputs.tf
@ -2,18 +2,18 @@ output "workspace_resource_name_suffix" {
  value = local.workspace_resource_name_suffix
 }

-output "WorkspaceOwnerAppRoleId" {
-  value = var.register_aad_application ? module.aad[0].app_role_workspace_owner_id : ""
+output "app_role_id_workspace_owner" {
+  value = var.register_aad_application ? module.aad[0].app_role_workspace_owner_id : var.app_role_id_workspace_owner
 }

-output "WorkspaceResearcherAppRoleId" {
-  value = var.register_aad_application ? module.aad[0].app_role_workspace_researcher_id : ""
+output "app_role_id_workspace_researcher" {
+  value = var.register_aad_application ? module.aad[0].app_role_workspace_researcher_id : var.app_role_id_workspace_researcher
 }

 output "client_id" {
-  value = var.register_aad_application ? module.aad[0].client_id : ""
+  value = var.register_aad_application ? module.aad[0].client_id : var.client_id
 }

 output "sp_id" {
-  value = var.register_aad_application ? module.aad[0].sp_id : ""
+  value = var.register_aad_application ? module.aad[0].sp_id : var.sp_id
 }
--- a/templates/workspaces/base/terraform/variables.tf
+++ b/templates/workspaces/base/terraform/variables.tf
@ -54,15 +54,40 @@ variable "register_aad_application" {
  description = "Create an AAD application automatically for the Workspace."
 }

-# These are used to authenticate into the AAD Tenant to create the AAD App
 variable "auth_tenant_id" {
-  type = string
+  type        = string
+  description = "Used to authenticate into the AAD Tenant to create the AAD App"
 }
 variable "auth_client_id" {
-  type = string
+  type        = string
+  description = "Used to authenticate into the AAD Tenant to create the AAD App"
 }
 variable "auth_client_secret" {
-  type = string
+  type        = string
+  description = "Used to authenticate into the AAD Tenant to create the AAD App"
+}
+
+# These variables are only passed in if you are not registering an AAD
+# application as they need passing back out
+variable "app_role_id_workspace_owner" {
+  type        = string
+  default     = ""
+  description = "The id of the application role WorkspaceOwner in the identity provider, this is passed in so that we may return it as an output."
+}
+variable "app_role_id_workspace_researcher" {
+  type        = string
+  default     = ""
+  description = "The id of the application role WorkspaceResearcher in the identity provider, this is passed in so that we may return it as an output."
+}
+variable "client_id" {
+  type        = string
+  default     = ""
+  description = "The client id of the workspace in the identity provider, this is passed in so that we may return it as an output."
+}
+variable "sp_id" {
+  type        = string
+  default     = ""
+  description = "The Service Principal in the Identity provider to be able to get claims, this is passed in so that we may return it as an output."
 }

 locals {
--- a/templates/workspaces/base/terraform/workspace.tf
+++ b/templates/workspaces/base/terraform/workspace.tf
@ -28,5 +28,9 @@ module "aad" {
  count                          = var.register_aad_application ? 1 : 0
  key_vault_id                   = azurerm_key_vault.kv.id
  workspace_resource_name_suffix = local.workspace_resource_name_suffix
-  depends_on                     = [azurerm_key_vault_access_policy.deployer]
+  depends_on = [
+    azurerm_key_vault_access_policy.deployer,
+    azurerm_key_vault_access_policy.resource_processor,
+    azurerm_private_endpoint.kvpe
+  ]
 }