Fix lost client_* state on magic link resend (#2196)

Closes https://github.com/firezone/firezone/issues/2012

elixir/apps/web/lib/web/live/auth/email.ex

@@ -7,7 +7,7 @@ defmodule Web.Auth.Email do
           "provider_id" => provider_id,
           "provider_identifier" => provider_identifier
         } = params,
-        _session,
+        session,
         socket
       ) do
     form = to_form(%{"secret" => nil})
@@ -19,7 +19,8 @@ defmodule Web.Auth.Email do
        account_id_or_slug: account_id_or_slug,
        provider_id: provider_id,
        resent: params["resent"],
-       client_platform: params["client_platform"]
+       client_platform: session["client_platform"] || params["client_platform"],
+       client_csrf_token: session["client_csrf_token"] || params["client_csrf_token"]
      ]}
   end
 
@@ -50,6 +51,7 @@ defmodule Web.Auth.Email do
                 provider_id={@provider_id}
                 provider_identifier={@provider_identifier}
                 client_platform={@client_platform}
+                client_csrf_token={@client_csrf_token}
               />
               <div class="flex">
                 <.dev_email_provider_link url="https://mail.google.com/mail/" name="Gmail" />
@@ -64,6 +66,7 @@ defmodule Web.Auth.Email do
               </p>
 
               <form
+                id="verify-sign-in-token"
                 action={
                   ~p"/#{@account_id_or_slug}/sign_in/providers/#{@provider_id}/verify_sign_in_token"
                 }
@@ -103,6 +106,7 @@ defmodule Web.Auth.Email do
                 provider_id={@provider_id}
                 provider_identifier={@provider_identifier}
                 client_platform={@client_platform}
+                client_csrf_token={@client_csrf_token}
               />
             </div>
           </div>
@@ -126,6 +130,7 @@ defmodule Web.Auth.Email do
     ~H"""
     <.form
       for={%{}}
+      id="resend-email"
       as={:email}
       class="inline"
       action={
@@ -137,15 +142,24 @@ defmodule Web.Auth.Email do
       <.input
         :if={not is_nil(@client_platform)}
         type="hidden"
-        name="email[client_platform]"
+        name="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>
+      />
+      <.input
+        :if={not is_nil(@client_csrf_token)}
+        type="hidden"
+        name="client_csrf_token"
+        value={@client_csrf_token}
+      />
+      <span>
+        Did not receive it?
+        <button
+          type="submit"
+          class="inline font-medium text-blue-600 dark:text-blue-500 hover:underline"
+        >
+          Resend email
+        </button>
+      </span>
     </.form>
     """
   end

elixir/apps/web/test/web/controllers/auth_controller_test.exs

@@ -1292,9 +1292,9 @@ defmodule Web.AuthControllerTest do
   end
 
   defp split_token(identity, size \\ 5) do
-    <<email_secret::binary-size(size), browser_secret::binary>> =
+    <<email_secret::binary-size(size), sign_in_nonce::binary>> =
       identity.provider_virtual_state.sign_in_token
 
-    {email_secret, browser_secret}
+    {email_secret, sign_in_nonce}
   end
 end

elixir/apps/web/test/web/live/auth/email_test.exs

@@ -1,12 +1,30 @@
 defmodule Web.Auth.EmailTest do
   use Web.ConnCase, async: true
 
-  test "renders email page", %{conn: conn} do
+  setup do
     Domain.Config.put_system_env_override(:outbound_email_adapter, Swoosh.Adapters.Postmark)
 
     account = Fixtures.Accounts.create_account()
     provider = Fixtures.Auth.create_email_provider(account: account)
+    actor = Fixtures.Actors.create_actor(account: account, type: :account_user)
 
