From 618b3d23d93af9773e6de3875d29b7f8e5dd71f7 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 1 Jun 2023 14:48:42 +0200 Subject: [PATCH 1/3] refactor(anti-spam): make anti-spam agnostic from Akismet Signed-off-by: Thomas Citharel --- config/config.exs | 2 + config/test.exs | 2 + lib/graphql/api/reports.ex | 2 +- lib/graphql/resolvers/comment.ex | 6 +- lib/graphql/resolvers/config.ex | 4 +- lib/graphql/resolvers/event.ex | 4 +- lib/graphql/resolvers/person.ex | 22 +++---- lib/graphql/resolvers/user.ex | 8 +-- .../mobilizon/maintenance/detect_spam.ex | 16 +++-- lib/service/{ => anti_spam}/akismet.ex | 10 +++- lib/service/anti_spam/anti_spam.ex | 17 ++++++ lib/service/anti_spam/provider.ex | 58 +++++++++++++++++++ test/support/mocks/anti_spam_mock.ex | 26 +++++++++ 13 files changed, 150 insertions(+), 27 deletions(-) rename lib/service/{ => anti_spam}/akismet.ex (97%) create mode 100644 lib/service/anti_spam/anti_spam.ex create mode 100644 lib/service/anti_spam/provider.ex create mode 100644 test/support/mocks/anti_spam_mock.ex diff --git a/config/config.exs b/config/config.exs index 59a6a4b6c..70e446dfb 100644 --- a/config/config.exs +++ b/config/config.exs @@ -378,6 +378,8 @@ config :mobilizon, Mobilizon.Service.GlobalSearch.SearchMobilizon, img_src: ["search.joinmobilizon.org"] ] +config :mobilizon, Mobilizon.Service.AntiSpam, service: Mobilizon.Service.AntiSpam.Akismet + # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. import_config "#{config_env()}.exs" diff --git a/config/test.exs b/config/test.exs index b5995e94a..38a71c467 100644 --- a/config/test.exs +++ b/config/test.exs @@ -90,6 +90,8 @@ config :junit_formatter, report_dir: "." config :mobilizon, :http_security, report_uri: "https://endpoint.com" +config :mobilizon, Mobilizon.Service.AntiSpam, service: Mobilizon.Service.AntiSpam.Mock + if System.get_env("DOCKER", "false") == "false" && File.exists?("./config/test.secret.exs") do import_config "test.secret.exs" end diff --git a/lib/graphql/api/reports.ex b/lib/graphql/api/reports.ex index 341beb7ee..561e269b5 100644 --- a/lib/graphql/api/reports.ex +++ b/lib/graphql/api/reports.ex @@ -8,7 +8,7 @@ defmodule Mobilizon.GraphQL.API.Reports do alias Mobilizon.Federation.ActivityPub.{Actions, Activity} alias Mobilizon.Reports, as: ReportsAction alias Mobilizon.Reports.{Note, Report, ReportStatus} - alias Mobilizon.Service.Akismet + alias Mobilizon.Service.AntiSpam.Akismet alias Mobilizon.Users.User import Mobilizon.Web.Gettext, only: [dgettext: 2] require Logger diff --git a/lib/graphql/resolvers/comment.ex b/lib/graphql/resolvers/comment.ex index 92aacea11..37f0144b5 100644 --- a/lib/graphql/resolvers/comment.ex +++ b/lib/graphql/resolvers/comment.ex @@ -7,7 +7,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Comment do alias Mobilizon.Actors.Actor alias Mobilizon.Discussions.Comment, as: CommentModel alias Mobilizon.Events.{Event, EventOptions} - alias Mobilizon.Service.Akismet + alias Mobilizon.Service.AntiSpam alias Mobilizon.Users.User import Mobilizon.Web.Gettext @@ -45,14 +45,14 @@ defmodule Mobilizon.GraphQL.Resolvers.Comment do if comment_moderation != :closed || actor_id == organizer_actor_id do args = Map.put(args, :actor_id, actor_id) - if Akismet.check_comment( + if AntiSpam.service().check_comment( args.text, preferred_username, !is_nil(Map.get(args, :in_reply_to_comment_id)), email, current_ip, user_agent - ) do + ) == :ham do do_create_comment(args) else {:error, diff --git a/lib/graphql/resolvers/config.ex b/lib/graphql/resolvers/config.ex index 8a1aa0e88..c38d0007e 100644 --- a/lib/graphql/resolvers/config.ex +++ b/lib/graphql/resolvers/config.ex @@ -5,7 +5,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Config do alias Mobilizon.Config alias Mobilizon.Events.Categories - alias Mobilizon.Service.{Akismet, FrontEndAnalytics} + alias Mobilizon.Service.{AntiSpam, FrontEndAnalytics} @doc """ Gets config. @@ -146,7 +146,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Config do features: %{ groups: Config.instance_group_feature_enabled?(), event_creation: Config.instance_event_creation_enabled?(), - antispam: Akismet.ready?() + antispam: AntiSpam.service().ready?() }, restrictions: %{ only_admin_can_create_groups: Config.only_admin_can_create_groups?(), diff --git a/lib/graphql/resolvers/event.ex b/lib/graphql/resolvers/event.ex index 052f0c87f..aaf18ef8c 100644 --- a/lib/graphql/resolvers/event.ex +++ b/lib/graphql/resolvers/event.ex @@ -13,7 +13,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do alias Mobilizon.Federation.ActivityPub.Activity alias Mobilizon.Federation.ActivityPub.Permission - alias Mobilizon.Service.Akismet + alias Mobilizon.Service.AntiSpam alias Mobilizon.Service.TimezoneDetector import Mobilizon.Users.Guards, only: [is_moderator: 1] import Mobilizon.Web.Gettext @@ -260,7 +260,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do args |> Map.put(:organizer_actor, organizer_actor) |> extract_timezone(user.id), {:askismet, :ham} <- {:askismet, - Akismet.check_event( + AntiSpam.service().check_event( args.description, organizer_actor.preferred_username, email, diff --git a/lib/graphql/resolvers/person.ex b/lib/graphql/resolvers/person.ex index 0e5c89cb3..138a8526d 100644 --- a/lib/graphql/resolvers/person.ex +++ b/lib/graphql/resolvers/person.ex @@ -8,7 +8,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Person do alias Mobilizon.{Actors, Events, Users} alias Mobilizon.Actors.{Actor, Follower, Member} alias Mobilizon.Events.Participant - alias Mobilizon.Service.Akismet + alias Mobilizon.Service.AntiSpam alias Mobilizon.Storage.{Page, Repo} alias Mobilizon.Users.User import Mobilizon.Web.Gettext @@ -133,18 +133,20 @@ defmodule Mobilizon.GraphQL.Resolvers.Person do def create_person( _parent, %{preferred_username: _preferred_username} = args, - %{context: %{current_user: user}} = _resolution + %{context: %{current_user: user} = context} = _resolution ) do args = Map.put(args, :user_id, user.id) + user_agent = Map.get(context, :user_agent, "") with args <- Map.update(args, :preferred_username, "", &String.downcase/1), - {:akismet, :ham} <- - {:akismet, - Akismet.check_profile( + {:spam, :ham} <- + {:spam, + AntiSpam.service().check_profile( args.preferred_username, args.summary, user.email, - user.current_sign_in_ip + user.current_sign_in_ip, + user_agent )}, {:picture, args} when is_map(args) <- {:picture, save_attached_pictures(args)}, {:ok, %Actor{} = new_person} <- Actors.new_person(args) do @@ -308,9 +310,9 @@ defmodule Mobilizon.GraphQL.Resolvers.Person do {:ok, %User{} = user} -> if is_nil(Users.get_actor_for_user(user)) do # No profile yet, we can create one - with {:akismet, :ham} <- - {:akismet, - Akismet.check_profile( + with {:spam, :ham} <- + {:spam, + AntiSpam.service().check_profile( args.preferred_username, args.summary, args.email, @@ -326,7 +328,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Person do {:error, _err} -> {:error, dgettext("errors", "Error while uploading pictures")} - {:akismet, _} -> + {:spam, _} -> {:error, dgettext("errors", "Your profile was detected as spam.")} end else diff --git a/lib/graphql/resolvers/user.ex b/lib/graphql/resolvers/user.ex index a6d224209..73d85e0ee 100644 --- a/lib/graphql/resolvers/user.ex +++ b/lib/graphql/resolvers/user.ex @@ -8,7 +8,7 @@ defmodule Mobilizon.GraphQL.Resolvers.User do alias Mobilizon.{Actors, Admin, Config, Events, FollowedGroupActivity, Users} alias Mobilizon.Actors.Actor alias Mobilizon.Federation.ActivityPub.Actions - alias Mobilizon.Service.Akismet + alias Mobilizon.Service.AntiSpam alias Mobilizon.Service.Auth.Authenticator alias Mobilizon.Storage.{Page, Repo} alias Mobilizon.Users.{Setting, User} @@ -161,8 +161,8 @@ defmodule Mobilizon.GraphQL.Resolvers.User do with {:ok, email} <- lowercase_domain(email), :registration_ok <- check_registration_config(email), :not_deny_listed <- check_registration_denylist(email), - {:akismet, :ham} <- - {:akismet, Akismet.check_user(email, current_ip, user_agent)}, + {:spam, :ham} <- + {:spam, AntiSpam.service().check_user(email, current_ip, user_agent)}, {:ok, %User{} = user} <- args |> Map.merge(%{email: email, current_sign_in_ip: current_ip, current_sign_in_at: now}) @@ -186,7 +186,7 @@ defmodule Mobilizon.GraphQL.Resolvers.User do "Your e-mail has been denied registration or uses a disallowed e-mail provider" )} - {:akismet, _} -> + {:spam, _} -> {:error, dgettext( "errors", diff --git a/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex b/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex index 9d436451b..1e3ee5c1d 100644 --- a/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex +++ b/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex @@ -6,7 +6,7 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do alias Mobilizon.{Actors, Config, Events, Users} alias Mobilizon.Actors.Actor alias Mobilizon.Events.Event - alias Mobilizon.Service.Akismet + alias Mobilizon.Service.AntiSpam import Mix.Tasks.Mobilizon.Common alias Mobilizon.Federation.ActivityPub.Actions alias Mobilizon.Web.Endpoint @@ -35,7 +35,7 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do start_mobilizon() - unless Akismet.ready?() do + unless anti_spam().ready?() do shell_error("Akismet is missing an API key in the configuration") end @@ -79,7 +79,7 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do email = if(is_nil(user), do: nil, else: user.email) ip = if(is_nil(user), do: nil, else: user.current_sign_in_ip || user.last_sign_in_ip) - case Akismet.check_profile(preferred_username, summary, email, ip) do + case anti_spam().check_profile(preferred_username, summary, email, ip, nil) do res when res in [:spam, :discard] -> handle_spam_profile(preferred_username, id, options) @@ -113,7 +113,13 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do {nil, nil} end - case Akismet.check_event(event_description, organizer_actor.preferred_username, email, ip) do + case anti_spam().check_event( + event_description, + organizer_actor.preferred_username, + email, + ip, + nil + ) do res when res in [:spam, :discard] -> handle_spam_event(event_id, title, uuid, organizer_actor.id, options) @@ -174,4 +180,6 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do defp verbose?(options), do: Keyword.get(options, :verbose, false) defp dry_run?(options), do: Keyword.get(options, :dry_run, false) + + defp anti_spam, do: AntiSpam.service() end diff --git a/lib/service/akismet.ex b/lib/service/anti_spam/akismet.ex similarity index 97% rename from lib/service/akismet.ex rename to lib/service/anti_spam/akismet.ex index 0b1b4dc9f..f17946e6c 100644 --- a/lib/service/akismet.ex +++ b/lib/service/anti_spam/akismet.ex @@ -1,4 +1,4 @@ -defmodule Mobilizon.Service.Akismet do +defmodule Mobilizon.Service.AntiSpam.Akismet do @moduledoc """ Validate user data """ @@ -9,12 +9,16 @@ defmodule Mobilizon.Service.Akismet do alias Mobilizon.Discussions.Comment alias Mobilizon.Events.Event alias Mobilizon.Reports.Report + alias Mobilizon.Service.AntiSpam.Provider alias Mobilizon.Users.User alias Mobilizon.Web.Endpoint require Logger + @behaviour Provider + @env Application.compile_env(:mobilizon, :env) + @impl Provider @spec check_user(String.t(), String.t(), String.t()) :: :ham | :spam | :discard | {:error, HTTPoison.Response.t()} def check_user(email, ip, user_agent) do @@ -27,6 +31,7 @@ defmodule Mobilizon.Service.Akismet do }) end + @impl Provider @spec check_profile(String.t(), String.t(), String.t() | nil, String.t(), String.t()) :: :ham | :spam | :discard | {:error, HTTPoison.Response.t()} def check_profile(username, summary, email \\ nil, ip \\ "127.0.0.1", user_agent \\ nil) do @@ -41,6 +46,7 @@ defmodule Mobilizon.Service.Akismet do }) end + @impl Provider @spec check_event(String.t(), String.t(), String.t() | nil, String.t(), String.t()) :: :ham | :spam | :discard | {:error, HTTPoison.Response.t()} def check_event(event_body, username, email \\ nil, ip \\ "127.0.0.1", user_agent \\ nil) do @@ -55,6 +61,7 @@ defmodule Mobilizon.Service.Akismet do }) end + @impl Provider @spec check_comment(String.t(), String.t(), boolean(), String.t() | nil, String.t(), String.t()) :: :ham | :spam | :discard | {:error, HTTPoison.Response.t()} def check_comment( @@ -108,6 +115,7 @@ defmodule Mobilizon.Service.Akismet do Application.get_env(:mobilizon, __MODULE__) |> get_in([:key]) end + @impl Provider def ready?, do: !is_nil(api_key()) @spec report_to_akismet_comment(Report.t()) :: AkismetComment.t() | {:error, atom()} diff --git a/lib/service/anti_spam/anti_spam.ex b/lib/service/anti_spam/anti_spam.ex new file mode 100644 index 000000000..3b491bcc4 --- /dev/null +++ b/lib/service/anti_spam/anti_spam.ex @@ -0,0 +1,17 @@ +defmodule Mobilizon.Service.AntiSpam do + @moduledoc """ + Module to load the service adapter defined inside the configuration. + + See `Mobilizon.Service.AntiSpam.Provider`. + """ + + @doc """ + Returns the appropriate service adapter. + + According to the config behind + `config :mobilizon, Mobilizon.Service.AntiSpam, + service: Mobilizon.Service.AntiSpam.Module` + """ + @spec service :: module + def service, do: get_in(Application.get_env(:mobilizon, __MODULE__), [:service]) +end diff --git a/lib/service/anti_spam/provider.ex b/lib/service/anti_spam/provider.ex new file mode 100644 index 000000000..a83401f8c --- /dev/null +++ b/lib/service/anti_spam/provider.ex @@ -0,0 +1,58 @@ +defmodule Mobilizon.Service.AntiSpam.Provider do + @moduledoc """ + Provider Behaviour for anti-spam detection. + + ## Supported backends + + * `Mobilizon.Service.AntiSpam.Akismet` [🔗](https://akismet.com/) + + """ + + @type spam_result :: :ham | :spam | :discard + @type result :: spam_result() | {:error, any()} + + @doc """ + Make sure the provider is ready + """ + @callback ready?() :: boolean() + + @doc """ + Check an user details + """ + @callback check_user(email :: String.t(), ip :: String.t(), user_agent :: String.t()) :: + result() + + @doc """ + Check a profile details + """ + @callback check_profile( + username :: String.t(), + summary :: String.t(), + email :: String.t() | nil, + ip :: String.t(), + user_agent :: String.t() | nil + ) :: result() + + @doc """ + Check an event details + """ + @callback check_event( + event_body :: String.t(), + username :: String.t(), + email :: String.t() | nil, + ip :: String.t(), + user_agent :: String.t() | nil + ) :: result() + + @doc """ + Check a comment details + """ + @callback check_comment( + comment_body :: String.t(), + username :: String.t(), + is_reply? :: boolean(), + email :: String.t() | nil, + ip :: String.t(), + user_agent :: String.t() | nil + ) :: result() +end diff --git a/test/support/mocks/anti_spam_mock.ex b/test/support/mocks/anti_spam_mock.ex new file mode 100644 index 000000000..d5dc4575a --- /dev/null +++ b/test/support/mocks/anti_spam_mock.ex @@ -0,0 +1,26 @@ +defmodule Mobilizon.Service.AntiSpam.Mock do + @moduledoc """ + Mock for Anti-spam Provider implementations. + """ + + alias Mobilizon.Service.AntiSpam.Provider + + @behaviour Provider + + @impl Provider + def ready?, do: true + + @impl Provider + def check_user(_email, _ip, _user_agent), do: :ham + + @impl Provider + def check_profile("spam", _summary, _email, _ip, _user_agent), do: :spam + def check_profile(_preferred_username, _summary, _email, _ip, _user_agent), do: :ham + + @impl Provider + def check_event("some spam event", _username, _email, _ip, _user_agent), do: :spam + def check_event(_event_body, _username, _email, _ip, _user_agent), do: :ham + + @impl Provider + def check_comment(_comment_body, _username, _is_reply?, _email, _ip, _user_agent), do: :ham +end From c971287624913c8555fe288af0df1175701e6209 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 1 Jun 2023 14:49:17 +0200 Subject: [PATCH 2/3] feat(anti-spam): allow to only scan for spam in profiles or events Signed-off-by: Thomas Citharel --- .../mobilizon/maintenance/detect_spam.ex | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex b/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex index 1e3ee5c1d..1af1902aa 100644 --- a/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex +++ b/lib/mix/tasks/mobilizon/maintenance/detect_spam.ex @@ -23,13 +23,17 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do dry_run: :boolean, verbose: :boolean, forward_reports: :boolean, - local_only: :boolean + local_only: :boolean, + only_profiles: :boolean, + only_events: :boolean ], aliases: [ d: :dry_run, v: :verbose, f: :forward_reports, - l: :local_only + l: :local_only, + p: :only_profiles, + e: :only_events ] ) @@ -41,23 +45,27 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do anonymous_actor_id = Config.anonymous_actor_id() - options - |> Keyword.get(:local_only, false) - |> profiles() - |> Stream.flat_map(& &1) - |> Stream.each(fn profile -> - process_profile(profile, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id)) - end) - |> Stream.run() + unless only_events?(options) do + options + |> Keyword.get(:local_only, false) + |> profiles() + |> Stream.flat_map(& &1) + |> Stream.each(fn profile -> + process_profile(profile, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id)) + end) + |> Stream.run() + end - options - |> Keyword.get(:local_only, false) - |> events() - |> Stream.flat_map(& &1) - |> Stream.each(fn event -> - process_event(event, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id)) - end) - |> Stream.run() + unless only_profiles?(options) do + options + |> Keyword.get(:local_only, false) + |> events() + |> Stream.flat_map(& &1) + |> Stream.each(fn event -> + process_event(event, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id)) + end) + |> Stream.run() + end end defp profiles(local_only) do @@ -180,6 +188,8 @@ defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpam do defp verbose?(options), do: Keyword.get(options, :verbose, false) defp dry_run?(options), do: Keyword.get(options, :dry_run, false) + defp only_profiles?(options), do: Keyword.get(options, :only_profiles, false) + defp only_events?(options), do: Keyword.get(options, :only_events, false) defp anti_spam, do: AntiSpam.service() end From ce15160e873518a6abd61a245b27e0dac879034e Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 1 Jun 2023 14:49:39 +0200 Subject: [PATCH 3/3] test(anti-spam): add tests for anti-spam detection command Signed-off-by: Thomas Citharel --- test/tasks/maintenance/detect_spam_test.exs | 55 +++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 test/tasks/maintenance/detect_spam_test.exs diff --git a/test/tasks/maintenance/detect_spam_test.exs b/test/tasks/maintenance/detect_spam_test.exs new file mode 100644 index 000000000..6942e22c9 --- /dev/null +++ b/test/tasks/maintenance/detect_spam_test.exs @@ -0,0 +1,55 @@ +defmodule Mix.Tasks.Mobilizon.Maintenance.DetectSpamTest do + use Mobilizon.DataCase + + import Mobilizon.Factory + + alias Mix.Tasks.Mobilizon.Maintenance.DetectSpam + + Mix.shell(Mix.Shell.Process) + + describe "detect spam" do + test "on all content" do + insert(:actor, preferred_username: "ham") + insert(:actor, preferred_username: "spam") + insert(:event, description: "some ham event", title: "some ham event") + spam_event = insert(:event, description: "some spam event", title: "some spam event") + + DetectSpam.run(["-v"]) + assert_received {:mix_shell, :info, [output_received]} + assert_received {:mix_shell, :info, [output_received2]} + assert_received {:mix_shell, :info, [output_received3]} + assert_received {:mix_shell, :info, [output_received4]} + assert_received {:mix_shell, :info, [output_received5]} + assert_received {:mix_shell, :info, [output_received6]} + assert_received {:mix_shell, :info, [output_received7]} + assert_received {:mix_shell, :info, [output_received8]} + assert_received {:mix_shell, :info, [output_received9]} + assert_received {:mix_shell, :info, [output_received10]} + assert_received {:mix_shell, :info, [output_received11]} + assert_received {:mix_shell, :info, [output_received12]} + + output = + MapSet.new([ + output_received, + output_received2, + output_received3, + output_received4, + output_received5, + output_received6, + output_received7, + output_received8, + output_received9, + output_received10, + output_received11, + output_received12 + ]) + + assert MapSet.member?(output, "Starting scanning of profiles") + assert MapSet.member?(output, "Starting scanning of events") + assert MapSet.member?(output, "Profile ham is fine") + assert MapSet.member?(output, "Event some ham event is fine") + assert MapSet.member?(output, "Detected profile spam as spam") + assert MapSet.member?(output, "Detected event some spam event as spam: #{spam_event.url}") + end + end +end