refactor: use GDS Internal Access/i.AI Auth API for auth layer#22
Conversation
…inside the settings class
…authorisation calls
| @@ -1,50 +1,71 @@ | |||
| import { isAuthorisedUser } from '@/lib/auth' | |||
There was a problem hiding this comment.
This import isn't used anymore, and that whole lib/auth can be deleted now
| '/monitoring', | ||
| '/privacy', | ||
| '/support', |
There was a problem hiding this comment.
We don't have a monitoring page, and can you remove /privacy and /support from the public urls for now?
There was a problem hiding this comment.
I've kept these in as we discussed on Slack - somethign is hitting /monitoring, and the other pages will continue to be behind GDS Internal Access authentication. It saves us making calls to the auth api for pages that don't require it
| authorization = jwt.encode(jwt_dict, "secret", algorithm="HS256", headers=jwt_headers) | ||
| authorization: str | None = x_amzn_oidc_data | ||
|
|
||
| if not authorization: |
There was a problem hiding this comment.
This isn't working for me locally because the frontend doesn't pass a fake auth header to the backend. We need to always load the dummy data locally, and skip this check for the header
There was a problem hiding this comment.
Thanks for spotting this, I've refactored to do this check after the check to see if the backend is running in a local environment.
| environment_variables = merge(local.shared_environment_variables, { | ||
| "APP_NAME" : "${local.name}-backend" | ||
| "APP_NAME" : "${local.name}-backend", | ||
| "AUTH_API_URL" : data.aws_ssm_parameter.auth_api_invoke_url.value, |
There was a problem hiding this comment.
This needs to be added to the shared_environment_variables even though the worker doesn't use it because the Settings class is shared across both services (or you can stick a placeholder in the worker)
It's something I'd like to change in future
There was a problem hiding this comment.
Doing this means that we'll also need to pass shared_environment_variables into the frontend - currently this doesn't happen. Is there any issue with doing this?
For now I've pushed a commit up where we explicitly pass the var in as "unused" for the worker. Happy to go either way, but didn't want to add shared vars to the frontend without checking first as presumably there's a reason for that.
2 participants
DO NOT MERGE => this change incurs a small amount of downtime and significant UX change, and needs to be carried out in a controlled way.
This change migrates away from i.AI keycloak as the authentication layer, using GDS Internal Access instead.
Authorisation is provided by i.AI's Auth API, instead of inspecting the Keycloak token for claims within the code's middleware/DI layer.
Core code changes:
UserDep./api/proxyrequests are recursively invoking the middleware. @i-dot-ai/minute-admins could you validate this change in particular please? All backend routes implementUserDep, so I don't see why we need to go through the frontend auth flow for these.