From dc0119c34749f74ab04aab2aae0583a3726e0cbd Mon Sep 17 00:00:00 2001
From: Jamil <jamilbk@users.noreply.github.com>
Date: Mon, 19 Feb 2024 13:53:47 -0800
Subject: [PATCH] Revert "feat(portal): Add sign-in success page for clients"
 (#3692)

Merged a bit too soon!
---
 .../domain/lib/domain/config/definitions.ex   | 10 +---
 elixir/apps/web/lib/web/auth.ex               |  6 +-
 .../apps/web/lib/web/live/sign_in/success.ex  | 58 -------------------
 elixir/apps/web/lib/web/router.ex             |  4 --
 elixir/apps/web/lib/web/sandbox.ex            |  7 +--
 .../web/test/support/acceptance_case/auth.ex  |  4 --
 .../test/web/acceptance/auth/email_test.exs   |  4 --
 .../acceptance/auth/openid_connect_test.exs   |  6 +-
 .../web/acceptance/auth/userpass_test.exs     |  4 --
 elixir/apps/web/test/web/auth_test.exs        |  8 +--
 .../web/controllers/auth_controller_test.exs  |  9 ++-
 .../web/test/web/live/sign_in/email_test.exs  |  2 +-
 .../test/web/live/sign_in/success_test.exs    | 44 --------------
 elixir/config/config.exs                      |  2 -
 elixir/config/runtime.exs                     |  2 -
 elixir/config/test.exs                        |  2 -
 16 files changed, 19 insertions(+), 153 deletions(-)
 delete mode 100644 elixir/apps/web/lib/web/live/sign_in/success.ex
 delete mode 100644 elixir/apps/web/test/web/live/sign_in/success_test.exs

diff --git a/elixir/apps/domain/lib/domain/config/definitions.ex b/elixir/apps/domain/lib/domain/config/definitions.ex
index ce3143400..ba97eea3f 100644
--- a/elixir/apps/domain/lib/domain/config/definitions.ex
+++ b/elixir/apps/domain/lib/domain/config/definitions.ex
@@ -89,8 +89,7 @@ defmodule Domain.Config.Definitions do
        ]},
       {"Clients",
        [
-         :clients_upstream_dns,
-         :client_redirect_delay
+         :clients_upstream_dns
        ]},
       {"Authorization",
        """
@@ -433,13 +432,6 @@ defmodule Domain.Config.Definitions do
     changeset: {Domain.Config.Configuration.ClientsUpstreamDNS, :changeset, []}
   )
 
-  @doc """
-  Delay time in milliseconds to wait before redirecting client on the sign in success page.
-
-  This is needed for acceptance tests.  In dev/staging/prod the default should work fine.
-  """
-  defconfig(:client_redirect_delay, :integer, default: 1)
-
   ##############################################
   ## Userpass / SAML / OIDC / Email authentication
   ##############################################
diff --git a/elixir/apps/web/lib/web/auth.ex b/elixir/apps/web/lib/web/auth.ex
index f49374c15..fb9bb2862 100644
--- a/elixir/apps/web/lib/web/auth.ex
+++ b/elixir/apps/web/lib/web/auth.ex
@@ -123,9 +123,13 @@ defmodule Web.Auth do
         identity_provider_identifier: identity.provider_identifier
       }
       |> Enum.reject(&is_nil(elem(&1, 1)))
+      |> URI.encode_query()
+
+    client_handler =
+      Domain.Config.fetch_env!(:web, :client_handler)
 
     Phoenix.Controller.redirect(conn,
-      to: ~p"/#{conn.assigns.account.slug}/signin_success?#{query}"
+      external: "#{client_handler}handle_client_sign_in_callback?#{query}"
     )
   end
 
diff --git a/elixir/apps/web/lib/web/live/sign_in/success.ex b/elixir/apps/web/lib/web/live/sign_in/success.ex
deleted file mode 100644
index 4c5641024..000000000
--- a/elixir/apps/web/lib/web/live/sign_in/success.ex
+++ /dev/null
@@ -1,58 +0,0 @@
-defmodule Web.SignIn.Success do
-  use Web, {:live_view, layout: {Web.Layouts, :public}}
-
-  def mount(params, _session, socket) do
-    if connected?(socket) do
-      delay = Domain.Config.fetch_env!(:web, :client_redirect_delay)
-      Process.send_after(self(), :redirect_client, delay)
-    end
-
-    query_params =
-      params
-      |> Map.take(
-        ~w[fragment state actor_name account_slug account_name identity_provider_identifier]
-      )
-
-    socket = assign(socket, :params, query_params)
-    {:ok, socket}
-  end
-
-  def render(assigns) do
-    ~H"""
-    <section>
-      <div class="flex flex-col items-center justify-center px-6 py-8 mx-auto lg:py-0">
-        <.logo />
-
-        <div class="w-full col-span-6 mx-auto bg-white rounded shadow md:mt-0 sm:max-w-lg xl:p-0">
-          <div class="p-6 space-y-4 lg:space-y-6 sm:p-8">
-            <h1 class="text-xl text-center leading-tight tracking-tight text-neutral-900 sm:text-2xl">
-              <span>
-                Sign in successful.
-              </span>
-            </h1>
-            <p class="text-center">You may close this window.</p>
-          </div>
-        </div>
-      </div>
-    </section>
-    """
-  end
-
-  def handle_info(:redirect_client, socket) do
-    {scheme, url} =
-      Domain.Config.fetch_env!(:web, :client_handler)
-      |> format_redirect_url()
-
-    query = URI.encode_query(socket.assigns.params)
-
-    {:noreply, redirect(socket, external: {scheme, "#{url}?#{query}"})}
-  end
-
-  defp format_redirect_url(raw_client_handler) do
-    uri = URI.parse(raw_client_handler)
-
-    maybe_host = if uri.host == "", do: "", else: "#{uri.host}:#{uri.port}/"
-
-    {uri.scheme, "//#{maybe_host}handle_client_sign_in_callback"}
-  end
-end
diff --git a/elixir/apps/web/lib/web/router.ex b/elixir/apps/web/lib/web/router.ex
index 34230f2e5..a50461609 100644
--- a/elixir/apps/web/lib/web/router.ex
+++ b/elixir/apps/web/lib/web/router.ex
@@ -89,10 +89,6 @@ defmodule Web.Router do
   scope "/:account_id_or_slug", Web do
     pipe_through [:browser, :account]
 
-    live_session :client_redirect, on_mount: [Web.Sandbox] do
-      live "/signin_success", SignIn.Success
-    end
-
     scope "/sign_in/providers/:provider_id" do
       # UserPass
       post "/verify_credentials", AuthController, :verify_credentials
diff --git a/elixir/apps/web/lib/web/sandbox.ex b/elixir/apps/web/lib/web/sandbox.ex
index 0a6ef9e6f..f83261ca4 100644
--- a/elixir/apps/web/lib/web/sandbox.ex
+++ b/elixir/apps/web/lib/web/sandbox.ex
@@ -31,16 +31,11 @@ defmodule Web.Sandbox do
   end
 
   def allow_live_ecto_sandbox(socket) do
-    user_agent = Phoenix.LiveView.get_connect_info(socket, :user_agent)
-
     if Phoenix.LiveView.connected?(socket) do
+      user_agent = Phoenix.LiveView.get_connect_info(socket, :user_agent)
       Sandbox.allow(Phoenix.Ecto.SQL.Sandbox, user_agent)
     end
 
-    with %{owner: test_pid} <- Phoenix.Ecto.SQL.Sandbox.decode_metadata(user_agent) do
-      Process.put(:last_caller_pid, test_pid)
-    end
-
     socket
   end
 end
diff --git a/elixir/apps/web/test/support/acceptance_case/auth.ex b/elixir/apps/web/test/support/acceptance_case/auth.ex
index 4de6ba572..267ba4ed0 100644
--- a/elixir/apps/web/test/support/acceptance_case/auth.ex
+++ b/elixir/apps/web/test/support/acceptance_case/auth.ex
@@ -112,10 +112,6 @@ defmodule Web.AcceptanceCase.Auth do
       Plug.Conn.send_resp(conn, 200, "Client redirected")
     end)
 
-    Bypass.stub(bypass, "GET", "/favicon.ico", fn conn ->
-      Plug.Conn.send_resp(conn, 404, "")
-    end)
-
     bypass
   end
 
diff --git a/elixir/apps/web/test/web/acceptance/auth/email_test.exs b/elixir/apps/web/test/web/acceptance/auth/email_test.exs
index 2f26f5951..d72328589 100644
--- a/elixir/apps/web/test/web/acceptance/auth/email_test.exs
+++ b/elixir/apps/web/test/web/acceptance/auth/email_test.exs
@@ -57,8 +57,6 @@ defmodule Web.Acceptance.SignIn.EmailTest do
 
     session
     |> email_login_flow(account, identity.provider_identifier, redirect_params)
-    |> assert_el(Query.text("Sign in successful"))
-    |> assert_path(~p"/#{account}/signin_success")
     |> assert_el(Query.text("Client redirected"))
     |> assert_path(~p"/handle_client_sign_in_callback")
 
@@ -112,8 +110,6 @@ defmodule Web.Acceptance.SignIn.EmailTest do
     # And then to a client
     session
     |> email_login_flow(account, identity.provider_identifier, redirect_params)
-    |> assert_el(Query.text("Sign in successful"))
-    |> assert_path(~p"/#{account}/signin_success")
     |> assert_el(Query.text("Client redirected"))
     |> assert_path(~p"/handle_client_sign_in_callback")
 
diff --git a/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs b/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs
index 7e086abae..44c9042c8 100644
--- a/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs
+++ b/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs
@@ -172,7 +172,7 @@ defmodule Web.Acceptance.Auth.OpenIDConnectTest do
         provider_identifier: entity_id
       )
 
-    # Sign In as a portal user
+    # Sign In as an portal user
     session
     |> visit(~p"/#{account}")
     |> assert_el(Query.text("Sign in to #{account.name}"))
@@ -187,8 +187,6 @@ defmodule Web.Acceptance.Auth.OpenIDConnectTest do
     |> visit(~p"/#{account}?#{redirect_params}")
     |> assert_el(Query.text("Sign in to #{account.name}"))
     |> click(Query.link("Sign in with Vault"))
-    |> assert_el(Query.text("Sign in successful"))
-    |> assert_path(~p"/#{account}/signin_success")
     |> assert_el(Query.text("Client redirected"))
     |> assert_path(~p"/handle_client_sign_in_callback")
 
@@ -242,8 +240,6 @@ defmodule Web.Acceptance.Auth.OpenIDConnectTest do
     |> assert_el(Query.text("Sign in to #{account.name}"))
     |> click(Query.link("Sign in with Vault"))
     |> Vault.userpass_flow(oidc_login, oidc_password)
-    |> assert_el(Query.text("Sign in successful"))
-    |> assert_path(~p"/#{account}/signin_success")
     |> assert_el(Query.text("Client redirected"))
     |> assert_path(~p"/handle_client_sign_in_callback")
 
diff --git a/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs b/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs
index feecd1e0b..66a5e083f 100644
--- a/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs
+++ b/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs
@@ -167,8 +167,6 @@ defmodule Web.Acceptance.Auth.UserPassTest do
 
     session
     |> password_login_flow(account, identity.provider_identifier, password, redirect_params)
-    |> assert_el(Query.text("Sign in successful"))
-    |> assert_path(~p"/#{account}/signin_success")
     |> assert_el(Query.text("Client redirected"))
     |> assert_path(~p"/handle_client_sign_in_callback")
 
@@ -232,8 +230,6 @@ defmodule Web.Acceptance.Auth.UserPassTest do
     # And then to a client
     session
     |> password_login_flow(account, identity.provider_identifier, password, redirect_params)
-    |> assert_el(Query.text("Sign in successful"))
-    |> assert_path(~p"/#{account}/signin_success")
     |> assert_el(Query.text("Client redirected"))
     |> assert_path(~p"/handle_client_sign_in_callback")
 
diff --git a/elixir/apps/web/test/web/auth_test.exs b/elixir/apps/web/test/web/auth_test.exs
index ec6e67962..41382a1d5 100644
--- a/elixir/apps/web/test/web/auth_test.exs
+++ b/elixir/apps/web/test/web/auth_test.exs
@@ -251,7 +251,7 @@ defmodule Web.AuthTest do
       assert conn.assigns.flash["error"] == "Please use a client application to access Firezone."
     end
 
-    test "redirects regular users to the sign in success page for client contexts", %{
+    test "redirects regular users to the deep link for client contexts", %{
       conn: conn,
       context: context,
       account: account,
@@ -270,7 +270,7 @@ defmodule Web.AuthTest do
         |> signed_in(provider, identity, context, encoded_fragment, redirect_params)
         |> redirected_to()
 
-      assert redirected_to =~ "#{account.slug}/signin_success"
+      assert redirected_to =~ "firezone-fd0020211111://handle_client_sign_in_callback"
       assert redirected_to =~ "fragment=#{URI.encode_www_form(encoded_fragment)}"
       assert redirected_to =~ "state=STATE"
       assert redirected_to =~ "account_slug=#{account.slug}"
@@ -299,7 +299,7 @@ defmodule Web.AuthTest do
         |> signed_in(provider, identity, context, encoded_fragment, redirect_params)
         |> redirected_to()
 
-      assert redirected_to =~ "#{account.slug}/signin_success"
+      assert redirected_to =~ "firezone-fd0020211111://handle_client_sign_in_callback"
       assert redirected_to =~ "fragment=#{URI.encode_www_form(encoded_fragment)}"
       assert redirected_to =~ "state=STATE"
       assert redirected_to =~ "account_slug=#{account.slug}"
@@ -780,7 +780,7 @@ defmodule Web.AuthTest do
       assert conn.halted
 
       assert redirected_to = redirected_to(conn)
-      assert redirected_to =~ "#{account.slug}/signin_success"
+      assert redirected_to =~ "firezone-fd0020211111://handle_client_sign_in_callback"
       assert redirected_to =~ "fragment=#{URI.encode_www_form(encoded_fragment)}"
       assert redirected_to =~ "state=STATE"
       assert redirected_to =~ "account_slug=#{account.slug}"
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 f4da550cd..04ca53ec2 100644
--- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs
+++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs
@@ -272,7 +272,8 @@ defmodule Web.AuthControllerTest do
 
       assert redirected_to = redirected_to(conn)
       assert redirected_to_uri = URI.parse(redirected_to)
-      assert redirected_to_uri.path == "/#{account.slug}/signin_success"
+      assert redirected_to_uri.scheme == "firezone-fd0020211111"
+      assert redirected_to_uri.host == "handle_client_sign_in_callback"
 
       assert %{
                "identity_provider_identifier" => identity_provider_identifier,
@@ -625,7 +626,8 @@ defmodule Web.AuthControllerTest do
       refute Map.has_key?(conn.cookies, "fz_auth_state_#{provider.id}")
 
       assert redirected_to = conn |> redirected_to() |> URI.parse()
-      assert redirected_to.path == "/#{account.slug}/signin_success"
+      assert redirected_to.scheme == "firezone-fd0020211111"
+      assert redirected_to.host == "handle_client_sign_in_callback"
 
       assert query_params = URI.decode_query(redirected_to.query)
       assert not is_nil(query_params["fragment"])
@@ -980,7 +982,8 @@ defmodule Web.AuthControllerTest do
         })
 
       assert redirected_to = conn |> redirected_to() |> URI.parse()
-      assert redirected_to.path == "/#{account.slug}/signin_success"
+      assert redirected_to.scheme == "firezone-fd0020211111"
+      assert redirected_to.host == "handle_client_sign_in_callback"
 
       assert query_params = URI.decode_query(redirected_to.query)
       assert not is_nil(query_params["fragment"])
diff --git a/elixir/apps/web/test/web/live/sign_in/email_test.exs b/elixir/apps/web/test/web/live/sign_in/email_test.exs
index 8cba65170..0b8748730 100644
--- a/elixir/apps/web/test/web/live/sign_in/email_test.exs
+++ b/elixir/apps/web/test/web/live/sign_in/email_test.exs
@@ -104,7 +104,7 @@ defmodule Web.SignIn.EmailTest do
       })
       |> submit_form(conn)
 
-    assert redirected_to(conn, 302) =~ "/#{account.slug}/signin_success"
+    assert redirected_to(conn, 302) =~ "firezone-fd0020211111://handle_client_sign_in_callback"
     refute conn.assigns.flash["error"]
   end
 
diff --git a/elixir/apps/web/test/web/live/sign_in/success_test.exs b/elixir/apps/web/test/web/live/sign_in/success_test.exs
deleted file mode 100644
index fb827d223..000000000
--- a/elixir/apps/web/test/web/live/sign_in/success_test.exs
+++ /dev/null
@@ -1,44 +0,0 @@
-defmodule Web.SignIn.SuccessTest do
-  use Web.ConnCase, async: true
-
-  setup do
-    account = Fixtures.Accounts.create_account()
-
-    %{account: account}
-  end
-
-  test "redirects to deep link URL", %{
-    account: account,
-    conn: conn
-  } do
-    query_params = %{
-      account_name: "account_name",
-      account_slug: "account_slug",
-      actor_name: "actor_name",
-      fragment: "fragment",
-      identity_provider_identifier: "identifier",
-      state: "state"
-    }
-
-    {:ok, lv, html} =
-      conn
-      |> live(~p"/#{account}/signin_success?#{query_params}")
-
-    assert html =~ "success"
-    assert html =~ "close this window"
-
-    client_redirect_delay = Domain.Config.fetch_env!(:web, :client_redirect_delay)
-
-    sorted_query_params =
-      query_params
-      |> Map.to_list()
-      |> Enum.sort()
-      |> URI.encode_query()
-
-    assert_redirect(
-      lv,
-      "firezone-fd0020211111://handle_client_sign_in_callback?#{sorted_query_params}",
-      client_redirect_delay + 500
-    )
-  end
-end
diff --git a/elixir/config/config.exs b/elixir/config/config.exs
index a8efc81e1..89c170d5a 100644
--- a/elixir/config/config.exs
+++ b/elixir/config/config.exs
@@ -129,8 +129,6 @@ config :web, Web.Plugs.SecureHeaders,
 
 config :web, api_url_override: "ws://localhost:13001/"
 
-config :web, client_redirect_delay: 1
-
 ###############################
 ##### API #####################
 ###############################
diff --git a/elixir/config/runtime.exs b/elixir/config/runtime.exs
index 7b1d98454..f19344a73 100644
--- a/elixir/config/runtime.exs
+++ b/elixir/config/runtime.exs
@@ -107,8 +107,6 @@ if config_env() == :prod do
 
   config :web, api_url_override: compile_config!(:api_url_override)
 
-  config :web, client_redirect_delay: compile_config!(:client_redirect_delay)
-
   ###############################
   ##### API #####################
   ###############################
diff --git a/elixir/config/test.exs b/elixir/config/test.exs
index 584f0faed..22fea885d 100644
--- a/elixir/config/test.exs
+++ b/elixir/config/test.exs
@@ -45,8 +45,6 @@ config :web, Web.Plugs.SecureHeaders,
     "script-src 'self' 'unsafe-inline' https://cdn.tailwindcss.com/"
   ]
 
-config :web, client_redirect_delay: 500
-
 ###############################
 ##### API #####################
 ###############################