Merge branch '35444-error-500-viewing-notes-with-anonymous-user' into 'master'
Resolve "Error 500 viewing notes with anonymous user" Closes #35444 See merge request !13037
This commit is contained in:
commit
3a26bce80e
|
@ -167,10 +167,14 @@ class Group < Namespace
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_owner?(user)
|
def has_owner?(user)
|
||||||
|
return false unless user
|
||||||
|
|
||||||
members_with_parents.owners.where(user_id: user).any?
|
members_with_parents.owners.where(user_id: user).any?
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_master?(user)
|
def has_master?(user)
|
||||||
|
return false unless user
|
||||||
|
|
||||||
members_with_parents.masters.where(user_id: user).any?
|
members_with_parents.masters.where(user_id: user).any?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -212,7 +216,7 @@ class Group < Namespace
|
||||||
end
|
end
|
||||||
|
|
||||||
def members_with_parents
|
def members_with_parents
|
||||||
GroupMember.non_request.where(source_id: ancestors.pluck(:id).push(id))
|
GroupMember.active.where(source_id: ancestors.pluck(:id).push(id)).where.not(user_id: nil)
|
||||||
end
|
end
|
||||||
|
|
||||||
def users_with_parents
|
def users_with_parents
|
||||||
|
|
|
@ -10,7 +10,8 @@ class ProjectPolicy < BasePolicy
|
||||||
|
|
||||||
desc "User is a project owner"
|
desc "User is a project owner"
|
||||||
condition :owner do
|
condition :owner do
|
||||||
@user && project.owner == @user || (project.group && project.group.has_owner?(@user))
|
(project.owner.present? && project.owner == @user) ||
|
||||||
|
project.group&.has_owner?(@user)
|
||||||
end
|
end
|
||||||
|
|
||||||
desc "Project has public builds enabled"
|
desc "Project has public builds enabled"
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Fix anonymous access to public projects in groups with pending invites
|
||||||
|
merge_request:
|
||||||
|
author:
|
|
@ -98,7 +98,7 @@ describe Ability, lib: true do
|
||||||
user2 = build(:user, external: true)
|
user2 = build(:user, external: true)
|
||||||
users = [user1, user2]
|
users = [user1, user2]
|
||||||
|
|
||||||
expect(project).to receive(:owner).twice.and_return(user1)
|
expect(project).to receive(:owner).at_least(:once).and_return(user1)
|
||||||
|
|
||||||
expect(described_class.users_that_can_read_project(users, project))
|
expect(described_class.users_that_can_read_project(users, project))
|
||||||
.to eq([user1])
|
.to eq([user1])
|
||||||
|
@ -109,7 +109,7 @@ describe Ability, lib: true do
|
||||||
user2 = build(:user, external: true)
|
user2 = build(:user, external: true)
|
||||||
users = [user1, user2]
|
users = [user1, user2]
|
||||||
|
|
||||||
expect(project.team).to receive(:members).twice.and_return([user1])
|
expect(project.team).to receive(:members).at_least(:once).and_return([user1])
|
||||||
|
|
||||||
expect(described_class.users_that_can_read_project(users, project))
|
expect(described_class.users_that_can_read_project(users, project))
|
||||||
.to eq([user1])
|
.to eq([user1])
|
||||||
|
@ -140,7 +140,7 @@ describe Ability, lib: true do
|
||||||
user2 = build(:user, external: true)
|
user2 = build(:user, external: true)
|
||||||
users = [user1, user2]
|
users = [user1, user2]
|
||||||
|
|
||||||
expect(project).to receive(:owner).twice.and_return(user1)
|
expect(project).to receive(:owner).at_least(:once).and_return(user1)
|
||||||
|
|
||||||
expect(described_class.users_that_can_read_project(users, project))
|
expect(described_class.users_that_can_read_project(users, project))
|
||||||
.to eq([user1])
|
.to eq([user1])
|
||||||
|
@ -151,7 +151,7 @@ describe Ability, lib: true do
|
||||||
user2 = build(:user, external: true)
|
user2 = build(:user, external: true)
|
||||||
users = [user1, user2]
|
users = [user1, user2]
|
||||||
|
|
||||||
expect(project.team).to receive(:members).twice.and_return([user1])
|
expect(project.team).to receive(:members).at_least(:once).and_return([user1])
|
||||||
|
|
||||||
expect(described_class.users_that_can_read_project(users, project))
|
expect(described_class.users_that_can_read_project(users, project))
|
||||||
.to eq([user1])
|
.to eq([user1])
|
||||||
|
|
|
@ -236,6 +236,7 @@ describe Group, models: true do
|
||||||
describe '#has_owner?' do
|
describe '#has_owner?' do
|
||||||
before do
|
before do
|
||||||
@members = setup_group_members(group)
|
@members = setup_group_members(group)
|
||||||
|
create(:group_member, :invited, :owner, group: group)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { expect(group.has_owner?(@members[:owner])).to be_truthy }
|
it { expect(group.has_owner?(@members[:owner])).to be_truthy }
|
||||||
|
@ -244,11 +245,13 @@ describe Group, models: true do
|
||||||
it { expect(group.has_owner?(@members[:reporter])).to be_falsey }
|
it { expect(group.has_owner?(@members[:reporter])).to be_falsey }
|
||||||
it { expect(group.has_owner?(@members[:guest])).to be_falsey }
|
it { expect(group.has_owner?(@members[:guest])).to be_falsey }
|
||||||
it { expect(group.has_owner?(@members[:requester])).to be_falsey }
|
it { expect(group.has_owner?(@members[:requester])).to be_falsey }
|
||||||
|
it { expect(group.has_owner?(nil)).to be_falsey }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#has_master?' do
|
describe '#has_master?' do
|
||||||
before do
|
before do
|
||||||
@members = setup_group_members(group)
|
@members = setup_group_members(group)
|
||||||
|
create(:group_member, :invited, :master, group: group)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { expect(group.has_master?(@members[:owner])).to be_falsey }
|
it { expect(group.has_master?(@members[:owner])).to be_falsey }
|
||||||
|
@ -257,6 +260,7 @@ describe Group, models: true do
|
||||||
it { expect(group.has_master?(@members[:reporter])).to be_falsey }
|
it { expect(group.has_master?(@members[:reporter])).to be_falsey }
|
||||||
it { expect(group.has_master?(@members[:guest])).to be_falsey }
|
it { expect(group.has_master?(@members[:guest])).to be_falsey }
|
||||||
it { expect(group.has_master?(@members[:requester])).to be_falsey }
|
it { expect(group.has_master?(@members[:requester])).to be_falsey }
|
||||||
|
it { expect(group.has_master?(nil)).to be_falsey }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#lfs_enabled?' do
|
describe '#lfs_enabled?' do
|
||||||
|
|
|
@ -127,6 +127,24 @@ describe ProjectPolicy, models: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when a project has pending invites, and the current user is anonymous' do
|
||||||
|
let(:group) { create(:group, :public) }
|
||||||
|
let(:project) { create(:empty_project, :public, namespace: group) }
|
||||||
|
let(:user_permissions) { [:create_project, :create_issue, :create_note, :upload_file] }
|
||||||
|
let(:anonymous_permissions) { guest_permissions - user_permissions }
|
||||||
|
|
||||||
|
subject { described_class.new(nil, project) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
create(:group_member, :invited, group: group)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not grant owner access' do
|
||||||
|
expect_allowed(*anonymous_permissions)
|
||||||
|
expect_disallowed(*user_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'abilities for non-public projects' do
|
context 'abilities for non-public projects' do
|
||||||
let(:project) { create(:empty_project, namespace: owner.namespace) }
|
let(:project) { create(:empty_project, namespace: owner.namespace) }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue