diff --git a/elixir/README.md b/elixir/README.md index b8d4f2f99..ccbcf5f06 100644 --- a/elixir/README.md +++ b/elixir/README.md @@ -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) ``` diff --git a/elixir/apps/api/test/api/relay/channel_test.exs b/elixir/apps/api/test/api/relay/channel_test.exs index 17588268a..5ad1b754c 100644 --- a/elixir/apps/api/test/api/relay/channel_test.exs +++ b/elixir/apps/api/test/api/relay/channel_test.exs @@ -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 diff --git a/elixir/apps/domain/lib/domain/auth.ex b/elixir/apps/domain/lib/domain/auth.ex index e2bcceb43..cedb78d97 100644 --- a/elixir/apps/domain/lib/domain/auth.ex +++ b/elixir/apps/domain/lib/domain/auth.ex @@ -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 diff --git a/elixir/apps/domain/lib/domain/auth/adapters/email.ex b/elixir/apps/domain/lib/domain/auth/adapters/email.ex index 070cae730..205c3dd7e 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/email.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/email.ex @@ -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 diff --git a/elixir/apps/domain/lib/domain/auth/adapters/openid_connect/state.ex b/elixir/apps/domain/lib/domain/auth/adapters/openid_connect/state.ex index b23b7e7b4..983cb7309 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/openid_connect/state.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/openid_connect/state.ex @@ -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 diff --git a/elixir/apps/domain/lib/domain/auth/adapters/token.ex b/elixir/apps/domain/lib/domain/auth/adapters/token.ex index 3502c3bd8..1c1677059 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/token.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/token.ex @@ -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) || %{} diff --git a/elixir/apps/domain/lib/domain/crypto.ex b/elixir/apps/domain/lib/domain/crypto.ex index 339cad780..55e1e11b6 100644 --- a/elixir/apps/domain/lib/domain/crypto.ex +++ b/elixir/apps/domain/lib/domain/crypto.ex @@ -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(<>, acc), + do: replace_ambiguous_characters(rest, <>) + end + + defp replace_ambiguous_characters(<>, acc), + do: replace_ambiguous_characters(rest, <>) + 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() diff --git a/elixir/apps/domain/lib/domain/gateways/gateway/changeset.ex b/elixir/apps/domain/lib/domain/gateways/gateway/changeset.ex index e46fc932c..b6b7d1b31 100644 --- a/elixir/apps/domain/lib/domain/gateways/gateway/changeset.ex +++ b/elixir/apps/domain/lib/domain/gateways/gateway/changeset.ex @@ -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) diff --git a/elixir/apps/domain/lib/domain/gateways/token/changeset.ex b/elixir/apps/domain/lib/domain/gateways/token/changeset.ex index 9654772dd..78e6c10f6 100644 --- a/elixir/apps/domain/lib/domain/gateways/token/changeset.ex +++ b/elixir/apps/domain/lib/domain/gateways/token/changeset.ex @@ -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") diff --git a/elixir/apps/domain/lib/domain/relays.ex b/elixir/apps/domain/lib/domain/relays.ex index 7aa40e538..24630d158 100644 --- a/elixir/apps/domain/lib/domain/relays.ex +++ b/elixir/apps/domain/lib/domain/relays.ex @@ -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 diff --git a/elixir/apps/domain/lib/domain/relays/token/changeset.ex b/elixir/apps/domain/lib/domain/relays/token/changeset.ex index 33c5f3e80..67fb693f3 100644 --- a/elixir/apps/domain/lib/domain/relays/token/changeset.ex +++ b/elixir/apps/domain/lib/domain/relays/token/changeset.ex @@ -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") diff --git a/elixir/apps/domain/priv/repo/seeds.exs b/elixir/apps/domain/priv/repo/seeds.exs index 2d125617a..f376dcd2b 100644 --- a/elixir/apps/domain/priv/repo/seeds.exs +++ b/elixir/apps/domain/priv/repo/seeds.exs @@ -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}} diff --git a/elixir/apps/domain/test/domain/auth/adapters/email_test.exs b/elixir/apps/domain/test/domain/auth/adapters/email_test.exs index 7ad87e490..16ed438c9 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/email_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/email_test.exs @@ -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" } ) diff --git a/elixir/apps/domain/test/domain/crypto_test.exs b/elixir/apps/domain/test/domain/crypto_test.exs index 886a693fd..91dabe187 100644 --- a/elixir/apps/domain/test/domain/crypto_test.exs +++ b/elixir/apps/domain/test/domain/crypto_test.exs @@ -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 diff --git a/elixir/apps/domain/test/support/fixtures/gateways.ex b/elixir/apps/domain/test/support/fixtures/gateways.ex index bd64999bb..65f46e0f0 100644 --- a/elixir/apps/domain/test/support/fixtures/gateways.ex +++ b/elixir/apps/domain/test/support/fixtures/gateways.ex @@ -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}} diff --git a/elixir/apps/web/lib/web/controllers/auth_controller.ex b/elixir/apps/web/lib/web/controllers/auth_controller.ex index a1506d94c..8d15f74c5 100644 --- a/elixir/apps/web/lib/web/controllers/auth_controller.ex +++ b/elixir/apps/web/lib/web/controllers/auth_controller.ex @@ -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() + <> = + 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 diff --git a/elixir/apps/web/lib/web/mailer/auth_email.ex b/elixir/apps/web/lib/web/mailer/auth_email.ex index 82d83c40f..9c1fb06c3 100644 --- a/elixir/apps/web/lib/web/mailer/auth_email.ex +++ b/elixir/apps/web/lib/web/mailer/auth_email.ex @@ -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 diff --git a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex index 0ff4a0635..ab50409a3 100644 --- a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex +++ b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.html.heex @@ -7,8 +7,8 @@

Here is the magic sign-in link - 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 "<%= @account.name %>". + It is valid for 15 minutes.

@@ -18,14 +18,25 @@

- Please copy the code and paste it into the Firezone application to proceed with the login: -

- <%= @secret %> -
- It is valid for 1 hour. + Please copy the code and paste it into the Firezone application to proceed with + signing in to "<%= @account.name %>":

- -

- If you didn't request this, you can safely discard this email. +

+ <%= @secret %>

+

It is valid for 15 minutes.

+ +

+ 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 %> +

diff --git a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex index 8f17c480a..455a367ac 100644 --- a/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex +++ b/elixir/apps/web/lib/web/mailer/auth_email/sign_in_link.text.heex @@ -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 %> diff --git a/elixir/apps/web/lib/web/plugs/secure_headers.ex b/elixir/apps/web/lib/web/plugs/secure_headers.ex index a7e29afdd..8b4c78966 100644 --- a/elixir/apps/web/lib/web/plugs/secure_headers.ex +++ b/elixir/apps/web/lib/web/plugs/secure_headers.ex @@ -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__) 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 6d142cece..198855695 100644 --- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs +++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs @@ -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 + <> = + identity.provider_virtual_state.sign_in_token + + {email_secret, browser_secret} + end end