From a5342256c35b2909ef63a305b9ce807ac7fbb6f1 Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Tue, 20 Aug 2024 13:05:19 -0600 Subject: [PATCH] feat(portal): Allow bulk-deleting synced actors (#6352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #6301 Closes #6217 Screenshot 2024-08-19 at 12 19 16 PM --- elixir/apps/domain/lib/domain/actors.ex | 23 ++++ .../domain/lib/domain/actors/actor/query.ex | 22 ++++ elixir/apps/domain/priv/repo/seeds.exs | 35 ++++- .../apps/domain/test/domain/actors_test.exs | 123 ++++++++++++++++++ .../identity_providers/openid_connect/show.ex | 49 ++++++- .../openid_connect/show_test.exs | 2 +- 6 files changed, 243 insertions(+), 11 deletions(-) diff --git a/elixir/apps/domain/lib/domain/actors.ex b/elixir/apps/domain/lib/domain/actors.ex index 37e08e20d..b40f60c42 100644 --- a/elixir/apps/domain/lib/domain/actors.ex +++ b/elixir/apps/domain/lib/domain/actors.ex @@ -352,6 +352,13 @@ defmodule Domain.Actors do |> Repo.aggregate(:count) end + def count_synced_actors_for_provider(%Auth.Provider{} = provider) do + Actor.Query.not_deleted() + |> Actor.Query.by_provider_id(provider.id) + |> Actor.Query.by_stale_for_provider(provider.id) + |> Repo.aggregate(:count) + end + def fetch_actor_by_id(id, %Auth.Subject{} = subject, opts \\ []) do with :ok <- Auth.ensure_has_permissions(subject, Authorizer.manage_actors_permission()), true <- Repo.valid_uuid?(id) do @@ -569,6 +576,22 @@ defmodule Domain.Actors do end end + def delete_stale_synced_actors_for_provider( + %Auth.Provider{} = provider, + %Auth.Subject{} = subject + ) do + with :ok <- Auth.ensure_has_permissions(subject, Authorizer.manage_actors_permission()) do + Actor.Query.not_deleted() + |> Authorizer.for_subject(subject) + |> Actor.Query.by_provider_id(provider.id) + |> Actor.Query.by_stale_for_provider(provider.id) + |> Repo.all() + |> Enum.each(fn actor -> + {:ok, _actor} = delete_actor(actor, subject) + end) + end + end + def actor_synced?(%Actor{last_synced_at: nil}), do: false def actor_synced?(%Actor{}), do: true diff --git a/elixir/apps/domain/lib/domain/actors/actor/query.ex b/elixir/apps/domain/lib/domain/actors/actor/query.ex index 270298376..422131006 100644 --- a/elixir/apps/domain/lib/domain/actors/actor/query.ex +++ b/elixir/apps/domain/lib/domain/actors/actor/query.ex @@ -30,6 +30,28 @@ defmodule Domain.Actors.Actor.Query do where(queryable, [actors: actors], actors.account_id == ^account_id) end + def by_provider_id(queryable, provider_id) do + queryable + |> join(:right, [actors: actors], identities in ^Domain.Auth.Identity.Query.all(), + on: identities.actor_id == actors.id, + as: :all_identities + ) + |> where( + [all_identities: all_identities], + all_identities.provider_id == ^provider_id + ) + end + + def by_stale_for_provider(queryable, provider_id) do + subquery = + Domain.Auth.Identity.Query.all() + |> where([identities: identities], identities.actor_id == parent_as(:actors).id) + |> where([identities: identities], identities.provider_id != ^provider_id) + + queryable + |> where([actors: actors], not exists(subquery)) + end + def by_type(queryable, {:in, types}) do where(queryable, [actors: actors], actors.type in ^types) end diff --git a/elixir/apps/domain/priv/repo/seeds.exs b/elixir/apps/domain/priv/repo/seeds.exs index 00c37f1a6..f2f98ee10 100644 --- a/elixir/apps/domain/priv/repo/seeds.exs +++ b/elixir/apps/domain/priv/repo/seeds.exs @@ -134,12 +134,16 @@ admin_actor_email = "firezone@localhost.local" name: "Firezone Unprivileged" }) -for i <- 1..10 do - Actors.create_actor(account, %{ - type: :account_user, - name: "Firezone Unprivileged #{i}" - }) -end +other_actors = + for i <- 1..10 do + {:ok, actor} = + Actors.create_actor(account, %{ + type: :account_user, + name: "Firezone Unprivileged #{i}" + }) + + actor + end {:ok, admin_actor} = Actors.create_actor(account, %{ @@ -203,6 +207,25 @@ admin_actor_oidc_identity ) |> Repo.update!() +for actor <- other_actors do + email = "user-#{System.unique_integer([:positive, :monotonic])}@localhost.local" + + {:ok, identity} = + Auth.create_identity(actor, oidc_provider, %{ + provider_identifier: email, + provider_identifier_confirmation: email + }) + + identity + |> Ecto.Changeset.change( + created_by: :provider, + provider_id: oidc_provider.id, + provider_identifier: email, + provider_state: %{"claims" => %{"email" => email, "group" => "users"}} + ) + |> Repo.update!() +end + # Other Account Users other_unprivileged_actor_email = "other-unprivileged-1@localhost.local" other_admin_actor_email = "other@localhost.local" diff --git a/elixir/apps/domain/test/domain/actors_test.exs b/elixir/apps/domain/test/domain/actors_test.exs index 46f2a2f78..2ecaa0fc6 100644 --- a/elixir/apps/domain/test/domain/actors_test.exs +++ b/elixir/apps/domain/test/domain/actors_test.exs @@ -2121,6 +2121,63 @@ defmodule Domain.ActorsTest do end end + describe "count_synced_actors_for_provider/1" do + test "returns 0 when there are no actors" do + account = Fixtures.Accounts.create_account() + provider = Fixtures.Auth.create_userpass_provider(account: account) + assert count_synced_actors_for_provider(provider) == 0 + end + + test "returns 0 when there are no synced actors" do + account = Fixtures.Accounts.create_account() + provider = Fixtures.Auth.create_userpass_provider(account: account) + Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + assert count_synced_actors_for_provider(provider) == 0 + end + + test "returns count of synced actors owned only by the given provider" do + account = Fixtures.Accounts.create_account() + + {provider, _bypass} = + Fixtures.Auth.start_and_create_openid_connect_provider(account: account) + + actor1 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor1, provider: provider) + + actor2 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor2, provider: provider) + + actor3 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor3) + + actor4 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor4, provider: provider) + Fixtures.Auth.create_identity(account: account, actor: actor4) + + assert count_synced_actors_for_provider(provider) == 2 + end + end + describe "fetch_actor_by_id/3" do test "returns error when actor is not found" do subject = Fixtures.Auth.create_subject() @@ -3145,6 +3202,72 @@ defmodule Domain.ActorsTest do end end + describe "delete_stale_synced_actors_for_provider/2" do + test "deletes actors synced with only the given provider" do + account = Fixtures.Accounts.create_account() + subject = Fixtures.Auth.create_subject(account: account) + + {provider, _bypass} = + Fixtures.Auth.start_and_create_openid_connect_provider(account: account) + + actor1 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor1, provider: provider) + + actor2 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor2, provider: provider) + + actor3 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor3) + + actor4 = + Fixtures.Actors.create_actor( + type: :account_admin_user, + account: account + ) + + Fixtures.Auth.create_identity(account: account, actor: actor4, provider: provider) + Fixtures.Auth.create_identity(account: account, actor: actor4) + + assert delete_stale_synced_actors_for_provider(provider, subject) == :ok + not_deleted_actors = Repo.all(Actors.Actor.Query.not_deleted()) + not_deleted_actor_ids = not_deleted_actors |> Enum.map(& &1.id) |> Enum.sort() + + assert not_deleted_actor_ids == Enum.sort([actor4.id, actor3.id, subject.actor.id]) + end + + test "returns error when subject cannot delete actors" do + account = Fixtures.Accounts.create_account() + + {provider, _bypass} = + Fixtures.Auth.start_and_create_openid_connect_provider(account: account) + + subject = + Fixtures.Auth.create_subject(account: account) + |> Fixtures.Auth.remove_permissions() + + assert delete_stale_synced_actors_for_provider(provider, subject) == + {:error, + {:unauthorized, + reason: :missing_permissions, + missing_permissions: [Actors.Authorizer.manage_actors_permission()]}} + end + end + describe "actor_synced?/1" do test "returns true when actor is synced" do actor = Fixtures.Actors.create_actor() diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/show.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/show.ex index 44c87c081..bd2267e55 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/show.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/show.ex @@ -1,16 +1,22 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.Show do use Web, :live_view import Web.Settings.IdentityProviders.Components - alias Domain.Auth + alias Domain.{Auth, Actors} def mount(%{"provider_id" => provider_id}, _session, socket) do with {:ok, provider} <- Auth.fetch_provider_by_id(provider_id, socket.assigns.subject, preload: [created_by_identity: [:actor]] ) do + safe_to_delete_actors_count = + if is_nil(provider.deleted_at), + do: 0, + else: Actors.count_synced_actors_for_provider(provider) + socket = assign(socket, provider: provider, + safe_to_delete_actors_count: safe_to_delete_actors_count, page_title: "Identity Provider #{provider.name}" ) @@ -103,6 +109,32 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.Show do <.flash_group flash={@flash} /> + <.flash + :if={not is_nil(@provider.deleted_at) and @safe_to_delete_actors_count > 0} + kind={:warning} + > + You have <%= @safe_to_delete_actors_count %> Actor(s) that were synced from this provider and do not have any other identities. + <.button_with_confirmation + id="delete_stale_actors" + style="danger" + icon="hero-trash-solid" + on_confirm="delete_stale_actors" + class="mt-4" + > + <:dialog_title>Delete Stale Actors + <:dialog_content> + Are you sure you want to delete all Actors that were synced synced from this provider and do not have any other identities? + + <:dialog_confirm_button> + Delete Actors + + <:dialog_cancel_button> + Cancel + + Delete Actors + + +
<.vertical_table id="provider"> <.vertical_table_row> @@ -165,7 +197,7 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.Show do <:dialog_title>Delete Identity Provider <:dialog_content> Are you sure you want to delete this provider? This will remove all - Actors and Groups associated with this provider. + Identities, Groups and Policies associated with this provider. <:dialog_confirm_button> Delete Identity Provider @@ -181,10 +213,19 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.Show do end def handle_event("delete", _params, socket) do - {:ok, _provider} = Auth.delete_provider(socket.assigns.provider, socket.assigns.subject) + {:ok, provider} = Auth.delete_provider(socket.assigns.provider, socket.assigns.subject) + {:noreply, push_navigate(socket, to: view_provider(socket.assigns.account, provider))} + end + + def handle_event("delete_stale_actors", _params, socket) do + :ok = + Actors.delete_stale_synced_actors_for_provider( + socket.assigns.provider, + socket.assigns.subject + ) {:noreply, - push_navigate(socket, to: ~p"/#{socket.assigns.account}/settings/identity_providers")} + push_navigate(socket, to: view_provider(socket.assigns.account, socket.assigns.provider))} end def handle_event("enable", _params, socket) do diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/show_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/show_test.exs index e17e7860d..12b98f7b6 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/show_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/show_test.exs @@ -160,7 +160,7 @@ defmodule Web.Live.Settings.IdentityProviders.OpenIDConnect.ShowTest do |> element("button[type=submit]", "Delete Identity Provider") |> render_click() - assert_redirected(lv, ~p"/#{account}/settings/identity_providers") + assert_redirected(lv, ~p"/#{account}/settings/identity_providers/openid_connect/#{provider}") assert Repo.get(Domain.Auth.Provider, provider.id).deleted_at end