Merge pull request educates#555 from GrahamDumpleton/lookup-service-r…

…esponse-codes

Fixup lookup service response codes.

lookup-service/service/routes/authnz.py

@@ -75,9 +75,9 @@ async def jwt_token_middleware(
             token = parts[1]
             decoded_token = decode_client_token(token)
         except jwt.ExpiredSignatureError:
-            return web.Response(text="JWT token has expired", status=403)
+            return web.Response(text="JWT token has expired", status=401)
         except jwt.InvalidTokenError:
-            return web.Response(text="JWT token is invalid", status=403)
+            return web.Response(text="JWT token is invalid", status=400)
 
         # Store the decoded token in the request object for later use.
 
@@ -110,10 +110,12 @@ async def wrapper(request: web.Request) -> web.Response:
         client = client_database.get_client(decoded_token["sub"])
 
         if not client:
-            return web.Response(text="Client not found", status=403)
+            return web.Response(text="Client details not found", status=401)
 
         if not client.validate_identity(decoded_token["jti"]):
-            return web.Response(text="Client identity not valid", status=403)
+            return web.Response(text="Client identity does not match", status=401)
+
+        request["remote_client"] = client
 
         # Continue processing the request.
 
@@ -130,33 +132,15 @@ def roles_accepted(
 
     def decorator(handler: Callable[..., web.Response]) -> web.Response:
         async def wrapper(request: web.Request) -> web.Response:
-            # Check if the decoded JWT token is present in the request object.
-
-            if "jwt_token" not in request:
-                return web.Response(text="JWT token not supplied", status=400)
-
-            decoded_token = request["jwt_token"]
-
-            # Lookup the client by the name of the client taken from the JWT
-            # token subject.
-
-            service_state = request.app["service_state"]
-            client_database = service_state.client_database
-
-            client_name = decoded_token["sub"]
-            client = client_database.get_client(client_name)
-
-            if not client:
-                return web.Response(text="Client not found", status=403)
-
             # Check if the client has one of the required roles.
 
+            client = request["remote_client"]
+
             matched_roles = client.has_required_role(*roles)
 
             if not matched_roles:
                 return web.Response(text="Client access not permitted", status=403)
 
-            request["remote_client"] = client
             request["client_roles"] = matched_roles
 
             # Continue processing the request.

lookup-service/service/routes/clients.py

@@ -36,7 +36,7 @@ async def api_get_v1_clients_details(request: web.Request) -> web.Response:
     client = client_database.get_client(client_name)
 
     if not client:
-        return web.Response(text="Client not available", status=403)
+        return web.Response(text="Client not available", status=404)
 
     details = {
         "name": client.name,

lookup-service/service/routes/clusters.py

@@ -37,7 +37,7 @@ async def api_get_v1_clusters_details(request: web.Request) -> web.Response:
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     details = {
         "name": cluster.name,
@@ -60,7 +60,7 @@ async def api_get_v1_clusters_kubeconfig(request: web.Request) -> web.Response:
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     kubeconfig = yaml.dump(cluster.kubeconfig)
 
@@ -80,7 +80,7 @@ async def api_get_v1_clusters_portals(request: web.Request) -> web.Response:
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     data = {
         "portals": [
@@ -116,12 +116,12 @@ async def api_get_v1_clusters_portals_details(request: web.Request) -> web.Respo
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     portal = cluster.get_portal(portal_name)
 
     if not portal:
-        return web.Response(text="Portal not available", status=403)
+        return web.Response(text="Portal not available", status=404)
 
     details = {
         "name": portal.name,
@@ -154,12 +154,12 @@ async def api_get_v1_clusters_portals_environments(
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     portal = cluster.get_portal(portal_name)
 
     if not portal:
-        return web.Response(text="Portal not available", status=403)
+        return web.Response(text="Portal not available", status=404)
 
     environments = portal.get_environments()
 
@@ -205,17 +205,17 @@ async def api_get_v1_clusters_portals_environments_details(
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     portal = cluster.get_portal(portal_name)
 
     if not portal:
-        return web.Response(text="Portal not available", status=403)
+        return web.Response(text="Portal not available", status=404)
 
     environment = portal.get_environment(environment_name)
 
     if not environment:
-        return web.Response(text="Environment not available", status=403)
+        return web.Response(text="Environment not available", status=404)
 
     details = {
         "name": environment.name,
@@ -254,17 +254,17 @@ async def api_get_v1_clusters_portals_environments_sessions(
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     portal = cluster.get_portal(portal_name)
 
     if not portal:
-        return web.Response(text="Portal not available", status=403)
+        return web.Response(text="Portal not available", status=404)
 
     environment = portal.get_environment(environment_name)
 
     if not environment:
-        return web.Response(text="Environment not available", status=403)
+        return web.Response(text="Environment not available", status=404)
 
     sessions = environment.get_sessions()
 
@@ -304,17 +304,17 @@ async def api_get_v1_clusters_portals_environments_users(
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     portal = cluster.get_portal(portal_name)
 
     if not portal:
-        return web.Response(text="Portal not available", status=403)
+        return web.Response(text="Portal not available", status=404)
 
     environment = portal.get_environment(environment_name)
 
     if not environment:
-        return web.Response(text="Environment not available", status=403)
+        return web.Response(text="Environment not available", status=404)
 
     sessions = environment.get_sessions()
 
@@ -347,17 +347,17 @@ async def api_get_v1_clusters_portals_environments_users_sessions(
     cluster = cluster_database.get_cluster(cluster_name)
 
     if not cluster:
-        return web.Response(text="Cluster not available", status=403)
+        return web.Response(text="Cluster not available", status=404)
 
     portal = cluster.get_portal(portal_name)
 
     if not portal:
-        return web.Response(text="Portal not available", status=403)
+        return web.Response(text="Portal not available", status=404)
 
     environment = portal.get_environment(environment_name)
 
     if not environment:
-        return web.Response(text="Environment not available", status=403)
+        return web.Response(text="Environment not available", status=404)
 
     sessions = environment.get_sessions()
 

lookup-service/service/routes/tenants.py

@@ -51,7 +51,7 @@ async def api_get_v1_tenants_details(request: web.Request) -> web.Response:
     tenant = tenant_database.get_tenant(tenant_name)
 
     if not tenant:
-        return web.Response(text="Tenant not available", status=403)
+        return web.Response(text="Tenant not available", status=404)
 
     details = {
         "name": tenant.name,
@@ -68,7 +68,6 @@ async def api_get_v1_tenants_portals(request: web.Request) -> web.Response:
 
     service_state = request.app["service_state"]
     tenant_database = service_state.tenant_database
-    client_database = service_state.client_database
 
     # Grab tenant name from path parameters. If the client has the tenant role
     # they can only access tenants they are mapped to.
@@ -78,18 +77,14 @@ async def api_get_v1_tenants_portals(request: web.Request) -> web.Response:
     if not tenant_name:
         return web.Response(text="Missing tenant name", status=400)
 
-    client_name = request["client_name"]
     client_roles = request["client_roles"]
 
     # Note that currently "tenant" is not within the allowed roles but leaving
     # this code here in case in future we allow access to this endpoint to
     # users with the "tenant" role.
 
     if "tenant" in client_roles:
-        client = client_database.get_client(client_name)
-
-        if not client:
-            return web.Response(text="Client not found", status=403)
+        client = request["remote_client"]
 
         if not client.allowed_access_to_tenant(tenant_name):
             return web.Response(text="Client access not permitted", status=403)
@@ -99,7 +94,7 @@ async def api_get_v1_tenants_portals(request: web.Request) -> web.Response:
     tenant = tenant_database.get_tenant(tenant_name)
 
     if not tenant:
-        return web.Response(text="Tenant not available", status=403)
+        return web.Response(text="Tenant not available", status=404)
 
     accessible_portals = tenant.portals_which_are_accessible()
 
@@ -132,7 +127,6 @@ async def api_get_v1_tenants_workshops(request: web.Request) -> web.Response:
 
     service_state = request.app["service_state"]
     tenant_database = service_state.tenant_database
-    client_database = service_state.client_database
 
     # Grab tenant name from path parameters. If the client has the tenant role
     # they can only access tenants they are mapped to.
@@ -142,14 +136,10 @@ async def api_get_v1_tenants_workshops(request: web.Request) -> web.Response:
     if not tenant_name:
         return web.Response(text="Missing tenant name", status=400)
 
-    client_name = request["client_name"]
     client_roles = request["client_roles"]
 
     if "tenant" in client_roles:
-        client = client_database.get_client(client_name)
-
-        if not client:
-            return web.Response(text="Client not found", status=403)
+        client = request["remote_client"]
 
         if not client.allowed_access_to_tenant(tenant_name):
             return web.Response(text="Client access not permitted", status=403)
@@ -159,7 +149,7 @@ async def api_get_v1_tenants_workshops(request: web.Request) -> web.Response:
     tenant = tenant_database.get_tenant(tenant_name)
 
     if not tenant:
-        return web.Response(text="Tenant not available", status=403)
+        return web.Response(text="Tenant not available", status=404)
 
     accessible_portals = tenant.portals_which_are_accessible()
 

lookup-service/service/routes/workshops.py

@@ -18,7 +18,6 @@ async def api_get_v1_workshops(request: web.Request) -> web.Response:
 
     service_state = request.app["service_state"]
     tenant_database = service_state.tenant_database
-    client_database = service_state.client_database
 
     # Get the tenant name from the query parameters. This is required when
     # the client role is "tenant".
@@ -36,21 +35,18 @@ async def api_get_v1_workshops(request: web.Request) -> web.Response:
 
             return web.Response(text="Missing tenant name", status=400)
 
-        client = client_database.get_client(client_name)
-
-        if not client:
-            return web.Response(text="Client not found", status=403)
+        client = request["remote_client"]
 
         if not client.allowed_access_to_tenant(tenant_name):
-            return web.Response(text="Client access not permitted", status=403)
+            return web.Response(text="Client not allowed access to tenant", status=403)
 
     # Work out the set of portals accessible by the specified tenant.
 
     if tenant_name:
         tenant = tenant_database.get_tenant(tenant_name)
 
         if not tenant:
-            return web.Response(text="Tenant not available", status=403)
+            return web.Response(text="Tenant not available", status=503)
 
         accessible_portals = tenant.portals_which_are_accessible()
 
@@ -144,7 +140,7 @@ async def api_post_v1_workshops(request: web.Request) -> web.Response:
     if not tenant:
         logger.error("Configuration for tenant %r could not be found", tenant_name)
 
-        return web.Response(text="Tenant not available", status=403)
+        return web.Response(text="Tenant not available", status=503)
 
     # Get the list of portals hosting the workshop and calculate the subset
     # that are accessible to the tenant.