diff --git a/config/test.exs b/config/test.exs index 794d4773d..8c4ac30d3 100644 --- a/config/test.exs +++ b/config/test.exs @@ -14,6 +14,7 @@ config :mobilizon, Mobilizon.Web.Endpoint, host: "mobilizon.test", scheme: "http" ], + debug_errors: true, secret_key_base: "some secret", server: false diff --git a/docs/administration/CLI tasks/manage_users.md b/docs/administration/CLI tasks/manage_users.md index 9a9e07dab..40c86fcf7 100644 --- a/docs/administration/CLI tasks/manage_users.md +++ b/docs/administration/CLI tasks/manage_users.md @@ -53,4 +53,5 @@ mix mobilizon.users.delete ### Options -* `--assume_yes`/`-y` Don't ask for confirmation \ No newline at end of file +* `--assume_yes`/`-y` Don't ask for confirmation +* `--keep_email`/`-k` Keep user entry with just email information (to prevent future registrations with same email) \ No newline at end of file diff --git a/js/src/views/Settings/AccountSettings.vue b/js/src/views/Settings/AccountSettings.vue index 6681abdb0..6b84d531f 100644 --- a/js/src/views/Settings/AccountSettings.vue +++ b/js/src/views/Settings/AccountSettings.vue @@ -288,7 +288,8 @@ export default class AccountSettings extends Vue { return !this.loggedUser.provider; } - static providerName(id: string): string { + // eslint-disable-next-line class-methods-use-this + providerName(id: string): string { if (SELECTED_PROVIDERS[id]) { return SELECTED_PROVIDERS[id]; } diff --git a/lib/graphql/resolvers/user.ex b/lib/graphql/resolvers/user.ex index 890ea2b68..fef857307 100644 --- a/lib/graphql/resolvers/user.ex +++ b/lib/graphql/resolvers/user.ex @@ -410,7 +410,7 @@ defmodule Mobilizon.GraphQL.Resolvers.User do {:moderator_actor, Users.get_actor_for_user(moderator_user)}, %User{disabled: false} = user <- Users.get_user(user_id), {:ok, %User{}} <- - do_delete_account(%User{} = user, Relay.get_actor()) do + do_delete_account(%User{} = user, actor_performing: Relay.get_actor()) do Admin.log_action(moderator_actor, "delete", user) else {:moderator_actor, nil} -> @@ -429,11 +429,11 @@ defmodule Mobilizon.GraphQL.Resolvers.User do {:confirmation_password, Map.get(args, :password)}, {:current_password, {:ok, _}} <- {:current_password, Authenticator.authenticate(email, password)} do - do_delete_account(user) + do_delete_account(user, reserve_email: false) else # If the user hasn't got any password (3rd-party auth) {:user_has_password, false} -> - do_delete_account(user) + do_delete_account(user, reserve_email: false) {:confirmation_password, nil} -> {:error, dgettext("errors", "The password provided is invalid")} @@ -447,23 +447,21 @@ defmodule Mobilizon.GraphQL.Resolvers.User do {:error, dgettext("errors", "You need to be logged-in to delete your account")} end - defp do_delete_account(%User{} = user, actor_performing \\ nil) do + @spec do_delete_account(User.t(), Keyword.t()) :: {:ok, User.t()} + defp do_delete_account(%User{} = user, options) do with actors <- Users.get_actors_for_user(user), activated <- not is_nil(user.confirmed_at), # Detach actors from user - :ok <- - if(activated, - do: :ok, - else: Enum.each(actors, fn actor -> Actors.update_actor(actor, %{user_id: nil}) end) - ), + :ok <- Enum.each(actors, fn actor -> Actors.update_actor(actor, %{user_id: nil}) end), # Launch a background job to delete actors :ok <- Enum.each(actors, fn actor -> - actor_performing = actor_performing || actor + actor_performing = Keyword.get(options, :actor_performing, actor) ActivityPub.delete(actor, actor_performing, true) end), # Delete user - {:ok, user} <- Users.delete_user(user, reserve_email: activated) do + {:ok, user} <- + Users.delete_user(user, reserve_email: Keyword.get(options, :reserve_email, activated)) do {:ok, user} end end diff --git a/lib/mix/tasks/mobilizon/create_bot.ex b/lib/mix/tasks/mobilizon/create_bot.ex index 0f0f20357..fce2b6166 100644 --- a/lib/mix/tasks/mobilizon/create_bot.ex +++ b/lib/mix/tasks/mobilizon/create_bot.ex @@ -11,7 +11,7 @@ defmodule Mix.Tasks.Mobilizon.CreateBot do require Logger - @shortdoc "Register user" + @shortdoc "Create bot" def run([email, name, summary, type, url]) do Mix.Task.run("app.start") diff --git a/lib/mix/tasks/mobilizon/users/delete.ex b/lib/mix/tasks/mobilizon/users/delete.ex index d95037dfb..d769601a0 100644 --- a/lib/mix/tasks/mobilizon/users/delete.ex +++ b/lib/mix/tasks/mobilizon/users/delete.ex @@ -15,23 +15,23 @@ defmodule Mix.Tasks.Mobilizon.Users.Delete do rest, strict: [ assume_yes: :boolean, - force: :boolean + keep_email: :boolean ], aliases: [ y: :assume_yes, - f: :force + k: :keep_email ] ) assume_yes? = Keyword.get(options, :assume_yes, false) - force? = Keyword.get(options, :force, false) + keep_email? = Keyword.get(options, :keep_email, false) Mix.Task.run("app.start") with {:ok, %User{} = user} <- Users.get_user_by_email(email), true <- assume_yes? or Mix.shell().yes?("Continue with deleting user #{user.email}?"), {:ok, %User{} = user} <- - Users.delete_user(user, reserve_email: !force?) do + Users.delete_user(user, reserve_email: keep_email?) do Mix.shell().info(""" The user #{user.email} has been deleted """) diff --git a/lib/mix/tasks/mobilizon/users/show.ex b/lib/mix/tasks/mobilizon/users/show.ex index bd1405fda..81864bfe6 100644 --- a/lib/mix/tasks/mobilizon/users/show.ex +++ b/lib/mix/tasks/mobilizon/users/show.ex @@ -20,6 +20,7 @@ defmodule Mix.Tasks.Mobilizon.Users.Show do Mix.shell().info(""" Informations for the user #{user.email}: - Activated: #{user.confirmed_at} + - Disabled: #{user.disabled} - Role: #{user.role} #{display_actors(actors)} """) diff --git a/lib/mobilizon/users/users.ex b/lib/mobilizon/users/users.ex index 21bc20c9b..458cb227e 100644 --- a/lib/mobilizon/users/users.ex +++ b/lib/mobilizon/users/users.ex @@ -280,7 +280,7 @@ defmodule Mobilizon.Users do Counts users. """ @spec count_users :: integer - def count_users, do: Repo.one(from(u in User, select: count(u.id))) + def count_users, do: Repo.one(from(u in User, select: count(u.id), where: u.disabled == false)) @doc """ Gets a settings for an user. @@ -383,7 +383,7 @@ defmodule Mobilizon.Users do defp user_by_email_query(email, true) do from( u in User, - where: u.email == ^email and not is_nil(u.confirmed_at), + where: u.email == ^email and not is_nil(u.confirmed_at) and not u.disabled, preload: :default_actor ) end diff --git a/test/graphql/resolvers/user_test.exs b/test/graphql/resolvers/user_test.exs index 13650c532..0f3dd5cd1 100644 --- a/test/graphql/resolvers/user_test.exs +++ b/test/graphql/resolvers/user_test.exs @@ -757,6 +757,23 @@ defmodule Mobilizon.GraphQL.Resolvers.UserTest do assert hd(res["errors"])["message"] == "This user can't reset their password" end + + test "test send_reset_password/3 for a deactivated user doesn't send email", %{conn: conn} do + {:ok, %User{email: email} = user} = + Users.register(%{email: "toto@tata.tld", password: "p4ssw0rd"}) + + Users.update_user(user, %{confirmed_at: DateTime.utc_now(), disabled: true}) + + res = + conn + |> AbsintheHelpers.graphql_query( + query: @send_reset_password_mutation, + variables: %{email: email} + ) + + assert hd(res["errors"])["message"] == + "No user with this email was found" + end end describe "Resolver: Reset user's password" do @@ -1379,7 +1396,7 @@ defmodule Mobilizon.GraphQL.Resolvers.UserTest do assert MapSet.new([actor1.id, actor2.id]) == MapSet.new([actor1_id, actor2_id]) - assert Users.get_user(user.id).disabled == true + assert is_nil(Users.get_user(user.id)) assert %{success: 2, failure: 0} == Oban.drain_queue(queue: :background) diff --git a/test/tasks/users_test.exs b/test/tasks/users_test.exs index 7206f670e..10d88ba59 100644 --- a/test/tasks/users_test.exs +++ b/test/tasks/users_test.exs @@ -52,13 +52,13 @@ defmodule Mix.Tasks.Mobilizon.UsersTest do test "delete existing user" do insert(:user, email: @email) Delete.run([@email, "-y"]) - assert {:ok, %User{disabled: true}} = Users.get_user_by_email(@email) + assert {:error, :user_not_found} = Users.get_user_by_email(@email) end test "full delete existing user" do insert(:user, email: @email) - Delete.run([@email, "-y", "-f"]) - assert {:error, :user_not_found} == Users.get_user_by_email(@email) + Delete.run([@email, "-y", "-k"]) + assert {:ok, %User{disabled: true}} = Users.get_user_by_email(@email) end test "delete non-existing user" do @@ -68,14 +68,18 @@ defmodule Mix.Tasks.Mobilizon.UsersTest do describe "show user" do test "show existing user" do - %User{confirmed_at: confirmed_at, role: role} = user = insert(:user, email: @email) + %User{confirmed_at: confirmed_at, role: role, disabled: disabled} = + user = insert(:user, email: @email) + actor1 = insert(:actor, user: user) actor2 = insert(:actor, user: user) output = - "Informations for the user #{@email}:\n - Activated: #{confirmed_at}\n - Role: #{role}\n Identities (2):\n - @#{ - actor1.preferred_username - } / \n - @#{actor2.preferred_username} / \n\n\n" + "Informations for the user #{@email}:\n - Activated: #{confirmed_at}\n - Disabled: #{ + disabled + }\n - Role: #{role}\n Identities (2):\n - @#{actor1.preferred_username} / \n - @#{ + actor2.preferred_username + } / \n\n\n" Show.run([@email]) assert_received {:mix_shell, :info, [output_received]}