feat: Support Keycloak auth #1309
Conversation
|
@lemald Apologies for the size of this. I tried to break it up by logical pieces as much as possible. I'd be happy to walk through it over video if that would be helpful. |
| user = | ||
| id | ||
| |> get_or_create_user(email, phone_number) | ||
| |> use_email_and_phone_from_token(email, phone_number) |
There was a problem hiding this comment.
question: just so I understand the transition plan correctly - it looks like you're using the email and phone number from the SSO response to create a new user if an old user isn't present, but otherwise you don't update those fields in the DB... though I'm guessing that since you're looking up by the ID in the SSO response this would only give you new users? Or is the MBTA SSO ID specially created to bridge the old and new accounts?
There was a problem hiding this comment.
Yeah, good questions, and maybe they imply that I should add some comments or something to describe this better.
I'll start with the last part: the plan is to export all existing user accounts into Keycloak, which will include the existing IDs. So lookups should cover all existing users, whether they created their account pre- or post-Keycloak transition. The case that won't be found in the database is a user who just created their account in Keycloak and is arriving here for the first time. So those are the ones we need to write to the database.
Getting to the bigger question, we have a system where the user changes their account details (email, phone) in Keycloak, but since we need to save that data locally to be able to send notifications, Keycloak sends a message for each of those updates via SQS. T-Alerts will regularly check SQS for new messages, and apply the given change to the local database so that the local data mirrors the authoritative source in Keycloak. But there's a possible/likely scenario where a user goes to Keycloak to edit their account, and then bounces back to T-Alerts. So the data in the token reflects the latest updates, but it's likely that we haven't yet gotten the message and updated our own database, so our local data might show the old values. So we want to use the new values they just set for what we show on the page since those are accurate. But since we know that the SQS system will eventually update the values in or DB to match, we don't need to actually worry about writing to the database here.
Sorry, that's long. I'll try to put something more concise into a comment.
There was a problem hiding this comment.
Ah, okay, that may be long but it helped clarify a lot! For me the key part was remembering that the application needs to sync the email / phone information even in the post-KeyCloak world so that you're able to send notifications.
|
@arkadyan okay, sorry for the delay but I've finally managed to make it through a full pass of review! I have a few comments and questions but overall this is looking good. |
|
@arkadyan okay, looks like the only things remaining are my comment about that additional test case and the failing test in CI, once those are addressed I think I'll be ready to approve. 👍 |
config/dev.exs
Outdated
|
|
||
| config :ueberauth, Ueberauth, | ||
| providers: [ | ||
| keycloak: {Draft.Ueberauth.Strategy.FakeKeycloak, []} |
There was a problem hiding this comment.
question(non-blocking): Just curious, where is Draft. coming from, here?
There was a problem hiding this comment.
It's the old application I copy-pasted the initial setup from. You are catching everything! (Thank you)
arkadyan
force-pushed
the
mss-keycloak
branch
2 times, most recently
from
5874509
to
95b3ab3
Compare
Defaults to "local", but could also be "keycloak".
Log in/out and register via either local or Keycloak based on the AUTHENTICATION_SOURCE environment variable. Create a user account if one doesn't already exist upon successful user login. Send welcome email upon completing options step. This echos the SMS behavior.
Receive update messages from SQS using the ex_aws_sqs library. Add support for setting a different AWS region.
These were barely used and mostly tested the local auth system that's going away.
Co-authored-by: Eddie Maldonado <[email protected]>
Use a custom error message directing the user to the link above.
Use a git branch for the ueberauth_oidc project until the change is released properly.
This includes our custom URI feature.
Update for both incoming logins as well as user updates.
* fix: Retrieve the auth session before logging out The user is actually logged in for this session action, so we want to retrieve the auth credentials so that we can properly log the user out. * feat: Store the ID token and use it to log the user out This logout URL is the new structure for Keycloak version 19. --------- Co-authored-by: Erin Moore <[email protected]>
Asana task: Add Keycloak auth support
Support auth via either local database (what we've always used up to now) or Keycloak (the new hotness), configured via an environment variable.
Once we've cut all environments over to using Keycloak and migrated all of our production users, the plan is to eventually remove local auth and only support Keycloak.