diff options
author | Sean McGivern <sean@gitlab.com> | 2017-07-19 12:58:56 +0000 |
---|---|---|
committer | James Edwards-Jones <jedwardsjones@gitlab.com> | 2017-07-19 16:43:12 +0100 |
commit | 92d475a6d52442ecd77b8cf0b1837cb6932732d0 (patch) | |
tree | 0cbc235f1e283a7bac7da794e26e927af6565ba1 | |
parent | c538b4ff8ead7ab2a24faf2bafce0c93a32c8ec8 (diff) | |
download | gitlab-ce-92d475a6d52442ecd77b8cf0b1837cb6932732d0.tar.gz |
Merge branch 'security-9-0-backport-33323-fix-incorrect-project-authorizations' into 'security-9-0'
Escape the underscore char inside the LIKE operator
See merge request !2133
-rw-r--r-- | app/models/concerns/routable.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/33323-fix-incorrect-project-authorizations | 4 | ||||
-rw-r--r-- | spec/services/users/refresh_authorized_projects_service_spec.rb | 30 |
3 files changed, 35 insertions, 1 deletions
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index 529fb5ce988..280493e7f25 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) 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/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index 08733d6dcf1..8c04c86b662 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 |