Merge branch 'registration-workflow' into 'develop'

Improve registration workflow, fixes #2175

Closes #2235 and #2175

See merge request pleroma/pleroma!3068

CHANGELOG.md

@@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - Polls now always return a `voters_count`, even if they are single-choice.
 - Admin Emails: The ap id is used as the user link in emails now.
+- Improved registration workflow for email confirmation and account approval modes.
+- **Breaking:** Changed `mix pleroma.user toggle_confirmed` to `mix pleroma.user confirm`
 - Search: When using Postgres 11+, Pleroma will use the `websearch_to_tsvector` function to parse search queries.
 - Emoji: Support the full Unicode 13.1 set of Emoji for reactions, plus regional indicators.
 

docs/administration/CLI_tasks/user.md

@@ -264,13 +264,13 @@
 === "OTP"
 
     ```sh
-     ./bin/pleroma_ctl user toggle_confirmed <nickname>
+     ./bin/pleroma_ctl user confirm <nickname>
     ```
 
 === "From Source"
 
     ```sh
-    mix pleroma.user toggle_confirmed <nickname>
+    mix pleroma.user confirm <nickname>
     ```
 
 ## Set confirmation status for all regular active users

lib/mix/tasks/pleroma/user.ex

@@ -345,11 +345,11 @@ def run(["delete_activities", nickname]) do
     end
   end
 
-  def run(["toggle_confirmed", nickname]) do
+  def run(["confirm", nickname]) do
     start_pleroma()
 
     with %User{} = user <- User.get_cached_by_nickname(nickname) do
-      {:ok, user} = User.toggle_confirmation(user)
+      {:ok, user} = User.confirm(user)
 
       message = if user.confirmation_pending, do: "needs", else: "doesn't need"
 

lib/pleroma/emails/user_email.ex

@@ -93,6 +93,19 @@ def account_confirmation_email(user) do
     |> html_body(html_body)
   end
 
