diff --git a/elixir/apps/web/lib/web/auth.ex b/elixir/apps/web/lib/web/auth.ex index eec423adf..7aa7e19ae 100644 --- a/elixir/apps/web/lib/web/auth.ex +++ b/elixir/apps/web/lib/web/auth.ex @@ -2,6 +2,16 @@ defmodule Web.Auth do use Web, :verified_routes alias Domain.{Auth, Accounts, Tokens} + # This cookie is used for client login. + @client_auth_cookie_name "fz_client_auth" + @client_auth_cookie_options [ + sign: true, + max_age: 2 * 60, + same_site: "Strict", + secure: true, + http_only: true + ] + # This is the cookie which will store recent account ids # that the user has signed in to. @recent_accounts_cookie_name "fz_recent_account_ids" @@ -113,17 +123,24 @@ defmodule Web.Auth do "nonce" => _nonce, "state" => state }) do - query = + client_auth_data = %{ - fragment: encoded_fragment, - state: state, actor_name: identity.actor.name, - identity_provider_identifier: identity.provider_identifier + fragment: encoded_fragment, + identity_provider_identifier: identity.provider_identifier, + state: state } - |> Enum.reject(&is_nil(elem(&1, 1))) + |> Map.reject(fn {_key, val} -> is_nil(val) end) - Phoenix.Controller.redirect(conn, - to: ~p"/#{conn.assigns.account.slug}/sign_in/success?#{query}" + redirect_url = ~p"/#{conn.assigns.account.slug}/sign_in/client_redirect" + + conn + |> put_client_auth_data_to_cookie(client_auth_data) + |> Phoenix.Controller.put_root_layout(false) + |> Phoenix.Controller.put_view(Web.SignInHTML) + |> Phoenix.Controller.render("client_redirect.html", + redirect_url: redirect_url, + layout: false ) end @@ -490,6 +507,22 @@ defmodule Web.Auth do %{} end + def get_client_auth_data_from_cookie(%Plug.Conn{} = conn) do + conn = Plug.Conn.fetch_cookies(conn, signed: [@client_auth_cookie_name]) + + case conn.cookies[@client_auth_cookie_name] do + %{actor_name: _, fragment: _, identity_provider_identifier: _, state: _} = client_auth_data -> + {:ok, client_auth_data, conn} + + _ -> + {:error, conn} + end + end + + defp put_client_auth_data_to_cookie(conn, state) do + Plug.Conn.put_resp_cookie(conn, @client_auth_cookie_name, state, @client_auth_cookie_options) + end + ########################### ## LiveView ########################### diff --git a/elixir/apps/web/lib/web/controllers/auth_controller.ex b/elixir/apps/web/lib/web/controllers/auth_controller.ex index aa5b4297a..783d46db2 100644 --- a/elixir/apps/web/lib/web/controllers/auth_controller.ex +++ b/elixir/apps/web/lib/web/controllers/auth_controller.ex @@ -179,10 +179,7 @@ defmodule Web.AuthController do """ def redirect_to_idp( conn, - %{ - "account_id_or_slug" => account_id_or_slug, - "provider_id" => provider_id - } = params + %{"account_id_or_slug" => account_id_or_slug, "provider_id" => provider_id} = params ) do with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id) do redirect_params = Web.Auth.take_sign_in_params(params) diff --git a/elixir/apps/web/lib/web/controllers/sign_in_controller.ex b/elixir/apps/web/lib/web/controllers/sign_in_controller.ex new file mode 100644 index 000000000..715907f93 --- /dev/null +++ b/elixir/apps/web/lib/web/controllers/sign_in_controller.ex @@ -0,0 +1,36 @@ +defmodule Web.SignInController do + use Web, :controller + + def client_redirect(conn, _params) do + account = conn.assigns.account + + with {:ok, client_auth_data, conn} <- Web.Auth.get_client_auth_data_from_cookie(conn) do + {scheme, url} = + Domain.Config.fetch_env!(:web, :client_handler) + |> format_redirect_url() + + query = + client_auth_data + |> Map.put_new(:account_slug, account.slug) + |> Map.put_new(:account_name, account.name) + |> URI.encode_query() + + redirect(conn, external: "#{scheme}://#{url}?#{query}") + else + {:error, conn} -> + redirect(conn, to: ~p"/#{account}/sign_in/client_auth_error") + end + end + + def client_auth_error(conn, _params) do + render(conn, :client_auth_error, layout: false) + 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/controllers/sign_in_html.ex b/elixir/apps/web/lib/web/controllers/sign_in_html.ex new file mode 100644 index 000000000..711b8cf75 --- /dev/null +++ b/elixir/apps/web/lib/web/controllers/sign_in_html.ex @@ -0,0 +1,75 @@ +defmodule Web.SignInHTML do + use Web, :html + + def client_redirect(assigns) do + ~H""" + + + + + + + + + + + + + + <.live_title suffix=" ยท Firezone"> + <%= assigns[:page_title] || "Firezone" %> + + + + +
+
+
+ <.logo /> + +
+
+

