From d714f39808f906ffa2b43de410a42c50694fdeaa Mon Sep 17 00:00:00 2001 From: Luna Fox Date: Sat, 11 Jul 2026 17:08:46 +0200 Subject: [PATCH] context logic - "pilot run" of tag changes controller/logic --- lib/philomena/attribution/actor.ex | 5 +- lib/philomena/authorization.ex | 12 +- lib/philomena/integer_id.ex | 48 ++++ lib/philomena/tag_changes.ex | 167 +++++++++++- lib/philomena/tag_changes/tag_change.ex | 2 + lib/philomena/users/user.ex | 2 + .../tag_change/full_revert_controller.ex | 76 +----- .../tag_change/revert_controller.ex | 45 +--- .../controllers/tag_change_controller.ex | 27 +- lib/philomena_web/integer_id.ex | 48 +--- test/philomena/authorization_test.exs | 18 ++ test/philomena/tag_changes_test.exs | 252 ++++++++++++++++++ test/support/fixtures/attribution_fixtures.ex | 15 ++ 13 files changed, 535 insertions(+), 182 deletions(-) create mode 100644 lib/philomena/integer_id.ex create mode 100644 test/philomena/tag_changes_test.exs diff --git a/lib/philomena/attribution/actor.ex b/lib/philomena/attribution/actor.ex index 92440cc02..3902a6232 100644 --- a/lib/philomena/attribution/actor.ex +++ b/lib/philomena/attribution/actor.ex @@ -18,11 +18,8 @@ defmodule Philomena.Attribution.Actor do @enforce_keys [:ip] defstruct user: nil, ip: nil, fingerprint: nil - # `%User{}` is used rather than `User.t()` to match the existing - # `t:Philomena.Users.principal/0` type: the `User` schema does not define a - # `t/0` type, so referencing it here would fail `--warnings-as-errors`. @type t :: %__MODULE__{ - user: %User{} | nil, + user: User.t() | nil, ip: EctoNetwork.INET.t(), fingerprint: String.t() | nil } diff --git a/lib/philomena/authorization.ex b/lib/philomena/authorization.ex index 1a640bfbb..4161f7fd2 100644 --- a/lib/philomena/authorization.ex +++ b/lib/philomena/authorization.ex @@ -16,13 +16,18 @@ defmodule Philomena.Authorization do special-casing is required here. """ + alias Philomena.Attribution.Actor + @doc """ Authorizes `actor` to perform `action` on `subject`. Returns `:ok` when Canada permits the action, otherwise `{:error, :unauthorized}`. - `actor` may be `nil` for an anonymous visitor. + `actor` may be `nil` for an anonymous visitor. It may also be a + `Philomena.Attribution.Actor`: permissions are decided by its `user` alone - + the IP and fingerprint attribute the action but grant nothing - so contexts + that take an attribution can pass it here unchanged. ## Examples @@ -32,9 +37,14 @@ defmodule Philomena.Authorization do iex> authorize(nil, :hide, image) {:error, :unauthorized} + iex> authorize(%Actor{user: moderator, ip: ip}, :revert, TagChange) + :ok + """ @spec authorize(actor :: any(), action :: atom(), subject :: any()) :: :ok | {:error, :unauthorized} + def authorize(%Actor{user: user}, action, subject), do: authorize(user, action, subject) + def authorize(actor, action, subject) do if Canada.Can.can?(actor, action, subject), do: :ok, else: {:error, :unauthorized} end diff --git a/lib/philomena/integer_id.ex b/lib/philomena/integer_id.ex new file mode 100644 index 000000000..ac5b9a6d3 --- /dev/null +++ b/lib/philomena/integer_id.ex @@ -0,0 +1,48 @@ +defmodule Philomena.IntegerId do + @moduledoc """ + Parsing of integer ids taken straight from request paths and query strings. + + Interpolating an unparsed path segment into `where(id: ^id)` raises rather + than returning no rows: `Ecto.Query.CastError` for a non-integer, and + `DBConnection.EncodeError` for a value too large for the `integer` column. + Callers use `parse/1` to turn both into an ordinary "no such row". + """ + + # Bounds of the Postgres `integer` (int4) columns these ids are stored in. + @int_min -2_147_483_648 + @int_max 2_147_483_647 + + @doc """ + Parses an id that an `integer` column could hold. + + Accepts an integer, or a string that is entirely an integer literal. Returns + `:error` for anything else, including values outside the column's range. + + ## Examples + + iex> Philomena.IntegerId.parse("42") + {:ok, 42} + + iex> Philomena.IntegerId.parse("not-a-number") + :error + + iex> Philomena.IntegerId.parse("99999999999999999999") + :error + + """ + @spec parse(any()) :: {:ok, integer()} | :error + def parse(id) when is_integer(id) do + if in_range?(id), do: {:ok, id}, else: :error + end + + def parse(id) when is_binary(id) do + case Integer.parse(id) do + {int, ""} -> parse(int) + _ -> :error + end + end + + def parse(_id), do: :error + + defp in_range?(id), do: id >= @int_min and id <= @int_max +end diff --git a/lib/philomena/tag_changes.ex b/lib/philomena/tag_changes.ex index 8bb8c4e9a..126de70e7 100644 --- a/lib/philomena/tag_changes.ex +++ b/lib/philomena/tag_changes.ex @@ -4,9 +4,15 @@ defmodule Philomena.TagChanges do """ import Ecto.Query, warn: false + import Philomena.Authorization, only: [authorize: 3] + alias Philomena.Repo alias PhilomenaQuery.Parse.IpParser alias PhilomenaQuery.Search + alias Philomena.Attribution.Actor + alias Philomena.IntegerId + alias Philomena.ModerationLogs + alias Philomena.ModerationLogs.Paths alias Philomena.TagChangeRevertWorker alias Philomena.TagChanges alias Philomena.TagChanges.TagChange @@ -18,7 +24,44 @@ defmodule Philomena.TagChanges do alias Philomena.Tags.Tag alias Philomena.Users.User - # Accepts a list of TagChanges.TagChange IDs. + @doc """ + Reverts the tag changes named by `ids` on behalf of `actor`. + + Changes on images hidden from users are silently skipped, and an empty or + fully-skipped list is a successful reversion of zero changes. + + Returns `{:ok, reverted_tag_changes}`, `{:error, :unauthorized}`, or + `{:error, :invalid_ids}` when `ids` is not a list. Failures inside the + batch update surface as their own `{:error, _}` shapes. + """ + @spec revert_tag_changes(Actor.t(), any()) :: + {:ok, [TagChange.t()]} | {:error, any()} + def revert_tag_changes(%Actor{} = actor, ids) do + with :ok <- authorize(actor, :revert, TagChange), + {:ok, tag_changes} <- mass_revert_for(actor, ids) do + ModerationLogs.create_moderation_log( + actor.user, + "TagChange.Revert:create", + Paths.profile_path(actor.user), + "Reverted #{length(tag_changes)} tag changes" + ) + + {:ok, tag_changes} + end + end + + defp mass_revert_for(actor, ids) when is_list(ids) do + mass_revert(ids, %{ + ip: actor.ip, + fingerprint: actor.fingerprint, + user_id: actor.user.id + }) + end + + defp mass_revert_for(_actor, _ids), do: {:error, :invalid_ids} + + # Accepts a list of TagChanges.TagChange IDs. This performs the actual reversion, + # and performs no authorization or logging. def mass_revert(ids, attributes) do tag_changes = Repo.all( @@ -65,14 +108,69 @@ defmodule Philomena.TagChanges do Images.batch_update(changes_per_image, attributes) end - def full_revert(%{user_id: _user_id, attributes: _attributes} = params), - do: Exq.enqueue(Exq, "indexing", TagChangeRevertWorker, [params]) + @doc """ + Enqueues a background reversion of every tag change made by one identity, + on behalf of `actor`. + + Returns `{:ok, target}`, `{:error, :unauthorized}`, or + `{:error, :invalid_target}` when `params` names no target. + """ + @spec full_revert(Actor.t(), map()) :: + {:ok, map()} | {:error, :unauthorized | :invalid_target} + def full_revert(%Actor{} = actor, params) do + with :ok <- authorize(actor, :revert, TagChange), + {:ok, target} <- full_revert_target(params) do + attributes = %{ + ip: to_string(actor.ip), + fingerprint: actor.fingerprint, + user_id: actor.user.id, + batch_size: 100 + } + + Exq.enqueue(Exq, "indexing", TagChangeRevertWorker, [ + Map.put(target, :attributes, attributes) + ]) - def full_revert(%{ip: _ip, attributes: _attributes} = params), - do: Exq.enqueue(Exq, "indexing", TagChangeRevertWorker, [params]) + log_full_revert(actor.user, target) - def full_revert(%{fingerprint: _fingerprint, attributes: _attributes} = params), - do: Exq.enqueue(Exq, "indexing", TagChangeRevertWorker, [params]) + {:ok, target} + end + end + + defp full_revert_target(%{"user_id" => user_id}), do: {:ok, %{user_id: user_id}} + defp full_revert_target(%{"ip" => ip}), do: {:ok, %{ip: ip}} + defp full_revert_target(%{"fingerprint" => fingerprint}), do: {:ok, %{fingerprint: fingerprint}} + defp full_revert_target(_params), do: {:error, :invalid_target} + + defp log_full_revert(user, target) do + {subject, subject_path} = + case target do + %{user_id: user_id} -> + full_revert_log_user(user_id) + + %{ip: ip} -> + {"ip #{ip}", Paths.ip_profile_path(ip)} + + %{fingerprint: fingerprint} -> + {"fingerprint #{fingerprint}", Paths.fingerprint_profile_path(fingerprint)} + end + + ModerationLogs.create_moderation_log( + user, + "TagChange.FullRevert:create", + subject_path, + "Reverted all tag changes for #{subject}" + ) + end + + defp full_revert_log_user(user_id) do + with {:ok, id} <- IntegerId.parse(user_id), + %User{} = user <- Repo.get(User, id) do + {"user #{user.name}", Paths.profile_path(user)} + else + _ -> {"user #{user_id}", "/tag_changes"} + end + end @doc """ Updates tag change search indices when a user's name changes. @@ -228,21 +326,51 @@ defmodule Philomena.TagChanges do end @doc """ - Deletes a TagChange. + Deletes the tag change named by the raw request `id` from the history, on + behalf of `actor` (a user, or `nil` for an anonymous visitor). + + An id that cannot name a row is `{:error, :not_found}`, while a well-formed id + that names no row authorizes `nil` - which no rule permits - and is + therefore `{:error, :unauthorized}`. ## Examples - iex> delete_tag_change(tag_change) + iex> delete_tag_change(moderator, "1") {:ok, %TagChange{}} - iex> delete_tag_change(tag_change) - {:error, %Ecto.Changeset{}} + iex> delete_tag_change(user, "1") + {:error, :unauthorized} + + iex> delete_tag_change(moderator, "not-an-integer") + {:error, :not_found} """ - def delete_tag_change(%TagChange{} = tag_change) do + @spec delete_tag_change(User.t() | nil, any()) :: + {:ok, TagChange.t()} + | {:error, :unauthorized | :not_found} + | {:error, Ecto.Changeset.t()} + def delete_tag_change(actor, id) do + case IntegerId.parse(id) do + {:ok, id} -> + tag_change = + TagChange + |> preload([:user, :image, tags: [:tag]]) + |> Repo.get(id) + + with :ok <- authorize(actor, :delete, tag_change) do + delete_loaded_tag_change(actor, tag_change) + end + + :error -> + {:error, :not_found} + end + end + + defp delete_loaded_tag_change(actor, %TagChange{} = tag_change) do case Repo.delete(tag_change) do {:ok, %TagChange{} = tc} = result -> Search.delete_document(tc.id, TagChange) + log_tag_change_deletion(actor, tc) result result -> @@ -250,6 +378,21 @@ defmodule Philomena.TagChanges do end end + defp log_tag_change_deletion(actor, %TagChange{user: user, image: image, tags: tags, ip: ip}) do + name = + case user do + %{name: name} -> name + _ -> to_string(ip) + end + + ModerationLogs.create_moderation_log( + actor, + "TagChange:delete", + Paths.image_path(image), + "Deleted tag change by #{name} containing #{length(tags)} tags on image #{image.id} from history" + ) + end + @doc """ Deletes tag changes that have no associated tags. ## Examples diff --git a/lib/philomena/tag_changes/tag_change.ex b/lib/philomena/tag_changes/tag_change.ex index 4bb0de04c..df3096c4d 100644 --- a/lib/philomena/tag_changes/tag_change.ex +++ b/lib/philomena/tag_changes/tag_change.ex @@ -1,6 +1,8 @@ defmodule Philomena.TagChanges.TagChange do use Ecto.Schema + @type t :: %__MODULE__{} + schema "tag_changes" do belongs_to :user, Philomena.Users.User belongs_to :image, Philomena.Images.Image diff --git a/lib/philomena/users/user.ex b/lib/philomena/users/user.ex index 91873b52e..f1ebc6556 100644 --- a/lib/philomena/users/user.ex +++ b/lib/philomena/users/user.ex @@ -22,6 +22,8 @@ defmodule Philomena.Users.User do alias Philomena.Donations.Donation alias Philomena.UserNameChanges.UserNameChange + @type t :: %__MODULE__{} + @derive {Phoenix.Param, key: :slug} @derive {Inspect, except: [:password]} schema "users" do diff --git a/lib/philomena_web/controllers/tag_change/full_revert_controller.ex b/lib/philomena_web/controllers/tag_change/full_revert_controller.ex index 71cca11b4..6286aea3c 100644 --- a/lib/philomena_web/controllers/tag_change/full_revert_controller.ex +++ b/lib/philomena_web/controllers/tag_change/full_revert_controller.ex @@ -1,84 +1,26 @@ defmodule PhilomenaWeb.TagChange.FullRevertController do use PhilomenaWeb, :controller - alias Philomena.Users.User - alias Philomena.TagChanges.TagChange alias Philomena.TagChanges - alias PhilomenaWeb.IntegerId - alias Philomena.Repo - plug :verify_authorized plug PhilomenaWeb.UserAttributionPlug - def create(%{assigns: %{attributes: attributes}} = conn, params) do - attributes = %{ - ip: to_string(attributes[:ip]), - fingerprint: attributes[:fingerprint], - user_id: attributes[:user].id, - batch_size: attributes[:batch_size] || 100 - } + action_fallback PhilomenaWeb.FallbackController - case revert_target(params) do - nil -> + def create(conn, params) do + case TagChanges.full_revert(conn.assigns.actor, params) do + {:ok, _target} -> conn - |> put_flash(:error, "Couldn't revert those tag changes!") + |> put_flash(:info, "Reversion of tag changes enqueued.") |> redirect(external: conn.assigns.referrer) - target -> - TagChanges.full_revert(Map.put(target, :attributes, attributes)) + {:error, :unauthorized} = error -> + error + {:error, :invalid_target} -> conn - |> put_flash(:info, "Reversion of tag changes enqueued.") - |> moderation_log( - details: &log_details/2, - data: %{user: conn.assigns.current_user, params: params} - ) + |> put_flash(:error, "Couldn't revert those tag changes!") |> redirect(external: conn.assigns.referrer) end end - - defp revert_target(%{"user_id" => user_id}), do: %{user_id: user_id} - defp revert_target(%{"ip" => ip}), do: %{ip: ip} - defp revert_target(%{"fingerprint" => fp}), do: %{fingerprint: fp} - defp revert_target(_params), do: nil - - defp verify_authorized(conn, _params) do - if Canada.Can.can?(conn.assigns.current_user, :revert, TagChange) do - conn - else - PhilomenaWeb.NotAuthorizedPlug.call(conn) - end - end - - defp log_details(_action, data) do - {subject, subject_path} = - case data.params do - %{"user_id" => user_id} -> - log_user(user_id) - - %{"ip" => ip} -> - {"ip #{ip}", ~p"/ip_profiles/#{ip}"} - - %{"fingerprint" => fp} -> - {"fingerprint #{fp}", ~p"/fingerprint_profiles/#{fp}"} - end - - %{body: "Reverted all tag changes for #{subject}", subject_path: subject_path} - end - - # The revert is enqueued for whatever id was named, so the log entry has to - # survive an id that names no user. - defp log_user(user_id) do - case load_user(user_id) do - nil -> {"user #{user_id}", ~p"/tag_changes"} - user -> {"user #{user.name}", ~p"/profiles/#{user}"} - end - end - - defp load_user(user_id) do - case IntegerId.parse(user_id) do - {:ok, id} -> Repo.get(User, id) - :error -> nil - end - end end diff --git a/lib/philomena_web/controllers/tag_change/revert_controller.ex b/lib/philomena_web/controllers/tag_change/revert_controller.ex index 6fbcd85e0..dc7df5314 100644 --- a/lib/philomena_web/controllers/tag_change/revert_controller.ex +++ b/lib/philomena_web/controllers/tag_change/revert_controller.ex @@ -1,53 +1,26 @@ defmodule PhilomenaWeb.TagChange.RevertController do use PhilomenaWeb, :controller - alias Philomena.TagChanges.TagChange alias Philomena.TagChanges - plug :verify_authorized plug PhilomenaWeb.UserAttributionPlug - def create(conn, %{"ids" => ids}) when is_list(ids) do - attributes = conn.assigns.attributes + action_fallback PhilomenaWeb.FallbackController - attributes = %{ - ip: attributes[:ip], - fingerprint: attributes[:fingerprint], - user_id: attributes[:user].id - } - - case TagChanges.mass_revert(ids, attributes) do + def create(conn, params) do + case TagChanges.revert_tag_changes(conn.assigns.actor, params["ids"]) do {:ok, tag_changes} -> conn |> put_flash(:info, "Successfully reverted #{length(tag_changes)} tag changes.") - |> moderation_log( - details: &log_details/2, - data: %{user: conn.assigns.current_user, count: length(tag_changes)} - ) |> redirect(external: conn.assigns.referrer) - _error -> - revert_failed(conn) - end - end - - def create(conn, _params), do: revert_failed(conn) - - defp revert_failed(conn) do - conn - |> put_flash(:error, "Couldn't revert those tag changes!") - |> redirect(external: conn.assigns.referrer) - end + {:error, :unauthorized} = error -> + error - defp verify_authorized(conn, _params) do - if Canada.Can.can?(conn.assigns.current_user, :revert, TagChange) do - conn - else - PhilomenaWeb.NotAuthorizedPlug.call(conn) + _error -> + conn + |> put_flash(:error, "Couldn't revert those tag changes!") + |> redirect(external: conn.assigns.referrer) end end - - defp log_details(_action, data) do - %{body: "Reverted #{data.count} tag changes", subject_path: ~p"/profiles/#{data.user}"} - end end diff --git a/lib/philomena_web/controllers/tag_change_controller.ex b/lib/philomena_web/controllers/tag_change_controller.ex index 6f676af7e..02733684f 100644 --- a/lib/philomena_web/controllers/tag_change_controller.ex +++ b/lib/philomena_web/controllers/tag_change_controller.ex @@ -2,12 +2,8 @@ defmodule PhilomenaWeb.TagChangeController do use PhilomenaWeb, :controller alias Philomena.TagChanges - alias Philomena.TagChanges.TagChange - plug :load_and_authorize_resource, - model: TagChange, - only: [:delete], - preload: [:user, :image, tags: [:tag]] + action_fallback PhilomenaWeb.FallbackController def index(conn, params) do tag_changes = @@ -26,28 +22,19 @@ defmodule PhilomenaWeb.TagChangeController do end def delete(conn, params) do - case TagChanges.delete_tag_change(conn.assigns.tag_change) do - {:ok, tag_change} -> + case TagChanges.delete_tag_change(conn.assigns.current_user, params["id"]) do + {:ok, _tag_change} -> conn |> put_flash(:info, "Successfully deleted tag change from history.") - |> moderation_log( - details: &log_details/2, - data: tag_change - ) |> redirect(to: params["redirect"]) - _ -> + {:error, %Ecto.Changeset{}} -> conn |> put_flash(:error, "Failed to delete tag change from history.") |> redirect(to: params["redirect"]) - end - end - defp log_details(_action, %{user: %{name: name}, image: image, tags: tags}) do - %{ - body: - "Deleted tag change by #{name} containing #{length(tags)} tags on image #{image.id} from history", - subject_path: ~p"/images/#{image}" - } + {:error, _} = error -> + error + end end end diff --git a/lib/philomena_web/integer_id.ex b/lib/philomena_web/integer_id.ex index 19b10f4da..a4f1feb93 100644 --- a/lib/philomena_web/integer_id.ex +++ b/lib/philomena_web/integer_id.ex @@ -1,48 +1,12 @@ defmodule PhilomenaWeb.IntegerId do @moduledoc """ - Parsing of integer ids taken straight from request paths and query strings. + Deprecated home of `Philomena.IntegerId`. - Interpolating an unparsed path segment into `where(id: ^id)` raises rather - than returning no rows: `Ecto.Query.CastError` for a non-integer, and - `DBConnection.EncodeError` for a value too large for the `integer` column. - Callers use `parse/1` to turn both into an ordinary "no such row". + Id parsing moved into the domain layer so that contexts, which cannot + reference `PhilomenaWeb`, can turn raw request ids into an ordinary + "no such row" themselves. This module remains for legacy callers before + everything is migrated to contexts. """ - # Bounds of the Postgres `integer` (int4) columns these ids are stored in. - @int_min -2_147_483_648 - @int_max 2_147_483_647 - - @doc """ - Parses an id that an `integer` column could hold. - - Accepts an integer, or a string that is entirely an integer literal. Returns - `:error` for anything else, including values outside the column's range. - - ## Examples - - iex> PhilomenaWeb.IntegerId.parse("42") - {:ok, 42} - - iex> PhilomenaWeb.IntegerId.parse("not-a-number") - :error - - iex> PhilomenaWeb.IntegerId.parse("99999999999999999999") - :error - - """ - @spec parse(any()) :: {:ok, integer()} | :error - def parse(id) when is_integer(id) do - if in_range?(id), do: {:ok, id}, else: :error - end - - def parse(id) when is_binary(id) do - case Integer.parse(id) do - {int, ""} -> parse(int) - _ -> :error - end - end - - def parse(_id), do: :error - - defp in_range?(id), do: id >= @int_min and id <= @int_max + defdelegate parse(id), to: Philomena.IntegerId end diff --git a/test/philomena/authorization_test.exs b/test/philomena/authorization_test.exs index 03ae8ddc6..2fd425e32 100644 --- a/test/philomena/authorization_test.exs +++ b/test/philomena/authorization_test.exs @@ -91,4 +91,22 @@ defmodule Philomena.AuthorizationTest do assert Authorization.authorize(nil, :show, image) == :ok end end + + describe "authorize/3 with an Attribution.Actor" do + # The struct's user alone decides permissions; the IP and fingerprint + # attribute the action but grant nothing. + import Philomena.AttributionFixtures, only: [actor: 0, actor: 1] + + test "resolves to the wrapped moderator", %{moderator: moderator} do + assert Authorization.authorize(actor(moderator), :edit, %Tag{}) == :ok + end + + test "resolves to the wrapped regular user", %{user: user} do + assert Authorization.authorize(actor(user), :edit, %Tag{}) == {:error, :unauthorized} + end + + test "an actor with no user is an anonymous visitor" do + assert Authorization.authorize(actor(), :edit, %Tag{}) == {:error, :unauthorized} + end + end end diff --git a/test/philomena/tag_changes_test.exs b/test/philomena/tag_changes_test.exs new file mode 100644 index 000000000..c60d2c6c7 --- /dev/null +++ b/test/philomena/tag_changes_test.exs @@ -0,0 +1,252 @@ +defmodule Philomena.TagChangesTest do + @moduledoc """ + Context-level tests for the actor-first `Philomena.TagChanges` API: + `delete_tag_change/2`, `revert_tag_changes/2`, and `full_revert/2`. + + These pin the authorization matrix (anonymous/user/moderator/admin), the + two global error shapes, and the moderation log entries - type strings, + bodies, and subject paths byte-for-byte - that each function writes on + success. The corresponding controller characterization tests pin the HTTP + behavior on top of these results. + """ + + use Philomena.DataCase, async: false + + # delete_tag_change/2 removes the record's search document, so this module + # follows the OpenSearch test rules (async: false, index cycled in setup). + @moduletag :search + + import Philomena.AttributionFixtures + import Philomena.ImagesFixtures + import Philomena.UsersFixtures + + import Ecto.Query + + alias Philomena.Images + alias Philomena.ModerationLogs.ModerationLog + alias Philomena.ModerationLogs.Paths + alias Philomena.Repo + alias Philomena.TagChanges + alias Philomena.TagChanges.TagChange + alias PhilomenaQuery.Search + + setup do + Search.clear_index!(TagChange) + # Valkey rate-limit counters are not rolled back by the SQL sandbox; reset + # the tag-change limit so accumulated counts don't trip check_limits. + reset_tag_change_limits() + :ok + end + + # Arranges an image whose tags went from "safe" to three tags, returning the + # image plus the single TagChange row that recorded the two adds. + defp tag_change!(user) do + image = image_fixture() + + {:ok, _} = + Images.update_tags(image, attribution(user), %{ + "old_tag_input" => "safe", + "tag_input" => "safe, added test tag, other added tag" + }) + + {image, Repo.one!(from tc in TagChange, where: tc.image_id == ^image.id)} + end + + defp image_tag_names(image) do + image + |> Repo.preload(:tags, force: true) + |> Map.fetch!(:tags) + |> Enum.map(& &1.name) + end + + defp only_moderation_log! do + Repo.one!(ModerationLog) + end + + describe "delete_tag_change/2" do + test "denies an anonymous actor" do + {_image, tc} = tag_change!(confirmed_user_fixture()) + + assert TagChanges.delete_tag_change(nil, "#{tc.id}") == {:error, :unauthorized} + assert Repo.get(TagChange, tc.id) + end + + test "denies a regular user" do + {_image, tc} = tag_change!(confirmed_user_fixture()) + + assert TagChanges.delete_tag_change(confirmed_user_fixture(), "#{tc.id}") == + {:error, :unauthorized} + + assert Repo.get(TagChange, tc.id) + end + + test "a moderator deletes the change and a moderation log is written" do + author = confirmed_user_fixture() + moderator = moderator_user_fixture() + {image, tc} = tag_change!(author) + + assert {:ok, %TagChange{}} = TagChanges.delete_tag_change(moderator, "#{tc.id}") + refute Repo.get(TagChange, tc.id) + + log = only_moderation_log!() + assert log.user_id == moderator.id + assert log.type == "TagChange:delete" + assert log.subject_path == "/images/#{image.id}" + + assert log.body == + "Deleted tag change by #{author.name} containing 2 tags on image #{image.id} from history" + end + + test "an admin may also delete" do + {_image, tc} = tag_change!(confirmed_user_fixture()) + + assert {:ok, %TagChange{}} = TagChanges.delete_tag_change(admin_user_fixture(), tc.id) + end + + test "deleting an anonymous change logs the author as anonymous" do + moderator = moderator_user_fixture() + {image, tc} = tag_change!(nil) + + assert {:ok, %TagChange{}} = TagChanges.delete_tag_change(moderator, "#{tc.id}") + refute Repo.get(TagChange, tc.id) + + assert only_moderation_log!().body == + "Deleted tag change by 203.0.113.1 containing 2 tags on image #{image.id} from history" + end + + test "a well-formed id naming no row is unauthorized, not not-found" do + # The former load-then-authorize plug authorized the nil load result, + # which no :delete rule permits; the context preserves that shape. + assert TagChanges.delete_tag_change(moderator_user_fixture(), "123456789") == + {:error, :unauthorized} + end + + test "an id that cannot name a row is not found" do + moderator = moderator_user_fixture() + + assert TagChanges.delete_tag_change(moderator, "not-an-integer") == {:error, :not_found} + + assert TagChanges.delete_tag_change(moderator, "99999999999999999999") == + {:error, :not_found} + end + end + + describe "revert_tag_changes/2" do + test "denies an anonymous actor" do + assert TagChanges.revert_tag_changes(actor(), ["1"]) == {:error, :unauthorized} + end + + test "denies a regular user before looking at the ids" do + # Authorization comes first, as it did when it was a plug: a bad ids + # shape from an unprivileged user is still unauthorized. + user_actor = actor(confirmed_user_fixture()) + + assert TagChanges.revert_tag_changes(user_actor, ["1"]) == {:error, :unauthorized} + assert TagChanges.revert_tag_changes(user_actor, "42") == {:error, :unauthorized} + end + + test "a moderator reverts the listed changes and a moderation log is written" do + moderator = moderator_user_fixture() + {image, tc} = tag_change!(confirmed_user_fixture()) + + assert "added test tag" in image_tag_names(image) + + assert {:ok, [%TagChange{}]} = + TagChanges.revert_tag_changes(actor(moderator), ["#{tc.id}"]) + + # Reverting the change removes the two tags it had added. + names = image_tag_names(image) + refute "added test tag" in names + refute "other added tag" in names + assert "safe" in names + + log = only_moderation_log!() + assert log.user_id == moderator.id + assert log.type == "TagChange.Revert:create" + # Slug encoding (e.g. `@` → `%40`) is pinned in the Paths tests. + assert log.subject_path == Paths.profile_path(moderator) + assert log.body == "Reverted 1 tag changes" + end + + test "an empty list is a successful reversion of zero changes" do + assert {:ok, []} = TagChanges.revert_tag_changes(actor(moderator_user_fixture()), []) + + assert only_moderation_log!().body == "Reverted 0 tag changes" + end + + test "a non-list ids value from a moderator is invalid" do + assert TagChanges.revert_tag_changes(actor(moderator_user_fixture()), "42") == + {:error, :invalid_ids} + + assert Repo.aggregate(ModerationLog, :count) == 0 + end + end + + describe "full_revert/2" do + test "denies an anonymous actor" do + assert TagChanges.full_revert(actor(), %{"user_id" => "1"}) == {:error, :unauthorized} + end + + test "denies a regular user before looking at the target" do + user_actor = actor(confirmed_user_fixture()) + + assert TagChanges.full_revert(user_actor, %{"user_id" => "1"}) == {:error, :unauthorized} + + assert TagChanges.full_revert(user_actor, %{"something" => "else"}) == + {:error, :unauthorized} + end + + test "a moderator enqueues a reversion for a user and the log names them" do + moderator = moderator_user_fixture() + target = confirmed_user_fixture() + + assert TagChanges.full_revert(actor(moderator), %{"user_id" => "#{target.id}"}) == + {:ok, %{user_id: "#{target.id}"}} + + log = only_moderation_log!() + assert log.user_id == moderator.id + assert log.type == "TagChange.FullRevert:create" + assert log.subject_path == Paths.profile_path(target) + assert log.body == "Reverted all tag changes for user #{target.name}" + end + + test "a user id naming no user still logs, against the tag changes listing" do + assert {:ok, _target} = + TagChanges.full_revert(actor(moderator_user_fixture()), %{ + "user_id" => "123456789" + }) + + log = only_moderation_log!() + assert log.subject_path == "/tag_changes" + assert log.body == "Reverted all tag changes for user 123456789" + end + + test "a moderator enqueues a reversion for an ip" do + assert {:ok, %{ip: "203.0.113.9"}} = + TagChanges.full_revert(actor(moderator_user_fixture()), %{"ip" => "203.0.113.9"}) + + log = only_moderation_log!() + assert log.type == "TagChange.FullRevert:create" + assert log.subject_path == "/ip_profiles/203.0.113.9" + assert log.body == "Reverted all tag changes for ip 203.0.113.9" + end + + test "a moderator enqueues a reversion for a fingerprint" do + assert {:ok, %{fingerprint: "c1774e9294a"}} = + TagChanges.full_revert(actor(moderator_user_fixture()), %{ + "fingerprint" => "c1774e9294a" + }) + + log = only_moderation_log!() + assert log.subject_path == "/fingerprint_profiles/c1774e9294a" + assert log.body == "Reverted all tag changes for fingerprint c1774e9294a" + end + + test "params naming no target are invalid" do + assert TagChanges.full_revert(actor(moderator_user_fixture()), %{"something" => "else"}) == + {:error, :invalid_target} + + assert Repo.aggregate(ModerationLog, :count) == 0 + end + end +end diff --git a/test/support/fixtures/attribution_fixtures.ex b/test/support/fixtures/attribution_fixtures.ex index d42b72c29..0e2529428 100644 --- a/test/support/fixtures/attribution_fixtures.ex +++ b/test/support/fixtures/attribution_fixtures.ex @@ -20,6 +20,21 @@ defmodule Philomena.AttributionFixtures do ] end + @doc """ + The same attribution as `attribution/1`, as the typed + `Philomena.Attribution.Actor` struct that actor-first context functions + take. + """ + def actor(user \\ nil) do + attrs = attribution(user) + + %Philomena.Attribution.Actor{ + ip: attrs[:ip], + fingerprint: attrs[:fingerprint], + user: attrs[:user] + } + end + @doc """ Clears the Valkey tag-change rate-limit counters for the given attribution.