refactor(portal): Refactor client login to use HTML meta refresh and cookie (#4617)

The client authentication had previously been using liveview and passing
params around using URL query params. One of the issues with using
liveview for this task was that there edge case issues on certain
clients with the websocket connection. Along with that, to have even
more security during the login process, the query param values that were
passed after the client was authenticated have been moved to an HTTP
cookie with very strict flags set.

The deep link redirection now uses a new HTTP endpoint that returns a
302 with the deep link as the location, which is triggered using a
`<meta http-equiv="refresh">` tag on the client.
This commit is contained in:
Brian Manifold
2024-04-16 15:47:16 -04:00
committed by GitHub
parent e7a20a7b16
commit 4ba3cedf37
13 changed files with 315 additions and 184 deletions

View File

@@ -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
###########################

View File

@@ -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)

View File

@@ -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

View File

@@ -0,0 +1,75 @@
defmodule Web.SignInHTML do
use Web, :html
def client_redirect(assigns) do
~H"""
<!DOCTYPE html>
<html lang="en" style="scrollbar-gutter: stable;">
<head>
<meta charset="utf-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta http-equiv="refresh" content={"0; url=#{@redirect_url}"} />
<meta name="viewport" content="width=client-width, initial-scale=1" />
<link rel="icon" href={~p"/favicon.ico"} sizes="any" />
<link rel="icon" href={~p"/images/favicon.svg"} type="image/svg+xml" />
<link rel="apple-touch-icon" href={~p"/images/apple-touch-icon.png"} />
<link rel="manifest" href={~p"/site.webmanifest"} />
<meta name="theme-color" content="#331700" />
<meta name="csrf-token" content={get_csrf_token()} />
<.live_title suffix=" · Firezone">
<%= assigns[:page_title] || "Firezone" %>
</.live_title>
<link
phx-track-static
rel="stylesheet"
nonce={@conn.private.csp_nonce}
href={~p"/assets/app.css"}
/>
</head>
<body class="bg-neutral-50">
<main class="h-auto pt-16">
<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>
</main>
</body>
</html>
"""
end
def client_auth_error(assigns) do
~H"""
<main class="h-auto pt-16">
<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 error!
</span>
</h1>
<p class="text-center">Please close this window and start the sign in process again.</p>
</div>
</div>
</div>
</section>
</main>
"""
end
end

View File

@@ -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"""
<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

View File

@@ -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

View File

@@ -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

View File

@@ -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()

View File

@@ -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

View File

@@ -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", %{

View File

@@ -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

View File

@@ -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

View File

@@ -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