From b9c11007a4e230ab28f0138afc98188b1956dfd3 Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Mon, 7 Aug 2023 17:15:35 -0500 Subject: [PATCH] Add client auth flow (#1868) Related to [#588](https://github.com/firezone/product/issues/588#issuecomment-1591730203) --- elixir/apps/web/lib/web/auth.ex | 75 ++++--- .../lib/web/controllers/auth_controller.ex | 101 +++++---- elixir/apps/web/lib/web/live/auth/sign_in.ex | 19 +- .../web/acceptance/auth/userpass_test.exs | 2 +- .../web/controllers/auth_controller_test.exs | 209 ++++++++++++++++++ elixir/config/config.exs | 6 + elixir/config/runtime.exs | 6 + 7 files changed, 339 insertions(+), 79 deletions(-) diff --git a/elixir/apps/web/lib/web/auth.ex b/elixir/apps/web/lib/web/auth.ex index 83e7f8d8f..6e5853049 100644 --- a/elixir/apps/web/lib/web/auth.ex +++ b/elixir/apps/web/lib/web/auth.ex @@ -6,7 +6,7 @@ defmodule Web.Auth do do: ~p"/#{subject.account}/dashboard" def put_subject_in_session(conn, %Auth.Subject{} = subject) do - {:ok, session_token} = Domain.Auth.create_session_token_from_subject(subject) + {:ok, session_token} = Auth.create_session_token_from_subject(subject) conn |> Plug.Conn.put_session(:signed_in_at, DateTime.utc_now()) @@ -15,38 +15,59 @@ defmodule Web.Auth do end @doc """ - This is a wrapper around `Domain.Auth.sign_in/5` that fails authentication and redirects - to app install instructions for the users that should not have access to the control plane UI. + Redirects the signed in user depending on the actor type. + + The account admin users are sent to dashboard or a return path if it's stored in session. + + The account users are only expected to authenticate using client apps. + If the platform is known, we direct them to the application through a deep link or an app link; + if not, we guide them to the install instructions accompanied by an error message. """ - def sign_in(conn, provider, provider_identifier, secret) do - case Domain.Auth.sign_in( - provider, - provider_identifier, - secret, - conn.assigns.user_agent, - conn.remote_ip - ) do - {:ok, %Auth.Subject{actor: %{type: :account_admin_user}} = subject} -> - {:ok, subject} + def signed_in_redirect( + conn, + %Auth.Subject{actor: %{type: :account_admin_user}} = subject, + _client_platform, + _client_csrf_token + ) do + redirect_to = Plug.Conn.get_session(conn, :user_return_to) || signed_in_path(subject) - {:ok, %Auth.Subject{}} -> - {:error, :invalid_actor_type} - - {:error, reason} -> - {:error, reason} - end + conn + |> Web.Auth.renew_session() + |> Web.Auth.put_subject_in_session(subject) + |> Plug.Conn.delete_session(:user_return_to) + |> Phoenix.Controller.redirect(to: redirect_to) end - def sign_in(conn, provider, payload) do - case Domain.Auth.sign_in(provider, payload, conn.assigns.user_agent, conn.remote_ip) do - {:ok, %Auth.Subject{actor: %{type: :account_admin_user}} = subject} -> - {:ok, subject} + def signed_in_redirect( + conn, + %Auth.Subject{actor: %{type: :account_user}} = subject, + client_platform, + client_csrf_token + ) do + platform_redirect_urls = + Domain.Config.fetch_env!(:web, __MODULE__) + |> Keyword.fetch!(:platform_redirect_urls) - {:ok, %Auth.Subject{}} -> - {:error, :invalid_actor_type} + if redirect_to = Map.get(platform_redirect_urls, client_platform) do + {:ok, client_token} = Auth.create_session_token_from_subject(subject) - {:error, reason} -> - {:error, reason} + query = + %{ + client_auth_token: client_token, + client_csrf_token: client_csrf_token + } + |> Enum.reject(&is_nil(elem(&1, 1))) + |> URI.encode_query() + + conn + |> Phoenix.Controller.redirect(external: "#{redirect_to}?#{query}") + else + conn + |> Phoenix.Controller.put_flash( + :info, + "Please use a client application to access Firezone." + ) + |> Phoenix.Controller.redirect(to: ~p"/#{conn.path_params["account_id_or_slug"]}/") end end diff --git a/elixir/apps/web/lib/web/controllers/auth_controller.ex b/elixir/apps/web/lib/web/controllers/auth_controller.ex index 74b1697e6..931427227 100644 --- a/elixir/apps/web/lib/web/controllers/auth_controller.ex +++ b/elixir/apps/web/lib/web/controllers/auth_controller.ex @@ -22,20 +22,24 @@ defmodule Web.AuthController do def verify_credentials(conn, %{ "account_id_or_slug" => account_id_or_slug, "provider_id" => provider_id, - "userpass" => %{ - "provider_identifier" => provider_identifier, - "secret" => secret - } + "userpass" => + %{ + "provider_identifier" => provider_identifier, + "secret" => secret + } = form }) do with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id), - {:ok, subject} <- Web.Auth.sign_in(conn, provider, provider_identifier, secret) do - redirect_to = get_session(conn, :user_return_to) || Auth.signed_in_path(subject) - - conn - |> Web.Auth.renew_session() - |> Web.Auth.put_subject_in_session(subject) - |> delete_session(:user_return_to) - |> redirect(to: redirect_to) + {:ok, subject} <- + Domain.Auth.sign_in( + provider, + provider_identifier, + secret, + conn.assigns.user_agent, + conn.remote_ip + ) do + client_platform = form["client_platform"] + client_csrf_token = form["client_csrf_token"] + Web.Auth.signed_in_redirect(conn, subject, client_platform, client_csrf_token) else {:error, :not_found} -> conn @@ -43,11 +47,6 @@ defmodule Web.AuthController do |> put_flash(:error, "You can not use this method to sign in.") |> redirect(to: "/#{account_id_or_slug}/sign_in") - {:error, :invalid_actor_type} -> - conn - |> put_flash(:info, "Please use client application to access Firezone.") - |> redirect(to: ~p"/#{account_id_or_slug}") - {:error, _reason} -> conn |> put_flash(:userpass_provider_identifier, String.slice(provider_identifier, 0, 160)) @@ -62,9 +61,10 @@ defmodule Web.AuthController do def request_magic_link(conn, %{ "account_id_or_slug" => account_id_or_slug, "provider_id" => provider_id, - "email" => %{ - "provider_identifier" => provider_identifier - } + "email" => + %{ + "provider_identifier" => provider_identifier + } = form }) do _ = with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id), @@ -75,7 +75,10 @@ defmodule Web.AuthController do |> Web.Mailer.deliver() end - redirect(conn, to: "/#{account_id_or_slug}/sign_in/providers/email/#{provider_id}") + conn + |> 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}") end @doc """ @@ -89,25 +92,27 @@ defmodule Web.AuthController do "secret" => secret }) do with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id), - {:ok, subject} <- Web.Auth.sign_in(conn, provider, identity_id, secret) do - redirect_to = get_session(conn, :user_return_to) || Auth.signed_in_path(subject) + {:ok, subject} <- + Domain.Auth.sign_in( + provider, + identity_id, + secret, + conn.assigns.user_agent, + conn.remote_ip + ) do + client_platform = get_session(conn, :client_platform) + client_csrf_token = get_session(conn, :client_csrf_token) conn - |> Web.Auth.renew_session() - |> Web.Auth.put_subject_in_session(subject) - |> delete_session(:user_return_to) - |> redirect(to: redirect_to) + |> delete_session(:client_platform) + |> delete_session(:client_csrf_token) + |> Web.Auth.signed_in_redirect(subject, client_platform, client_csrf_token) else {:error, :not_found} -> conn |> put_flash(:error, "You can not use this method to sign in.") |> redirect(to: "/#{account_id_or_slug}/sign_in") - {:error, :invalid_actor_type} -> - conn - |> put_flash(:info, "Please use client application to access Firezone.") - |> redirect(to: ~p"/#{account_id_or_slug}") - {:error, _reason} -> conn |> put_flash(:error, "The sign in link is invalid or expired.") @@ -119,11 +124,17 @@ defmodule Web.AuthController do This controller redirects user to IdP during sign in for authentication while persisting verification state to prevent various attacks on OpenID Connect. """ - def redirect_to_idp(conn, %{ - "account_id_or_slug" => account_id_or_slug, - "provider_id" => provider_id - }) do + def redirect_to_idp( + conn, + %{ + "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 + conn = put_session(conn, :client_platform, params["client_platform"]) + conn = put_session(conn, :client_csrf_token, params["client_csrf_token"]) + redirect_url = url(~p"/#{provider.account_id}/sign_in/providers/#{provider.id}/handle_callback") @@ -165,25 +176,21 @@ defmodule Web.AuthController do } with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id), - {:ok, subject} <- Web.Auth.sign_in(conn, provider, payload) do - redirect_to = get_session(conn, :user_return_to) || Auth.signed_in_path(subject) + {:ok, subject} <- + Domain.Auth.sign_in(provider, payload, conn.assigns.user_agent, conn.remote_ip) do + client_platform = get_session(conn, :client_platform) + client_csrf_token = get_session(conn, :client_csrf_token) conn - |> Web.Auth.renew_session() - |> Web.Auth.put_subject_in_session(subject) - |> delete_session(:user_return_to) - |> redirect(to: redirect_to) + |> delete_session(:client_platform) + |> delete_session(:client_csrf_token) + |> Web.Auth.signed_in_redirect(subject, client_platform, client_csrf_token) else {:error, :not_found} -> conn |> put_flash(:error, "You can not use this method to sign in.") |> redirect(to: "/#{account_id_or_slug}/sign_in") - {:error, :invalid_actor_type} -> - conn - |> put_flash(:info, "Please use client application to access Firezone.") - |> redirect(to: ~p"/#{account_id_or_slug}") - {:error, _reason} -> conn |> put_flash(:error, "You can not authenticate to this account.") diff --git a/elixir/apps/web/lib/web/live/auth/sign_in.ex b/elixir/apps/web/lib/web/live/auth/sign_in.ex index 8079f5272..e753bc46a 100644 --- a/elixir/apps/web/lib/web/live/auth/sign_in.ex +++ b/elixir/apps/web/lib/web/live/auth/sign_in.ex @@ -2,7 +2,7 @@ defmodule Web.Auth.SignIn do use Web, {:live_view, layout: {Web.Layouts, :public}} alias Domain.{Auth, Accounts} - def mount(%{"account_id_or_slug" => account_id_or_slug}, _session, socket) do + def mount(%{"account_id_or_slug" => account_id_or_slug} = params, _session, socket) do with {:ok, account} <- Accounts.fetch_account_by_id_or_slug(account_id_or_slug), {:ok, [_ | _] = providers} <- Auth.list_active_providers_for_account(account) do providers_by_adapter = @@ -19,6 +19,7 @@ defmodule Web.Auth.SignIn do {:ok, socket, temporary_assigns: [ + params: Map.take(params, ["client_platform", "client_csrf_token"]), account: account, providers_by_adapter: providers_by_adapter, page_title: "Sign in" @@ -53,6 +54,7 @@ defmodule Web.Auth.SignIn do <.providers_group_form adapter="openid_connect" providers={@providers_by_adapter[:openid_connect]} + params={@params} /> @@ -65,6 +67,7 @@ defmodule Web.Auth.SignIn do adapter="userpass" provider={List.first(@providers_by_adapter[:userpass])} flash={@flash} + params={@params} /> @@ -77,6 +80,7 @@ defmodule Web.Auth.SignIn do adapter="email" provider={List.first(@providers_by_adapter[:email])} flash={@flash} + params={@params} /> @@ -100,7 +104,7 @@ defmodule Web.Auth.SignIn do def providers_group_form(%{adapter: "openid_connect"} = assigns) do ~H"""
- <.openid_connect_button :for={provider <- @providers} provider={provider} /> + <.openid_connect_button :for={provider <- @providers} provider={provider} params={@params} />
""" end @@ -118,6 +122,8 @@ defmodule Web.Auth.SignIn do id="userpass_form" phx-update="ignore" > + <.input :for={{key, value} <- @params} type="hidden" name={key} value={value} /> + <.input field={@userpass_form[:provider_identifier]} type="text" @@ -156,6 +162,8 @@ defmodule Web.Auth.SignIn do id="email_form" phx-update="ignore" > + <.input :for={{key, value} <- @params} type="hidden" name={key} value={value} /> + <.input field={@email_form[:provider_identifier]} type="email" @@ -175,7 +183,9 @@ defmodule Web.Auth.SignIn do def openid_connect_button(assigns) do ~H""" - + dark:border-gray-600 dark:hover:text-white dark:hover:bg-gray-700]} + > Log in with <%= @provider.name %> """ 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 ed6189592..c9b2661f0 100644 --- a/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs +++ b/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs @@ -134,7 +134,7 @@ defmodule Web.Acceptance.Auth.UserPassTest do session |> password_login_flow(account, identity.provider_identifier, password) - |> assert_path(~p"/#{account}") + |> assert_path(~p"/#{account}/") end defp password_login_flow(session, account, username, password) 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 f0ecf99b0..e7451d224 100644 --- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs +++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs @@ -214,6 +214,103 @@ defmodule Web.AuthControllerTest do assert subject.identity.last_seen_remote_ip.address == {127, 0, 0, 1} assert subject.identity.last_seen_at end + + test "redirects to the platform link when credentials are valid for account users", %{ + conn: conn + } do + account = AccountsFixtures.create_account() + provider = AuthFixtures.create_userpass_provider(account: account) + password = "Firezone1234" + + actor = + ActorsFixtures.create_actor( + type: :account_user, + account: account, + provider: provider + ) + + identity = + AuthFixtures.create_identity( + actor: actor, + account: account, + provider: provider, + provider_virtual_state: %{"password" => password, "password_confirmation" => password} + ) + + csrf_token = Ecto.UUID.generate() + + conn = + conn + |> post( + ~p"/#{provider.account_id}/sign_in/providers/#{provider.id}/verify_credentials", + %{ + "userpass" => %{ + "provider_identifier" => identity.provider_identifier, + "secret" => password, + "client_platform" => "android", + "client_csrf_token" => csrf_token + } + } + ) + + assert conn.assigns.flash == %{} + + assert is_nil(get_session(conn, :user_return_to)) + + assert redirected_to = redirected_to(conn) + assert redirected_to_uri = URI.parse(redirected_to) + assert redirected_to_uri.scheme == "https" + assert redirected_to_uri.host == "app.firez.one" + assert redirected_to_uri.path == "/handle_client_auth_callback" + + assert %{ + "client_auth_token" => _token, + "client_csrf_token" => ^csrf_token + } = URI.decode_query(redirected_to_uri.query) + end + + test "redirects account users to app install page when mobile platform is invalid", %{ + conn: conn + } do + account = AccountsFixtures.create_account() + provider = AuthFixtures.create_userpass_provider(account: account) + password = "Firezone1234" + + actor = + ActorsFixtures.create_actor( + type: :account_user, + account: account, + provider: provider + ) + + identity = + AuthFixtures.create_identity( + actor: actor, + account: account, + provider: provider, + provider_virtual_state: %{"password" => password, "password_confirmation" => password} + ) + + conn = + conn + |> post( + ~p"/#{provider.account_id}/sign_in/providers/#{provider.id}/verify_credentials", + %{ + "userpass" => %{ + "provider_identifier" => identity.provider_identifier, + "secret" => password, + "client_platform" => "platform" + } + } + ) + + assert conn.assigns.flash == %{ + "info" => "Please use a client application to access Firezone." + } + + assert redirected_to(conn) == ~p"/#{account.id}/" + assert is_nil(get_session(conn, :user_return_to)) + end end describe "request_magic_link/2" do @@ -247,6 +344,38 @@ defmodule Web.AuthControllerTest do assert redirected_to(conn) == "/#{account.id}/sign_in/providers/email/#{provider.id}" end + test "persists client platform name", %{conn: conn} do + account = AccountsFixtures.create_account() + provider = AuthFixtures.create_email_provider(account: account) + identity = AuthFixtures.create_identity(account: account, provider: provider) + + conn = + post( + conn, + ~p"/#{provider.account_id}/sign_in/providers/#{provider.id}/request_magic_link", + %{ + "email" => %{ + "provider_identifier" => identity.provider_identifier, + "client_platform" => "platform" + } + } + ) + + 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=" + end) + + assert redirected_to(conn) == "/#{account.id}/sign_in/providers/email/#{provider.id}" + assert get_session(conn, :client_platform) == "platform" + end + test "does not return error if provider is not found", %{conn: conn} do account = AccountsFixtures.create_account() provider_id = Ecto.UUID.generate() @@ -374,6 +503,32 @@ defmodule Web.AuthControllerTest do assert redirected_to(conn) == "/#{account.id}/dashboard" end + test "redirects to the platform link when credentials are valid for account users", %{ + conn: conn + } do + account = AccountsFixtures.create_account() + provider = AuthFixtures.create_email_provider(account: account) + + identity = + AuthFixtures.create_identity( + actor_default_type: :account_user, + account: account, + provider: provider + ) + + conn = + conn + |> put_session(:client_platform, "apple") + |> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{ + "identity_id" => identity.id, + "secret" => identity.provider_virtual_state.sign_in_token + }) + + assert conn.assigns.flash == %{} + assert redirected_to(conn) =~ "firezone://handle_client_auth_callback?client_auth_token=" + assert is_nil(get_session(conn, :client_platform)) + end + test "renews the session when credentials are valid", %{conn: conn} do account = AccountsFixtures.create_account() provider = AuthFixtures.create_email_provider(account: account) @@ -455,6 +610,21 @@ defmodule Web.AuthControllerTest do "state" => state } end + + test "persists client platform name", %{conn: conn} do + account = AccountsFixtures.create_account() + + {provider, _bypass} = + AuthFixtures.start_openid_providers(["google"]) + |> AuthFixtures.create_openid_connect_provider(account: account) + + conn = + get(conn, ~p"/#{account.id}/sign_in/providers/#{provider.id}/redirect", %{ + "client_platform" => "platform" + }) + + assert get_session(conn, :client_platform) == "platform" + end end describe "handle_idp_callback/2" do @@ -595,6 +765,45 @@ defmodule Web.AuthControllerTest do assert subject.identity.last_seen_remote_ip.address == {127, 0, 0, 1} assert subject.identity.last_seen_at end + + test "redirects to the platform link when credentials are valid for account users", %{ + account: account, + provider: provider, + bypass: bypass, + conn: conn, + redirected_conn: redirected_conn + } do + identity = + AuthFixtures.create_identity( + actor_default_type: :account_user, + account: account, + provider: provider + ) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + cookie_key = "fz_auth_state_#{provider.id}" + redirected_conn = fetch_cookies(redirected_conn) + {state, _verifier} = redirected_conn.cookies[cookie_key] |> :erlang.binary_to_term([:safe]) + %{value: signed_state} = redirected_conn.resp_cookies[cookie_key] + + conn = + conn + |> put_req_cookie(cookie_key, signed_state) + |> put_session(:foo, "bar") + |> put_session(:preferred_locale, "en_US") + |> put_session(:client_platform, "apple") + |> get(~p"/#{account.id}/sign_in/providers/#{provider.id}/handle_callback", %{ + "state" => state, + "code" => "MyFakeCode" + }) + + assert conn.assigns.flash == %{} + assert redirected_to(conn) =~ "firezone://handle_client_auth_callback?client_auth_token=" + assert is_nil(get_session(conn, :client_platform)) + end end describe "sign_out/2" do diff --git a/elixir/config/config.exs b/elixir/config/config.exs index 316b644e0..a1d0e9327 100644 --- a/elixir/config/config.exs +++ b/elixir/config/config.exs @@ -90,6 +90,12 @@ config :web, external_trusted_proxies: [], private_clients: [%{__struct__: Postgrex.INET, address: {172, 28, 0, 0}, netmask: 16}] +config :web, Web.Auth, + platform_redirect_urls: %{ + "apple" => "firezone://handle_client_auth_callback", + "android" => "https://app.firez.one/handle_client_auth_callback" + } + config :web, Web.Plugs.SecureHeaders, csp_policy: [ "default-src 'self' 'nonce-${nonce}'", diff --git a/elixir/config/runtime.exs b/elixir/config/runtime.exs index f1ef799d2..b15d81c9b 100644 --- a/elixir/config/runtime.exs +++ b/elixir/config/runtime.exs @@ -81,6 +81,12 @@ if config_env() == :prod do cookie_signing_salt: compile_config!(:cookie_signing_salt), cookie_encryption_salt: compile_config!(:cookie_encryption_salt) + config :web, Web.Auth, + platform_redirect_urls: %{ + "apple" => "firezone://handle_client_auth_callback", + "android" => "#{external_url_scheme}://#{external_url_host}/handle_client_auth_callback" + } + ############################### ##### API ##################### ###############################