From 1a3a3602d113735b8a8833b5a1eae05ee276d6dc Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 14:36:55 +0200 Subject: [PATCH 1/9] Change CLI delete user task to completly delete user by default And document the new option Signed-off-by: Thomas Citharel --- docs/administration/CLI tasks/manage_users.md | 3 ++- lib/mix/tasks/mobilizon/users/delete.ex | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) 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/lib/mix/tasks/mobilizon/users/delete.ex b/lib/mix/tasks/mobilizon/users/delete.ex index d95037dfb..545481835 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 + f: :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 """) From 8035fb743d67596d4c22421c058609d8dd2064fd Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 14:39:05 +0200 Subject: [PATCH 2/9] Don't count deactivated users in statistics Signed-off-by: Thomas Citharel --- lib/mobilizon/users/users.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mobilizon/users/users.ex b/lib/mobilizon/users/users.ex index 21bc20c9b..d4c02aa93 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. From 92bff34df8384cf9d0641294035bf832953d9a5c Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:05:32 +0200 Subject: [PATCH 3/9] Fix AccountSettings for 3rd-party auth Signed-off-by: Thomas Citharel --- js/src/views/Settings/AccountSettings.vue | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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]; } From 19c82c76bad463ee42e1e20a76d1437aa55a8b78 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:06:00 +0200 Subject: [PATCH 4/9] Debug errors in test mode as well Signed-off-by: Thomas Citharel --- config/test.exs | 1 + 1 file changed, 1 insertion(+) 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 From 3c6916360dcd78043c7bf433aa9cff67705481fc Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:07:51 +0200 Subject: [PATCH 5/9] Completely delete user when user request self destruction Signed-off-by: Thomas Citharel --- lib/graphql/resolvers/user.ex | 20 +++++++++----------- test/graphql/resolvers/user_test.exs | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) 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/test/graphql/resolvers/user_test.exs b/test/graphql/resolvers/user_test.exs index 13650c532..d989a4b42 100644 --- a/test/graphql/resolvers/user_test.exs +++ b/test/graphql/resolvers/user_test.exs @@ -1379,7 +1379,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) From edfcacf5ac9c8f5e6eb1e98929e2e92a720f02fe Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:20:07 +0200 Subject: [PATCH 6/9] Fix a typo in create_bot task Signed-off-by: Thomas Citharel --- lib/mix/tasks/mobilizon/create_bot.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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") From 53c4f5dfdeaca4bd30934e5dfd94d6abda55b7f6 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:20:30 +0200 Subject: [PATCH 7/9] Show user disabled status in mix task Signed-off-by: Thomas Citharel --- lib/mix/tasks/mobilizon/users/show.ex | 1 + 1 file changed, 1 insertion(+) 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)} """) From 71476ee5875ba9cab684d56d6b62d3726ec3fed4 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:21:00 +0200 Subject: [PATCH 8/9] Don't sent reset email when user is disabled Signed-off-by: Thomas Citharel --- lib/mobilizon/users/users.ex | 2 +- test/graphql/resolvers/user_test.exs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/mobilizon/users/users.ex b/lib/mobilizon/users/users.ex index d4c02aa93..458cb227e 100644 --- a/lib/mobilizon/users/users.ex +++ b/lib/mobilizon/users/users.ex @@ -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 d989a4b42..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 From bba9e6bf59d404178614fa0d293d716a5460db6a Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Oct 2020 15:53:18 +0200 Subject: [PATCH 9/9] Fix tests with user tasks Signed-off-by: Thomas Citharel --- lib/mix/tasks/mobilizon/users/delete.ex | 2 +- test/tasks/users_test.exs | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/mix/tasks/mobilizon/users/delete.ex b/lib/mix/tasks/mobilizon/users/delete.ex index 545481835..d769601a0 100644 --- a/lib/mix/tasks/mobilizon/users/delete.ex +++ b/lib/mix/tasks/mobilizon/users/delete.ex @@ -19,7 +19,7 @@ defmodule Mix.Tasks.Mobilizon.Users.Delete do ], aliases: [ y: :assume_yes, - f: :keep_email + k: :keep_email ] ) 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]}