diff options
-rw-r--r-- | app/models/concerns/routable.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/33323-fix-incorrect-project-authorizations | 4 | ||||
-rw-r--r-- | spec/models/concerns/routable_spec.rb | 23 | ||||
-rw-r--r-- | spec/services/users/refresh_authorized_projects_service_spec.rb | 30 |
4 files changed, 60 insertions, 3 deletions
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index aca99feee53..be145b7a453 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -78,7 +78,7 @@ module Routable # Returns an ActiveRecord::Relation. def member_descendants(user_id) joins(:route). - joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%') + joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(REPLACE(r2.path, '_', '\\_'), '/%') INNER JOIN members ON members.source_id = r2.source_id AND members.source_type = r2.source_type"). where('members.user_id = ?', user_id) @@ -95,7 +95,7 @@ module Routable # Returns an ActiveRecord::Relation. def member_self_and_descendants(user_id) joins(:route). - joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%') + joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(REPLACE(r2.path, '_', '\\_'), '/%') OR routes.path = r2.path INNER JOIN members ON members.source_id = r2.source_id AND members.source_type = r2.source_type"). @@ -146,7 +146,7 @@ module Routable wheres = paths.map do |path| "#{connection.quote(path)} = routes.path OR - #{connection.quote(path)} LIKE CONCAT(routes.path, '/%')" + #{connection.quote(path)} LIKE CONCAT(REPLACE(routes.path, '_', '\\_'), '/%')" end joins(:route).where(wheres.join(' OR ')) diff --git a/changelogs/unreleased/33323-fix-incorrect-project-authorizations b/changelogs/unreleased/33323-fix-incorrect-project-authorizations new file mode 100644 index 00000000000..332cceebfd5 --- /dev/null +++ b/changelogs/unreleased/33323-fix-incorrect-project-authorizations @@ -0,0 +1,4 @@ +--- +title: Fix incorrect project authorizations +merge_request: +author: diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index f191605dbdb..012907cdc11 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -89,6 +89,29 @@ describe Group, 'Routable' do subject { described_class.member_self_and_descendants(user.id) } it { is_expected.to match_array [group, nested_group] } + + context 'when the group has special chars in its path' do + let(:user1) { create(:user) } + let(:group1) { create(:group, name: 'demo', path: 'demo') } + let(:nested_group1) { create(:group, name: 'nest', path: 'nest', parent: group1) } + let!(:project1) { create(:empty_project, group: nested_group1) } + + let(:user2) { create(:user) } + let(:group2) { create(:group, name: '____', path: '____') } + let(:nested_group2) { create(:group, name: 'test', path: 'test', parent: group2) } + let!(:project2) { create(:empty_project, group: nested_group2) } + + before do + group1.add_master(user1) + group2.add_master(user2) + end + + it 'only returns the right groups' do + groups = described_class.member_self_and_descendants(user2.id) + + expect(groups).to match_array([group2, nested_group2]) + end + end end describe '.member_hierarchy' do diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index b19374ef1a2..70b6034a6af 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -115,6 +115,36 @@ describe Users::RefreshAuthorizedProjectsService do expect(user.authorized_projects_populated).to eq(true) end + + context 'when the group has special chars in its path' do + let(:user1) { create(:user) } + let(:group1) { create(:group, name: 'demo', path: 'demo') } + let(:nested_group1) { create(:group, name: 'nest', path: 'nest', parent: group1) } + let!(:project1) { create(:empty_project, group: nested_group1) } + + let(:user2) { create(:user) } + let(:group2) { create(:group, name: '____', path: '____') } + let(:nested_group2) { create(:group, name: 'test', path: 'test', parent: group2) } + let!(:project2) { create(:empty_project, group: nested_group2) } + + before do + group1.add_master(user1) + group2.add_master(user2) + + described_class.new(user1).execute + described_class.new(user2).execute + end + + it "it doesn't give authorization to foreign projects" do + expect(user1.authorized_projects).not_to include(project2) + expect(user2.authorized_projects).not_to include(project1) + end + + it 'only gives authorization to the right projects' do + expect(user1.authorized_projects).to match_array([project1]) + expect(user2.authorized_projects).to match_array([project2]) + end + end end describe '#fresh_access_levels_per_project' do |