diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-30 16:13:25 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-06-30 14:41:15 -0400 |
commit | 70fd0177692d1d67965f099d7814cb3bd95b6dd1 (patch) | |
tree | c915436e1646a0c497a577e19d70d8b680338223 | |
parent | ad421b3ac65d7bd0679ee37546011dc0b2601199 (diff) | |
download | gitlab-ce-70fd0177692d1d67965f099d7814cb3bd95b6dd1.tar.gz |
Merge branch '18033-private-repo-mentions' into 'master'
Ensure logged-out users can't see private refs
https://gitlab.com/gitlab-org/gitlab-ce/issues/18033
I'm still not sure what to do about the CHANGELOG on security issues - should I add to a patch release? This issue was assigned to 8.10.
See merge request !1974
(cherry picked from commit 3a6ebb1fd624c216a4ce65380e64072793b7ccda)
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/concerns/mentionable.rb | 2 | ||||
-rw-r--r-- | app/services/todo_service.rb | 2 | ||||
-rw-r--r-- | spec/models/concerns/mentionable_spec.rb | 37 |
4 files changed, 40 insertions, 2 deletions
diff --git a/CHANGELOG b/CHANGELOG index 1e1a5ca9b12..761f423a88a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.9.4 - Fix privilege escalation issue with OAuth external users. + - Ensure references to private repos aren't shown to logged-out users. v 8.9.3 - Fix encrypted data backwards compatibility after upgrading attr_encrypted gem. !4963 diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index f00b5b8497c..8cac47246db 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -45,7 +45,7 @@ module Mentionable def all_references(current_user = nil, text = nil, extractor: nil) extractor ||= Gitlab::ReferenceExtractor. - new(project, current_user || author) + new(project, current_user) if text extractor.analyze(text, author: author) diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 239bd17a035..6bb0a72d30e 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -237,7 +237,7 @@ class TodoService end def filter_mentioned_users(project, target, author) - mentioned_users = target.mentioned_users + mentioned_users = target.mentioned_users(author) mentioned_users = reject_users_without_access(mentioned_users, project, target) mentioned_users.delete(author) mentioned_users.uniq diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index cb33edde820..0344dae8b5d 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -29,6 +29,43 @@ describe Issue, "Mentionable" do it { is_expected.not_to include(user2) } end + describe '#referenced_mentionables' do + context 'with an issue on a private project' do + let(:project) { create(:empty_project, :public) } + let(:issue) { create(:issue, project: project) } + let(:public_issue) { create(:issue, project: project) } + let(:private_project) { create(:empty_project, :private) } + let(:private_issue) { create(:issue, project: private_project) } + let(:user) { create(:user) } + + def referenced_issues(current_user) + text = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" + + issue.referenced_mentionables(current_user, text) + end + + context 'when the current user can see the issue' do + before { private_project.team << [user, Gitlab::Access::DEVELOPER] } + + it 'includes the reference' do + expect(referenced_issues(user)).to contain_exactly(private_issue, public_issue) + end + end + + context 'when the current user cannot see the issue' do + it 'does not include the reference' do + expect(referenced_issues(user)).to contain_exactly(public_issue) + end + end + + context 'when there is no current user' do + it 'does not include the reference' do + expect(referenced_issues(nil)).to contain_exactly(public_issue) + end + end + end + end + describe '#create_cross_references!' do let(:project) { create(:project) } let(:author) { double('author') } |