github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Protect the magic links from account spoofing (#1990)

Browse Source 
Closes https://github.com/firezone/product/issues/644
This commit is contained in:
Andrew Dryga
2023-09-07 18:23:33 -06:00
committed by GitHub
parent ed94e41050
commit 8398e3013b
21 changed files with 620 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
elixir/README.md
View File

@@ -189,7 +189,7 @@ executing this example:
:ok = Domain.Gateways.connect_gateway(gateway)
[relay | _rest_relays] = Domain.Repo.all(Domain.Relays.Relay)
relay_secret = Domain.Crypto.rand_string()
relay_secret = Domain.Crypto.random_token()
:ok = Domain.Relays.connect_relay(relay, relay_secret)
```

4
elixir/apps/api/test/api/relay/channel_test.exs
View File

@@ -4,7 +4,7 @@ defmodule API.Relay.ChannelTest do
setup do
relay = Fixtures.Relays.create_relay()
stamp_secret = Domain.Crypto.rand_string()
stamp_secret = Domain.Crypto.random_token()
{:ok, _, socket} =
API.Relay.Socket
@@ -26,7 +26,7 @@ defmodule API.Relay.ChannelTest do
group = Fixtures.Relays.create_global_group()
relay = Fixtures.Relays.create_relay(group: group)
stamp_secret = Domain.Crypto.rand_string()
stamp_secret = Domain.Crypto.random_token()
{:ok, _, _socket} =
API.Relay.Socket

15
elixir/apps/domain/lib/domain/auth.ex
View File

@@ -448,10 +448,23 @@ defmodule Domain.Auth do
end
end
def fetch_identity_by_provider_and_identifier(%Provider{} = provider, provider_identifier) do
def fetch_identity_by_provider_and_identifier(
%Provider{} = provider,
provider_identifier,
opts \\ []
) do
{preload, _opts} = Keyword.pop(opts, :preload, [])
Identity.Query.by_provider_id(provider.id)
|> Identity.Query.by_provider_identifier(provider_identifier)
|> Repo.fetch()
|> case do
{:ok, identity} ->
{:ok, Repo.preload(identity, preload)}
{:error, reason} ->
{:error, reason}
end
end
@doc false

39
elixir/apps/domain/lib/domain/auth/adapters/email.ex
View File

@@ -7,6 +7,7 @@ defmodule Domain.Auth.Adapters.Email do
@behaviour Adapter.Local
@sign_in_token_expiration_seconds 15 * 60
@sign_in_token_max_attempts 5
def start_link(_init_arg) do
Supervisor.start_link(__MODULE__, nil, name: __MODULE__)
@@ -64,11 +65,16 @@ defmodule Domain.Auth.Adapters.Email do
end
defp identity_create_state(%Provider{} = _provider) do
sign_in_token = Domain.Crypto.rand_string()
email_token = Domain.Crypto.random_token(5, encoder: :user_friendly)
nonce = Domain.Crypto.random_token(27)
sign_in_token = String.downcase(email_token) <> nonce
salt = Domain.Crypto.random_token(16)
{
%{
"sign_in_token_hash" => Domain.Crypto.hash(sign_in_token),
"sign_in_token_salt" => salt,
"sign_in_token_hash" => Domain.Crypto.hash(sign_in_token <> salt),
"sign_in_token_created_at" => DateTime.utc_now()
},
%{
@@ -100,18 +106,25 @@ defmodule Domain.Auth.Adapters.Email do
identity.provider_state["sign_in_token_created_at"] ||
identity.provider_state[:sign_in_token_created_at]
sign_in_token_salt =
identity.provider_state["sign_in_token_salt"] ||
identity.provider_state[:sign_in_token_salt]
cond do
is_nil(sign_in_token_hash) ->
:invalid_secret
is_nil(sign_in_token_salt) ->
:invalid_secret
is_nil(sign_in_token_created_at) ->
:invalid_secret
sign_in_token_expired?(sign_in_token_created_at) ->
:expired_secret
not Domain.Crypto.equal?(token, sign_in_token_hash) ->
:invalid_secret
not Domain.Crypto.equal?(token <> sign_in_token_salt, sign_in_token_hash) ->
track_failed_attempt!(identity)
true ->
Identity.Changeset.update_identity_provider_state(identity, %{}, %{})
@@ -124,6 +137,24 @@ defmodule Domain.Auth.Adapters.Email do
end
end
defp track_failed_attempt!(%Identity{} = identity) do
attempts = identity.provider_state["sign_in_failed_attempts"] || 0
attempt = attempts + 1
{error, provider_state} =
if attempt > @sign_in_token_max_attempts do
{:invalid_secret, %{}}
else
provider_state = Map.put(identity.provider_state, "sign_in_failed_attempts", attempts + 1)
{:invalid_secret, provider_state}
end
Identity.Changeset.update_identity_provider_state(identity, provider_state, %{})
|> Repo.update!()
error
end
defp sign_in_token_expired?(%DateTime{} = sign_in_token_created_at) do
now = DateTime.utc_now()
DateTime.diff(now, sign_in_token_created_at, :second) > @sign_in_token_expiration_seconds

2
elixir/apps/domain/lib/domain/auth/adapters/openid_connect/state.ex
View File

@@ -1,6 +1,6 @@
defmodule Domain.Auth.Adapters.OpenIDConnect.State do
def new do
Domain.Crypto.rand_string()
Domain.Crypto.random_token(32)
end
def equal?(state1, state2) do

2
elixir/apps/domain/lib/domain/auth/adapters/token.ex
View File

@@ -38,7 +38,7 @@ defmodule Domain.Auth.Adapters.Token do
end
defp put_hash_and_expiration(changeset) do
secret = Domain.Crypto.rand_token(32)
secret = Domain.Crypto.random_token(32)
secret_hash = Domain.Crypto.hash(secret)
data = Map.get(changeset.data, :provider_virtual_state) || %{}

62
elixir/apps/domain/lib/domain/crypto.ex
View File

@@ -2,29 +2,65 @@ defmodule Domain.Crypto do
@wg_psk_length 32
def psk do
rand_base64(@wg_psk_length)
random_token(@wg_psk_length, encoder: :base64)
end
def rand_string(length \\ 16) do
rand_base64(length, :url)
|> binary_part(0, length)
def random_token(length \\ 16, opts \\ []) do
generator = Keyword.get(opts, :generator, :binary)
default_encoder = if generator == :numeric, do: :raw, else: :url_encode64
encoder = Keyword.get(opts, :encoder, default_encoder)
generate_random_token(length, generator)
|> encode_random_token(length, encoder)
end
def rand_token(length \\ 8) do
rand_base64(length, :url)
defp generate_random_token(bytes, :binary) do
:crypto.strong_rand_bytes(bytes)
end
defp rand_base64(length, :url) do
:crypto.strong_rand_bytes(length)
# XXX: we want to add `padding: false` to shorten URLs
|> Base.url_encode64()
defp generate_random_token(length, :numeric) do
n =
:math.pow(10, length)
|> round()
|> :rand.uniform()
|> floor()
|> Kernel.-(1)
:io_lib.format("~#{length}..0B", [n])
|> List.to_string()
end
defp rand_base64(length) do
:crypto.strong_rand_bytes(length)
|> Base.encode64()
defp encode_random_token(binary, _length, :raw) do
binary
end
defp encode_random_token(binary, _length, :url_encode64) do
Base.url_encode64(binary, padding: false)
end
defp encode_random_token(binary, _length, :base64) do
Base.encode64(binary)
end
defp encode_random_token(binary, length, :user_friendly) do
encode_random_token(binary, length, :url_encode64)
|> String.downcase()
|> replace_ambiguous_characters()
|> String.slice(0, length)
end
defp replace_ambiguous_characters(binary, acc \\ "")
defp replace_ambiguous_characters("", acc), do: acc
for {mapping, replacement} <- Enum.zip(~c"-+/lO0=", ~c"ptusxyz") do
defp replace_ambiguous_characters(<<unquote(mapping)::utf8, rest::binary>>, acc),
do: replace_ambiguous_characters(rest, <<acc::binary, unquote(replacement)::utf8>>)
end
defp replace_ambiguous_characters(<<char::utf8, rest::binary>>, acc),
do: replace_ambiguous_characters(rest, <<acc::binary, char::utf8>>)
def hash(value), do: Argon2.hash_pwd_salt(value)
def equal?(token, hash) when is_nil(token) or is_nil(hash), do: Argon2.no_user_verify()

4
elixir/apps/domain/lib/domain/gateways/gateway/changeset.ex
View File

@@ -23,7 +23,9 @@ defmodule Domain.Gateways.Gateway.Changeset do
def upsert(%Gateways.Token{} = token, attrs) do
%Gateways.Gateway{}
|> cast(attrs, @upsert_fields)
|> put_default_value(:name_suffix, fn -> Domain.Crypto.rand_string(5) end)
|> put_default_value(:name_suffix, fn ->
Domain.Crypto.random_token(5, encoder: :user_friendly)
end)
|> changeset()
|> validate_required(@required_fields)
|> validate_base64(:public_key)

2
elixir/apps/domain/lib/domain/gateways/token/changeset.ex
View File

@@ -8,7 +8,7 @@ defmodule Domain.Gateways.Token.Changeset do
%Gateways.Token{}
|> change()
|> put_change(:account_id, account.id)
|> put_change(:value, Domain.Crypto.rand_string(64))
|> put_change(:value, Domain.Crypto.random_token(64))
|> put_hash(:value, to: :hash)
|> assoc_constraint(:group)
|> check_constraint(:hash, name: :hash_not_null, message: "can't be blank")

2
elixir/apps/domain/lib/domain/relays.ex
View File

@@ -279,7 +279,7 @@ defmodule Domain.Relays do
def generate_username_and_password(%Relay{stamp_secret: stamp_secret}, %DateTime{} = expires_at)
when is_binary(stamp_secret) do
expires_at = DateTime.to_unix(expires_at, :second)
salt = Domain.Crypto.rand_string()
salt = Domain.Crypto.random_token()
password = generate_hash(expires_at, stamp_secret, salt)
%{username: "#{expires_at}:#{salt}", password: password, expires_at: expires_at}
end

2
elixir/apps/domain/lib/domain/relays/token/changeset.ex
View File

@@ -7,7 +7,7 @@ defmodule Domain.Relays.Token.Changeset do
def create do
%Relays.Token{}
|> change()
|> put_change(:value, Domain.Crypto.rand_string(64))
|> put_change(:value, Domain.Crypto.random_token(64))
|> put_hash(:value, to: :hash)
|> assoc_constraint(:group)
|> check_constraint(:hash, name: :hash_not_null, message: "can't be blank")

4
elixir/apps/domain/priv/repo/seeds.exs
View File

@@ -350,7 +350,7 @@ IO.puts("")
{:ok, gateway1} =
Gateways.upsert_gateway(gateway_group_token, %{
external_id: Ecto.UUID.generate(),
name_suffix: "gw-#{Domain.Crypto.rand_string(5)}",
name_suffix: "gw-#{Domain.Crypto.random_token(5, encoder: :user_friendly)}",
public_key: :crypto.strong_rand_bytes(32) |> Base.encode64(),
last_seen_user_agent: "iOS/12.7 (iPhone) connlib/0.7.412",
last_seen_remote_ip: %Postgrex.INET{address: {189, 172, 73, 153}}
@@ -359,7 +359,7 @@ IO.puts("")
{:ok, gateway2} =
Gateways.upsert_gateway(gateway_group_token, %{
external_id: Ecto.UUID.generate(),
name_suffix: "gw-#{Domain.Crypto.rand_string(5)}",
name_suffix: "gw-#{Domain.Crypto.random_token(5, encoder: :user_friendly)}",
public_key: :crypto.strong_rand_bytes(32) |> Base.encode64(),
last_seen_user_agent: "iOS/12.7 (iPhone) connlib/0.7.412",
last_seen_remote_ip: %Postgrex.INET{address: {164, 112, 78, 62}}

27
elixir/apps/domain/test/domain/auth/adapters/email_test.exs
View File

@@ -24,12 +24,13 @@ defmodule Domain.Auth.Adapters.EmailTest do
assert %{
provider_state: %{
"sign_in_token_created_at" => %DateTime{},
"sign_in_token_hash" => sign_in_token_hash
"sign_in_token_hash" => sign_in_token_hash,
"sign_in_token_salt" => sign_in_token_salt
},
provider_virtual_state: %{sign_in_token: sign_in_token}
} = changeset.changes
assert Domain.Crypto.equal?(sign_in_token, sign_in_token_hash)
assert Domain.Crypto.equal?(sign_in_token <> sign_in_token_salt, sign_in_token_hash)
end
test "trims provider identifier", %{provider: provider, changeset: changeset} do
@@ -80,14 +81,15 @@ defmodule Domain.Auth.Adapters.EmailTest do
assert %{
"sign_in_token_created_at" => sign_in_token_created_at,
"sign_in_token_hash" => sign_in_token_hash
"sign_in_token_hash" => sign_in_token_hash,
"sign_in_token_salt" => sign_in_token_salt
} = identity.provider_state
assert %{
sign_in_token: sign_in_token
} = identity.provider_virtual_state
assert Domain.Crypto.equal?(sign_in_token, sign_in_token_hash)
assert Domain.Crypto.equal?(sign_in_token <> sign_in_token_salt, sign_in_token_hash)
assert %DateTime{} = sign_in_token_created_at
end
end
@@ -114,6 +116,18 @@ defmodule Domain.Auth.Adapters.EmailTest do
assert identity.provider_virtual_state == %{}
end
test "removes token after 5 failed attempts", %{
identity: identity
} do
for i <- 1..5 do
assert verify_secret(identity, "foo") == {:error, :invalid_secret}
assert %{"sign_in_failed_attempts" => ^i} = Repo.one(Auth.Identity).provider_state
end
assert verify_secret(identity, "foo") == {:error, :invalid_secret}
assert Repo.one(Auth.Identity).provider_state == %{}
end
test "returns error when token is expired", %{
account: account,
provider: provider
@@ -125,8 +139,9 @@ defmodule Domain.Auth.Adapters.EmailTest do
account: account,
provider: provider,
provider_state: %{
"sign_in_token_hash" => Domain.Crypto.hash("dummy_token"),
"sign_in_token_created_at" => DateTime.to_iso8601(forty_seconds_ago)
"sign_in_token_hash" => Domain.Crypto.hash("dummy_token" <> "salty"),
"sign_in_token_created_at" => DateTime.to_iso8601(forty_seconds_ago),
"sign_in_token_salt" => "salty"
}
)

64
elixir/apps/domain/test/domain/crypto_test.exs
View File

@@ -8,38 +8,44 @@ defmodule Domain.CryptoTest do
end
end
describe "rand_string/1" do
test "it returns a string of default length" do
assert 16 == String.length(rand_string())
describe "random_token/2" do
test "generates random number" do
assert random_token(16, generator: :numeric) != random_token(16, generator: :numeric)
end
test "it returns a string of proper length" do
for length <- [1, 32, 32_768] do
assert length == String.length(rand_string(length))
test "generates random string" do
assert random_token(16, generator: :binary) != random_token(16, generator: :binary)
end
test "generates a random number of given length" do
for length <- [1, 2, 4, 16, 32], _i <- 0..100 do
assert length == String.length(random_token(length, generator: :numeric))
end
end
test "generates a random binary of given byte size" do
for length <- [1, 2, 4, 16, 32], _i <- 0..100 do
assert length == byte_size(random_token(length, encoder: :raw))
end
end
test "returns base64 url encoded token by default" do
# 2 padding bytes are stripped
assert String.length(random_token(1)) == 2
assert String.length(random_token(3)) == 4
end
test "returns base64 encoded token doesn't remove padding" do
assert String.length(random_token(1, encoder: :base64)) == 4
end
test "user friendly encoder does not print ambiguous or upcased characters" do
for _ <- 1..100 do
token = random_token(16, encoder: :user_friendly)
assert String.downcase(token) == token
assert String.printable?(token)
refute String.contains?(token, ["-", "+", "/", "l", "I", "O", "0"])
end
end
end
describe "rand_token/1" do
test "returns a token of default length" do
# 8 bytes is 12 chars in Base64
assert 12 == String.length(rand_token())
end
test "returns a token of length 4 when bytes is 1" do
assert 4 == String.length(rand_token(1))
end
test "returns a token of length 4 when bytes is 3" do
assert 4 == String.length(rand_token(3))
end
test "returns a token of length 40_000 when bytes is 32_768" do
assert 44 == String.length(rand_token(32))
end
test "returns a token of length 44 when bytes is 32" do
assert 43_692 == String.length(rand_token(32_768))
end
end
end

2
elixir/apps/domain/test/support/fixtures/gateways.ex
View File

@@ -72,7 +72,7 @@ defmodule Domain.Fixtures.Gateways do
def gateway_attrs(attrs \\ %{}) do
Enum.into(attrs, %{
external_id: Ecto.UUID.generate(),
name_suffix: "gw-#{Domain.Crypto.rand_string(5)}",
name_suffix: "gw-#{Domain.Crypto.random_token(5, encoder: :user_friendly)}",
public_key: unique_public_key(),
last_seen_user_agent: "iOS/12.7 (iPhone) connlib/0.7.412",
last_seen_remote_ip: %Postgrex.INET{address: {189, 172, 73, 153}}

90
elixir/apps/web/lib/web/controllers/auth_controller.ex
View File

@@ -3,6 +3,17 @@ defmodule Web.AuthController do
alias Web.Auth
alias Domain.Auth.Adapters.OpenIDConnect
# This is the cookie which will store recent account ids
# that the user has signed in to.
@remember_me_cookie_name "fz_recent_account_ids"
@remember_me_cookie_options [
sign: true,
max_age: 365 * 24 * 60 * 60,
same_site: "Lax",
secure: true,
http_only: true
]
# This is the cookie which will be used to store the
# state and code verifier for OpenID Connect IdP's
@state_cookie_key_prefix "fz_auth_state_"
@@ -41,7 +52,10 @@ defmodule Web.AuthController do
) do
client_platform = params["client_platform"]
client_csrf_token = params["client_csrf_token"]
Web.Auth.signed_in_redirect(conn, subject, client_platform, client_csrf_token)
conn
|> persist_recent_account(subject.account)
|> Web.Auth.signed_in_redirect(subject, client_platform, client_csrf_token)
else
{:error, :not_found} ->
conn
@@ -70,15 +84,31 @@ defmodule Web.AuthController do
}
} = params
) do
_ =
conn =
with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id),
{:ok, identity} <-
Domain.Auth.fetch_identity_by_provider_and_identifier(provider, provider_identifier),
Domain.Auth.fetch_identity_by_provider_and_identifier(provider, provider_identifier,
preload: :account
),
{:ok, identity} <- Domain.Auth.Adapters.Email.request_sign_in_token(identity) do
sign_in_link_params = Map.take(params, ["client_platform", "client_csrf_token"])
Web.Mailer.AuthEmail.sign_in_link_email(identity, sign_in_link_params)
|> Web.Mailer.deliver()
<<email_secret::binary-size(5), nonce::binary>> =
identity.provider_virtual_state.sign_in_token
{:ok, _} =
Web.Mailer.AuthEmail.sign_in_link_email(
identity,
email_secret,
conn.assigns.user_agent,
conn.remote_ip,
sign_in_link_params
)
|> Web.Mailer.deliver()
put_session(conn, :sign_in_nonce, nonce)
else
_ -> conn
end
redirect_params =
@@ -110,15 +140,16 @@ defmodule Web.AuthController do
"account_id_or_slug" => account_id_or_slug,
"provider_id" => provider_id,
"identity_id" => identity_id,
"secret" => secret
"secret" => email_secret
} = params
) do
with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id),
nonce = get_session(conn, :sign_in_nonce) || "=",
{:ok, subject} <-
Domain.Auth.sign_in(
provider,
identity_id,
secret,
String.downcase(email_secret) <> nonce,
conn.assigns.user_agent,
conn.remote_ip
) do
@@ -128,6 +159,8 @@ defmodule Web.AuthController do
conn
|> delete_session(:client_platform)
|> delete_session(:client_csrf_token)
|> delete_session(:sign_in_nonce)
|> persist_recent_account(subject.account)
|> Web.Auth.signed_in_redirect(subject, client_platform, client_csrf_token)
else
{:error, :not_found} ->
@@ -206,6 +239,8 @@ defmodule Web.AuthController do
conn
|> delete_session(:client_platform)
|> delete_session(:client_csrf_token)
|> delete_session(:sign_in_nonce)
|> persist_recent_account(subject.account)
|> Web.Auth.signed_in_redirect(subject, client_platform, client_csrf_token)
else
{:error, :not_found} ->
@@ -246,6 +281,7 @@ defmodule Web.AuthController do
def sign_out(conn, %{"account_id_or_slug" => account_id_or_slug}) do
# TODO: post logout redirect url
conn
|> maybe_delete_recent_account()
|> Auth.sign_out()
|> redirect(to: ~p"/#{account_id_or_slug}/sign_in")
end
@@ -270,4 +306,44 @@ defmodule Web.AuthController do
# |> Plug.Conn.configure_session(drop: true)
# |> Phoenix.Controller.redirect(to: ~p"/")
# end
defp maybe_delete_recent_account(%{assigns: %{subject: subject}} = conn) do
update_recent_accounts(conn, fn recent_account_ids ->
recent_account_ids -- [subject.account.id]
end)
end
defp maybe_delete_recent_account(conn) do
conn
end
defp persist_recent_account(conn, %Domain.Accounts.Account{} = account) do
update_recent_accounts(conn, fn recent_account_ids ->
[account.id] ++ recent_account_ids
end)
end
defp update_recent_accounts(conn, callback) when is_function(callback, 1) do
conn = fetch_cookies(conn, signed: [@remember_me_cookie_name])
recent_account_ids =
if recent_account_ids = Map.get(conn.cookies, @remember_me_cookie_name) do
:erlang.binary_to_term(recent_account_ids, [:safe])
else
[]
end
recent_account_ids =
recent_account_ids
|> callback.()
|> Enum.take(5)
|> :erlang.term_to_binary()
put_resp_cookie(
conn,
@remember_me_cookie_name,
recent_account_ids,
@remember_me_cookie_options
)
end
end

23
elixir/apps/web/lib/web/mailer/auth_email.ex
View File

@@ -6,25 +6,38 @@ defmodule Web.Mailer.AuthEmail do
embed_templates "auth_email/*.html", suffix: "_html"
embed_templates "auth_email/*.text", suffix: "_text"
def sign_in_link_email(%Domain.Auth.Identity{} = identity, params \\ %{}) do
def sign_in_link_email(
%Domain.Auth.Identity{} = identity,
email_secret,
user_agent,
remote_ip,
params \\ %{}
) do
params =
Map.merge(params, %{
identity_id: identity.id,
secret: identity.provider_virtual_state.sign_in_token
secret: email_secret
})
sign_in_link =
url(
~p"/#{identity.account_id}/sign_in/providers/#{identity.provider_id}/verify_sign_in_token?#{params}"
~p"/#{identity.account}/sign_in/providers/#{identity.provider_id}/verify_sign_in_token?#{params}"
)
default_email()
|> subject("Firezone Sign In Link")
|> to(identity.provider_identifier)
|> render_body(__MODULE__, :sign_in_link,
account: identity.account,
client_platform: params["client_platform"],
secret: identity.provider_virtual_state.sign_in_token,
link: sign_in_link
sign_in_token_created_at:
Cldr.DateTime.to_string!(identity.provider_state["sign_in_token_created_at"], Web.CLDR,
format: :short
) <> " UTC",
secret: email_secret,
link: sign_in_link,
user_agent: user_agent,
remote_ip: "#{:inet.ntoa(remote_ip)}"
)
end
end

31
elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex
View File

@@ -7,8 +7,8 @@
<div :if={is_nil(@client_platform)}>
<p>
Here is the <a href={@link} target="_blank">magic sign-in link</a>
you requested. It is valid for 1 hour.
If you didn't request this, you can safely discard this email.
you requested to sign in to <b>"<%= @account.name %>"</b>.
It is valid for 15 minutes.
</p>
<small>
@@ -18,14 +18,25 @@
<div :if={not is_nil(@client_platform)}>
<p>
Please copy the code and paste it into the Firezone application to proceed with the login:
<div style="font-weight:bold; margin-top:1rem; margin-bottom:1rem;">
<code><%= @secret %></code>
</div>
It is valid for 1 hour.
Please copy the code and paste it into the Firezone application to proceed with
signing in to <b>"<%= @account.name %>"</b>:
</p>
<p>
If you didn't request this, you can safely discard this email.
<p style="font-weight:bold; margin-top:1rem; margin-bottom:1rem;">
<code><%= @secret %></code>
</p>
<p>It is valid for 15 minutes.</p>
</div>
<p>
If you did not request this action and have received this email in error, you can safely ignore
and discard this email. However, if you continue to receive multiple unsolicited emails of this nature,
we strongly recommend contacting your system administrator to report the issue.
</p>
<p>
<b>Request details:</b>
<br /> Time: <%= @sign_in_token_created_at %>
<br /> IP address: <%= @remote_ip %>
<br /> User Agent: <%= @user_agent %>
<br /> Account ID: <%= @account.id %>
</p>

19
elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex
View File

@@ -1,15 +1,24 @@
Dear Firezone user,
<%= if is_nil(@client_platform) do %>
Here is the magic sign-in link you requested:
Here is the magic sign-in link you requested to sign in to "<%= @account.name %>":
<%= @link %>
Please copy this link and open it in your browser. It is valid for 1 hour.
Please copy this link and open it in your browser. It is valid for 15 minutes.
<% else %>
Please copy the code and paste it into the Firezone application to proceed with the login:
Please copy the code and paste it into the Firezone application to proceed with
the sign in to "<%= @account.name %>":
<%= @secret %>
It is valid for 1 hour.
It is valid for 15 minutes.
<% end %>
If you didn't request this, you can safely discard this email.
If you did not request this action and have received this email in error, you can safely ignore
and discard this email. However, if you continue to receive multiple unsolicited emails of this nature,
we strongly recommend contacting your system administrator to report the issue.
Request details:
Time: <%= @sign_in_token_created_at %>
IP address: <%= @remote_ip %>
User Agent: <%= @user_agent %>
Account ID: <%= @account.id %>

2
elixir/apps/web/lib/web/plugs/secure_headers.ex
View File

@@ -10,7 +10,7 @@ defmodule Web.Plugs.SecureHeaders do
end
defp put_csp_nonce_and_header(conn) do
csp_nonce = Domain.Crypto.rand_string(8)
csp_nonce = Domain.Crypto.random_token(8)
policy =
Application.fetch_env!(:web, __MODULE__)

321
elixir/apps/web/test/web/controllers/auth_controller_test.exs
View File

@@ -310,6 +310,45 @@ defmodule Web.AuthControllerTest do
assert redirected_to(conn) == ~p"/#{account.id}/"
assert is_nil(get_session(conn, :user_return_to))
end
test "persists account into list of recent accounts when credentials are valid", %{
conn: conn
} do
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_userpass_provider(account: account)
password = "Firezone1234"
actor =
Fixtures.Actors.create_actor(
type: :account_user,
account: account,
provider: provider
)
identity =
Fixtures.Auth.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 %{"fz_recent_account_ids" => fz_recent_account_ids} = conn.cookies
assert :erlang.binary_to_term(fz_recent_account_ids) == [identity.account_id]
end
end
describe "request_magic_link/2" do
@@ -458,6 +497,67 @@ defmodule Web.AuthControllerTest do
assert flash(conn, :error) == "The sign in link is invalid or expired."
end
test "redirects back to the form when browser token is invalid", %{conn: conn} do
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
actor =
Fixtures.Actors.create_actor(
type: :account_admin_user,
account: account,
provider: provider
)
identity = Fixtures.Auth.create_identity(account: account, provider: provider, actor: actor)
{email_token, _sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:sign_in_nonce, "foo")
|> put_session(:user_return_to, "/foo/bar")
|> get(
~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token",
%{
"identity_id" => identity.id,
"secret" => email_token
}
)
assert redirected_to(conn) == "/#{account.id}/sign_in"
assert flash(conn, :error) == "The sign in link is invalid or expired."
end
test "redirects back to the form when browser token is not set", %{conn: conn} do
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
actor =
Fixtures.Actors.create_actor(
type: :account_admin_user,
account: account,
provider: provider
)
identity = Fixtures.Auth.create_identity(account: account, provider: provider, actor: actor)
{email_token, _sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:user_return_to, "/foo/bar")
|> get(
~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token",
%{
"identity_id" => identity.id,
"secret" => email_token
}
)
assert redirected_to(conn) == "/#{account.id}/sign_in"
assert flash(conn, :error) == "The sign in link is invalid or expired."
end
test "redirects to the return to path when credentials are valid", %{conn: conn} do
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
@@ -471,14 +571,49 @@ defmodule Web.AuthControllerTest do
identity = Fixtures.Auth.create_identity(account: account, provider: provider, actor: actor)
{email_token, sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> put_session(:user_return_to, "/foo/bar")
|> get(
~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token",
%{
"identity_id" => identity.id,
"secret" => identity.provider_virtual_state.sign_in_token
"secret" => email_token
}
)
assert conn.assigns.flash == %{}
assert redirected_to(conn) == "/foo/bar"
assert is_nil(get_session(conn, :user_return_to))
end
test "emailed part of the token is not case sensitive", %{conn: conn} do
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
actor =
Fixtures.Actors.create_actor(
type: :account_admin_user,
account: account,
provider: provider
)
identity = Fixtures.Auth.create_identity(account: account, provider: provider, actor: actor)
{email_token, sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> put_session(:user_return_to, "/foo/bar")
|> get(
~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token",
%{
"identity_id" => identity.id,
"secret" => String.upcase(email_token)
}
)
@@ -500,10 +635,14 @@ defmodule Web.AuthControllerTest do
provider: provider
)
{email_token, sign_in_nonce} = split_token(identity)
conn =
get(conn, ~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{
"identity_id" => identity.id,
"secret" => identity.provider_virtual_state.sign_in_token
"secret" => email_token
})
assert redirected_to(conn) == "/#{account.slug}/dashboard"
@@ -522,12 +661,15 @@ defmodule Web.AuthControllerTest do
provider: provider
)
{email_token, sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> 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
"secret" => email_token
})
assert conn.assigns.flash == %{}
@@ -556,11 +698,14 @@ defmodule Web.AuthControllerTest do
provider: provider
)
{email_token, sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{
"identity_id" => identity.id,
"secret" => identity.provider_virtual_state.sign_in_token,
"secret" => email_token,
"client_platform" => "apple"
})
@@ -590,16 +735,19 @@ defmodule Web.AuthControllerTest do
identity = Fixtures.Auth.create_identity(account: account, provider: provider, actor: actor)
{email_token, sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:foo, "bar")
|> put_session(:session_token, "foo")
|> put_session(:preferred_locale, "en_US")
|> put_session(:sign_in_nonce, sign_in_nonce)
|> get(
~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token",
%{
"identity_id" => identity.id,
"secret" => identity.provider_virtual_state.sign_in_token
"secret" => email_token
}
)
@@ -616,6 +764,33 @@ defmodule Web.AuthControllerTest do
assert subject.identity.last_seen_remote_ip.address == {127, 0, 0, 1}
assert subject.identity.last_seen_at
end
test "persists account into list of recent accounts when credentials are valid", %{
conn: conn
} do
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
identity =
Fixtures.Auth.create_identity(
actor: [type: :account_admin_user],
account: account,
provider: provider
)
{email_token, sign_in_nonce} = split_token(identity)
conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{
"identity_id" => identity.id,
"secret" => email_token
})
assert %{"fz_recent_account_ids" => fz_recent_account_ids} = conn.cookies
assert :erlang.binary_to_term(fz_recent_account_ids) == [identity.account_id]
end
end
describe "redirect_to_idp/2" do
@@ -857,6 +1032,43 @@ defmodule Web.AuthControllerTest do
assert not is_nil(query_params["client_auth_token"])
assert query_params["identity_provider_identifier"] == identity.provider_identifier
end
test "persists account into list of recent accounts when credentials are valid", %{
account: account,
provider: provider,
bypass: bypass,
conn: conn,
redirected_conn: redirected_conn
} do
identity =
Fixtures.Auth.create_identity(
actor: [type: :account_admin_user],
account: account,
provider: provider
)
{token, _claims} = Mocks.OpenIDConnect.generate_openid_connect_token(provider, identity)
Mocks.OpenIDConnect.expect_refresh_token(bypass, %{"id_token" => token})
Mocks.OpenIDConnect.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")
|> get(~p"/#{account.id}/sign_in/providers/#{provider.id}/handle_callback", %{
"state" => state,
"code" => "MyFakeCode"
})
assert %{"fz_recent_account_ids" => fz_recent_account_ids} = conn.cookies
assert :erlang.binary_to_term(fz_recent_account_ids) == [identity.account_id]
end
end
describe "sign_out/2" do
@@ -875,6 +1087,9 @@ defmodule Web.AuthControllerTest do
assert redirected_to(conn) == "/#{account.id}/sign_in"
assert conn.private.plug_session == %{"preferred_locale" => "en_US"}
assert %{"fz_recent_account_ids" => fz_recent_account_ids} = conn.cookies
assert :erlang.binary_to_term(fz_recent_account_ids) == []
end
test "broadcasts to the given live_socket_id", %{conn: conn} do
@@ -897,6 +1112,51 @@ defmodule Web.AuthControllerTest do
assert_receive %Phoenix.Socket.Broadcast{event: "disconnect", topic: ^live_socket_id}
end
test "removes current account id from list of recent ones", %{conn: conn} do
Domain.Config.put_system_env_override(:outbound_email_adapter, Swoosh.Adapters.Postmark)
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
identity =
Fixtures.Auth.create_identity(
actor: [type: :account_admin_user],
account: account,
provider: provider
)
{email_token, sign_in_nonce} = split_token(identity)
authorized_conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{
"identity_id" => identity.id,
"secret" => email_token
})
|> fetch_cookies()
recent_account_ids =
authorized_conn.cookies["fz_recent_account_ids"]
|> :erlang.binary_to_term()
assert recent_account_ids == [account.id]
%{value: signed_state} = authorized_conn.resp_cookies["fz_recent_account_ids"]
conn =
conn
|> put_req_cookie("fz_recent_account_ids", signed_state)
|> authorize_conn(identity)
|> put_session(:preferred_locale, "en_US")
|> get(~p"/#{account}/sign_out")
assert redirected_to(conn) == "/#{account.id}/sign_in"
assert conn.private.plug_session == %{"preferred_locale" => "en_US"}
assert %{"fz_recent_account_ids" => fz_recent_account_ids} = conn.cookies
assert :erlang.binary_to_term(fz_recent_account_ids) == []
end
test "works even if user is already logged out", %{conn: conn} do
account = Fixtures.Accounts.create_account()
@@ -907,6 +1167,55 @@ defmodule Web.AuthControllerTest do
assert redirected_to(conn) == "/#{account.id}/sign_in"
assert conn.private.plug_session == %{"preferred_locale" => "en_US"}
refute Map.has_key?(conn.cookies, "fz_recent_account_ids")
end
end
test "keeps up to 5 recent accounts the used signed in to", %{conn: conn} do
Domain.Config.put_system_env_override(:outbound_email_adapter, Swoosh.Adapters.Postmark)
{conn, account_ids} =
Enum.reduce(1..6, {conn, []}, fn _i, {conn, account_ids} ->
account = Fixtures.Accounts.create_account()
provider = Fixtures.Auth.create_email_provider(account: account)
identity =
Fixtures.Auth.create_identity(
actor: [type: :account_admin_user],
account: account,
provider: provider
)
{email_token, sign_in_nonce} = split_token(identity)
authorized_conn =
conn
|> put_session(:sign_in_nonce, sign_in_nonce)
|> get(~p"/#{account}/sign_in/providers/#{provider}/verify_sign_in_token", %{
"identity_id" => identity.id,
"secret" => email_token
})
|> fetch_cookies()
%{value: signed_state} = authorized_conn.resp_cookies["fz_recent_account_ids"]
conn = put_req_cookie(conn, "fz_recent_account_ids", signed_state)
{conn, [account.id] ++ account_ids}
end)
conn = %{conn | secret_key_base: Web.Endpoint.config(:secret_key_base)}
conn = fetch_cookies(conn, signed: ["fz_recent_account_ids"])
assert %{"fz_recent_account_ids" => fz_recent_account_ids} = conn.cookies
recent_account_ids = :erlang.binary_to_term(fz_recent_account_ids)
assert length(recent_account_ids) == 5
assert recent_account_ids == Enum.take(account_ids, 5)
end
defp split_token(identity, size \\ 5) do
<<email_secret::binary-size(size), browser_secret::binary>> =
identity.provider_virtual_state.sign_in_token
{email_secret, browser_secret}
end
end