+ + Sign in successful. + +

+

You may close this window.

+
+
+
+
+
+ + + """ + end + + def client_auth_error(assigns) do + ~H""" +
+
+
+ <.logo /> + +
+
+

+ + Sign in error! + +

+

Please close this window and start the sign in process again.

+
+
+
+
+
+ """ + end +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 cb29e2050..000000000 --- a/elixir/apps/web/lib/web/live/sign_in/success.ex +++ /dev/null @@ -1,70 +0,0 @@ -defmodule Web.SignIn.Success do - use Web, {:live_view, layout: {Web.Layouts, :public}} - - def mount( - %{ - "fragment" => _, - "state" => _, - "actor_name" => _, - "identity_provider_identifier" => _ - } = params, - _session, - socket - ) do - if connected?(socket) do - Process.send_after(self(), :redirect_client, 100) - end - - query_params = - params - |> Map.take(~w[fragment state actor_name identity_provider_identifier]) - |> Map.put("account_slug", socket.assigns.account.slug) - |> Map.put("account_name", socket.assigns.account.name) - - socket = assign(socket, :params, query_params) - {:ok, socket} - end - - def mount(_params, _session, _socket) do - raise Web.LiveErrors.InvalidParamsError - end - - def render(assigns) do - ~H""" -
-
- <.logo /> - -
-
-

- - Sign in successful. - -

-

You may close this window.