+  def approval_pending_email(user) do
+    html_body = """
+    <h3>Awaiting Approval</h3>
+    <p>Your account at #{instance_name()} is being reviewed by staff. You will receive another email once your account is approved.</p>
+    """
+
+    new()
+    |> to(recipient(user))
+    |> from(sender())
+    |> subject("Your account is awaiting approval")
+    |> html_body(html_body)
+  end
+
   @doc """
   Email used in digest email notifications
   Includes Mentions and New Followers data

lib/pleroma/user.ex

@@ -806,18 +806,50 @@ def register(%Ecto.Changeset{} = changeset) do
     end
   end
 
-  def post_register_action(%User{} = user) do
+  def post_register_action(%User{confirmation_pending: true} = user) do
+    with {:ok, _} <- try_send_confirmation_email(user) do
+      {:ok, user}
+    end
+  end
+
+  def post_register_action(%User{approval_pending: true} = user) do
+    with {:ok, _} <- send_user_approval_email(user),
+         {:ok, _} <- send_admin_approval_emails(user) do
+      {:ok, user}
+    end
+  end
+
+  def post_register_action(%User{approval_pending: false, confirmation_pending: false} = user) do
     with {:ok, user} <- autofollow_users(user),
          {:ok, _} <- autofollowing_users(user),
          {:ok, user} <- set_cache(user),
          {:ok, _} <- send_welcome_email(user),
          {:ok, _} <- send_welcome_message(user),
-         {:ok, _} <- send_welcome_chat_message(user),
-         {:ok, _} <- try_send_confirmation_email(user) do
+         {:ok, _} <- send_welcome_chat_message(user) do
       {:ok, user}
     end
   end
 
+  defp send_user_approval_email(user) do
+    user
+    |> Pleroma.Emails.UserEmail.approval_pending_email()
+    |> Pleroma.Emails.Mailer.deliver_async()
+
+    {:ok, :enqueued}
+  end
+
+  defp send_admin_approval_emails(user) do
+    all_superusers()
+    |> Enum.filter(fn user -> not is_nil(user.email) end)
+    |> Enum.each(fn superuser ->
+      superuser
+      |> Pleroma.Emails.AdminEmail.new_unapproved_registration(user)
+      |> Pleroma.Emails.Mailer.deliver_async()
+    end)
+
+    {:ok, :enqueued}
+  end
+
   def send_welcome_message(user) do
     if User.WelcomeMessage.enabled?() do
       User.WelcomeMessage.post_message(user)
@@ -1590,10 +1622,33 @@ def approve(users) when is_list(users) do
     end)
   end
 
-  def approve(%User{} = user) do
-    change(user, approval_pending: false)
-    |> update_and_set_cache()
+  def approve(%User{approval_pending: true} = user) do
+    with chg <- change(user, approval_pending: false),
+         {:ok, user} <- update_and_set_cache(chg) do
+      post_register_action(user)
+      {:ok, user}
     end
+  end
+
+  def approve(%User{} = user), do: {:ok, user}
+
+  def confirm(users) when is_list(users) do
+    Repo.transaction(fn ->
+      Enum.map(users, fn user ->
+        with {:ok, user} <- confirm(user), do: user
+      end)
+    end)
+  end
+
+  def confirm(%User{confirmation_pending: true} = user) do
+    with chg <- confirmation_changeset(user, need_confirmation: false),
+         {:ok, user} <- update_and_set_cache(chg) do
+      post_register_action(user)
+      {:ok, user}
+    end
+  end
+
+  def confirm(%User{} = user), do: {:ok, user}
 
   def update_notification_settings(%User{} = user, settings) do
     user
@@ -2081,18 +2136,6 @@ def touch_last_digest_emailed_at(user) do
     updated_user
   end
 
-  @spec toggle_confirmation(User.t()) :: {:ok, User.t()} | {:error, Changeset.t()}
-  def toggle_confirmation(%User{} = user) do
-    user
-    |> confirmation_changeset(need_confirmation: !user.confirmation_pending)
-    |> update_and_set_cache()
-  end
-
-  @spec toggle_confirmation([User.t()]) :: [{:ok, User.t()} | {:error, Changeset.t()}]
-  def toggle_confirmation(users) do
-    Enum.map(users, &toggle_confirmation/1)
-  end
-
   @spec need_confirmation(User.t(), boolean()) :: {:ok, User.t()} | {:error, Changeset.t()}
   def need_confirmation(%User{} = user, bool) do
     user

lib/pleroma/web/admin_api/controllers/admin_api_controller.ex

@@ -415,7 +415,7 @@ def reload_emoji(conn, _params) do
   def confirm_email(%{assigns: %{user: admin}} = conn, %{"nicknames" => nicknames}) do
     users = Enum.map(nicknames, &User.get_cached_by_nickname/1)
 
-    User.toggle_confirmation(users)
+    User.confirm(users)
 
     ModerationLog.insert_log(%{actor: admin, subject: users, action: "confirm_email"})
 

lib/pleroma/web/twitter_api/controller.ex

@@ -31,10 +31,7 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
   def confirm_email(conn, %{"user_id" => uid, "token" => token}) do
     with %User{} = user <- User.get_cached_by_id(uid),
          true <- user.local and user.confirmation_pending and user.confirmation_token == token,
-         {:ok, _} <-
-           user
-           |> User.confirmation_changeset(need_confirmation: false)
-           |> User.update_and_set_cache() do
+         {:ok, _} <- User.confirm(user) do
       redirect(conn, to: "/")
     end
   end

lib/pleroma/web/twitter_api/twitter_api.ex

@@ -45,7 +45,6 @@ defp create_user(params, opts) do
 
     case User.register(changeset) do
       {:ok, user} ->
-        maybe_notify_admins(user)
         {:ok, user}
 
       {:error, changeset} ->
@@ -58,18 +57,6 @@ defp create_user(params, opts) do
     end
   end
 
-  defp maybe_notify_admins(%User{} = account) do
-    if Pleroma.Config.get([:instance, :account_approval_required]) do
-      User.all_superusers()
-      |> Enum.filter(fn user -> not is_nil(user.email) end)
-      |> Enum.each(fn superuser ->
-        superuser
-        |> Pleroma.Emails.AdminEmail.new_unapproved_registration(account)
-        |> Pleroma.Emails.Mailer.deliver_async()
-      end)
-    end
-  end
-
   def password_reset(nickname_or_email) do
     with true <- is_binary(nickname_or_email),
          %User{local: true, email: email, deactivated: false} = user when is_binary(email) <-

test/mix/tasks/pleroma/user_test.exs

@@ -462,24 +462,24 @@ test "it prints an error message when user is not exist" do
     end
   end
 
-  describe "running toggle_confirmed" do
+  describe "running confirm" do
     test "user is confirmed" do
       %{id: id, nickname: nickname} = insert(:user, confirmation_pending: false)
 
-      assert :ok = Mix.Tasks.Pleroma.User.run(["toggle_confirmed", nickname])
+      assert :ok = Mix.Tasks.Pleroma.User.run(["confirm", nickname])
       assert_received {:mix_shell, :info, [message]}
-      assert message == "#{nickname} needs confirmation."
+      assert message == "#{nickname} doesn't need confirmation."
 
       user = Repo.get(User, id)
-      assert user.confirmation_pending
-      assert user.confirmation_token
+      refute user.confirmation_pending
+      refute user.confirmation_token
     end
 
     test "user is not confirmed" do
       %{id: id, nickname: nickname} =
         insert(:user, confirmation_pending: true, confirmation_token: "some token")
 
-      assert :ok = Mix.Tasks.Pleroma.User.run(["toggle_confirmed", nickname])
+      assert :ok = Mix.Tasks.Pleroma.User.run(["confirm", nickname])
       assert_received {:mix_shell, :info, [message]}
       assert message == "#{nickname} doesn't need confirmation."
 
@@ -489,7 +489,7 @@ test "user is not confirmed" do
     end
 
     test "it prints an error message when user is not exist" do
-      Mix.Tasks.Pleroma.User.run(["toggle_confirmed", "foo"])
+      Mix.Tasks.Pleroma.User.run(["confirm", "foo"])
 
       assert_received {:mix_shell, :error, [message]}
       assert message =~ "No local user"

test/pleroma/emails/user_email_test.exs

@@ -45,4 +45,15 @@ test "build account confirmation email" do
     assert email.html_body =~
              Router.Helpers.confirm_email_url(Endpoint, :confirm_email, user.id, "conf-token")
   end
+
+  test "build approval pending email" do
+    config = Pleroma.Config.get(:instance)
+    user = insert(:user)
+    email = UserEmail.approval_pending_email(user)
+
+    assert email.from == {config[:name], config[:notify_email]}
+    assert email.to == [{user.name, user.email}]
+    assert email.subject == "Your account is awaiting approval"
+    assert email.html_body =~ "Awaiting Approval"
+  end
 end

test/pleroma/user_test.exs

@@ -535,6 +535,22 @@ test "it sends a confirm email" do
       |> assert_email_sent()
     end
 
+    test "sends a pending approval email" do
+      clear_config([:instance, :account_approval_required], true)
+
+      {:ok, user} =
+        User.register_changeset(%User{}, @full_user_data)
+        |> User.register()
+
+      ObanHelpers.perform_all()
+
+      assert_email_sent(
+        from: Pleroma.Config.Helpers.sender(),
+        to: {user.name, user.email},
+        subject: "Your account is awaiting approval"
+      )
+    end
+
     test "it requires an email, name, nickname and password, bio is optional when account_activation_required is enabled" do
       Pleroma.Config.put([:instance, :account_activation_required], true)
 
@@ -1393,6 +1409,98 @@ test "approves a list of users" do
         assert false == user.approval_pending
       end)
     end
+
+    test "it sends welcome email if it is set" do
+      clear_config([:welcome, :email, :enabled], true)
+      clear_config([:welcome, :email, :sender], "tester@test.me")
+
+      user = insert(:user, approval_pending: true)
+      welcome_user = insert(:user, email: "tester@test.me")
+      instance_name = Pleroma.Config.get([:instance, :name])
+
+      User.approve(user)
+
+      ObanHelpers.perform_all()
+
+      assert_email_sent(
+        from: {instance_name, welcome_user.email},
+        to: {user.name, user.email},
+        html_body: "Welcome to #{instance_name}"
+      )
+    end
+
+    test "approving an approved user does not trigger post-register actions" do
+      clear_config([:welcome, :email, :enabled], true)
+
+      user = insert(:user, approval_pending: false)
+      User.approve(user)
+
+      ObanHelpers.perform_all()
+
+      assert_no_email_sent()
+    end
+  end
+
+  describe "confirm" do
+    test "confirms a user" do
+      user = insert(:user, confirmation_pending: true)
+      assert true == user.confirmation_pending
+      {:ok, user} = User.confirm(user)
+      assert false == user.confirmation_pending
+    end
+
+    test "confirms a list of users" do
+      unconfirmed_users = [
+        insert(:user, confirmation_pending: true),
+        insert(:user, confirmation_pending: true),
+        insert(:user, confirmation_pending: true)
+      ]
+
+      {:ok, users} = User.confirm(unconfirmed_users)
+
+      assert Enum.count(users) == 3
+
+      Enum.each(users, fn user ->
+        assert false == user.confirmation_pending
+      end)
+    end
+
+    test "sends approval emails when `approval_pending: true`" do
+      admin = insert(:user, is_admin: true)
+      user = insert(:user, confirmation_pending: true, approval_pending: true)
+      User.confirm(user)
+
+      ObanHelpers.perform_all()
+
+      user_email = Pleroma.Emails.UserEmail.approval_pending_email(user)
+      admin_email = Pleroma.Emails.AdminEmail.new_unapproved_registration(admin, user)
+
+      notify_email = Pleroma.Config.get([:instance, :notify_email])
+      instance_name = Pleroma.Config.get([:instance, :name])
+
+      # User approval email
+      assert_email_sent(
+        from: {instance_name, notify_email},
+        to: {user.name, user.email},
+        html_body: user_email.html_body
+      )
+
+      # Admin email
+      assert_email_sent(
+        from: {instance_name, notify_email},
+        to: {admin.name, admin.email},
+        html_body: admin_email.html_body
+      )
+    end
+
+    test "confirming a confirmed user does not trigger post-register actions" do
+      user = insert(:user, confirmation_pending: false, approval_pending: true)
+      User.confirm(user)
+
+      ObanHelpers.perform_all()
+
+      assert_no_email_sent()
+    end
   end
 
   describe "delete" do
@@ -1889,24 +1997,6 @@ test "Only includes users with no read notifications" do
     end
   end
 
-  describe "toggle_confirmation/1" do
-    test "if user is confirmed" do
-      user = insert(:user, confirmation_pending: false)
-      {:ok, user} = User.toggle_confirmation(user)
-
-      assert user.confirmation_pending
-      assert user.confirmation_token
-    end
-
-    test "if user is unconfirmed" do
-      user = insert(:user, confirmation_pending: true, confirmation_token: "some token")
-      {:ok, user} = User.toggle_confirmation(user)
-
-      refute user.confirmation_pending
-      refute user.confirmation_token
-    end
-  end
-
   describe "ensure_keys_present" do
     test "it creates keys for a user and stores them in info" do
       user = insert(:user)

test/pleroma/web/twitter_api/twitter_api_test.exs

@@ -80,13 +80,9 @@ test "it sends confirmation email if :account_activation_required is specified i
   end
 
   test "it sends an admin email if :account_approval_required is specified in instance config" do
-    admin = insert(:user, is_admin: true)
-    setting = Pleroma.Config.get([:instance, :account_approval_required])
+    clear_config([:instance, :account_approval_required], true)
 
-    unless setting do
-      Pleroma.Config.put([:instance, :account_approval_required], true)
-      on_exit(fn -> Pleroma.Config.put([:instance, :account_approval_required], setting) end)
-    end
+    admin = insert(:user, is_admin: true)
 
     data = %{
       :username => "lain",
@@ -103,15 +99,24 @@ test "it sends an admin email if :account_approval_required is specified in inst
 
     assert user.approval_pending
 
-    email = Pleroma.Emails.AdminEmail.new_unapproved_registration(admin, user)
+    user_email = Pleroma.Emails.UserEmail.approval_pending_email(user)
+    admin_email = Pleroma.Emails.AdminEmail.new_unapproved_registration(admin, user)
 
     notify_email = Pleroma.Config.get([:instance, :notify_email])
     instance_name = Pleroma.Config.get([:instance, :name])
 
+    # User approval email
+    Swoosh.TestAssertions.assert_email_sent(
+      from: {instance_name, notify_email},
+      to: {user.name, user.email},
+      html_body: user_email.html_body
+    )
+
+    # Admin email
     Swoosh.TestAssertions.assert_email_sent(
       from: {instance_name, notify_email},
       to: {admin.name, admin.email},
-      html_body: email.html_body
+      html_body: admin_email.html_body
     )
   end