From 508b803d9869cef2ec2dc479e45dda3cdf26a76c Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Wed, 16 Aug 2023 15:40:22 -0600 Subject: [PATCH] Change magic link flow to require copy-pasting the magic link code on mobile platforms (#1916) Signed-off-by: Andrew Dryga Co-authored-by: Jamil --- .../web/lib/web/components/form_components.ex | 5 +- .../lib/web/controllers/auth_controller.ex | 32 ++-- elixir/apps/web/lib/web/live/auth/email.ex | 149 +++++++++++++++--- elixir/apps/web/lib/web/mailer/auth_email.ex | 6 +- .../mailer/auth_email/sign_in_link.html.heex | 32 +++- .../mailer/auth_email/sign_in_link.text.heex | 10 +- .../web/controllers/auth_controller_test.exs | 30 ++-- .../web/test/web/live/auth/email_test.exs | 3 +- 8 files changed, 212 insertions(+), 55 deletions(-) diff --git a/elixir/apps/web/lib/web/components/form_components.ex b/elixir/apps/web/lib/web/components/form_components.ex index 170bc61fa..21b037bda 100644 --- a/elixir/apps/web/lib/web/components/form_components.ex +++ b/elixir/apps/web/lib/web/components/form_components.ex @@ -179,7 +179,7 @@ defmodule Web.FormComponents do def input(assigns) do ~H"""
- <.label for={@id}><%= @label %> + <.label :if={not is_nil(@label)} for={@id}><%= @label %> """ + attr :rest, :global slot :inner_block, required: true def submit_button(assigns) do @@ -314,7 +315,7 @@ defmodule Web.FormComponents do inline-flex items-center px-5 py-2.5 mt-4 sm:mt-6 text-sm font-medium text-center text-white bg-primary-700 rounded-lg focus:ring-4 focus:ring-primary-200 dark:focus:ring-primary-900 hover:bg-primary-800 - ]}> + ]} {@rest}> <%= render_slot(@inner_block) %> """ diff --git a/elixir/apps/web/lib/web/controllers/auth_controller.ex b/elixir/apps/web/lib/web/controllers/auth_controller.ex index 185d3aa41..ab43664cb 100644 --- a/elixir/apps/web/lib/web/controllers/auth_controller.ex +++ b/elixir/apps/web/lib/web/controllers/auth_controller.ex @@ -60,14 +60,17 @@ defmodule Web.AuthController do @doc """ This is a callback for the Email provider which sends login link. """ - def request_magic_link(conn, %{ - "account_id_or_slug" => account_id_or_slug, - "provider_id" => provider_id, - "email" => - %{ - "provider_identifier" => provider_identifier - } = form - }) do + def request_magic_link( + conn, + %{ + "account_id_or_slug" => account_id_or_slug, + "provider_id" => provider_id, + "email" => + %{ + "provider_identifier" => provider_identifier + } = form + } = params + ) do _ = with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id), {:ok, identity} <- @@ -79,12 +82,23 @@ defmodule Web.AuthController do |> Web.Mailer.deliver() end + redirect_params = Map.take(form, ["client_platform", "provider_identifier"]) + conn + |> maybe_put_resent_flash(params) |> put_session(:client_platform, form["client_platform"]) |> put_session(:client_csrf_token, form["client_csrf_token"]) - |> redirect(to: "/#{account_id_or_slug}/sign_in/providers/email/#{provider_id}") + |> redirect( + to: ~p"/#{account_id_or_slug}/sign_in/providers/email/#{provider_id}?#{redirect_params}" + ) end + defp maybe_put_resent_flash(conn, %{"resend" => "true"}), + do: put_flash(conn, :info, "Email was resent.") + + defp maybe_put_resent_flash(conn, _params), + do: conn + @doc """ This is a callback for the Email provider which handles both form submission and redirect login link to authenticate a user. diff --git a/elixir/apps/web/lib/web/live/auth/email.ex b/elixir/apps/web/lib/web/live/auth/email.ex index e98cce727..932557cd6 100644 --- a/elixir/apps/web/lib/web/live/auth/email.ex +++ b/elixir/apps/web/lib/web/live/auth/email.ex @@ -1,6 +1,32 @@ defmodule Web.Auth.Email do use Web, {:live_view, layout: {Web.Layouts, :public}} + def mount( + %{ + "account_id_or_slug" => account_id_or_slug, + "provider_id" => provider_id, + "provider_identifier" => provider_identifier + } = params, + _session, + socket + ) do + form = to_form(%{"secret" => nil}) + + {:ok, socket, + temporary_assigns: [ + form: form, + provider_identifier: provider_identifier, + account_id_or_slug: account_id_or_slug, + provider_id: provider_id, + resent: params["resent"], + client_platform: params["client_platform"] + ]} + end + + def handle_info(:hide_resent_flash, socket) do + {:noreply, assign(socket, :resent, nil)} + end + def render(assigns) do ~H"""
@@ -12,17 +38,72 @@ defmodule Web.Auth.Email do

