Change magic link flow to require copy-pasting the magic link code on mobile platforms (#1916)

Signed-off-by: Andrew Dryga <andrew@dryga.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>

elixir/apps/web/lib/web/components/form_components.ex

@@ -179,7 +179,7 @@ defmodule Web.FormComponents do
   def input(assigns) do
     ~H"""
     <div phx-feedback-for={@name}>
-      <.label for={@id}><%= @label %></.label>
+      <.label :if={not is_nil(@label)} for={@id}><%= @label %></.label>
       <input
         type={@type}
         name={@name}
@@ -306,6 +306,7 @@ defmodule Web.FormComponents do
     </.submit_button>
   """
 
+  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) %>
     </button>
     """

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.

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"""
     <section class="bg-gray-50 dark:bg-gray-900">
@@ -12,17 +38,72 @@ defmodule Web.Auth.Email do
             <h1 class="text-xl font-bold leading-tight tracking-tight text-gray-900 sm:text-2xl dark:text-white">
               Please check your email
             </h1>
-            <p>
-              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.
-            </p>
-            <p>
-              Did not receive it? <a href="?reset">Resend email</a>.
-            </p>
-            <div class="flex">
-              <.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")} />
+
+            <div :if={is_nil(@client_platform)}>
+              <p>
+                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.
+              </p>
+              <.resend
+                account_id_or_slug={@account_id_or_slug}
+                provider_id={@provider_id}
+                provider_identifier={@provider_identifier}
+                client_platform={@client_platform}
+              />
+              <div class="flex">
+                <.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" />
+              </div>
+            </div>
+            <div :if={not is_nil(@client_platform)}>
+              <p>
+                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.
+              </p>
+
+              <form
+                action={
+                  ~p"/#{@account_id_or_slug}/sign_in/providers/#{@provider_id}/verify_sign_in_token"
+                }
+                method="get"
+                class="my-4 flex"
+              >
+                <.input type="hidden" name="identity_id" value={@provider_identifier} />
+
+                <input
+                  type="text"
+                  name="secret"
+                  id="secret"
+                  class={[
+                    "block p-2.5 w-full text-sm",
+                    "bg-gray-50 text-gray-900",
+                    "rounded-l-lg border-gray-300 focus:border-primary-600 focus:ring-primary-600"
+                  ]}
+                  required
+                  placeholder="Enter token from email"
+                />
+
+                <button
+                  type="submit"
+                  class={[
+                    "block p-2.5",
+                    "text-sm text-white font-medium",
+                    "items-center text-center",
+                    "bg-primary-700 rounded-r-lg",
+                    "focus:ring-4 focus:ring-primary-200 hover:bg-primary-800"
+                  ]}
+                >
+                  Submit
+                </button>
+              </form>
+              <.resend
+                account_id_or_slug={@account_id_or_slug}
+                provider_id={@provider_id}
+                provider_identifier={@provider_identifier}
+                client_platform={@client_platform}
+              />
             </div>
           </div>
         </div>
@@ -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?
+      <button
+        type="submit"
+        class="inline font-medium text-blue-600 dark:text-blue-500 hover:underline"
+      >
+        Resend email
+      </button>
+    </.form>
+    """
+  end
+
+  defp email_provider_link(assigns) do
     ~H"""
     <a
       href={@url}
-      class="w-1/2 m-2 inline-flex items-center justify-center py-2.5 px-5 text-sm font-medium text-gray-900 focus:outline-none bg-white rounded-lg border border-gray-200 hover:bg-gray-100 hover:text-gray-900 focus:z-10 focus:ring-4 focus:ring-gray-200 dark:focus:ring-gray-700 dark:bg-gray-800 dark:text-gray-400 dark:border-gray-600 dark:hover:text-white dark:hover:bg-gray-700"
+      class={[
+        "w-1/2 m-2 inline-flex items-center justify-center py-2.5 px-5",
+        "text-sm font-medium text-gray-900 bg-white ",
+        "rounded-lg border border-gray-200",
+        "focus:outline-none focus:z-10 focus:ring-4 focus:ring-gray-200",
+        "hover:text-gray-900 hover:bg-gray-100"
+      ]}
     >
       Open <%= @name %>
     </a>
     """
   end
-
-  def mount(_params, _session, socket) do
-    {:ok, socket}
-  end
 end

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

elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex

@@ -4,12 +4,28 @@
   Dear Firezone user,
 </p>
 
-<p>
-  Here is the <a href={@link} target="_blank">magic sign-in link</a>
-  you requested. It is valid for 1 hour.
-  If you didn't request this, you can safely discard this email.
-</p>
+<div :if={is_nil(@client_platform)}>
+  <p>
+    Here is the <a href={@link} target="_blank">magic sign-in link</a>
+    you requested. It is valid for 1 hour.
+    If you didn't request this, you can safely discard this email.
+  </p>
 
-<small>
-  If the link didn't work, please copy this link and open it in your browser. <%= @link %>
-</small>
+  <small>
+    If the link didn't work, please copy this link and open it in your browser. <%= @link %>
+  </small>
+</div>
+
+<div :if={not is_nil(@client_platform)}>
+  <p>
+    Please copy the code and paste it into the Firezone application to proceed with the login:
+    <div style="font-weight:bold; margin-top:1rem; margin-bottom:1rem;">
+      <code><%= @secret %></code>
+    </div>
+    It is valid for 1 hour.
+  </p>
+
+  <p>
+    If you didn't request this, you can safely discard this email.
+  </p>
+</div>

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.

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
 

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")