Skip to content

Commit

Permalink
Review error response codes and correct as necessary.
Browse files Browse the repository at this point in the history
  • Loading branch information
GrahamDumpleton committed Aug 20, 2024
1 parent 5463def commit fbb5f99
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 64 deletions.
26 changes: 5 additions & 21 deletions lookup-service/service/routes/authnz.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,13 @@ 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=401)
return web.Response(text="Client details not found", status=401)

if not client.validate_identity(decoded_token["jti"]):
return web.Response(text="Client identity does not match", status=401)

request["remote_client"] = client

# Continue processing the request.

return await handler(request)
Expand All @@ -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=401)

# 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.
Expand Down
2 changes: 1 addition & 1 deletion lookup-service/service/routes/clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
38 changes: 19 additions & 19 deletions lookup-service/service/routes/clusters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -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": [
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down
20 changes: 5 additions & 15 deletions lookup-service/service/routes/tenants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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)
Expand All @@ -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()

Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand All @@ -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()

Expand Down
12 changes: 4 additions & 8 deletions lookup-service/service/routes/workshops.py
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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()

Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit fbb5f99

Please sign in to comment.