Please check your email

-

- Should the provided email be registered, a sign-in link will be dispatched to your email account. - Please click this link to proceed with your login. -

-

- Did not receive it? Resend email. -

-
- <.dev_email_provider_link url="https://mail.google.com/mail/" name="Gmail" /> - <.email_provider_link url="https://mail.google.com/mail/" name="Gmail" /> - <.email_provider_link url="https://outlook.live.com/mail/" name="Outlook" /> + <.flash flash={@flash} kind={:info} phx-click={JS.hide(transition: "fade-out")} /> + +
+

+ Should the provided email be registered, a sign-in link will be dispatched to your email account. + Please click this link to proceed with your login. +

+ <.resend + account_id_or_slug={@account_id_or_slug} + provider_id={@provider_id} + provider_identifier={@provider_identifier} + client_platform={@client_platform} + /> +
+ <.dev_email_provider_link url="https://mail.google.com/mail/" name="Gmail" /> + <.email_provider_link url="https://mail.google.com/mail/" name="Gmail" /> + <.email_provider_link url="https://outlook.live.com/mail/" name="Outlook" /> +
+
+
+

+ Should the provided email be registered, a sign-in token will be dispatched to your email account. + Please copy and paste this token into the form below to proceed with your login. +

+ +
+ <.input type="hidden" name="identity_id" value={@provider_identifier} /> + + + + +
+ <.resend + account_id_or_slug={@account_id_or_slug} + provider_id={@provider_id} + provider_identifier={@provider_identifier} + client_platform={@client_platform} + />
@@ -32,27 +113,57 @@ defmodule Web.Auth.Email do end if Mix.env() in [:dev, :test] do - def dev_email_provider_link(assigns) do + defp dev_email_provider_link(assigns) do ~H""" <.email_provider_link url={~p"/dev/mailbox"} name="Local" /> """ end else - def dev_email_provider_link(assigns), do: ~H"" + defp dev_email_provider_link(assigns), do: ~H"" end - def email_provider_link(assigns) do + defp resend(assigns) do + ~H""" + <.form + for={%{}} + as={:email} + class="inline" + action={ + ~p"/#{@account_id_or_slug}/sign_in/providers/#{@provider_id}/request_magic_link?resend=true" + } + method="post" + > + <.input type="hidden" name="email[provider_identifier]" value={@provider_identifier} /> + <.input + :if={not is_nil(@client_platform)} + type="hidden" + name="email[client_platform]" + value={@client_platform} + /> Did not receive it? + + + """ + end + + defp email_provider_link(assigns) do ~H""" Open <%= @name %> """ end - - def mount(_params, _session, socket) do - {:ok, socket} - end end diff --git a/elixir/apps/web/lib/web/mailer/auth_email.ex b/elixir/apps/web/lib/web/mailer/auth_email.ex index a39e03b51..82d83c40f 100644 --- a/elixir/apps/web/lib/web/mailer/auth_email.ex +++ b/elixir/apps/web/lib/web/mailer/auth_email.ex @@ -21,6 +21,10 @@ defmodule Web.Mailer.AuthEmail do default_email() |> subject("Firezone Sign In Link") |> to(identity.provider_identifier) - |> render_body(__MODULE__, :sign_in_link, link: sign_in_link) + |> render_body(__MODULE__, :sign_in_link, + client_platform: params["client_platform"], + secret: identity.provider_virtual_state.sign_in_token, + link: sign_in_link + ) end end diff --git a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex index 114d0116b..0ff4a0635 100644 --- a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex +++ b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex @@ -4,12 +4,28 @@ Dear Firezone user,

-

- Here is the magic sign-in link - you requested. It is valid for 1 hour. - If you didn't request this, you can safely discard this email. -

+
+

