Skip to content

Commit

Permalink
alternate login
Browse files Browse the repository at this point in the history
  • Loading branch information
ErinLMoore committed Jun 14, 2024
1 parent 0993057 commit c964537
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 18 deletions.
9 changes: 9 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,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()]
def get_by_alternate_id(%{id: id, mbta_uuid: mbta_uuid}) do
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
23 changes: 23 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,29 @@ 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 two users if both present (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
49 changes: 32 additions & 17 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 @@ -32,10 +32,9 @@ defmodule ConciergeSite.AuthController do
phone_number = PhoneNumber.strip_us_country_code(phone_number)

role = parse_role({:ok, userinfo})
Logger.warn("Auth Controller: #{inspect(id)}")
Logger.warn("Auth Controller: #{inspect(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)

Expand All @@ -45,9 +44,15 @@ defmodule ConciergeSite.AuthController do

{: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)

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

def callback(%{assigns: %{ueberauth_failure: failure}} = conn, _params) do
Expand All @@ -68,13 +73,14 @@ 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
Logger.warn("Inside create user: #{inspect([id, email, phone_number, role])}")
Logger.warn("Actual user id: #{inspect(User.for_email(email).id)}")
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(User.id(), String.t(), String.t() | nil, String.t()) :: User.t()
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 @@ -84,8 +90,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}")
:find_user_error
end
end

Expand All @@ -95,7 +107,10 @@ 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() | :find_user_error, String.t(), String.t() | nil, String.t()) ::
User.t() | nil
defp use_props_from_token(:find_user_error, _, _, _), do: 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: "rider2@example.com",
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 c964537

Please sign in to comment.