+    {:ok, identity} =
+      Fixtures.Auth.create_identity(account: account, actor: actor, provider: provider)
+      |> Domain.Auth.Adapters.Email.request_sign_in_token()
+
+    %{
+      account: account,
+      provider: provider,
+      actor: actor,
+      identity: identity
+    }
+  end
+
+  test "renders delivery confirmation page for browser users", %{
+    account: account,
+    provider: provider,
+    conn: conn
+  } do
     {:ok, lv, html} =
       live(conn, ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo")
 
@@ -14,4 +32,160 @@ defmodule Web.Auth.EmailTest do
     assert has_element?(lv, ~s|a[href="https://mail.google.com/mail/"]|, "Open Gmail")
     assert has_element?(lv, ~s|a[href="https://outlook.live.com/mail/"]|, "Open Outlook")
   end
+
+  test "renders token form for Apple users", %{
+    account: account,
+    provider: provider,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, lv, _html} =
+      live(
+        conn,
+        ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo&client_platform=apple"
+      )
+
+    assert has_element?(lv, ~s|form#verify-sign-in-token|)
+    assert has_element?(lv, "button", "Submit")
+
+    <<secret::binary-size(5), nonce::binary>> = identity.provider_virtual_state.sign_in_token
+
+    conn =
+      conn
+      |> put_session(:sign_in_nonce, nonce)
+      |> put_session(:client_platform, "apple")
+      |> put_session(:client_csrf_token, "foo")
+
+    conn =
+      lv
+      |> form("#verify-sign-in-token", %{
+        identity_id: identity.id,
+        secret: secret
+      })
+      |> submit_form(conn)
+
+    assert redirected_to(conn, 302) =~ "firezone://handle_client_auth_callback"
+    refute conn.assigns.flash["error"]
+  end
+
+  test "renders token form for Android users", %{
+    account: account,
+    provider: provider,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, lv, _html} =
+      live(
+        conn,
+        ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo&client_platform=android"
+      )
+
+    <<secret::binary-size(5), nonce::binary>> = identity.provider_virtual_state.sign_in_token
+
+    conn =
+      conn
+      |> put_session(:sign_in_nonce, nonce)
+      |> put_session(:client_platform, "android")
+      |> put_session(:client_csrf_token, "foo")
+
+    conn =
+      lv
+      |> form("#verify-sign-in-token", %{
+        identity_id: identity.id,
+        secret: secret
+      })
+      |> submit_form(conn)
+
+    assert "/handle_client_auth_callback" <> _ = redirected_to(conn, 302)
+    refute conn.assigns.flash["error"]
+  end
+
+  test "renders error on invalid token", %{
+    account: account,
+    provider: provider,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, lv, _html} =
+      live(
+        conn,
+        ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo&client_platform=android"
+      )
+
+    <<_secret::binary-size(5), nonce::binary>> = identity.provider_virtual_state.sign_in_token
+
+    conn =
+      conn
+      |> put_session(:sign_in_nonce, nonce)
+      |> put_session(:client_platform, "android")
+      |> put_session(:client_csrf_token, "foo")
+
+    conn =
+      lv
+      |> form("#verify-sign-in-token", %{
+        identity_id: identity.id,
+        secret: "foo"
+      })
+      |> submit_form(conn)
+
+    assert redirected_to(conn, 302) == ~p"/#{account}"
+    assert conn.assigns.flash["error"] == "The sign in link is invalid or expired."
+  end
+
+  test "allows to resend magic link", %{
+    account: account,
+    provider: provider,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, lv, _html} =
+      live(conn, ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo")
+
+    assert has_element?(lv, ~s|button[type="submit"]|, "Resend email")
+
+    conn =
+      lv
+      |> form("#resend-email", %{email: %{provider_identifier: identity.provider_identifier}})
+      |> submit_form(conn)
+
+    assert conn.assigns.flash["info"] == "Email was resent."
+
+    assert redirected_to = redirected_to(conn, 302)
+    assert redirected_to =~ ~p"/#{account}/sign_in/providers/email/#{provider}"
+    assert redirected_to =~ "provider_identifier="
+    refute get_session(conn, :client_platform)
+  end
+
+  test "does not loose client platform param on email resend", %{
+    account: account,
+    provider: provider,
+    identity: identity,
+    conn: conn
+  } do
+    conn = put_session(conn, :client_platform, "apple")
+
+    {:ok, lv, _html} =
+      live(
+        conn,
+        ~p"/#{account}/sign_in/providers/email/#{provider}?provider_identifier=foo&client_platform=apple"
+      )
+
+    assert has_element?(lv, ~s|button[type="submit"]|, "Resend email")
+
+    conn =
+      lv
+      |> form("#resend-email", %{
+        email: %{provider_identifier: identity.provider_identifier},
+        client_platform: "apple"
+      })
+      |> submit_form(conn)
+
+    assert conn.assigns.flash["info"] == "Email was resent."
+
+    assert redirected_to = redirected_to(conn, 302)
+    assert redirected_to =~ ~p"/#{account}/sign_in/providers/email/#{provider}"
+    assert redirected_to =~ "provider_identifier="
+    assert redirected_to =~ "client_platform=apple"
+    assert get_session(conn, :client_platform) == "apple"
+  end
 end