+ Here is the magic sign-in link + you requested. It is valid for 1 hour. + If you didn't request this, you can safely discard this email. +

- - If the link didn't work, please copy this link and open it in your browser. <%= @link %> - + + If the link didn't work, please copy this link and open it in your browser. <%= @link %> + +
+ +
+

+ Please copy the code and paste it into the Firezone application to proceed with the login: +

+ <%= @secret %> +
+ It is valid for 1 hour. +

+ +

+ If you didn't request this, you can safely discard this email. +

+
diff --git a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex index 690cd9d9b..8f17c480a 100644 --- a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex +++ b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex @@ -1,11 +1,15 @@ -Magic sign-in link - Dear Firezone user, - +<%= if is_nil(@client_platform) do %> Here is the magic sign-in link you requested: <%= @link %> Please copy this link and open it in your browser. It is valid for 1 hour. +<% else %> +Please copy the code and paste it into the Firezone application to proceed with the login: +<%= @secret %> + +It is valid for 1 hour. +<% end %> If you didn't request this, you can safely discard this email. diff --git a/elixir/apps/web/test/web/controllers/auth_controller_test.exs b/elixir/apps/web/test/web/controllers/auth_controller_test.exs index 49d547ae0..5327e0a79 100644 --- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs +++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs @@ -341,7 +341,9 @@ defmodule Web.AuthControllerTest do assert email.text_body =~ "secret=" end) - assert redirected_to(conn) == "/#{account.id}/sign_in/providers/email/#{provider.id}" + assert redirected_to(conn) == + "/#{account.id}/sign_in/providers/email/#{provider.id}?" <> + "provider_identifier=#{URI.encode_www_form(identity.provider_identifier)}" end test "persists client platform name", %{conn: conn} do @@ -363,17 +365,18 @@ defmodule Web.AuthControllerTest do assert_email_sent(fn email -> assert email.subject == "Firezone Sign In Link" - - verify_sign_in_token_path = - "/#{account.id}/sign_in/providers/#{provider.id}/verify_sign_in_token" - - assert email.text_body =~ "#{verify_sign_in_token_path}" - assert email.text_body =~ "identity_id=#{identity.id}" - assert email.text_body =~ "secret=" - assert email.text_body =~ "client_platform=platform" + assert email.text_body =~ "Please copy the code and paste it into" end) - assert redirected_to(conn) == "/#{account.id}/sign_in/providers/email/#{provider.id}" + assert url = redirected_to(conn) + uri = URI.parse(url) + assert uri.path == "/#{account.id}/sign_in/providers/email/#{provider.id}" + + assert URI.decode_query(uri.query) == %{ + "client_platform" => "platform", + "provider_identifier" => identity.provider_identifier + } + assert get_session(conn, :client_platform) == "platform" end @@ -388,7 +391,9 @@ defmodule Web.AuthControllerTest do %{"email" => %{"provider_identifier" => "foo"}} ) - assert redirected_to(conn) == "/#{account.id}/sign_in/providers/email/#{provider_id}" + assert redirected_to(conn) == + "/#{account.id}/sign_in/providers/email/#{provider_id}?" <> + "provider_identifier=foo" end test "does not return error if identity is not found", %{conn: conn} do @@ -402,7 +407,8 @@ defmodule Web.AuthControllerTest do %{"email" => %{"provider_identifier" => "foo"}} ) - assert redirected_to(conn) == "/#{account.id}/sign_in/providers/email/#{provider.id}" + assert redirected_to(conn) == + "/#{account.id}/sign_in/providers/email/#{provider.id}?provider_identifier=foo" end end diff --git a/elixir/apps/web/test/web/live/auth/email_test.exs b/elixir/apps/web/test/web/live/auth/email_test.exs index c28884f2b..5c07894e5 100644 --- a/elixir/apps/web/test/web/live/auth/email_test.exs +++ b/elixir/apps/web/test/web/live/auth/email_test.exs @@ -8,7 +8,8 @@ defmodule Web.Auth.EmailTest do account = AccountsFixtures.create_account() provider = AuthFixtures.create_email_provider(account: account) - {:ok, lv, html} = live(conn, ~p"/#{account}/sign_in/providers/email/#{provider}") + {:ok, lv, html} = + live(conn, ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo") assert html =~ "Please check your email" assert has_element?(lv, ~s|a[href="https://mail.google.com/mail/"]|, "Open Gmail")