DEV: Make Guardian#can_see? default to false for unwatched objects (#20412)

When invoking e.g. `can_see?(Foo.new)`, the guardian checks if there's a method `#can_see_foo?` defined and if so uses that to determine whether the user can see it or not.

When such a method is not defined, the guardian currently returns `true`, but it is probably a better call (pun intended) to make it "safe by default" and return `false` instead. I.e. if you can't explicitly see it, you can't see it at all.

This change makes the change to `Guardian#can_see?` to fall back to `false` if no visibility check method is defined.

For `#can_see_user?` and `#can_see_tag?` we don't have any particular logic that prevents viewing. We previously relied on the implicit `true` value, but since that's now change to `false`, I have explicitly implemented these two methods in `UserGuardian` and `TagGuardian` modules. If in the future we want to add some logic for it, this would be the place.

To be clear, **the behaviour remains the same**, but the `true` value is now explicit rather than implicit.
This commit is contained in:
Ted Johansson 2023-02-24 15:57:01 +08:00 committed by GitHub
parent 9fb5bfd93d
commit b50b63808c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 26 additions and 1 deletions

View File

@ -162,7 +162,7 @@ class Guardian
def can_see?(obj)
if obj
see_method = method_name_for :see, obj
(see_method ? public_send(see_method, obj) : true)
see_method && public_send(see_method, obj)
end
end

View File

@ -2,6 +2,10 @@
#mixin for all guardian methods dealing with tagging permissions
module TagGuardian
def can_see_tag?(_tag)
true
end
def can_create_tag?
SiteSetting.tagging_enabled &&
@user.has_trust_level_or_staff?(SiteSetting.min_trust_to_create_tag)

View File

@ -118,6 +118,10 @@ module UserGuardian
user && can_administer_user?(user)
end
def can_see_user?(_user)
true
end
def can_see_profile?(user)
return false if user.blank?
return true if !SiteSetting.allow_users_to_hide_profile?

View File

@ -91,6 +91,12 @@ RSpec.describe UserGuardian do
end
end
describe "#can_see_user?" do
it "is always true" do
expect(Guardian.new.can_see_user?(anything)).to eq(true)
end
end
describe "#can_see_profile?" do
it "is false for no user" do
expect(Guardian.new.can_see_profile?(nil)).to eq(false)

View File

@ -861,6 +861,11 @@ RSpec.describe Guardian do
expect(Guardian.new.can_see?(nil)).to be_falsey
end
it "returns false when no visibility method is defined for the object" do
unguarded_object = 42
expect(Guardian.new.can_see?(unguarded_object)).to be_falsey
end
describe "a Category" do
it "allows public categories" do
public_category = Fabricate(:category, read_restricted: false)
@ -3532,6 +3537,12 @@ RSpec.describe Guardian do
context "when min_trust_to_create_tag is 3" do
before { SiteSetting.min_trust_to_create_tag = 3 }
describe "#can_see_tag?" do
it "is always true" do
expect(Guardian.new.can_see_tag?(anything)).to be_truthy
end
end
describe "can_create_tag" do
it "returns false if trust level is too low" do
expect(Guardian.new(trust_level_2).can_create_tag?).to be_falsey