From 396f2ef5846494b409a6a8dad77365fa1a06a10f Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Wed, 10 Jan 2024 13:52:03 -0600 Subject: [PATCH] Redirect to deep link when user is already signed in (#3156) Closes #2032 --- elixir/apps/domain/lib/domain/auth.ex | 2 +- elixir/apps/web/lib/web/auth.ex | 14 ++++- elixir/apps/web/test/web/auth_test.exs | 77 ++++++++++++++++++++------ 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/elixir/apps/domain/lib/domain/auth.ex b/elixir/apps/domain/lib/domain/auth.ex index e638e48dc..5c67453f4 100644 --- a/elixir/apps/domain/lib/domain/auth.ex +++ b/elixir/apps/domain/lib/domain/auth.ex @@ -604,7 +604,7 @@ defmodule Domain.Auth do def create_service_account_token( %Actors.Actor{type: :service_account, account_id: account_id} = actor, %Subject{account: %{id: account_id}} = subject, - attrs \\ %{} + attrs ) do attrs = Map.merge(attrs, %{ diff --git a/elixir/apps/web/lib/web/auth.ex b/elixir/apps/web/lib/web/auth.ex index 40fc6aed1..d5100e2e6 100644 --- a/elixir/apps/web/lib/web/auth.ex +++ b/elixir/apps/web/lib/web/auth.ex @@ -288,7 +288,7 @@ defmodule Web.Auth do context = get_auth_context(conn, :browser) with account when not is_nil(account) <- Map.get(conn.assigns, :account), - sessions <- Plug.Conn.get_session(conn, :sessions, []), + sessions = Plug.Conn.get_session(conn, :sessions, []), {:ok, encoded_fragment} <- fetch_token(sessions, account.id, context.type), {:ok, subject} <- Auth.authenticate(encoded_fragment, context), true <- subject.account.id == account.id do @@ -410,16 +410,24 @@ defmodule Web.Auth do """ def redirect_if_user_is_authenticated(%Plug.Conn{} = conn, _opts) do if subject = conn.assigns[:subject] do - redirect_to = signed_in_path(subject.account) + redirect_params = take_sign_in_params(conn.query_params) + encoded_fragment = fetch_subject_token!(conn, subject) + identity = %{subject.identity | actor: subject.actor} conn - |> Phoenix.Controller.redirect(to: redirect_to) + |> signed_in_redirect(identity, subject.context, encoded_fragment, redirect_params) |> Plug.Conn.halt() else conn end end + defp fetch_subject_token!(conn, %Auth.Subject{} = subject) do + sessions = Plug.Conn.get_session(conn, :sessions, []) + {:ok, encoded_fragment} = fetch_token(sessions, subject.account.id, subject.context.type) + encoded_fragment + end + @doc """ Used for routes that require the user to be authenticated. diff --git a/elixir/apps/web/test/web/auth_test.exs b/elixir/apps/web/test/web/auth_test.exs index e2de9b4a2..38dac85c8 100644 --- a/elixir/apps/web/test/web/auth_test.exs +++ b/elixir/apps/web/test/web/auth_test.exs @@ -38,7 +38,7 @@ defmodule Web.AuthTest do user_identity = %{user_identity | actor: user_actor} - {:ok, user_token} = Domain.Auth.create_token(admin_identity, context, nonce, nil) + {:ok, user_token} = Domain.Auth.create_token(user_identity, context, nonce, nil) user_encoded_fragment = Domain.Tokens.encode_fragment!(user_token) user_subject = @@ -75,7 +75,7 @@ defmodule Web.AuthTest do user_encoded_fragment: encoded_token } do conn = put_account_session(conn, :browser, account.id, encoded_token) - assert get_session(conn, "sessions") == [{:browser, account.id, encoded_token}] + assert get_session(conn, :sessions) == [{:browser, account.id, encoded_token}] end test "persists a client token in session", %{ @@ -104,42 +104,42 @@ defmodule Web.AuthTest do end test "appends a new tokens to a session", %{ - conn: conn, - account: account + conn: conn } do - session = {:client, Ecto.UUID.generate(), "buz"} + account_id1 = Ecto.UUID.generate() + account_id2 = Ecto.UUID.generate() + + session = {:client, account_id1, "buz"} conn = conn |> put_session(:sessions, [session]) - |> put_account_session(:browser, account.id, "foo") - |> put_account_session(:client, account.id, "bar") + |> put_account_session(:browser, account_id1, "foo") + |> put_account_session(:browser, account_id2, "bar") - assert [ - ^session, - {:browser, account_id, "foo"}, - {:client, account_id, "bar"} - ] = get_session(conn, "sessions") - - assert account_id == account.id + assert get_session(conn, "sessions") == [ + session, + {:browser, account_id1, "foo"}, + {:browser, account_id2, "bar"} + ] end - test "doesn't store more than 15 last sessions", %{ + test "doesn't store more than 6 last sessions", %{ conn: conn, account: account } do sessions = for i <- 1..15 do - {:client, Ecto.UUID.generate(), "foo_#{i}"} + {:browser, Ecto.UUID.generate(), "foo_#{i}"} end conn = conn |> put_session(:sessions, sessions) - |> put_account_session(:client, account.id, "bar") + |> put_account_session(:browser, account.id, "bar") assert get_session(conn, "sessions") == - Enum.take(sessions, -5) ++ [{:client, account.id, "bar"}] + Enum.take(sessions, -5) ++ [{:browser, account.id, "bar"}] end end @@ -680,10 +680,13 @@ defmodule Web.AuthTest do test "redirects if user is authenticated to the signed in path", %{ conn: conn, account: account, + context: context, + admin_encoded_fragment: encoded_fragment, admin_subject: subject } do conn = conn + |> put_account_session(context.type, account.id, encoded_fragment) |> assign(:subject, subject) |> fetch_query_params() |> redirect_if_user_is_authenticated([]) @@ -692,6 +695,44 @@ defmodule Web.AuthTest do assert redirected_to(conn) == ~p"/#{account}/sites" end + test "redirects if client is authenticated to the deep link", %{ + conn: conn, + account: account, + nonce: nonce, + context: context, + admin_identity: admin_identity + } do + context = %{context | type: :client} + + {:ok, client_token} = Domain.Auth.create_token(admin_identity, context, nonce, nil) + client_fragment = Domain.Tokens.encode_fragment!(client_token) + {:ok, client_subject} = Domain.Auth.authenticate(nonce <> client_fragment, context) + + redirect_params = %{"as" => "client", "state" => "STATE", "nonce" => nonce} + + conn = + %{ + conn + | path_params: %{"account_id_or_slug" => account.slug}, + query_params: redirect_params + } + |> put_account_session(context.type, account.id, client_fragment) + |> assign(:subject, client_subject) + |> redirect_if_user_is_authenticated([]) + + assert conn.halted + + assert redirected_to = redirected_to(conn) + assert redirected_to =~ "firezone-fd0020211111://handle_client_sign_in_callback" + assert redirected_to =~ "fragment=#{URI.encode_www_form(client_fragment)}" + assert redirected_to =~ "state=STATE" + assert redirected_to =~ "account_slug=#{account.slug}" + assert redirected_to =~ "account_name=#{URI.encode_www_form(account.name)}" + + assert redirected_to =~ + "identity_provider_identifier=#{URI.encode_www_form(admin_identity.provider_identifier)}" + end + test "does not redirect if user is not authenticated", %{conn: conn} do conn = redirect_if_user_is_authenticated(conn, []) refute conn.halted