Issue admin token on demand to avoid an expired token in e2e tests (#…

…2572)

* admin token is retrieved via direct method call in some of the tests
* fix 500 error on bad token
* raising informative http exceptions on token validation errors

CHANGELOG.md

@@ -25,13 +25,15 @@ ENHANCEMENTS:
 * UI is deployed by default ([#2554](https://github.com/microsoft/AzureTRE/pull/2554))
 * Remove manual/makefile option to install Gitea/Nexus ([#2573](https://github.com/microsoft/AzureTRE/pull/2573))
 * Exact Terraform provider versions in bundles ([#2579](https://github.com/microsoft/AzureTRE/pull/2579))
+* Stabilize E2E tests by issuing the access token prior using it, hence, reducing the change of expired token ([#2572](https://github.com/microsoft/AzureTRE/pull/2572))
 
 BUG FIXES:
 
 * API health check is also returned by accessing the root path at / ([#2469](https://github.com/microsoft/AzureTRE/pull/2469))
 * Temporary disable AppInsight's private endpoint in base workspace ([#2543](https://github.com/microsoft/AzureTRE/pull/2543))
 * Resource Processor execution optimization (`porter show`) for long-standing services ([#2542](https://github.com/microsoft/AzureTRE/pull/2542))
 * Move AML Compute deployment to use AzApi Terraform Provider {[#2555]((https://github.com/microsoft/AzureTRE/pull/2555))
+* Invalid token exceptions in the API app are catched, throwing 401 instead of 500 Internal server error ([#2572](https://github.com/microsoft/AzureTRE/pull/2572))
 
 ## 0.4.2 (August 23, 2022)
 

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.4.29"
+__version__ = "0.4.30"

api_app/resources/strings.py

@@ -99,6 +99,9 @@
 AUTH_CONFIGURATION_NOT_AVAILABLE_FOR_WORKSPACE = "Auth configuration not available for workspace"
 AUTH_UNABLE_TO_VALIDATE_TOKEN = "Unable to decode or validate token"
 INVALID_AUTH_PROVIDER = "Invalid authentication provider"
+INVALID_SIGNATURE = "Invalid token signature"
+EXPIRED_SIGNATURE = "Expired token signature"
+INVALID_TOKEN = "Invalid token"
 
 UNABLE_TO_REPLACE_CURRENT_TEMPLATE = "Unable to replace the existing 'current' template with this name"
 UNABLE_TO_PROCESS_REQUEST = "Unable to process request"

api_app/services/aad_authentication.py

@@ -67,7 +67,18 @@ async def __call__(self, request: Request) -> User:
             try:
                 decoded_token = self._decode_token(token, config.API_AUDIENCE)
             except jwt.exceptions.InvalidSignatureError:
-                logging.debug("Failed to decode using TRE API app registration")
+                logging.debug("Failed to decode using TRE API app registration (Invalid Signatrue)")
+                raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strings.INVALID_SIGNATURE)
+            except jwt.exceptions.ExpiredSignatureError:
+                logging.debug("Failed to decode using TRE API app registration (Expired Signature)")
+                raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strings.EXPIRED_SIGNATURE)
+            except jwt.exceptions.InvalidTokenError:
+                # any other token validation exception, we want to catch all of these...
+                logging.debug("Failed to decode using TRE API app registration (Invalid token)")
+                raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strings.INVALID_TOKEN)
+            except Exception as e:
+                # Unexpected token decoding/validation exception. making sure we are not crashing (with 500)
+                logging.debug(e)
                 pass
 
         # Failed to decode token using either app registration

e2e_tests/conftest.py

@@ -1,11 +1,5 @@
-from json import JSONDecodeError
 import pytest
 
-from httpx import AsyncClient
-from starlette import status
-
-import config
-
 pytestmark = pytest.mark.asyncio
 
 
@@ -19,30 +13,3 @@ def verify(pytestconfig):
         return True
     elif pytestconfig.getoption("verify").lower() == "false":
         return False
-
-
-@pytest.fixture
-async def admin_token(verify) -> str:
-    async with AsyncClient(verify=verify) as client:
-        responseJson = ""
-        headers = {'Content-Type': "application/x-www-form-urlencoded"}
-        if config.TEST_ACCOUNT_CLIENT_ID != "" and config.TEST_ACCOUNT_CLIENT_SECRET != "":
-            # Use Client Credentials flow
-            payload = f"grant_type=client_credentials&client_id={config.TEST_ACCOUNT_CLIENT_ID}&client_secret={config.TEST_ACCOUNT_CLIENT_SECRET}&scope=api://{config.API_CLIENT_ID}/.default"
-            url = f"https://login.microsoftonline.com/{config.AAD_TENANT_ID}/oauth2/v2.0/token"
-
-        else:
-            # Use Resource Owner Password Credentials flow
-            payload = f"grant_type=password&resource={config.API_CLIENT_ID}&username={config.TEST_USER_NAME}&password={config.TEST_USER_PASSWORD}&scope=api://{config.API_CLIENT_ID}/user_impersonation&client_id={config.TEST_APP_ID}"
-            url = f"https://login.microsoftonline.com/{config.AAD_TENANT_ID}/oauth2/token"
-
-        response = await client.post(url, headers=headers, content=payload)
-        try:
-            responseJson = response.json()
-        except JSONDecodeError:
-            assert False, "Failed to parse response as JSON: {}".format(response.content)
-
-        assert "access_token" in responseJson, "Failed to get access_token: {}".format(response.content)
-        token = responseJson["access_token"]
-        assert token is not None, "Token not returned"
-        return token if (response.status_code == status.HTTP_200_OK) else None

e2e_tests/helpers.py

@@ -1,3 +1,5 @@
+from json import JSONDecodeError
+
 import asyncio
 from typing import Optional
 from contextlib import asynccontextmanager
@@ -101,3 +103,29 @@ async def check_aad_auth_redirect(endpoint, verify) -> None:
 
         valid_redirection_contains = ["login", "microsoftonline", "oauth2", "authorize"]
         assert all(word in location for word in valid_redirection_contains), "Redirect URL doesn't apper to be valid"
+
+
+async def get_admin_token(verify) -> str:
+    async with AsyncClient(verify=verify) as client:
+        responseJson = ""
+        headers = {'Content-Type': "application/x-www-form-urlencoded"}
+        if config.TEST_ACCOUNT_CLIENT_ID != "" and config.TEST_ACCOUNT_CLIENT_SECRET != "":
+            # Use Client Credentials flow
+            payload = f"grant_type=client_credentials&client_id={config.TEST_ACCOUNT_CLIENT_ID}&client_secret={config.TEST_ACCOUNT_CLIENT_SECRET}&scope=api://{config.API_CLIENT_ID}/.default"
+            url = f"https://login.microsoftonline.com/{config.AAD_TENANT_ID}/oauth2/v2.0/token"
+
+        else:
+            # Use Resource Owner Password Credentials flow
+            payload = f"grant_type=password&resource={config.API_CLIENT_ID}&username={config.TEST_USER_NAME}&password={config.TEST_USER_PASSWORD}&scope=api://{config.API_CLIENT_ID}/user_impersonation&client_id={config.TEST_APP_ID}"
+            url = f"https://login.microsoftonline.com/{config.AAD_TENANT_ID}/oauth2/token"
+
+        response = await client.post(url, headers=headers, content=payload)
+        try:
+            responseJson = response.json()
+        except JSONDecodeError:
+            assert False, "Failed to parse response as JSON: {}".format(response.content)
+
+        assert "access_token" in responseJson, "Failed to get access_token: {}".format(response.content)
+        token = responseJson["access_token"]
+        assert token is not None, "Token not returned"
+        return token if (response.status_code == status.HTTP_200_OK) else None

e2e_tests/resources/resource.py

@@ -67,8 +67,8 @@ async def disable_and_delete_resource(endpoint, access_token, verify):
 
 async def wait_for(func, client, operation_endpoint, headers, failure_states: list):
     done, done_state, message = await func(client, operation_endpoint, headers)
+    LOGGER.info(f'WAITING FOR OP: {operation_endpoint}')
     while not done:
-        LOGGER.info(f'WAITING FOR OP: {operation_endpoint}')
         await asyncio.sleep(30)
 
         done, done_state, message = await func(client, operation_endpoint, headers)

e2e_tests/test_airlock.py

@@ -13,6 +13,7 @@
 from airlock.request import post_request, get_request, upload_blob_using_sas, wait_for_status
 from airlock import strings as airlock_strings
 
+from helpers import get_admin_token
 
 pytestmark = pytest.mark.asyncio
 LOGGER = logging.getLogger(__name__)
@@ -23,8 +24,9 @@
 @pytest.mark.airlock
 @pytest.mark.extended
 @pytest.mark.timeout(2000)
-async def test_airlock_import_flow(admin_token, verify) -> None:
+async def test_airlock_import_flow(verify) -> None:
 
+    admin_token = await get_admin_token(verify)
     if config.TEST_AIRLOCK_WORKSPACE_ID != "":
         workspace_id = config.TEST_AIRLOCK_WORKSPACE_ID
         workspace_path = f"/workspaces/{workspace_id}"
@@ -125,4 +127,5 @@ async def test_airlock_import_flow(admin_token, verify) -> None:
     if config.TEST_AIRLOCK_WORKSPACE_ID == "":
         # 8. delete workspace
         LOGGER.info("Deleting workspace")
+        admin_token = await get_admin_token(verify)
         await disable_and_delete_resource(f'/api{workspace_path}', admin_token, verify)

e2e_tests/test_performance.py

@@ -5,12 +5,14 @@
 from resources.resource import disable_and_delete_resource, post_resource
 from resources import strings
 
+from helpers import get_admin_token
+
 pytestmark = pytest.mark.asyncio
 
 
 @pytest.mark.performance
 @pytest.mark.timeout(3000)
-async def test_parallel_resource_creations(admin_token, verify) -> None:
+async def test_parallel_resource_creations(verify) -> None:
     """Creates N workspaces in parallel, and creates a workspace service in each, in parallel"""
 
     number_workspaces = 2
@@ -28,6 +30,7 @@ async def test_parallel_resource_creations(admin_token, verify) -> None:
             }
         }
 
+        admin_token = await get_admin_token(verify)
         task = asyncio.create_task(post_resource(payload=payload, endpoint=strings.API_WORKSPACES, access_token=admin_token, verify=verify))
         tasks.append(task)
 
@@ -45,7 +48,7 @@ async def test_parallel_resource_creations(admin_token, verify) -> None:
 @pytest.mark.skip
 @pytest.mark.performance
 @pytest.mark.timeout(3000)
-async def test_bulk_updates_to_ensure_each_resource_updated_in_series(admin_token, verify) -> None:
+async def test_bulk_updates_to_ensure_each_resource_updated_in_series(verify) -> None:
     """Optionally creates a workspace and workspace service,
     then creates N number of VMs in parallel, patches each, and deletes them"""
 
@@ -69,6 +72,7 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(admin_toke
             }
         }
 
+        admin_token = await get_admin_token(verify)
         workspace_path, workspace_id = await post_resource(payload, strings.API_WORKSPACES, admin_token, verify)
     else:
         workspace_path = f"/workspaces/{config.PERF_TEST_WORKSPACE_ID}"
@@ -140,6 +144,7 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(admin_toke
 
     await asyncio.gather(*tasks)
 
+    admin_token = await get_admin_token(verify)
     # clear up workspace + service (if we created them)
     if config.PERF_TEST_WORKSPACE_SERVICE_ID == "":
         await disable_and_delete_resource(f'/api{workspace_service_path}', workspace_owner_token, verify)

e2e_tests/test_shared_service_templates.py

@@ -6,6 +6,7 @@
 import config
 from helpers import get_auth_header, get_template
 from resources import strings
+from helpers import get_admin_token
 
 shared_service_templates = [
     (strings.FIREWALL_SHARED_SERVICE),
@@ -15,8 +16,9 @@
 
 @pytest.mark.smoke
 @pytest.mark.parametrize("template_name", shared_service_templates)
-async def test_get_shared_service_templates(template_name, admin_token, verify) -> None:
+async def test_get_shared_service_templates(template_name, verify) -> None:
     async with AsyncClient(verify=verify) as client:
+        admin_token = await get_admin_token(verify)
         response = await client.get(f"https://{config.TRE_ID}.{config.RESOURCE_LOCATION}.cloudapp.azure.com{strings.API_SHARED_SERVICE_TEMPLATES}", headers=get_auth_header(admin_token))
 
         template_names = [templates["name"] for templates in response.json()["templates"]]
@@ -25,6 +27,7 @@ async def test_get_shared_service_templates(template_name, admin_token, verify)
 
 @pytest.mark.smoke
 @pytest.mark.parametrize("template_name", shared_service_templates)
-async def test_get_shared_service_template(template_name, admin_token, verify) -> None:
+async def test_get_shared_service_template(template_name, verify) -> None:
+    admin_token = await get_admin_token(verify)
     async with get_template(template_name, strings.API_SHARED_SERVICE_TEMPLATES, admin_token, verify) as response:
         assert (response.status_code == status.HTTP_200_OK), f"GET Request for {template_name} failed"

e2e_tests/test_shared_services.py

@@ -4,13 +4,13 @@
 from resources.resource import disable_and_delete_resource, post_resource
 from helpers import get_shared_service_id_by_name
 from resources import strings
-
+from helpers import get_admin_token
 
 LOGGER = logging.getLogger(__name__)
 
 
 @pytest.mark.shared_services
-async def test_patch_firewall(admin_token, verify):
+async def test_patch_firewall(verify):
     template_name = strings.FIREWALL_SHARED_SERVICE
 
     patch_payload = {
@@ -70,6 +70,7 @@ async def test_patch_firewall(admin_token, verify):
         "templateName": template_name,
     }
 
+    admin_token = await get_admin_token(verify)
     shared_service_firewall = await get_shared_service_id_by_name(
         template_name, verify, admin_token
     )
@@ -93,7 +94,8 @@ async def test_patch_firewall(admin_token, verify):
 @pytest.mark.shared_services
 @pytest.mark.timeout(65 * 60)
 @pytest.mark.parametrize("template_name", shared_service_templates_to_create)
-async def test_create_shared_service(template_name, admin_token, verify) -> None:
+async def test_create_shared_service(template_name, verify) -> None:
+    admin_token = await get_admin_token(verify)
     # Check that the shared service hasn't already been created
     shared_service = await get_shared_service_id_by_name(
         template_name, verify, admin_token
@@ -122,6 +124,7 @@ async def test_create_shared_service(template_name, admin_token, verify) -> None
         verify=verify,
     )
 
+    admin_token = await get_admin_token(verify)
     await disable_and_delete_resource(
         f"/api{shared_service_path}", admin_token, verify
     )

e2e_tests/test_workspace_service_templates.py

@@ -6,7 +6,7 @@
 import config
 from helpers import get_auth_header, get_template
 from resources import strings
-
+from helpers import get_admin_token
 
 pytestmark = pytest.mark.asyncio
 
@@ -20,8 +20,9 @@
 
 @pytest.mark.smoke
 @pytest.mark.parametrize("template_name", workspace_service_templates)
-async def test_get_workspace_service_templates(template_name, admin_token, verify) -> None:
+async def test_get_workspace_service_templates(template_name, verify) -> None:
     async with AsyncClient(verify=verify) as client:
+        admin_token = await get_admin_token(verify)
         response = await client.get(f"https://{config.TRE_ID}.{config.RESOURCE_LOCATION}.cloudapp.azure.com{strings.API_WORKSPACE_SERVICE_TEMPLATES}", headers=get_auth_header(admin_token))
 
         template_names = [templates["name"] for templates in response.json()["templates"]]
@@ -30,13 +31,14 @@ async def test_get_workspace_service_templates(template_name, admin_token, verif
 
 @pytest.mark.smoke
 @pytest.mark.parametrize("template_name", workspace_service_templates)
-async def test_get_workspace_service_template(template_name, admin_token, verify) -> None:
+async def test_get_workspace_service_template(template_name, verify) -> None:
+    admin_token = await get_admin_token(verify)
     async with get_template(template_name, strings.API_WORKSPACE_SERVICE_TEMPLATES, admin_token, verify) as response:
         assert (response.status_code == status.HTTP_200_OK), f"GET Request for {template_name} failed"
 
 
 @pytest.mark.smoke
-async def test_create_workspace_service_templates(admin_token, verify) -> None:
+async def test_create_workspace_service_templates(verify) -> None:
     async with AsyncClient(verify=verify) as client:
         payload = {
             "name": f"{strings.TEST_WORKSPACE_SERVICE_TEMPLATE}",
@@ -53,6 +55,7 @@ async def test_create_workspace_service_templates(admin_token, verify) -> None:
             }
         }
 
+        admin_token = await get_admin_token(verify)
         response = await client.post(f"https://{config.TRE_ID}.{config.RESOURCE_LOCATION}.cloudapp.azure.com{strings.API_WORKSPACE_SERVICE_TEMPLATES}", headers=get_auth_header(admin_token), json=payload)
 
         assert (response.status_code == status.HTTP_201_CREATED or response.status_code == status.HTTP_409_CONFLICT), "The workspace service template creation service returned unexpected response."

e2e_tests/test_workspace_services.py

@@ -5,14 +5,14 @@
 from resources.workspace import get_workspace_auth_details
 from resources.resource import disable_and_delete_resource, post_resource
 from resources import strings
-
+from helpers import get_admin_token
 
 pytestmark = pytest.mark.asyncio
 
 
 @pytest.mark.extended
 @pytest.mark.timeout(75 * 60)
-async def test_create_guacamole_service_into_base_workspace(admin_token, verify) -> None:
+async def test_create_guacamole_service_into_base_workspace(verify) -> None:
 
     payload = {
         "templateName": strings.BASE_WORKSPACE,
@@ -27,6 +27,7 @@ async def test_create_guacamole_service_into_base_workspace(admin_token, verify)
     if config.TEST_WORKSPACE_APP_PLAN != "":
         payload["properties"]["app_service_plan_sku"] = config.TEST_WORKSPACE_APP_PLAN
 
+    admin_token = await get_admin_token(verify)
     workspace_path, workspace_id = await post_resource(payload, strings.API_WORKSPACES, access_token=admin_token, verify=verify)
     workspace_owner_token, scope_uri = await get_workspace_auth_details(admin_token=admin_token, workspace_id=workspace_id, verify=verify)
 
@@ -67,12 +68,13 @@ async def test_create_guacamole_service_into_base_workspace(admin_token, verify)
 
     await disable_and_delete_resource(f'/api{workspace_service_path}', workspace_owner_token, verify)
 
+    admin_token = await get_admin_token(verify)
     await disable_and_delete_resource(f'/api{workspace_path}', admin_token, verify)
 
 
 @pytest.mark.extended_aad
 @pytest.mark.timeout(75 * 60)
-async def test_create_guacamole_service_into_aad_workspace(admin_token, verify) -> None:
+async def test_create_guacamole_service_into_aad_workspace(verify) -> None:
     """This test will create a Guacamole service but will create a workspace and automatically register the AAD Application"""
 
     payload = {
@@ -87,6 +89,7 @@ async def test_create_guacamole_service_into_aad_workspace(admin_token, verify)
     if config.TEST_WORKSPACE_APP_PLAN != "":
         payload["properties"]["app_service_plan_sku"] = config.TEST_WORKSPACE_APP_PLAN
 
+    admin_token = await get_admin_token(verify)
     workspace_path, workspace_id = await post_resource(payload, strings.API_WORKSPACES, access_token=admin_token, verify=verify)
     workspace_owner_token, scope_uri = await get_workspace_auth_details(admin_token=admin_token, workspace_id=workspace_id, verify=verify)
 
@@ -127,6 +130,7 @@ async def test_create_guacamole_service_into_aad_workspace(admin_token, verify)
 
     await disable_and_delete_resource(f'/api{workspace_service_path}', workspace_owner_token, verify)
 
+    admin_token = await get_admin_token(verify)
     await disable_and_delete_resource(f'/api{workspace_path}', admin_token, verify)