diff --git a/lib/bike_brigade/authentication_messenger.ex b/lib/bike_brigade/authentication_messenger.ex index 1666315a..16814974 100644 --- a/lib/bike_brigade/authentication_messenger.ex +++ b/lib/bike_brigade/authentication_messenger.ex @@ -29,6 +29,20 @@ defmodule BikeBrigade.AuthenticationMessenger do GenServer.call(pid, {:validate_token, phone, token_attempt}) end + def clear_token(phone), do: clear_token(@name, phone) + + def clear_token(pid, phone) do + GenServer.cast(pid, {:clear, phone}) + end + + if Mix.env() == :test do + def get_state(), do: get_state(@name) + + def get_state(pid) do + GenServer.call(pid, :get_state) + end + end + # Server (callbacks) @impl GenServer @@ -53,7 +67,7 @@ defmodule BikeBrigade.AuthenticationMessenger do def handle_call({:validate_token, phone, token_attempt}, _from, state) do # TODO refactor - if !dev?() do + if dev?() do unless Map.has_key?(state, phone) do {:reply, {:error, :token_expired}, state} else @@ -68,16 +82,29 @@ defmodule BikeBrigade.AuthenticationMessenger do end end + if Mix.env() == :test do + @impl GenServer + def handle_call(:get_state, _from, state) do + {:reply, state, state} + end + end + @impl GenServer def handle_info({:expire, phone}, state) do {:noreply, Map.delete(state, phone)} end + @impl GenServer + def handle_cast({:clear, phone}, state) do + {:noreply, Map.delete(state, phone)} + end + defp send_message(phone, token) do msg = [ from: Messaging.outbound_number(), to: phone, - body: "Your BikeBrigade access code is #{token}.\n\n@#{BikeBrigadeWeb.Endpoint.host()} ##{token}" + body: + "Your BikeBrigade access code is #{token}.\n\n@#{BikeBrigadeWeb.Endpoint.host()} ##{token}" ] SmsService.send_sms(msg) diff --git a/lib/bike_brigade_web/controllers/authentication.ex b/lib/bike_brigade_web/controllers/authentication_controller.ex similarity index 53% rename from lib/bike_brigade_web/controllers/authentication.ex rename to lib/bike_brigade_web/controllers/authentication_controller.ex index ffb149db..2d2980b8 100644 --- a/lib/bike_brigade_web/controllers/authentication.ex +++ b/lib/bike_brigade_web/controllers/authentication_controller.ex @@ -1,4 +1,4 @@ -defmodule BikeBrigadeWeb.Authentication do +defmodule BikeBrigadeWeb.AuthenticationController do use BikeBrigadeWeb, :controller import Plug.Conn @@ -6,7 +6,59 @@ defmodule BikeBrigadeWeb.Authentication do alias BikeBrigade.Accounts alias BikeBrigade.AuthenticationMessenger - def login(conn, %{"login" => %{"phone" => phone, "token_attempt" => token_attempt}}) do + defmodule Login do + use BikeBrigade.Schema + import Ecto.Changeset + + alias BikeBrigade.EctoPhoneNumber + + @primary_key false + embedded_schema do + field :phone, EctoPhoneNumber.Canadian + field :token_attempt, :string + end + + def validate_phone(attrs) do + %Login{} + |> cast(attrs, [:phone]) + |> validate_required([:phone]) + |> validate_user_exists(:phone) + |> Ecto.Changeset.apply_action(:insert) + end + + defp validate_user_exists(changeset, field) when is_atom(field) do + validate_change(changeset, field, fn _, phone -> + case Accounts.get_user_by_phone(phone) do + nil -> [{field, "We can't find your number. Have you signed up for Bike Brigade?"}] + _ -> [] + end + end) + end + end + + def show(conn, %{"login" => attrs}) do + case Login.validate_phone(attrs) do + {:ok, login} -> + changeset = Ecto.Changeset.change(login) + + conn + |> render("show.html", state: :token, changeset: changeset, layout: false) + + {:error, %Ecto.Changeset{} = changeset} -> + conn + |> render("show.html", state: :phone, changeset: changeset, layout: false) + end + end + + def show(conn, _params) do + changeset = Ecto.Changeset.change(%Login{}) + + conn + |> render("show.html", state: :phone, changeset: changeset, layout: false) + end + + def login(conn, %{"login" => %{"phone" => phone, "token_attempt" => token_attempt}}) + when not is_nil(token_attempt) do case AuthenticationMessenger.validate_token(phone, token_attempt) do :ok -> user = Accounts.get_user_by_phone(phone) @@ -24,10 +76,36 @@ defmodule BikeBrigadeWeb.Authentication do {:error, :token_invalid} -> conn |> put_flash(:error, "Access code is invalid. Please try again.") - |> redirect(to: ~p"/login?#{%{phone: phone}}") + |> redirect(to: ~p"/login?#{%{login: %{phone: phone}}}") end end + def login(conn, %{"login" => %{"phone" => phone}}) do + with {:ok, login} <- Login.validate_phone(%{"phone" => phone}), + :ok <- AuthenticationMessenger.generate_token(login.phone) do + changeset = Ecto.Changeset.change(login) + + conn + |> render("show.html", state: :token, changeset: changeset, layout: false) + else + {:error, %Ecto.Changeset{} = changeset} -> + conn + |> render("show.html", state: :phone, changeset: changeset, layout: false) + + {:error, err} -> + conn + |> put_flash(:error, err) + |> redirect(to: ~p"/login") + end + end + + def cancel(conn, %{"phone" => phone}) do + AuthenticationMessenger.clear_token(phone) + + conn + |> redirect(to: ~p"/login") + end + @doc "Set the session token and live socket for the user" def do_login(conn, user) do conn diff --git a/lib/bike_brigade_web/controllers/authentication_html.ex b/lib/bike_brigade_web/controllers/authentication_html.ex new file mode 100644 index 00000000..c2dc716c --- /dev/null +++ b/lib/bike_brigade_web/controllers/authentication_html.ex @@ -0,0 +1,6 @@ +defmodule BikeBrigadeWeb.AuthenticationHTML do + use BikeBrigadeWeb, :html + + embed_templates "authentication_html/*" + +end diff --git a/lib/bike_brigade_web/live/login_live.html.heex b/lib/bike_brigade_web/controllers/authentication_html/show.html.heex similarity index 86% rename from lib/bike_brigade_web/live/login_live.html.heex rename to lib/bike_brigade_web/controllers/authentication_html/show.html.heex index 7cc45c6f..ab0dde06 100644 --- a/lib/bike_brigade_web/live/login_live.html.heex +++ b/lib/bike_brigade_web/controllers/authentication_html/show.html.heex @@ -1,3 +1,7 @@ +<.flash kind={:info} title="Success!" flash={@flash} /> +<.flash kind={:warn} title="Alert!" flash={@flash} /> +<.flash kind={:error} title="Error!" flash={@flash} /> +
@@ -18,7 +22,7 @@ :if={@state == :phone} for={@changeset} id="login-form" - phx-submit="submit_phone" + action={~p"/login"} > <.input type="tel" field={{f, :phone}} label="Phone Number" /> @@ -78,6 +82,14 @@ pattern="\d{6}" /> <:actions> + <.button + color={:white} + class="w-full" + href={~p"/login?phone=#{@changeset.data.phone}"} + method="delete" + > + Cancel + <.button type="submit" class="w-full">Sign in diff --git a/lib/bike_brigade_web/core_components.ex b/lib/bike_brigade_web/core_components.ex index 3511b263..10d10793 100644 --- a/lib/bike_brigade_web/core_components.ex +++ b/lib/bike_brigade_web/core_components.ex @@ -37,7 +37,7 @@ defmodule BikeBrigadeWeb.CoreComponents do values: [:none, :small, :normal, :medium, :full] attr :class, :string, default: nil - attr :rest, :global, include: ~w(href patch navigate disabled replace) + attr :rest, :global, include: ~w(href patch navigate disabled replace method) slot :inner_block, required: true @button_base_classes [ diff --git a/lib/bike_brigade_web/live/login_live.ex b/lib/bike_brigade_web/live/login_live.ex deleted file mode 100644 index 3a4e51df..00000000 --- a/lib/bike_brigade_web/live/login_live.ex +++ /dev/null @@ -1,75 +0,0 @@ -defmodule BikeBrigadeWeb.LoginLive do - use BikeBrigadeWeb, {:live_view, layout: :public} - use Phoenix.HTML - - alias BikeBrigade.AuthenticationMessenger - alias BikeBrigade.Accounts - - defmodule Login do - use BikeBrigade.Schema - import Ecto.Changeset - - alias BikeBrigade.EctoPhoneNumber - - @primary_key false - embedded_schema do - field :phone, EctoPhoneNumber.Canadian - field :token_attempt, :string - end - - def validate_phone(attrs) do - %Login{} - |> cast(attrs, [:phone]) - |> validate_required([:phone]) - |> validate_user_exists(:phone) - |> Ecto.Changeset.apply_action(:insert) - end - - defp validate_user_exists(changeset, field) when is_atom(field) do - validate_change(changeset, field, fn _, phone -> - case Accounts.get_user_by_phone(phone) do - nil -> [{field, "We can't find your number. Have you signed up for Bike Brigade?"}] - _ -> [] - end - end) - end - end - - @impl Phoenix.LiveView - def mount(_params, _session, socket) do - {:ok, assign(socket, page_title: "Login")} - end - - @impl Phoenix.LiveView - def handle_params(%{"phone" => phone}, _uri, socket) do - {:noreply, - socket - |> assign(:state, :token) - |> assign(:changeset, Ecto.Changeset.change(%Login{phone: phone}))} - end - - @impl Phoenix.LiveView - def handle_params(_params, _uri, socket) do - {:noreply, - socket - |> assign(:state, :phone) - |> assign(:changeset, Ecto.Changeset.change(%Login{}))} - end - - @impl Phoenix.LiveView - def handle_event("submit_phone", %{"login" => attrs}, socket) do - with {:ok, login} <- Login.validate_phone(attrs), - :ok <- AuthenticationMessenger.generate_token(login.phone) do - {:noreply, assign(socket, state: :token, changeset: Ecto.Changeset.change(login))} - else - {:error, %Ecto.Changeset{} = changeset} -> - {:noreply, assign(socket, changeset: changeset)} - - {:error, err} -> - {:noreply, - socket - |> put_flash(:error, err) - |> push_patch(to: ~p"/login")} - end - end -end diff --git a/lib/bike_brigade_web/router.ex b/lib/bike_brigade_web/router.ex index cdbcf075..d968c534 100644 --- a/lib/bike_brigade_web/router.ex +++ b/lib/bike_brigade_web/router.ex @@ -12,7 +12,7 @@ defmodule BikeBrigadeWeb.Router do def handle_errors(_conn, %{reason: %BikeBrigadeWeb.DeliveryExpiredError{}}), do: :ok def handle_errors(conn, err), do: super(conn, err) - import BikeBrigadeWeb.Authentication, + import BikeBrigadeWeb.AuthenticationController, only: [ require_authenticated_user: 2, require_dispatcher: 2, @@ -82,8 +82,9 @@ defmodule BikeBrigadeWeb.Router do scope "/", BikeBrigadeWeb do pipe_through [:browser, :redirect_if_user_is_authenticated] - live "/login", LoginLive, :index - post "/login", Authentication, :login + get "/login", AuthenticationController, :show + post "/login", AuthenticationController, :login + delete "/login", AuthenticationController, :cancel end # this scope is for authenticated users that aren't dispatchers (ie: riders). @@ -101,7 +102,7 @@ defmodule BikeBrigadeWeb.Router do live "/leaderboard", StatsLive.Leaderboard, :show end - delete "/logout", Authentication, :logout + delete "/logout", AuthenticationController, :logout end scope "/", BikeBrigadeWeb do diff --git a/test/bike_brigade_web/controllers/authentication_controller_test.exs b/test/bike_brigade_web/controllers/authentication_controller_test.exs new file mode 100644 index 00000000..eba8c9a3 --- /dev/null +++ b/test/bike_brigade_web/controllers/authentication_controller_test.exs @@ -0,0 +1,66 @@ +defmodule BikeBrigadeWeb.AuthenticationControllerTest do + use BikeBrigadeWeb.ConnCase + alias BikeBrigade.{Accounts, AuthenticationMessenger} + + describe "login" do + setup do + rider = fixture(:rider) + {:ok, user} = Accounts.create_user_for_rider(rider) + + %{user: user} + end + + test "shows the login page", %{conn: conn} do + conn = get(conn, ~p"/login") + assert html_response(conn, 200) =~ "Sign into your Bike Brigade account" + end + + test "errors when you send an invalid number", %{conn: conn} do + conn = post(conn, ~p"/login", %{login: %{phone: "5555555555555555"}}) + assert html_response(conn, 200) =~ "phone number is not valid for Canada" + end + + test "errors when you send a number that doesn't exist", %{conn: conn} do + conn = post(conn, ~p"/login", %{login: %{phone: "6475555555"}}) + + assert html_response(conn, 200) =~ + "We can't find your number. Have you signed up for Bike Brigade?" + end + + test "shows the token page when you give a valid number", %{conn: conn, user: user} do + conn = post(conn, ~p"/login", %{login: %{phone: user.phone}}) + assert html_response(conn, 200) =~ "We sent an authentication code to your phone number" + end + + test "errors when you send an invalid token", %{conn: conn, user: user} do + conn = post(conn, ~p"/login", %{login: %{phone: user.phone}}) + + token = Map.get(BikeBrigade.AuthenticationMessenger.get_state(), user.phone) + conn = post(conn, ~p"/login", %{login: %{phone: user.phone, token_attempt: "not a token"}}) + + assert redirected_to(conn) == ~p"/login?login[phone]=#{user.phone}" + conn = get(conn, ~p"/login?login[phone]=#{user.phone}") + assert html_response(conn, 200) =~ "Access code is invalid. Please try again." + + # Token is not regenerated after error + assert Map.get(BikeBrigade.AuthenticationMessenger.get_state(), user.phone) == token + end + + test "logs you in with valid token", %{conn: conn, user: user} do + conn = post(conn, ~p"/login", %{login: %{phone: user.phone}}) + token = Map.get(BikeBrigade.AuthenticationMessenger.get_state(), user.phone) + conn = post(conn, ~p"/login", %{login: %{phone: user.phone, token_attempt: token}}) + assert redirected_to(conn) == ~p"/" + + conn = get(conn, ~p"/home") + assert html_response(conn, 200) =~ "Welcome!" + end + + test "lets you cancel a login", %{conn: conn, user: user} do + conn = post(conn, ~p"/login", %{login: %{phone: user.phone}}) + conn = delete(conn, ~p"/login", %{phone: user.phone}) + assert redirected_to(conn) == ~p"/login" + assert Map.get(BikeBrigade.AuthenticationMessenger.get_state(), user.phone) == nil + end + end +end diff --git a/test/bike_brigade_web/live/login_live_test.exs b/test/bike_brigade_web/live/login_live_test.exs deleted file mode 100644 index f8f3513d..00000000 --- a/test/bike_brigade_web/live/login_live_test.exs +++ /dev/null @@ -1,95 +0,0 @@ -defmodule BikeBrigadeWeb.LoginLiveTest do - use BikeBrigadeWeb.ConnCase - - alias BikeBrigade.SmsService.FakeSmsService - - import Phoenix.LiveViewTest - - describe "Log in page" do - test "renders log in page", %{conn: conn} do - {:ok, _lv, html} = live(conn, ~p"/login") - - assert html =~ "Sign into your Bike Brigade account" - assert html =~ "Sign Up!" - assert html =~ "Need help? Email" - end - - test "redirects if already logged in", %{conn: conn} do - result = - conn - |> login_user(fixture(:user)) - |> live(~p"/login") - |> follow_redirect(conn, "/") - - assert {:ok, _conn} = result - end - end - - describe "user login" do - test "shows the login page with an error if the phone is unregistered", %{ - conn: conn - } do - {:ok, lv, _html} = live(conn, ~p"/login") - - result = - lv - |> form("#login-form", login: %{phone: "+16475555555"}) - |> render_submit() - - assert result =~ "We can't find your number. Have you signed up for Bike Brigade?" - end - - test "redirects to the login page with a flash error if the token is invalid", %{ - conn: conn - } do - user = fixture(:user) - {:ok, lv, _html} = live(conn, ~p"/login") - - result = - lv - |> form("#login-form", login: %{phone: user.phone}) - |> render_submit() - - assert result =~ "We sent an authentication code to your phone number" - - form = form(lv, "#login-form2", login: %{phone: user.phone, token_attempt: "not a token"}) - conn = submit_form(form, conn) - - assert Phoenix.Flash.get(conn.assigns.flash, :error) == - "Access code is invalid. Please try again." - - assert redirected_to(conn) == ~p"/login?phone=#{user.phone}" - end - - test "redirects if user logins with valid credentials", %{conn: conn} do - user = fixture(:user) - {:ok, lv, _html} = live(conn, ~p"/login") - - result = - lv - |> form("#login-form", login: %{phone: user.phone}) - |> render_submit() - - assert result =~ "We sent an authentication code to your phone number" - - form = - form(lv, "#login-form2", - login: %{phone: user.phone, token_attempt: get_authentication_code()} - ) - - conn = submit_form(form, conn) - - assert redirected_to(conn) == ~p"/" - end - end - - def get_authentication_code() do - message = - FakeSmsService.last_message() - |> Keyword.fetch!(:body) - - [_, authentication_code] = Regex.run(~r[Your BikeBrigade access code is (\d+)\.], message) - - authentication_code - end -end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 255c79d3..3f530e98 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -103,6 +103,6 @@ defmodule BikeBrigadeWeb.ConnCase do def login_user(conn, user) do conn |> Phoenix.ConnTest.init_test_session(%{}) - |> BikeBrigadeWeb.Authentication.do_login(user) + |> BikeBrigadeWeb.AuthenticationController.do_login(user) end end