Use keycloak to handle auth - #540
Conversation
Deploying bats-ai with
|
| Latest commit: |
f25d3f8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://09263910.bats-ai.pages.dev |
| Branch Preview URL: | https://mln-nabat-auth.bats-ai.pages.dev |
a8afe9e to
cf70d8b
Compare
Provides a docker-compose service with a seeded NABAT realm, plus scripts to reproduce and print NABat's Keycloak redirect flow, so the auth code-exchange work can be tested without access to a real NABat-batai deployment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fsp5Rp9WquLPhQqjjTsFG8
c60edac to
aa70563
Compare
aa70563 to
99f9463
Compare
BryonLewis
left a comment
There was a problem hiding this comment.
Couple of questions and comments on my first pass.
One thing (maybe not for this PR) is maybe doing a review of all the ways we deteremine we are in 'NABat' mode. I think we have a mix of windows.location, looking at routing paths and others. I know that some can't using the routing path because it happens before the route initializes but an audit and check to align it and have a single or lower number of ways to determine the client is in NABAT mode I think would help for code readability.
Related to the above, removing the apiToken from some of the NABat components may require another way to determine if it is in nabat mode.
| ) | ||
|
|
||
| nabat_recording = NABatRecording.objects.filter(recording_id=payload.recordingId) | ||
| api_token = get_auth_header(request) |
There was a problem hiding this comment.
should we return early here if the api_token is not found? I think the task eventually knows it's missing and will fail but we know the information right here so an early return could prevent the task from ever queueing up.
There was a problem hiding this comment.
Good call. Added in.
| @@ -0,0 +1,125 @@ | |||
| { | |||
There was a problem hiding this comment.
This all looks fine, but I'm wondering if this folder should have a README.md to explain how to use it. I.E the flow of dev testing with the docker compose started up, running using the script and a simple explaination. It's in this PR but I think something persisting may help.
There was a problem hiding this comment.
Added a README. let me know if you have suggestions for ways to improve it.
| const nabatRefreshToken = ref(""); | ||
|
|
||
| axiosInstance.interceptors.request.use((config) => { | ||
| if (config.url?.startsWith("nabat") && nabatApiToken.value) { |
There was a problem hiding this comment.
could this be fragile with it being either '/nabat' or 'nabat'?
There was a problem hiding this comment.
I've updated the check to use a regex that should accommodate either case
| const nabatApiToken = ref(""); | ||
| const nabatRefreshToken = ref(""); | ||
|
|
||
| axiosInstance.interceptors.request.use((config) => { | ||
| if (config.url?.startsWith("nabat") && nabatApiToken.value) { | ||
| config.headers.Authorization = `Bearer ${nabatApiToken.value}`; | ||
| } | ||
| return config; | ||
| }); | ||
|
|
There was a problem hiding this comment.
more a questions, we have a lot of state and it's mostly related to UI elements should we move this keycloak/nabat stuff to it's own state file?
Summary of Changes
Modifies authentication/authorization with the external NABat API by moving away from a query parameter-passed API token towards obtaining a new set of (API/refresh) tokens via an external Keycloak server.
Developer Changes
In order to simulate the token exchange, a local keycloak service can be used. The new
docker-compose-keycloak.ymlfile can be chained to your existingdocker compose -f ... upcommand to spin up the service when you need it.Keycloak-specific configuration and tools live in
/dev/keycloak.NABAT-realm.jsonconfigures 2 client applications with keycloak: the BatAI application and a dummy application that is a stand-in for NABat. Theprint-nabat-auth-url.shscript can be used to generate a URL that mocks the redirect flow that NABat currently employs.New environment variables have also been set up in both
.envfiles used for development, and they have been added to the proper Django settings files.Frontend Changes
The entrypoint for NABat redirects has been changed to a new component:
NABatAuthorization.vue. This components strips the following information out of the URL:recordingId,surveyEventId,iss,code. These are all needed by the backend to reconstruct the redirect URL for a valid token exchange.Tokens are now stored in the state, and do not need to be passed as a query param for every relevant API request. The
axiosInstanceused for the client-side API now adds anAuthorization: Bearerheader to each NABat-related API call using the token stored in the current state.Backend Changes
The API token is now pulled out of the request header rather than a query parameter. A new endpoint has been added which handles the token exchange.
Out of scope
Adding automated refreshes for the API/refresh token is out of scope for this PR and will be handled by subsequent development. Behavior for token expiration (a simple warning to the user) has not been changed.