From 595a970493a9bae0dcc4b84de338766c2cc61a75 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 04:46:03 +0000 Subject: [PATCH 1/8] user: use pattern matching to determine if user is local or remote instead of the previous hairy logic --- lib/pleroma/user.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 85d0f9fce..ce909601d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -49,7 +49,8 @@ def auth_active?(%User{} = user) do !Pleroma.Config.get([:instance, :account_activation_required]) end - def remote_or_auth_active?(%User{} = user), do: !user.local || auth_active?(user) + def remote_or_auth_active?(%User{local: false}), do: true + def remote_or_auth_active?(%User{local: true} = user), do: auth_active?(user) def visible_for?(%User{} = user, for_user \\ nil) do User.remote_or_auth_active?(user) || (for_user && for_user.id == user.id) || From 4124c9aa4aae4622f7a939caa84f01ca0760057c Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 05:02:00 +0000 Subject: [PATCH 2/8] tests: user: add regression test for remote_or_auth_active?/1 --- test/user_test.exs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/user_test.exs b/test/user_test.exs index 74accb7c8..419a576dc 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -767,4 +767,18 @@ test "finds a user whose name is nil" do |> Map.put(:search_distance, nil) end end + + test "remote_or_auth_active?/1 works correctly" do + Pleroma.Config.put([:instance, :account_activation_required], true) + + local_user = insert(:user, local: true, info: %{confirmation_pending: true}) + confirmed_user = insert(:user, local: true, info: %{confirmation_pending: false}) + remote_user = insert(:user, local: false) + + refute User.remote_or_auth_active?(local_user) + assert User.remote_or_auth_active?(confirmed_user) + assert User.remote_or_auth_active?(remote_user) + + Pleroma.Config.put([:instance, :account_activation_required], false) + end end From 2af67353c5014edcc24bf2ec27b2bc871bd80eb7 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 06:21:21 +0000 Subject: [PATCH 3/8] user: harden auth_active?/1, superuser?/1, visible_for?/1 --- lib/pleroma/user.ex | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index ce909601d..5491e8b9a 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -44,21 +44,28 @@ defmodule Pleroma.User do timestamps() end - def auth_active?(%User{} = user) do - (user.info && !user.info.confirmation_pending) || - !Pleroma.Config.get([:instance, :account_activation_required]) - end + def auth_active?(%User{info: %User.Info{confirmation_pending: false}}), do: true + + def auth_active?(%User{info: %User.Info{confirmation_pending: true}}), + do: !Pleroma.Config.get([:instance, :account_activation_required]) + + def auth_active?(_), do: false def remote_or_auth_active?(%User{local: false}), do: true def remote_or_auth_active?(%User{local: true} = user), do: auth_active?(user) - def visible_for?(%User{} = user, for_user \\ nil) do - User.remote_or_auth_active?(user) || (for_user && for_user.id == user.id) || - User.superuser?(for_user) + def visible_for?(user, for_user \\ nil) + + def visible_for?(%User{id: user_id}, %User{id: for_id}) when user_id == for_id, do: true + + def visible_for?(%User{} = user, for_user) do + remote_or_auth_active?(user) || superuser?(for_user) end - def superuser?(nil), do: false - def superuser?(%User{} = user), do: user.info && User.Info.superuser?(user.info) + def visible_for?(_, _), do: false + + def superuser?(%User{info: %User.Info{} = info}), do: User.Info.superuser?(info) + def superuser?(_), do: false def avatar_url(user) do case user.avatar do From 74f48beec3bf78cd94cb6db5cbdb937505891eab Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 06:36:50 +0000 Subject: [PATCH 4/8] user: remove entirely redundant remote_or_auth_active?/1. auth_active?/1 can check remote users and return true directly. --- lib/pleroma/user.ex | 7 +++---- test/user_test.exs | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 5491e8b9a..636c56312 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -44,6 +44,8 @@ defmodule Pleroma.User do timestamps() end + def auth_active?(%User{local: false}), do: false + def auth_active?(%User{info: %User.Info{confirmation_pending: false}}), do: true def auth_active?(%User{info: %User.Info{confirmation_pending: true}}), @@ -51,15 +53,12 @@ def auth_active?(%User{info: %User.Info{confirmation_pending: true}}), def auth_active?(_), do: false - def remote_or_auth_active?(%User{local: false}), do: true - def remote_or_auth_active?(%User{local: true} = user), do: auth_active?(user) - def visible_for?(user, for_user \\ nil) def visible_for?(%User{id: user_id}, %User{id: for_id}) when user_id == for_id, do: true def visible_for?(%User{} = user, for_user) do - remote_or_auth_active?(user) || superuser?(for_user) + auth_active?(user) || superuser?(for_user) end def visible_for?(_, _), do: false diff --git a/test/user_test.exs b/test/user_test.exs index 419a576dc..1c4e84914 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -768,16 +768,16 @@ test "finds a user whose name is nil" do end end - test "remote_or_auth_active?/1 works correctly" do + test "auth_active?/1 works correctly" do Pleroma.Config.put([:instance, :account_activation_required], true) local_user = insert(:user, local: true, info: %{confirmation_pending: true}) confirmed_user = insert(:user, local: true, info: %{confirmation_pending: false}) remote_user = insert(:user, local: false) - refute User.remote_or_auth_active?(local_user) - assert User.remote_or_auth_active?(confirmed_user) - assert User.remote_or_auth_active?(remote_user) + refute User.auth_active?(local_user) + assert User.auth_active?(confirmed_user) + assert User.auth_active?(remote_user) Pleroma.Config.put([:instance, :account_activation_required], false) end From 0015d43e13508155462a9e5d18f19725fadd8931 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 06:41:25 +0000 Subject: [PATCH 5/8] user: factor out illogical User.Info.superuser?/1. any actual callee will be dealing with a User struct to begin with, so just check the child struct inside User.superuser?/1 with pattern matching. --- lib/pleroma/user.ex | 3 ++- lib/pleroma/user/info.ex | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 636c56312..4d0f68cd6 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -63,7 +63,8 @@ def visible_for?(%User{} = user, for_user) do def visible_for?(_, _), do: false - def superuser?(%User{info: %User.Info{} = info}), do: User.Info.superuser?(info) + def superuser?(%User{local: true, info: %User.Info{is_admin: true}}), do: true + def superuser?(%User{local: true, info: %User.Info{is_moderator: true}}), do: true def superuser?(_), do: false def avatar_url(user) do diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 2f419a5a2..7c79dfcff 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -41,8 +41,6 @@ defmodule Pleroma.User.Info do # subject _> Where is this used? end - def superuser?(info), do: info.is_admin || info.is_moderator - def set_activation_status(info, deactivated) do params = %{deactivated: deactivated} From f15183178c15bb01f6cd49f28f2177bfd26bdac8 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 06:45:17 +0000 Subject: [PATCH 6/8] user: fix auth_active?/1 for remote users --- lib/pleroma/user.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 4d0f68cd6..06049dc62 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -44,7 +44,7 @@ defmodule Pleroma.User do timestamps() end - def auth_active?(%User{local: false}), do: false + def auth_active?(%User{local: false}), do: true def auth_active?(%User{info: %User.Info{confirmation_pending: false}}), do: true From f2a4f89abef810f1106afa3a9ef82fa748bc783e Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 06:50:31 +0000 Subject: [PATCH 7/8] tests: user: add tests for superuser?/1 --- test/user_test.exs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/user_test.exs b/test/user_test.exs index 1c4e84914..582374fb9 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -781,4 +781,32 @@ test "auth_active?/1 works correctly" do Pleroma.Config.put([:instance, :account_activation_required], false) end + + describe "superuser?/1" do + test "returns false for unprivileged users" do + user = insert(:user, local: true) + + refute User.superuser?(user) + end + + test "returns false for remote users" do + user = insert(:user, local: false) + remote_admin_user = insert(:user, local: false, info: %{is_admin: true}) + + refute User.superuser?(user) + refute User.superuser?(remote_admin_user) + end + + test "returns true for local moderators" do + user = insert(:user, local: true, info: %{is_moderator: true}) + + assert User.superuser?(user) + end + + test "returns true for local admins" do + user = insert(:user, local: true, info: %{is_admin: true}) + + assert User.superuser?(user) + end + end end From 567651fb3fcacbe5bb2f9c19deb9655edaaad076 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 9 Jan 2019 07:03:32 +0000 Subject: [PATCH 8/8] test: user: add tests for visible_for?/2 --- test/user_test.exs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/user_test.exs b/test/user_test.exs index 582374fb9..542eaaaeb 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -809,4 +809,41 @@ test "returns true for local admins" do assert User.superuser?(user) end end + + describe "visible_for?/2" do + test "returns true when the account is itself" do + user = insert(:user, local: true) + + assert User.visible_for?(user, user) + end + + test "returns false when the account is unauthenticated and auth is required" do + Pleroma.Config.put([:instance, :account_activation_required], true) + + user = insert(:user, local: true, info: %{confirmation_pending: true}) + other_user = insert(:user, local: true) + + refute User.visible_for?(user, other_user) + + Pleroma.Config.put([:instance, :account_activation_required], false) + end + + test "returns true when the account is unauthenticated and auth is not required" do + user = insert(:user, local: true, info: %{confirmation_pending: true}) + other_user = insert(:user, local: true) + + assert User.visible_for?(user, other_user) + end + + test "returns true when the account is unauthenticated and being viewed by a privileged account (auth required)" do + Pleroma.Config.put([:instance, :account_activation_required], true) + + user = insert(:user, local: true, info: %{confirmation_pending: true}) + other_user = insert(:user, local: true, info: %{is_admin: true}) + + assert User.visible_for?(user, other_user) + + Pleroma.Config.put([:instance, :account_activation_required], false) + end + end end