From a753af688ae0c8f9fdc6372a2c1205d6d5b130d4 Mon Sep 17 00:00:00 2001 From: SimpleTest Date: Fri, 24 Jul 2026 21:07:08 +0300 Subject: [PATCH] fix(moderation): enforce staff privilege boundaries --- lib/who_need_help/accounts.ex | 3 ++ lib/who_need_help/accounts/user.ex | 9 ++++ lib/who_need_help_web/live/moderation_live.ex | 11 ++++ test/who_need_help/trust_safety_test.exs | 53 ++++++++++++++++--- .../live/mutual_aid_live_test.exs | 2 + 5 files changed, 72 insertions(+), 6 deletions(-) diff --git a/lib/who_need_help/accounts.ex b/lib/who_need_help/accounts.ex index 0eaecf8..a5bf87d 100644 --- a/lib/who_need_help/accounts.ex +++ b/lib/who_need_help/accounts.ex @@ -211,6 +211,9 @@ defmodule WhoNeedHelp.Accounts do requested_status = attrs["moderation_status"] || attrs[:moderation_status] cond do + user.role in [:moderator, :admin] and not admin_authorized?(moderator) -> + {:error, :forbidden} + user.id == moderator.id and requested_status in [:restricted, :suspended, "restricted", "suspended"] -> {:error, :cannot_restrict_self} diff --git a/lib/who_need_help/accounts/user.ex b/lib/who_need_help/accounts/user.ex index 988d53f..ea0814a 100644 --- a/lib/who_need_help/accounts/user.ex +++ b/lib/who_need_help/accounts/user.ex @@ -102,6 +102,15 @@ defmodule WhoNeedHelp.Accounts.User do |> cast(attrs, [:moderation_status, :moderation_note]) |> validate_required([:moderation_status]) |> validate_length(:moderation_note, max: 1_000) + |> require_moderation_note() + end + + defp require_moderation_note(changeset) do + if get_field(changeset, :moderation_status) in [:restricted, :suspended] do + validate_required(changeset, :moderation_note) + else + changeset + end end def role_changeset(user, attrs) do diff --git a/lib/who_need_help_web/live/moderation_live.ex b/lib/who_need_help_web/live/moderation_live.ex index ef73cc6..53dfd16 100644 --- a/lib/who_need_help_web/live/moderation_live.ex +++ b/lib/who_need_help_web/live/moderation_live.ex @@ -316,6 +316,10 @@ defmodule WhoNeedHelpWeb.ModerationLive do defp report_activity(_report), do: nil + defp can_moderate_account?(%{role: :admin}, _target), do: true + defp can_moderate_account?(%{role: :moderator}, %{role: :user}), do: true + defp can_moderate_account?(_actor, _target), do: false + @impl true def render(assigns) do ~H""" @@ -685,6 +689,7 @@ defmodule WhoNeedHelpWeb.ModerationLive do {status_label(user.moderation_status)} <.form + :if={can_moderate_account?(@current_scope.user, user)} for={user_form} phx-submit="moderate-user" phx-value-id={user.id} @@ -705,6 +710,12 @@ defmodule WhoNeedHelpWeb.ModerationLive do /> <.button class="btn btn-sm btn-primary self-end">{gettext("Save")} +

+ {gettext("Administrator access is required to moderate staff accounts.")} +

<.form :if={@current_scope.user.role == :admin} for={role_form} diff --git a/test/who_need_help/trust_safety_test.exs b/test/who_need_help/trust_safety_test.exs index c1da46b..10caa3e 100644 --- a/test/who_need_help/trust_safety_test.exs +++ b/test/who_need_help/trust_safety_test.exs @@ -452,7 +452,10 @@ defmodule WhoNeedHelp.TrustSafetyTest do test "restricted accounts cannot perform trust-sensitive actions", context do context.helper - |> WhoNeedHelp.Accounts.User.moderation_changeset(%{moderation_status: :restricted}) + |> WhoNeedHelp.Accounts.User.moderation_changeset(%{ + moderation_status: :restricted, + moderation_note: "Test restriction" + }) |> Repo.update!() assert {:error, :account_not_eligible} = @@ -587,7 +590,7 @@ defmodule WhoNeedHelp.TrustSafetyTest do ) end - test "restricted moderators lose authorization and the last active admin cannot be suspended", + test "restricted moderators lose authorization and moderators cannot suspend administrators", context do admin = user_fixture(display_name: "Administrator") @@ -609,7 +612,8 @@ defmodule WhoNeedHelp.TrustSafetyTest do restricted = moderator |> WhoNeedHelp.Accounts.User.moderation_changeset(%{ - "moderation_status" => "restricted" + "moderation_status" => "restricted", + "moderation_note" => "Test moderator restriction" }) |> Repo.update!() @@ -634,7 +638,8 @@ defmodule WhoNeedHelp.TrustSafetyTest do assert {:ok, suspended} = Trust.moderate_user(user_scope_fixture(active_moderator), context.helper.id, %{ - "moderation_status" => "suspended" + "moderation_status" => "suspended", + "moderation_note" => "Confirmed safety policy violation" }) assert suspended.moderation_status == :suspended @@ -649,14 +654,50 @@ defmodule WhoNeedHelp.TrustSafetyTest do active: true ) - assert {:error, :last_admin} = + assert {:error, :forbidden} = Trust.moderate_user(user_scope_fixture(active_moderator), admin.id, %{ - "moderation_status" => "suspended" + "moderation_status" => "suspended", + "moderation_note" => "Attempted staff restriction" }) assert Repo.get!(WhoNeedHelp.Accounts.User, admin.id).moderation_status == :active end + test "moderators cannot restrict staff and restrictions require an internal note", context do + admin = + user_fixture(display_name: "Staff administrator") + |> Ecto.Changeset.change(role: :admin) + |> Repo.update!() + + moderator = + user_fixture(display_name: "Staff moderator") + |> Ecto.Changeset.change(role: :moderator) + |> Repo.update!() + + assert {:error, :forbidden} = + Trust.moderate_user(user_scope_fixture(moderator), admin.id, %{ + "moderation_status" => "restricted", + "moderation_note" => "A moderator must not restrict an administrator" + }) + + assert {:error, %Ecto.Changeset{} = changeset} = + Trust.moderate_user(user_scope_fixture(admin), context.helper.id, %{ + "moderation_status" => "restricted", + "moderation_note" => "" + }) + + assert "can't be blank" in errors_on(changeset).moderation_note + assert Repo.get!(WhoNeedHelp.Accounts.User, context.helper.id).moderation_status == :active + + assert {:ok, restricted} = + Trust.moderate_user(user_scope_fixture(admin), moderator.id, %{ + "moderation_status" => "restricted", + "moderation_note" => "Administrator-reviewed staff restriction" + }) + + assert restricted.moderation_status == :restricted + end + test "the first administrator bootstrap is one-time and audited", context do assert {:ok, admin} = Release.bootstrap_admin(context.helper.email) assert admin.id == context.helper.id diff --git a/test/who_need_help_web/live/mutual_aid_live_test.exs b/test/who_need_help_web/live/mutual_aid_live_test.exs index 19c5480..31552f8 100644 --- a/test/who_need_help_web/live/mutual_aid_live_test.exs +++ b/test/who_need_help_web/live/mutual_aid_live_test.exs @@ -1139,6 +1139,8 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do assert html =~ "Accounts" assert html =~ "Please review the matched conversation." assert html =~ "Bicycle repair" + refute has_element?(view, "#moderation-user-status-#{user.id}") + assert html =~ "Administrator access is required to moderate staff accounts." assert has_element?( view,