Skip to content

Commit

Permalink
Login in with mbta_uuid (#1449)
Browse files Browse the repository at this point in the history
* Alternate login with mbta_uuid
  • Loading branch information
ErinLMoore authored Jun 17, 2024
1 parent e5bd81a commit 4b64a8e
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 15 deletions.
12 changes: 12 additions & 0 deletions apps/alert_processor/lib/model/user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,18 @@ 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(), mbta_uuid: id() | nil}) :: [t()]
def get_by_alternate_id(%{id: id, mbta_uuid: mbta_uuid}) do
# can get accounts using id or mbta_uuid
# which is the old-style ID for accounts from the time before SSO.

from(u in __MODULE__,
where: u.id in [^id, ^mbta_uuid]
)
|> Repo.all()
|> Enum.filter(&(!is_nil(&1)))
end

@spec for_email(String.t()) :: t | nil
def for_email(email) do
email =
Expand Down
28 changes: 28 additions & 0 deletions apps/alert_processor/test/alert_processor/model/user_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,34 @@ defmodule AlertProcessor.Model.UserTest do
end
end

describe "get_by_alternate_id/1" do
test "returns a user by id if present" do
user = insert(:user)
assert User.get_by_alternate_id(%{id: user.id, mbta_uuid: nil}) == [user]
end

test "returns a user by mbta_uuid if present" do
user = insert(:user)
assert User.get_by_alternate_id(%{id: nil, mbta_uuid: user.id}) == [user]
end

test "returns a user if both id's are the same " do
user = insert(:user)
assert User.get_by_alternate_id(%{id: user.id, mbta_uuid: user.id}) == [user]
end

test "returns two users if both ids present and different (unlikely)" do
user = insert(:user)
user2 = insert(:user)
assert User.get_by_alternate_id(%{id: user.id, mbta_uuid: user2.id}) == [user, user2]
end

test "returns empty_list if no matching user" do
bad_id = UUID.uuid4()
assert User.get_by_alternate_id(%{id: bad_id, mbta_uuid: nil}) == []
end
end

describe "for_email/1" do
test "returns a user if present" do
user = insert(:user)
Expand Down
56 changes: 42 additions & 14 deletions apps/concierge_site/lib/controllers/auth_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ defmodule ConciergeSite.AuthController do
extra: %{
raw_info: %{
claims: %{"sub" => id},
userinfo: userinfo
userinfo: %{"mbta_uuid" => mbta_uuid} = userinfo
}
}
} = auth
Expand All @@ -34,19 +34,26 @@ defmodule ConciergeSite.AuthController do
role = parse_role({:ok, userinfo})

user =
id
%{id: id, mbta_uuid: mbta_uuid}
|> get_or_create_user(email, phone_number, role)
|> use_props_from_token(email, phone_number, role)

logout_params = %{
post_logout_redirect_uri: page_url(conn, :landing)
}

{:ok, logout_uri} = UeberauthOidcc.initiate_logout_url(auth, logout_params)

conn
|> put_session("logout_uri", logout_uri)
|> SessionHelper.sign_in(user)
case user do
nil ->
SessionHelper.sign_out(conn, skip_oidc_sign_out: true)

_ ->
user = use_props_from_token(user, email, phone_number, role)

conn
|> put_session("logout_uri", logout_uri)
|> SessionHelper.sign_in(user)
end
end

def callback(%{assigns: %{ueberauth_failure: failure}} = conn, _params) do
Expand All @@ -67,11 +74,20 @@ defmodule ConciergeSite.AuthController do
SessionHelper.sign_out(conn)
end

@spec get_or_create_user(User.id(), String.t(), String.t() | nil, String.t()) :: User.t()
defp get_or_create_user(id, email, phone_number, role) do
case User.get(id) do
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(), mbta_uuid: User.id() | nil},
String.t(),
String.t() | nil,
String.t()
) ::
User.t() | nil
defp get_or_create_user(%{id: id, mbta_uuid: mbta_uuid} = id_map, email, phone_number, role) do
# This checks both the normal id from Keycloak, and the legacy mbta_uuid. We should get either 0 or 1 users back.
user_list = User.get_by_alternate_id(id_map)

case length(user_list) do
0 ->
# If neither ID is found, the user just created their account in Keycloak so we need to add them to our database
Repo.insert!(%User{
id: id,
email: email,
Expand All @@ -81,8 +97,14 @@ defmodule ConciergeSite.AuthController do

User.get(id)

user ->
user
1 ->
# If 1 user is found, we want to return that user
hd(user_list)

2 ->
# If 2 users are found, something weird happened. Log and return nil. User will be redirected to landing page.
Logger.warn("User with 2 ids found. sub id: #{id}, mbta_uuid: #{mbta_uuid}")
nil
end
end

Expand All @@ -92,7 +114,13 @@ defmodule ConciergeSite.AuthController do
# able to use them to send notifications), but in cases where the user just
# changed one of these fields in Keycloak, we might not have had time receive
# that message yet, so the values in the token are more authoritative.
@spec use_props_from_token(User.t(), String.t(), String.t() | nil, String.t()) :: User.t()
@spec use_props_from_token(
User.t(),
String.t(),
String.t() | nil,
String.t()
) ::
User.t() | nil
defp use_props_from_token(user, email, phone_number, role) do
%User{
user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,40 @@ defmodule ConciergeSite.Web.AuthControllerTest do
} = Guardian.Plug.current_resource(conn)
end

test "can use mbta_uuid user",
%{conn: conn, rider: %User{id: id, email: email, phone_number: phone_number}} do
auth = auth_for(nil, email, phone_number, ["user"], id)

conn =
conn
|> assign(:ueberauth_auth, auth)
|> get("/auth/keycloak/callback")

assert %User{id: ^id, email: ^email, phone_number: ^phone_number} =
Guardian.Plug.current_resource(conn)
end

@tag capture_log: true
test "redirects if we somehow get 2 users",
%{conn: conn, rider: %User{id: id, email: email, phone_number: phone_number}} do
user2 =
Repo.insert!(%User{
email: "[email protected]",
phone_number: "5551234567",
role: "user"
})

auth = auth_for(id, email, phone_number, ["user"], user2.id)

conn =
conn
|> assign(:ueberauth_auth, auth)
|> get("/auth/keycloak/callback")

assert is_nil(Guardian.Plug.current_resource(conn))
assert redirected_to(conn) == "/"
end

test "doesn't allow admin access if the token says they are now just a user",
%{conn: conn} do
was_an_admin =
Expand Down Expand Up @@ -110,7 +144,7 @@ defmodule ConciergeSite.Web.AuthControllerTest do
end

@spec auth_for(User.id(), String.t(), String.t() | nil) :: Auth.t()
defp auth_for(id, email, phone_number, roles \\ ["user"]) do
defp auth_for(id, email, phone_number, roles \\ ["user"], mbta_uuid \\ nil) do
%Auth{
uid: email,
provider: :keycloak,
Expand Down Expand Up @@ -147,6 +181,7 @@ defmodule ConciergeSite.Web.AuthControllerTest do
"name" => "John Rider",
"phone_number" => phone_number,
"preferred_username" => email,
"mbta_uuid" => mbta_uuid,
"resource_access" => %{
"t-alerts" => %{
"roles" => roles
Expand Down

0 comments on commit 4b64a8e

Please sign in to comment.