From 3cbc18d0d605d34dbb809ced63b9acfa87021808 Mon Sep 17 00:00:00 2001 From: Po Chen Date: Thu, 26 May 2022 15:43:09 +1000 Subject: [PATCH] Sync users with OIDC provider (#605) * tmp [skip ci] * add connection [skip ci] * implement periodic refresh * format [skip ci] * no need to guard * make credo happy * remove filter * updates * extract component * prevent changing admin's connection permission * add to show as well * show connections * hide oidc connections section if nothing is there * add test * leave site id out for now may not need it if we create different users for different sites, even for the same email * move delay randomization to manager * add test * address comments * re-enable user when sign-ed via oidc * fix test * remove unused alias * fix tests * update tests * fix re-enable * add tests * Update vpn_connection_component_test.exs * revert formats * tests * remove alias * remove special case for admins * remove auto re-enable --- apps/fz_http/lib/fz_http/application.ex | 10 +-- apps/fz_http/lib/fz_http/devices.ex | 2 +- apps/fz_http/lib/fz_http/oidc.ex | 36 +++++++++ apps/fz_http/lib/fz_http/oidc/connection.ex | 25 ++++++ .../lib/fz_http/oidc/refresh_manager.ex | 51 ++++++++++++ apps/fz_http/lib/fz_http/oidc/refresher.ex | 78 +++++++++++++++++++ apps/fz_http/lib/fz_http/telemetry.ex | 8 ++ apps/fz_http/lib/fz_http/users.ex | 11 +++ apps/fz_http/lib/fz_http/users/user.ex | 4 +- .../controllers/auth_controller.ex | 5 ++ .../live/user_live/index.html.heex | 6 ++ .../fz_http_web/live/user_live/show.html.heex | 54 ++++++++----- .../fz_http_web/live/user_live/show_live.ex | 4 +- .../user_live/vpn_connection_component.ex | 47 +++++++++++ .../fz_http_web/templates/root/auth.html.heex | 16 ++-- .../shared/oidc_connections_table.html.heex | 26 +++++++ .../lib/fz_http_web/views/root_view.ex | 4 +- ...20220519034545_create_oidc_connections.exs | 17 ++++ ...20220520072323_add_disabled_at_to_user.exs | 9 +++ .../test/fz_http/oidc/refresher_test.exs | 58 ++++++++++++++ apps/fz_http/test/fz_http/users_test.exs | 35 ++++++++- .../fz_http_web/live/user_live/show_test.exs | 34 ++++++++ .../vpn_connection_component_test.exs | 22 ++++++ .../test/support/mock_openid_connect.ex | 4 +- apps/fz_http/test/support/test_helpers.ex | 6 +- 25 files changed, 525 insertions(+), 47 deletions(-) create mode 100644 apps/fz_http/lib/fz_http/oidc.ex create mode 100644 apps/fz_http/lib/fz_http/oidc/connection.ex create mode 100644 apps/fz_http/lib/fz_http/oidc/refresh_manager.ex create mode 100644 apps/fz_http/lib/fz_http/oidc/refresher.ex create mode 100644 apps/fz_http/lib/fz_http_web/live/user_live/vpn_connection_component.ex create mode 100644 apps/fz_http/lib/fz_http_web/templates/shared/oidc_connections_table.html.heex create mode 100644 apps/fz_http/priv/repo/migrations/20220519034545_create_oidc_connections.exs create mode 100644 apps/fz_http/priv/repo/migrations/20220520072323_add_disabled_at_to_user.exs create mode 100644 apps/fz_http/test/fz_http/oidc/refresher_test.exs create mode 100644 apps/fz_http/test/fz_http_web/live/user_live/vpn_connection_component_test.exs diff --git a/apps/fz_http/lib/fz_http/application.ex b/apps/fz_http/lib/fz_http/application.ex index d48cafae9..6daf0d025 100644 --- a/apps/fz_http/lib/fz_http/application.ex +++ b/apps/fz_http/lib/fz_http/application.ex @@ -36,9 +36,11 @@ defmodule FzHttp.Application do {Phoenix.PubSub, name: FzHttp.PubSub}, FzHttpWeb.Presence, FzHttp.ConnectivityCheckService, - FzHttp.VpnSessionScheduler + FzHttp.VpnSessionScheduler, + {OpenIDConnect.Worker, openid_connect_providers}, + {DynamicSupervisor, name: FzHttp.RefresherSupervisor, strategy: :one_for_one}, + FzHttp.OIDC.RefreshManager ] - |> append_if(openid_connect_providers, {OpenIDConnect.Worker, openid_connect_providers}) end defp children(:test) do @@ -51,8 +53,4 @@ defmodule FzHttp.Application do FzHttpWeb.Presence ] end - - defp append_if(list, condition, item) do - if condition, do: list ++ [item], else: list - end end diff --git a/apps/fz_http/lib/fz_http/devices.ex b/apps/fz_http/lib/fz_http/devices.ex index 2fdf4f663..c8723d1fa 100644 --- a/apps/fz_http/lib/fz_http/devices.ex +++ b/apps/fz_http/lib/fz_http/devices.ex @@ -88,7 +88,7 @@ defmodule FzHttp.Devices do preload: :user ) |> Enum.filter(fn device -> - device.user.role == :admin || !Users.vpn_session_expired?(device.user, vpn_duration) + !device.user.disabled_at && !Users.vpn_session_expired?(device.user, vpn_duration) end) |> Enum.map(fn device -> %{ diff --git a/apps/fz_http/lib/fz_http/oidc.ex b/apps/fz_http/lib/fz_http/oidc.ex new file mode 100644 index 000000000..1c39f617b --- /dev/null +++ b/apps/fz_http/lib/fz_http/oidc.ex @@ -0,0 +1,36 @@ +defmodule FzHttp.OIDC do + @moduledoc """ + The OIDC context. + """ + + import Ecto.Query, warn: false + + alias FzHttp.{OIDC.Connection, Repo, Users.User} + + def list_connections(%User{id: id}) do + Repo.all(from Connection, where: [user_id: ^id]) + end + + def get_connection!(user_id, provider) do + Repo.get_by!(Connection, user_id: user_id, provider: provider) + end + + def get_connection(user_id, provider) do + Repo.get_by(Connection, user_id: user_id, provider: provider) + end + + def create_connection(user_id, provider, refresh_token) do + %Connection{user_id: user_id} + |> Connection.changeset(%{provider: provider, refresh_token: refresh_token}) + |> Repo.insert( + conflict_target: [:user_id, :provider], + on_conflict: {:replace, [:refresh_token]} + ) + end + + def update_connection(%Connection{} = connection, attrs) do + connection + |> Connection.changeset(attrs) + |> Repo.update() + end +end diff --git a/apps/fz_http/lib/fz_http/oidc/connection.ex b/apps/fz_http/lib/fz_http/oidc/connection.ex new file mode 100644 index 000000000..19233845c --- /dev/null +++ b/apps/fz_http/lib/fz_http/oidc/connection.ex @@ -0,0 +1,25 @@ +defmodule FzHttp.OIDC.Connection do + @moduledoc """ + OIDC connections + """ + + use Ecto.Schema + import Ecto.Changeset + + schema "oidc_connections" do + field :provider, :string + field :refresh_response, :map + field :refresh_token, :string + field :refreshed_at, :utc_datetime_usec + field :user_id, :id + + timestamps(type: :utc_datetime_usec) + end + + @doc false + def changeset(connection, attrs) do + connection + |> cast(attrs, [:provider, :refresh_token, :refreshed_at, :refresh_response]) + |> validate_required([:provider, :refresh_token]) + end +end diff --git a/apps/fz_http/lib/fz_http/oidc/refresh_manager.ex b/apps/fz_http/lib/fz_http/oidc/refresh_manager.ex new file mode 100644 index 000000000..236fafd7f --- /dev/null +++ b/apps/fz_http/lib/fz_http/oidc/refresh_manager.ex @@ -0,0 +1,51 @@ +defmodule FzHttp.OIDC.RefreshManager do + @moduledoc """ + Manager module for refreshing OIDC connections + """ + use GenServer, restart: :permanent + + import Ecto.Query + alias FzHttp.{Repo, Users.User} + + @spawn_interval 60 * 60 * 1000 + @max_delay_after_spawn 15 + + def start_link(_) do + GenServer.start_link(__MODULE__, []) + end + + def init(_) do + {:ok, [], {:continue, :schedule}} + end + + def handle_continue(:schedule, state) do + spawn_refresher() + {:noreply, state} + end + + def handle_info(:spawn_refresher, user_id) do + spawn_refresher() + {:noreply, user_id} + end + + defp schedule do + Process.send_after(self(), :spawn_refresher, @spawn_interval) + end + + defp spawn_refresher do + schedule() + + from(u in User, where: is_nil(u.disabled_at)) + |> Repo.all() + |> Enum.each(&do_spawn/1) + end + + defp do_spawn(%{id: id} = _user) do + delay_after_spawn = Enum.random(1..@max_delay_after_spawn) * 1000 + + DynamicSupervisor.start_child( + FzHttp.RefresherSupervisor, + {FzHttp.OIDC.Refresher, {id, delay_after_spawn}} + ) + end +end diff --git a/apps/fz_http/lib/fz_http/oidc/refresher.ex b/apps/fz_http/lib/fz_http/oidc/refresher.ex new file mode 100644 index 000000000..7473bb52c --- /dev/null +++ b/apps/fz_http/lib/fz_http/oidc/refresher.ex @@ -0,0 +1,78 @@ +defmodule FzHttp.OIDC.Refresher do + @moduledoc """ + Worker module for refreshing OIDC connections + """ + use GenServer, restart: :temporary + + import Ecto.{Changeset, Query} + alias FzHttp.{OIDC, OIDC.Connection, Repo, Users} + require Logger + + def start_link(init_opts) do + GenServer.start_link(__MODULE__, init_opts) + end + + def init({user_id, delay}) do + {:ok, user_id, {:continue, {:delay, delay}}} + end + + def handle_continue({:delay, delay}, user_id) do + Process.sleep(delay) + refresh(user_id) + end + + def refresh(user_id) do + connections = Repo.all(from Connection, where: [user_id: ^user_id]) + Enum.each(connections, &do_refresh(user_id, &1)) + {:stop, :shutdown, user_id} + end + + defp do_refresh(user_id, %{provider: provider, refresh_token: refresh_token} = conn) do + provider = String.to_existing_atom(provider) + + Logger.info("Refreshing user\##{user_id} @ #{provider}...") + + result = + openid_connect().fetch_tokens( + provider, + %{grant_type: "refresh_token", refresh_token: refresh_token} + ) + + refresh_response = + case result do + {:ok, refreshed} -> + refreshed + + {:error, :fetch_tokens, %{body: body}} -> + %{error: body} + + _ -> + %{error: "unknown error"} + end + + OIDC.update_connection(conn, %{ + refreshed_at: DateTime.utc_now(), + refresh_response: refresh_response + }) + + with %{error: _} <- refresh_response do + user = Users.get_user!(user_id) + + Logger.info("Disabling user #{user.email} due to OIDC token refresh failure...") + + user + |> change() + |> put_change(:disabled_at, DateTime.utc_now()) + |> prepare_changes(fn changeset -> + FzHttp.Telemetry.disable_user(user, "oidc refresh failure") + FzHttpWeb.Endpoint.broadcast("users_socket:#{user.id}", "disconnect", %{}) + changeset + end) + |> Repo.update!() + end + end + + defp openid_connect do + Application.fetch_env!(:fz_http, :openid_connect) + end +end diff --git a/apps/fz_http/lib/fz_http/telemetry.ex b/apps/fz_http/lib/fz_http/telemetry.ex index fb8de85f7..37f35eae4 100644 --- a/apps/fz_http/lib/fz_http/telemetry.ex +++ b/apps/fz_http/lib/fz_http/telemetry.ex @@ -86,6 +86,14 @@ defmodule FzHttp.Telemetry do ) end + def disable_user(user, reason) do + telemetry_module().capture( + "disable_user", + common_fields() ++ + [user_email_hash: hash(user.email), reason: reason] + ) + end + def fz_http_started do telemetry_module().capture( "fz_http_started", diff --git a/apps/fz_http/lib/fz_http/users.ex b/apps/fz_http/lib/fz_http/users.ex index 217a5cbf2..f1b129f36 100644 --- a/apps/fz_http/lib/fz_http/users.ex +++ b/apps/fz_http/lib/fz_http/users.ex @@ -3,6 +3,7 @@ defmodule FzHttp.Users do The Users context. """ + import Ecto.Changeset import Ecto.Query, warn: false alias FzCommon.{FzCrypto, FzMap} @@ -141,6 +142,16 @@ defmodule FzHttp.Users do update_user(user, %{last_signed_in_at: DateTime.utc_now(), last_signed_in_method: method}) end + def enable_vpn_connection(user, %{provider: :identity}), do: user + def enable_vpn_connection(user, %{provider: :magic_link}), do: user + + def enable_vpn_connection(user, %{provider: _oidc_provider}) do + user + |> change() + |> put_change(:disabled_at, nil) + |> Repo.update!() + end + @doc """ Returns DateTime that VPN sessions expire based on last_signed_in_at and the security.require_auth_for_vpn_frequency setting. diff --git a/apps/fz_http/lib/fz_http/users/user.ex b/apps/fz_http/lib/fz_http/users/user.ex index 80587acf3..07e05da80 100644 --- a/apps/fz_http/lib/fz_http/users/user.ex +++ b/apps/fz_http/lib/fz_http/users/user.ex @@ -10,7 +10,7 @@ defmodule FzHttp.Users.User do import Ecto.Changeset import FzHttp.Users.PasswordHelpers - alias FzHttp.Devices.Device + alias FzHttp.{Devices.Device, OIDC.Connection} schema "users" do field :uuid, Ecto.UUID, autogenerate: true @@ -21,6 +21,7 @@ defmodule FzHttp.Users.User do field :password_hash, :string field :sign_in_token, :string field :sign_in_token_created_at, :utc_datetime_usec + field :disabled_at, :utc_datetime_usec # VIRTUAL FIELDS field :device_count, :integer, virtual: true @@ -29,6 +30,7 @@ defmodule FzHttp.Users.User do field :current_password, :string, virtual: true has_many :devices, Device, on_delete: :delete_all + has_many :oidc_connections, Connection, on_delete: :delete_all timestamps(type: :utc_datetime_usec) end diff --git a/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex b/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex index ef7820e1e..3e4187d6a 100644 --- a/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex +++ b/apps/fz_http/lib/fz_http_web/controllers/auth_controller.ex @@ -67,6 +67,11 @@ defmodule FzHttpWeb.AuthController do {:ok, claims} <- openid_connect.verify(provider, tokens["id_token"]) do case UserFromAuth.find_or_create(provider, claims) do {:ok, user} -> + # only first-time connect will include refresh token + with %{"refresh_token" => refresh_token} <- tokens do + FzHttp.OIDC.create_connection(user.id, provider_key, refresh_token) + end + conn |> configure_session(renew: true) |> put_session(:live_socket_id, "users_socket:#{user.id}") diff --git a/apps/fz_http/lib/fz_http_web/live/user_live/index.html.heex b/apps/fz_http/lib/fz_http_web/live/user_live/index.html.heex index a7c6bd602..0b8dcb07e 100644 --- a/apps/fz_http/lib/fz_http_web/live/user_live/index.html.heex +++ b/apps/fz_http/lib/fz_http_web/live/user_live/index.html.heex @@ -20,6 +20,7 @@ Email Devices + VPN Connection Last Signed In Last Signed In Method Created @@ -33,6 +34,11 @@ <%= live_patch(user.email, to: Routes.user_show_path(@socket, :show, user)) %> <%= user.device_count %> + + <.live_component id={"allowed-to-connect-#{user.id}"} + module={FzHttpWeb.UserLive.VPNConnectionComponent } + user={user} disabled={true} /> + diff --git a/apps/fz_http/lib/fz_http_web/live/user_live/show.html.heex b/apps/fz_http/lib/fz_http_web/live/user_live/show.html.heex index 2f46697ea..f9d8aec06 100644 --- a/apps/fz_http/lib/fz_http_web/live/user_live/show.html.heex +++ b/apps/fz_http/lib/fz_http_web/live/user_live/show.html.heex @@ -40,6 +40,14 @@ <%= render FzHttpWeb.SharedView, "user_details.html", user: @user %> +<%= if length(@connections) > 0 do %> +
+

OIDC Connections

+ + <%= render FzHttpWeb.SharedView, "oidc_connections_table.html", connections: @connections %> +
+<% end %> +

Devices

@@ -62,25 +70,31 @@

Danger Zone

- + <.live_component id="allowed-to-connect" + module={FzHttpWeb.UserLive.VPNConnectionComponent } + user={@user} label="Permission to Connect VPN" /> - +
+ + + +
diff --git a/apps/fz_http/lib/fz_http_web/live/user_live/show_live.ex b/apps/fz_http/lib/fz_http_web/live/user_live/show_live.ex index 8b14fff12..7cfb09d5f 100644 --- a/apps/fz_http/lib/fz_http_web/live/user_live/show_live.ex +++ b/apps/fz_http/lib/fz_http_web/live/user_live/show_live.ex @@ -5,18 +5,20 @@ defmodule FzHttpWeb.UserLive.Show do """ use FzHttpWeb, :live_view - alias FzHttp.{Devices, Repo, Users} + alias FzHttp.{Devices, OIDC, Repo, Users} alias FzHttpWeb.ErrorHelpers @impl Phoenix.LiveView def mount(%{"id" => user_id} = _params, _session, socket) do user = Users.get_user!(user_id) devices = Devices.list_devices(user) + connections = OIDC.list_connections(user) {:ok, socket |> assign(:devices, devices) |> assign(:device_config, socket.assigns[:device_config]) + |> assign(:connections, connections) |> assign(:user, user) |> assign(:page_title, "Users")} end diff --git a/apps/fz_http/lib/fz_http_web/live/user_live/vpn_connection_component.ex b/apps/fz_http/lib/fz_http_web/live/user_live/vpn_connection_component.ex new file mode 100644 index 000000000..2b0e4e250 --- /dev/null +++ b/apps/fz_http/lib/fz_http_web/live/user_live/vpn_connection_component.ex @@ -0,0 +1,47 @@ +defmodule FzHttpWeb.UserLive.VPNConnectionComponent do + @moduledoc """ + Handles user form. + """ + use FzHttpWeb, :live_component + + import Ecto.Changeset + alias FzHttp.Repo + + @impl Phoenix.LiveComponent + def render(assigns) do + ~H""" + + """ + end + + @impl Phoenix.LiveComponent + def handle_event("toggle_disabled_at", params, socket) do + to_disable = !params["value"] + + user = + socket.assigns.user + |> change() + |> put_change( + :disabled_at, + if(to_disable, do: DateTime.utc_now(), else: nil) + ) + |> prepare_changes(fn + %{changes: %{disabled_at: nil}} = changeset -> + changeset + + %{data: user} = changeset -> + FzHttp.Telemetry.disable_user(user, "disabled by admin") + FzHttpWeb.Endpoint.broadcast("users_socket:#{user.id}", "disconnect", %{}) + changeset + end) + |> Repo.update!() + + {:noreply, assign(socket, :user, user)} + end +end diff --git a/apps/fz_http/lib/fz_http_web/templates/root/auth.html.heex b/apps/fz_http/lib/fz_http_web/templates/root/auth.html.heex index e627ece7f..5caeb9122 100644 --- a/apps/fz_http/lib/fz_http_web/templates/root/auth.html.heex +++ b/apps/fz_http/lib/fz_http_web/templates/root/auth.html.heex @@ -7,15 +7,13 @@ Please sign in via one of the methods below.

- <%= if @openid_connect_providers > 0 do %> - <%= for {provider, config} <- @openid_connect_providers do %> -

- <%= link( - "Sign in with #{Keyword.get(config, :label)}", - to: authorization_uri(@openid_connect, provider), - class: "button") %> -

- <% end %> + <%= for {provider, config} <- @openid_connect_providers do %> +

+ <%= link( + "Sign in with #{config[:label]}", + to: authorization_uri(@openid_connect, provider), + class: "button") %> +

<% end %> <%= if @local_enabled do %> diff --git a/apps/fz_http/lib/fz_http_web/templates/shared/oidc_connections_table.html.heex b/apps/fz_http/lib/fz_http_web/templates/shared/oidc_connections_table.html.heex new file mode 100644 index 000000000..7601ad3e0 --- /dev/null +++ b/apps/fz_http/lib/fz_http_web/templates/shared/oidc_connections_table.html.heex @@ -0,0 +1,26 @@ + + + + + + + + + + <%= for conn <- @connections do %> + + + + + + <% end %> + +
ProviderRefreshed AtRefresh Result
+ <%= conn.provider %> + + <%= if match?(%{"error" => _}, conn.refresh_response) do %> + ERROR: <%= conn.refresh_response["error"] %> + <% else %> + OK + <% end %> +
diff --git a/apps/fz_http/lib/fz_http_web/views/root_view.ex b/apps/fz_http/lib/fz_http_web/views/root_view.ex index 039618306..7d9bce84c 100644 --- a/apps/fz_http/lib/fz_http_web/views/root_view.ex +++ b/apps/fz_http/lib/fz_http_web/views/root_view.ex @@ -5,7 +5,9 @@ defmodule FzHttpWeb.RootView do def authorization_uri(oidc, provider) do params = %{ - state: FzCrypto.rand_string() + state: FzCrypto.rand_string(), + # needed for google + access_type: "offline" } oidc.authorization_uri(provider, params) diff --git a/apps/fz_http/priv/repo/migrations/20220519034545_create_oidc_connections.exs b/apps/fz_http/priv/repo/migrations/20220519034545_create_oidc_connections.exs new file mode 100644 index 000000000..6a15c83fa --- /dev/null +++ b/apps/fz_http/priv/repo/migrations/20220519034545_create_oidc_connections.exs @@ -0,0 +1,17 @@ +defmodule FzHttp.Repo.Migrations.CreateOidcConnections do + use Ecto.Migration + + def change do + create table(:oidc_connections) do + add :provider, :string, null: false + add :refresh_token, :string + add :refreshed_at, :utc_datetime_usec + add :refresh_response, :map + add :user_id, references(:users, on_delete: :nothing), null: false + + timestamps(type: :utc_datetime_usec) + end + + create unique_index(:oidc_connections, [:user_id, :provider]) + end +end diff --git a/apps/fz_http/priv/repo/migrations/20220520072323_add_disabled_at_to_user.exs b/apps/fz_http/priv/repo/migrations/20220520072323_add_disabled_at_to_user.exs new file mode 100644 index 000000000..f8b99180a --- /dev/null +++ b/apps/fz_http/priv/repo/migrations/20220520072323_add_disabled_at_to_user.exs @@ -0,0 +1,9 @@ +defmodule FzHttp.Repo.Migrations.AddDisabledAtToUser do + use Ecto.Migration + + def change do + alter table("users") do + add :disabled_at, :utc_datetime_usec + end + end +end diff --git a/apps/fz_http/test/fz_http/oidc/refresher_test.exs b/apps/fz_http/test/fz_http/oidc/refresher_test.exs new file mode 100644 index 000000000..f6bce4f4b --- /dev/null +++ b/apps/fz_http/test/fz_http/oidc/refresher_test.exs @@ -0,0 +1,58 @@ +defmodule FzHttp.OIDC.RefresherTest do + use FzHttp.DataCase, async: true + + import Mox + + alias FzHttp.{OIDC.Refresher, Repo} + + setup :create_user + + setup %{user: user} do + conn = + Repo.insert!(%FzHttp.OIDC.Connection{ + user_id: user.id, + provider: "test", + refresh_token: "REFRESH_TOKEN" + }) + + {:ok, conn: conn} + end + + describe "refresh failed" do + setup do + expect(OpenIDConnect.Mock, :fetch_tokens, fn _, _ -> + {:error, :fetch_tokens, "TEST_ERROR"} + end) + + :ok + end + + test "disable user", %{user: user, conn: conn} do + Refresher.refresh(user.id) + user = Repo.reload(user) + conn = Repo.reload(conn) + + assert user.disabled_at + assert %{"error" => _} = conn.refresh_response + end + end + + describe "refresh succeeded" do + setup do + expect(OpenIDConnect.Mock, :fetch_tokens, fn _, _ -> + {:ok, %{data: "success"}} + end) + + :ok + end + + test "does not change user", %{user: user, conn: conn} do + Refresher.refresh(user.id) + user = Repo.reload(user) + conn = Repo.reload(conn) + + refute user.disabled_at + refute match?(%{"error" => _}, conn.refresh_response) + end + end +end diff --git a/apps/fz_http/test/fz_http/users_test.exs b/apps/fz_http/test/fz_http/users_test.exs index a68e652c2..66646d159 100644 --- a/apps/fz_http/test/fz_http/users_test.exs +++ b/apps/fz_http/test/fz_http/users_test.exs @@ -1,7 +1,7 @@ defmodule FzHttp.UsersTest do use FzHttp.DataCase, async: true - alias FzHttp.Users + alias FzHttp.{Repo, Users} describe "consume_sign_in_token/1 valid token" do setup [:create_user_with_valid_sign_in_token] @@ -222,7 +222,7 @@ defmodule FzHttp.UsersTest do end describe "delete_user/1" do - setup [:create_user] + setup :create_user test "raises Ecto.NoResultsError when a deleted user is fetched", %{user: user} do Users.delete_user(user) @@ -234,7 +234,7 @@ defmodule FzHttp.UsersTest do end describe "change_user/1" do - setup [:create_user] + setup :create_user test "returns changeset", %{user: user} do assert %Ecto.Changeset{} = Users.change_user(user) @@ -246,4 +246,33 @@ defmodule FzHttp.UsersTest do assert %Ecto.Changeset{} = Users.new_user() end end + + describe "enable_vpn_connection/2" do + import Ecto.Changeset + + setup :create_user + + setup %{user: user} do + user = user |> change |> put_change(:disabled_at, DateTime.utc_now()) |> Repo.update!() + {:ok, user: user} + end + + @tag :unprivileged + test "enable via OIDC", %{user: user} do + Users.enable_vpn_connection(user, %{provider: :oidc}) + + user = Repo.reload(user) + + assert %{disabled_at: nil} = user + end + + @tag :unprivileged + test "no change via password", %{user: user} do + Users.enable_vpn_connection(user, %{provider: :identity}) + + user = Repo.reload(user) + + assert user.disabled_at + end + end end diff --git a/apps/fz_http/test/fz_http_web/live/user_live/show_test.exs b/apps/fz_http/test/fz_http_web/live/user_live/show_test.exs index 6a6149a81..c8d784eaa 100644 --- a/apps/fz_http/test/fz_http_web/live/user_live/show_test.exs +++ b/apps/fz_http/test/fz_http_web/live/user_live/show_test.exs @@ -614,4 +614,38 @@ defmodule FzHttpWeb.UserLive.ShowTest do assert test_view =~ "should be at least 12 character(s)" end end + + describe "disable/enable user" do + import Ecto.Changeset + alias FzHttp.Repo + + test "enable user", %{admin_conn: conn, unprivileged_user: user} do + user = user |> change |> put_change(:disabled_at, DateTime.utc_now()) |> Repo.update!() + path = Routes.user_show_path(conn, :show, user.id) + + {:ok, view, _html} = live(conn, path) + + view + |> element("input[type=checkbox]") + |> render_click() + + user = Repo.reload(user) + + refute user.disabled_at + end + + test "disable user", %{admin_conn: conn, unprivileged_user: user} do + path = Routes.user_show_path(conn, :show, user.id) + + {:ok, view, _html} = live(conn, path) + + view + |> element("input[type=checkbox]") + |> render_click() + + user = Repo.reload(user) + + assert user.disabled_at + end + end end diff --git a/apps/fz_http/test/fz_http_web/live/user_live/vpn_connection_component_test.exs b/apps/fz_http/test/fz_http_web/live/user_live/vpn_connection_component_test.exs new file mode 100644 index 000000000..56d4c0f9d --- /dev/null +++ b/apps/fz_http/test/fz_http_web/live/user_live/vpn_connection_component_test.exs @@ -0,0 +1,22 @@ +defmodule FzHttpWeb.UserLive.VPNConnectionComponentTest do + use FzHttpWeb.ConnCase, async: true + + alias FzHttpWeb.UserLive.VPNConnectionComponent + + describe "admin" do + setup :create_user + + test "checkbox is disabled", %{user: user} do + assert render_component(VPNConnectionComponent, id: "1", user: user) =~ ~r"\bdisabled\b" + end + end + + describe "unprivileged" do + setup :create_user + + @tag :unprivileged + test "checkbox is not disabled", %{user: user} do + refute render_component(VPNConnectionComponent, id: "1", user: user) =~ ~r"\bdisabled\b" + end + end +end diff --git a/apps/fz_http/test/support/mock_openid_connect.ex b/apps/fz_http/test/support/mock_openid_connect.ex index ac8f8cf88..c78997648 100644 --- a/apps/fz_http/test/support/mock_openid_connect.ex +++ b/apps/fz_http/test/support/mock_openid_connect.ex @@ -3,6 +3,6 @@ defmodule OpenIDConnect.MockBehaviour do Mock Behaviour for OpenIDConnect so that we can use Mox """ @callback authorization_uri(any, map) :: String.t() - @callback fetch_tokens(any, map) :: {:ok, any} | {:error, any} - @callback verify(any, map) :: {:ok, any} | {:error, any} + @callback fetch_tokens(any, map) :: {:ok, any} | {:error, :fetch_tokens, any} + @callback verify(any, map) :: {:ok, any} | {:error, :verify, any} end diff --git a/apps/fz_http/test/support/test_helpers.ex b/apps/fz_http/test/support/test_helpers.ex index 841710fc6..b92072e4d 100644 --- a/apps/fz_http/test/support/test_helpers.ex +++ b/apps/fz_http/test/support/test_helpers.ex @@ -146,7 +146,9 @@ defmodule FzHttp.TestHelpers do {:ok, user: user} end - def create_users(%{count: count}) do + def create_users(opts) do + count = opts[:count] || 5 + users = Enum.map(1..count, fn i -> UsersFixtures.user(%{email: "userlist#{i}@test"}) @@ -155,8 +157,6 @@ defmodule FzHttp.TestHelpers do {:ok, users: users} end - def create_users(_), do: create_users(%{count: 5}) - def clear_users(_) do {count, _result} = Repo.delete_all(User) {:ok, count: count}