Merge branch 'spam-detection-improvement' into 'main'

refactor(anti-spam): make anti-spam agnostic from Akismet

See merge request framasoft/mobilizon!1403

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"

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

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

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,

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?(),

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,

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} <-
+         {:spam, :ham} <-
-           {:akismet,
+           {:spam,
-            Akismet.check_profile(
+            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} <-
+          with {:spam, :ham} <-
-                 {:akismet,
+                 {:spam,
-                  Akismet.check_profile(
+                  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

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} <-
+         {:spam, :ham} <-
-           {:akismet, Akismet.check_user(email, current_ip, user_agent)},
+           {: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",

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
@@ -23,41 +23,49 @@ 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
         ]
       )
 
     start_mobilizon()
 
-    unless Akismet.ready?() do
+    unless anti_spam().ready?() do
       shell_error("Akismet is missing an API key in the configuration")
     end
 
     anonymous_actor_id = Config.anonymous_actor_id()
 
-    options
+    unless only_events?(options) do
-    |> Keyword.get(:local_only, false)
+      options
-    |> profiles()
+      |> Keyword.get(:local_only, false)
-    |> Stream.flat_map(& &1)
+      |> profiles()
-    |> Stream.each(fn profile ->
+      |> Stream.flat_map(& &1)
-      process_profile(profile, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id))
+      |> Stream.each(fn profile ->
-    end)
+        process_profile(profile, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id))
-    |> Stream.run()
+      end)
+      |> Stream.run()
+    end
 
-    options
+    unless only_profiles?(options) do
-    |> Keyword.get(:local_only, false)
+      options
-    |> events()
+      |> Keyword.get(:local_only, false)
-    |> Stream.flat_map(& &1)
+      |> events()
-    |> Stream.each(fn event ->
+      |> Stream.flat_map(& &1)
-      process_event(event, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id))
+      |> Stream.each(fn event ->
-    end)
+        process_event(event, Keyword.put(options, :anonymous_actor_id, anonymous_actor_id))
-    |> Stream.run()
+      end)
+      |> Stream.run()
+    end
   end
 
   defp profiles(local_only) do
@@ -79,7 +87,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 +121,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 +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

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()}

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

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

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

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