diff --git a/elixir/apps/web/lib/web/auth.ex b/elixir/apps/web/lib/web/auth.ex index c3153eaa0..38ba5b5b8 100644 --- a/elixir/apps/web/lib/web/auth.ex +++ b/elixir/apps/web/lib/web/auth.ex @@ -41,6 +41,16 @@ defmodule Web.Auth do Plug.Conn.put_session(conn, :sessions, sessions) end + defp delete_account_session(conn, context_type, account_id) do + sessions = + Plug.Conn.get_session(conn, :sessions, []) + |> Enum.reject(fn {session_context_type, session_account_id, _encoded_fragment} -> + session_context_type == context_type and session_account_id == account_id + end) + + Plug.Conn.put_session(conn, :sessions, sessions) + end + # Signing In and Out @doc """ @@ -295,17 +305,24 @@ defmodule Web.Auth do context_type = fetch_auth_context_type!(params) context = get_auth_context(conn, context_type) - with account when not is_nil(account) <- Map.get(conn.assigns, :account), - sessions = Plug.Conn.get_session(conn, :sessions, []), - {:ok, encoded_fragment} <- fetch_token(sessions, account.id, context.type), - {:ok, subject} <- Auth.authenticate(encoded_fragment, context), - true <- subject.account.id == account.id do - conn - |> Plug.Conn.put_session(:live_socket_id, "sessions:#{subject.token_id}") - |> Plug.Conn.assign(:subject, subject) + if account = Map.get(conn.assigns, :account) do + sessions = Plug.Conn.get_session(conn, :sessions, []) + + with {:ok, encoded_fragment} <- fetch_token(sessions, account.id, context.type), + {:ok, subject} <- Auth.authenticate(encoded_fragment, context), + true <- subject.account.id == account.id do + conn + |> Plug.Conn.put_session(:live_socket_id, "sessions:#{subject.token_id}") + |> Plug.Conn.assign(:subject, subject) + else + {:error, :unauthorized} -> + delete_account_session(conn, context.type, account.id) + + _ -> + conn + end else - {:error, :unauthorized} -> renew_session(conn) - _ -> conn + conn end end diff --git a/elixir/apps/web/lib/web/controllers/auth_controller.ex b/elixir/apps/web/lib/web/controllers/auth_controller.ex index e8d5ed0e0..c68ea69bb 100644 --- a/elixir/apps/web/lib/web/controllers/auth_controller.ex +++ b/elixir/apps/web/lib/web/controllers/auth_controller.ex @@ -185,10 +185,11 @@ defmodule Web.AuthController do } = params ) do with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id) do + redirect_params = Web.Auth.take_sign_in_params(params) + redirect_url = url(~p"/#{provider.account_id}/sign_in/providers/#{provider.id}/handle_callback") - redirect_params = Web.Auth.take_sign_in_params(params) redirect_to_idp(conn, redirect_url, provider, %{}, redirect_params) else {:error, :not_found} -> diff --git a/elixir/apps/web/lib/web/router.ex b/elixir/apps/web/lib/web/router.ex index d1ae431ae..98a164007 100644 --- a/elixir/apps/web/lib/web/router.ex +++ b/elixir/apps/web/lib/web/router.ex @@ -84,6 +84,10 @@ defmodule Web.Router do ## Email live "/sign_in/providers/email/:provider_id", SignIn.Email end + end + + scope "/:account_id_or_slug", Web do + pipe_through [:browser, :account] scope "/sign_in/providers/:provider_id" do # UserPass diff --git a/elixir/apps/web/test/web/acceptance/auth/email_test.exs b/elixir/apps/web/test/web/acceptance/auth/email_test.exs index 0c02c48c3..5d77253a8 100644 --- a/elixir/apps/web/test/web/acceptance/auth/email_test.exs +++ b/elixir/apps/web/test/web/acceptance/auth/email_test.exs @@ -118,7 +118,7 @@ defmodule Web.Acceptance.SignIn.EmailTest do |> visit(~p"/#{account}/sites") |> assert_el(Query.css("#user-menu-button")) - # Browser is stored correctly + # Browser session is stored correctly {:ok, cookie} = Auth.fetch_session_cookie(session) assert [{:browser, account_id, _fragment}] = cookie["sessions"] assert account_id == account.id diff --git a/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs b/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs index 19d96f473..8a48204d0 100644 --- a/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs +++ b/elixir/apps/web/test/web/acceptance/auth/openid_connect_test.exs @@ -110,4 +110,128 @@ defmodule Web.Acceptance.Auth.OpenIDConnectTest do assert Domain.Auth.authenticate(redirect_params["nonce"], context) == {:error, :unauthorized} assert {:ok, _subject} = Domain.Auth.authenticate(token, context) end + + feature "allows to log in to the browser and then to the client", %{ + session: session + } do + nonce = Ecto.UUID.generate() + state = Ecto.UUID.generate() + + Auth.mock_client_sign_in_callback() + + redirect_params = %{ + "as" => "client", + "state" => "state_#{state}", + "nonce" => "nonce_#{nonce}" + } + + account = Fixtures.Accounts.create_account() + provider = Vault.setup_oidc_provider(account, @endpoint.url) + + oidc_login = "firezone-1" + oidc_password = "firezone1234_oidc" + email = Fixtures.Auth.email() + + {:ok, entity_id} = Vault.upsert_user(oidc_login, email, oidc_password) + + actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + + identity = + Fixtures.Auth.create_identity( + actor: actor, + account: account, + provider: provider, + provider_identifier: entity_id + ) + + # Sign In as an portal user + session + |> visit(~p"/#{account}") + |> assert_el(Query.text("Sign into #{account.name}")) + |> click(Query.link("Sign in with Vault")) + |> Vault.userpass_flow(oidc_login, oidc_password) + |> assert_el(Query.css("#user-menu-button")) + |> Auth.assert_authenticated(identity) + |> assert_path(~p"/#{account.slug}/sites") + + # And then to a client + session + |> visit(~p"/#{account}?#{redirect_params}") + |> assert_el(Query.text("Sign into #{account.name}")) + |> click(Query.link("Sign in with Vault")) + |> assert_el(Query.text("Client redirected")) + |> assert_path(~p"/handle_client_sign_in_callback") + + # The browser sessions stays active + session + |> visit(~p"/#{account}/sites") + |> assert_el(Query.css("#user-menu-button")) + + # Browser session is stored correctly + {:ok, cookie} = Auth.fetch_session_cookie(session) + assert [{:browser, account_id, _fragment}] = cookie["sessions"] + assert account_id == account.id + end + + feature "allows to log in to the client and then to the browser", %{ + session: session + } do + nonce = Ecto.UUID.generate() + state = Ecto.UUID.generate() + + Auth.mock_client_sign_in_callback() + + redirect_params = %{ + "as" => "client", + "state" => "state_#{state}", + "nonce" => "nonce_#{nonce}" + } + + account = Fixtures.Accounts.create_account() + provider = Vault.setup_oidc_provider(account, @endpoint.url) + + oidc_login = "firezone-1" + oidc_password = "firezone1234_oidc" + email = Fixtures.Auth.email() + + {:ok, entity_id} = Vault.upsert_user(oidc_login, email, oidc_password) + + actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + + identity = + Fixtures.Auth.create_identity( + actor: actor, + account: account, + provider: provider, + provider_identifier: entity_id + ) + + # And then to a client + session + |> visit(~p"/#{account}?#{redirect_params}") + |> assert_el(Query.text("Sign into #{account.name}")) + |> click(Query.link("Sign in with Vault")) + |> Vault.userpass_flow(oidc_login, oidc_password) + |> assert_el(Query.text("Client redirected")) + |> assert_path(~p"/handle_client_sign_in_callback") + + # Sign In as an portal user + session + |> visit(~p"/#{account}") + |> assert_el(Query.text("Sign into #{account.name}")) + |> click(Query.link("Sign in with Vault")) + |> assert_el(Query.css("#user-menu-button")) + |> Auth.assert_authenticated(identity) + |> assert_path(~p"/#{account.slug}/sites") + + # The browser sessions stays active + session + |> visit(~p"/#{account}/sites") + |> assert_el(Query.css("#user-menu-button")) + + # Browser session is stored correctly + {:ok, cookie} = Auth.fetch_session_cookie(session) + assert [{:browser, account_id, _fragment}] = cookie["sessions"] + assert account_id == account.id + end end diff --git a/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs b/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs index 1788ff6b9..8be5fada5 100644 --- a/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs +++ b/elixir/apps/web/test/web/acceptance/auth/userpass_test.exs @@ -193,6 +193,57 @@ defmodule Web.Acceptance.Auth.UserPassTest do assert {:ok, _subject} = Domain.Auth.authenticate(token, context) end + feature "allows to log in using email link to the client even with active browser session", %{ + session: session + } do + nonce = Ecto.UUID.generate() + state = Ecto.UUID.generate() + + Auth.mock_client_sign_in_callback() + + redirect_params = %{ + "as" => "client", + "state" => "state_#{state}", + "nonce" => "nonce_#{nonce}" + } + + account = Fixtures.Accounts.create_account() + actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + provider = Fixtures.Auth.create_userpass_provider(account: account) + password = "Firezone1234" + + identity = + Fixtures.Auth.create_identity( + account: account, + provider: provider, + actor: actor, + provider_virtual_state: %{"password" => password, "password_confirmation" => password} + ) + + # Sign In as an portal user + session + |> password_login_flow(account, identity.provider_identifier, password) + |> assert_el(Query.css("#user-menu-button")) + |> assert_path(~p"/#{account.slug}/sites") + |> Auth.assert_authenticated(identity) + + # And then to a client + session + |> password_login_flow(account, identity.provider_identifier, password, redirect_params) + |> assert_el(Query.text("Client redirected")) + |> assert_path(~p"/handle_client_sign_in_callback") + + # The browser sessions stays active + session + |> visit(~p"/#{account}/sites") + |> assert_el(Query.css("#user-menu-button")) + + # Browser session is stored correctly + {:ok, cookie} = Auth.fetch_session_cookie(session) + assert [{:browser, account_id, _fragment}] = cookie["sessions"] + assert account_id == account.id + end + defp password_login_flow(session, account, username, password, redirect_params \\ %{}) do session |> visit(~p"/#{account}?#{redirect_params}") diff --git a/elixir/apps/web/test/web/auth_test.exs b/elixir/apps/web/test/web/auth_test.exs index 19361759a..0a0029b59 100644 --- a/elixir/apps/web/test/web/auth_test.exs +++ b/elixir/apps/web/test/web/auth_test.exs @@ -714,20 +714,22 @@ defmodule Web.AuthTest do refute Map.has_key?(conn.assigns, :subject) end - test "renews session when token is invalid", %{ + test "removes invalid tokens from session", %{ conn: conn, - account: account, - context: context + account: account } do conn = %{conn | remote_ip: {100, 64, 100, 58}} - |> put_session(:sessions, [{context.type, account.id, "invalid"}]) + |> put_session(:sessions, [ + {:client, account.id, "valid"}, + {:browser, account.id, "invalid"} + ]) |> assign(:account, account) |> fetch_subject([]) refute Map.has_key?(conn.assigns, :subject) - assert get_session(conn, :sessions) == [] + assert get_session(conn, :sessions) == [{:client, account.id, "valid"}] end end 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 cda417046..04ca53ec2 100644 --- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs +++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs @@ -759,11 +759,32 @@ defmodule Web.AuthControllerTest do conn = get(conn, ~p"/#{account.id}/sign_in/providers/#{provider.id}/redirect", redirect_params) - {params, _state, _verifier} = + assert to = redirected_to(conn) + uri = URI.parse(to) + assert uri.host == "localhost" + assert uri.path == "/authorize" + + callback_url = + url(~p"/#{account.id}/sign_in/providers/#{provider.id}/handle_callback") + + {params, state, verifier} = conn.cookies["fz_auth_state_#{provider.id}"] |> :erlang.binary_to_term([:safe]) assert params == redirect_params + + code_challenge = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_challenge(verifier) + + assert URI.decode_query(uri.query) == %{ + "access_type" => "offline", + "client_id" => provider.adapter_config["client_id"], + "code_challenge" => code_challenge, + "code_challenge_method" => "S256", + "redirect_uri" => callback_url, + "response_type" => "code", + "scope" => "openid email profile", + "state" => state + } end end