-
-
-
-
- """ - 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 59e8aeb2d..6398df67c 100644 --- a/elixir/apps/web/lib/web/router.ex +++ b/elixir/apps/web/lib/web/router.ex @@ -89,9 +89,8 @@ defmodule Web.Router do scope "/:account_id_or_slug", Web do pipe_through [:browser, :account] - live_session :client_redirect, on_mount: [Web.Sandbox, {Web.Auth, :mount_account}] do - live "/sign_in/success", SignIn.Success - end + get "/sign_in/client_redirect", SignInController, :client_redirect + get "/sign_in/client_auth_error", SignInController, :client_auth_error scope "/sign_in/providers/:provider_id" do # UserPass diff --git a/elixir/apps/web/test/support/conn_case.ex b/elixir/apps/web/test/support/conn_case.ex index 582320ecc..cc908e981 100644 --- a/elixir/apps/web/test/support/conn_case.ex +++ b/elixir/apps/web/test/support/conn_case.ex @@ -117,6 +117,49 @@ defmodule Web.ConnCase do {conn_with_cookie, state, verifier} end + def put_client_auth_state( + conn, + account, + %{adapter: :email} = provider, + identity, + params \\ %{} + ) do + params = + Map.merge( + %{ + "email" => %{"provider_identifier" => identity.provider_identifier}, + "as" => "client", + "nonce" => "nonce", + "state" => "state" + }, + params + ) + + redirected_conn = + post(conn, ~p"/#{account}/sign_in/providers/#{provider.id}/request_magic_link", params) + + assert_received {:email, email} + [_match, secret] = Regex.run(~r/secret=([^&\n]*)/, email.text_body) + + auth_state_cookie_key = "fz_auth_state_#{provider.id}" + %{value: signed_state} = redirected_conn.resp_cookies[auth_state_cookie_key] + + verified_conn = + conn + |> put_req_cookie("fz_auth_state_#{provider.id}", signed_state) + |> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{ + "identity_id" => identity.id, + "secret" => secret + }) + + client_cookie_key = "fz_client_auth" + %{value: signed_client_auth} = verified_conn.resp_cookies[client_cookie_key] + + conn + |> put_req_cookie("fz_client_auth", signed_client_auth) + |> put_req_cookie("fz_auth_state_#{provider.id}", signed_state) + end + ### Helpers to test LiveView forms def find_inputs(html, selector) do 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 d72328589..e6c8ef336 100644 --- a/elixir/apps/web/test/web/acceptance/auth/email_test.exs +++ b/elixir/apps/web/test/web/acceptance/auth/email_test.exs @@ -37,7 +37,7 @@ defmodule Web.Acceptance.SignIn.EmailTest do |> Auth.assert_authenticated(identity) end - feature "allows to sign in using email link to the client", %{session: session} do + feature "allows client to sign in using email link", %{session: session} do Domain.Config.put_env_override(:outbound_email_adapter_configured?, true) nonce = Ecto.UUID.generate() state = Ecto.UUID.generate() diff --git a/elixir/apps/web/test/web/auth_test.exs b/elixir/apps/web/test/web/auth_test.exs index 7c60c0ec0..bfcc11607 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 non-admin users to the sign in success page for client contexts", %{ conn: conn, context: context, account: account, @@ -264,21 +264,24 @@ defmodule Web.AuthTest do redirect_params = %{"as" => "client", "state" => "STATE", "nonce" => nonce} - redirected_to = + response = %{conn | path_params: %{"account_id_or_slug" => account.slug}} + |> put_private(:phoenix_endpoint, @endpoint) + |> Web.Plugs.SecureHeaders.call([]) |> fetch_flash() |> signed_in(provider, identity, context, encoded_fragment, redirect_params) - |> redirected_to() + |> Phoenix.ConnTest.response(200) - assert redirected_to =~ "#{account.slug}/sign_in/success" - assert redirected_to =~ "fragment=#{URI.encode_www_form(encoded_fragment)}" - assert redirected_to =~ "state=STATE" + assert response =~ "Sign in successful" - assert redirected_to =~ - "identity_provider_identifier=#{URI.encode_www_form(identity.provider_identifier)}" + assert response + |> Floki.attribute("meta", "content") + |> Enum.any?(fn value -> + &(&1 == "0; url=/#{account.slug}/sign_in/client_redirect") + end) end - test "redirects admin users to the deep link for client contexts", %{ + test "redirects admin users to the sign in success page for client contexts", %{ conn: conn, context: context, account: account, @@ -291,18 +294,21 @@ defmodule Web.AuthTest do redirect_params = %{"as" => "client", "state" => "STATE", "nonce" => nonce} - redirected_to = + response = %{conn | path_params: %{"account_id_or_slug" => account.slug}} + |> put_private(:phoenix_endpoint, @endpoint) + |> Web.Plugs.SecureHeaders.call([]) |> fetch_flash() |> signed_in(provider, identity, context, encoded_fragment, redirect_params) - |> redirected_to() + |> Phoenix.ConnTest.response(200) - assert redirected_to =~ "#{account.slug}/sign_in/success" - assert redirected_to =~ "fragment=#{URI.encode_www_form(encoded_fragment)}" - assert redirected_to =~ "state=STATE" + assert response =~ "Sign in successful" - assert redirected_to =~ - "identity_provider_identifier=#{URI.encode_www_form(identity.provider_identifier)}" + assert response + |> Floki.attribute("meta", "content") + |> Enum.any?(fn value -> + &(&1 == "0; url=/#{account.slug}/sign_in/client_redirect") + end) end test "redirects admin user to the post-login path for browser contexts", %{ @@ -748,7 +754,7 @@ defmodule Web.AuthTest do assert redirected_to(conn) == ~p"/#{account}/sites" end - test "redirects if client is authenticated to the deep link", %{ + test "redirects to sign in success page if client is authenticated", %{ conn: conn, account: account, nonce: nonce, @@ -769,19 +775,22 @@ defmodule Web.AuthTest do | path_params: %{"account_id_or_slug" => account.slug}, params: redirect_params } + |> put_private(:phoenix_endpoint, @endpoint) + |> Web.Plugs.SecureHeaders.call([]) |> put_session(:sessions, [{context.type, account.id, encoded_fragment}]) |> assign(:subject, client_subject) |> redirect_if_user_is_authenticated([]) assert conn.halted - assert redirected_to = redirected_to(conn) - assert redirected_to =~ "#{account.slug}/sign_in/success" - assert redirected_to =~ "fragment=#{URI.encode_www_form(encoded_fragment)}" - assert redirected_to =~ "state=STATE" + assert response = response(conn, 200) + assert response =~ "Sign in successful" - assert redirected_to =~ - "identity_provider_identifier=#{URI.encode_www_form(admin_identity.provider_identifier)}" + assert response + |> Floki.attribute("meta", "content") + |> Enum.any?(fn value -> + &(&1 == "0; url=/#{account.slug}/sign_in/client_redirect") + end) end test "does not redirect if user is not authenticated", %{conn: conn} do 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 e7a9a090c..f25e2490f 100644 --- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs +++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs @@ -270,18 +270,17 @@ defmodule Web.AuthControllerTest do assert conn.assigns.flash == %{} - assert redirected_to = redirected_to(conn) - assert redirected_to_uri = URI.parse(redirected_to) - assert redirected_to_uri.path == "/#{account.slug}/sign_in/success" + assert response = response(conn, 200) + assert response =~ "Sign in successful" - assert %{ - "identity_provider_identifier" => identity_provider_identifier, - "actor_name" => actor_name, - "fragment" => _fragment - } = URI.decode_query(redirected_to_uri.query) + cookie_key = "fz_client_auth" + conn = fetch_cookies(conn, signed: [cookie_key]) + client_auth_data = conn.cookies[cookie_key] - assert actor.name == actor_name - assert identity.provider_identifier == identity_provider_identifier + assert client_auth_data[:state] == "STATE" + assert client_auth_data[:fragment] + assert client_auth_data[:actor_name] == actor.name + assert client_auth_data[:identity_provider_identifier] == identity.provider_identifier end test "persists account into list of recent accounts when credentials are valid", %{ @@ -621,19 +620,22 @@ defmodule Web.AuthControllerTest do "secret" => secret }) + client_auth_cookie_key = "fz_client_auth" + assert conn.assigns.flash == %{} refute Map.has_key?(conn.cookies, "fz_auth_state_#{provider.id}") + assert Map.has_key?(conn.cookies, client_auth_cookie_key) - assert redirected_to = conn |> redirected_to() |> URI.parse() - assert redirected_to.path == "/#{account.slug}/sign_in/success" + assert response = response(conn, 200) + assert response =~ "Sign in successful" - assert query_params = URI.decode_query(redirected_to.query) - assert not is_nil(query_params["fragment"]) - refute query_params["fragment"] =~ redirect_params["nonce"] - assert query_params["state"] == redirect_params["state"] - refute query_params["nonce"] - assert query_params["actor_name"] == Repo.preload(identity, :actor).actor.name - assert query_params["identity_provider_identifier"] == identity.provider_identifier + conn = fetch_cookies(conn, signed: [client_auth_cookie_key]) + client_auth_data = conn.cookies[client_auth_cookie_key] + + assert client_auth_data[:state] == redirect_params["state"] + assert not is_nil(client_auth_data[:fragment]) + assert client_auth_data[:actor_name] == Repo.preload(identity, :actor).actor.name + assert client_auth_data[:identity_provider_identifier] == identity.provider_identifier end test "appends a new valid session when credentials are valid", %{ @@ -947,7 +949,7 @@ defmodule Web.AuthControllerTest do assert redirected_to(conn) == "/#{account.slug}/foo" end - test "redirects clients to deep link on success", %{ + test "redirects clients to sign in success page on success", %{ account: account, provider: provider, bypass: bypass, @@ -979,16 +981,17 @@ defmodule Web.AuthControllerTest do "code" => "MyFakeCode" }) - assert redirected_to = conn |> redirected_to() |> URI.parse() - assert redirected_to.path == "/#{account.slug}/sign_in/success" + cookie_key = "fz_client_auth" + conn = fetch_cookies(conn, signed: [cookie_key]) + client_auth_data = conn.cookies[cookie_key] - assert query_params = URI.decode_query(redirected_to.query) - assert not is_nil(query_params["fragment"]) - refute query_params["fragment"] =~ "NONCE" - assert query_params["state"] == "STATE" - refute query_params["nonce"] - assert query_params["actor_name"] == Repo.preload(identity, :actor).actor.name - assert query_params["identity_provider_identifier"] == identity.provider_identifier + assert response = response(conn, 200) + assert response =~ "Sign in successful" + + assert client_auth_data[:state] == "STATE" + assert not is_nil(client_auth_data[:fragment]) + assert client_auth_data[:actor_name] == Repo.preload(identity, :actor).actor.name + assert client_auth_data[:identity_provider_identifier] == identity.provider_identifier end test "persists the valid auth token in session on success", %{ diff --git a/elixir/apps/web/test/web/controllers/sign_in_controller_test.exs b/elixir/apps/web/test/web/controllers/sign_in_controller_test.exs new file mode 100644 index 000000000..614387db6 --- /dev/null +++ b/elixir/apps/web/test/web/controllers/sign_in_controller_test.exs @@ -0,0 +1,51 @@ +defmodule Web.SignInControllerTest do + use Web.ConnCase, async: true + + setup do + Domain.Config.put_env_override(:outbound_email_adapter_configured?, true) + account = Fixtures.Accounts.create_account() + + {:ok, account: account} + end + + describe "client_redirect/2" do + test "renders 302 with deep link location on proper cookie values", %{ + conn: conn, + account: account + } do + provider = Fixtures.Auth.create_email_provider(account: account) + actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + identity = Fixtures.Auth.create_identity(account: account, provider: provider, actor: actor) + + conn_with_cookies = + conn + |> put_client_auth_state(account, provider, identity, %{ + "state" => "STATE", + "nonce" => "NONCE" + }) + |> get(~p"/#{account}/sign_in/client_redirect") + + assert redirected_to = redirected_to(conn_with_cookies, 302) + assert redirected_to =~ "firezone-fd0020211111://handle_client_sign_in_callback" + + assert redirected_uri = URI.parse(redirected_to) + assert query_params = URI.decode_query(redirected_uri.query) + assert not is_nil(query_params["fragment"]) + refute query_params["fragment"] =~ "NONCE" + assert query_params["state"] == "STATE" + refute query_params["nonce"] + assert query_params["actor_name"] == actor.name + assert query_params["identity_provider_identifier"] == identity.provider_identifier + assert query_params["account_name"] == account.name + assert query_params["account_slug"] == account.slug + end + + test "redirects to sign in page when cookie not present", %{account: account} do + conn = + build_conn() + |> get(~p"/#{account}/sign_in/client_redirect") + + assert redirected_to(conn, 302) =~ ~p"/#{account}/sign_in/client_auth_error" + end + end +end 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 6f7a71ec9..8dd34780d 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,8 @@ defmodule Web.SignIn.EmailTest do }) |> submit_form(conn) - assert redirected_to(conn, 302) =~ "/#{account.slug}/sign_in/success" + assert response = response(conn, 200) + assert response =~ "Sign in successful" 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 c260ed2ac..000000000 --- a/elixir/apps/web/test/web/live/sign_in/success_test.exs +++ /dev/null @@ -1,46 +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 = %{ - "actor_name" => "actor_name", - "fragment" => "fragment", - "identity_provider_identifier" => "identifier", - "state" => "state" - } - - {:ok, lv, html} = - conn - |> live(~p"/#{account}/sign_in/success?#{query_params}") - - assert html =~ "success" - assert html =~ "close this window" - - expected_query_params = - query_params - |> Map.put("account_name", account.name) - |> Map.put("account_slug", account.slug) - - {path, _flash} = assert_redirect(lv, 500) - uri = URI.parse(path) - assert URI.decode_query(uri.query) == expected_query_params - end - - test "returns 422 error when params are missing", %{ - account: account, - conn: conn - } do - assert_raise Web.LiveErrors.InvalidParamsError, fn -> - live(conn, ~p"/#{account}/sign_in/success") - end - end -end