Login in with mbta_uuid #1449
Login in with mbta_uuid #1449
Conversation
ErinLMoore
force-pushed
the
nil-error-elm
branch
4 times, most recently
from
bb1cb7f
to
871882f
Compare
ErinLMoore
force-pushed
the
nil-error-elm
branch
from
871882f
to
d2cb217
Compare
| @@ -240,6 +240,15 @@ defmodule AlertProcessor.Model.User do | |||
| @spec get(id()) :: t() | nil | |||
| def get(id), do: Repo.get(__MODULE__, id) | |||
|
|
|||
| @spec get_by_alternate_id(%{id: id() | nil, mbta_uuid: id() | nil}) :: [t()] | |||
There was a problem hiding this comment.
suggestion (non-blocking): It might be helpful to have a quick description here describing how mbta_uuid is the old-style ID for accounts from the time before SSO.
| @spec get_by_alternate_id(%{id: id() | nil, mbta_uuid: id() | nil}) :: [t()] | ||
| def get_by_alternate_id(%{id: id, mbta_uuid: mbta_uuid}) do | ||
| from(u in __MODULE__, | ||
| where: u.id in [^id, ^mbta_uuid] |
There was a problem hiding this comment.
question: I think it will be possible for a little while for id and mbta_uuid to be the same value. If that's the case, will this just do the "right" think and return a single result (if there is one)? I'm guessing that's probably the case? Would it be worth a quick test case maybe just to demonstrate the possibility and show it behaving correctly?
There was a problem hiding this comment.
I was surprised, but Repo.all() apparently won't return duplicate records, so it does behave correctly in that case. I'll add a test around it.
| conn | ||
| |> put_session("logout_uri", logout_uri) | ||
| |> SessionHelper.sign_in(user) | ||
| case user do |
There was a problem hiding this comment.
suggestion (non-blocking): If you were to move this case statement up so that it's examining the results of get_or_create_user, then you could skip a bunch of the following lines in the case where you get the result :find_user_error, and use_props_from_token wouldn't have to be changed to handle that case.
| nil -> | ||
| # The user just created their account in Keycloak so we need to add them to our database | ||
| @spec get_or_create_user( | ||
| %{id: User.id() | nil, mbta_uuid: User.id() | nil}, |
There was a problem hiding this comment.
suggestion: I don't think id can ever be nil here, right? (Which is good, because otherwise you'd have to do something about it when you go to create a new account.) This also applies to User.get_by_alternate_id.
ErinLMoore
force-pushed
the
nil-error-elm
branch
2 times, most recently
from
aa23bf1
to
6400747
Compare
ErinLMoore
force-pushed
the
nil-error-elm
branch
from
6400747
to
db0ca50
Compare
Asana ticket
Lets a user login in from keycloak if their user id matches either the
subid or thembta_uuid. Does not log in and redirects to landing page if both are somehow found.