diff options
author | Robert Speicher <robert@gitlab.com> | 2015-12-28 00:09:16 +0300 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2015-12-28 00:09:16 +0300 |
commit | a97a2d27205637ea2ff6da112dabc1499c37ccfe (patch) | |
tree | b49b508683a5cc184e4fd010072baff4e0a82515 | |
parent | a52746649d1db4f52ae4e989dcf654ef4af57905 (diff) | |
parent | 9a0e16f4548bca25f6efc6cd7a4dd0af42b60042 (diff) |
Merge branch 'mention-all' into 'master'
Only allow group/project members to mention `@all`
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/3473
See merge request !2205
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/projects_controller.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/mentionable.rb | 2 | ||||
-rw-r--r-- | lib/banzai/filter/redactor_filter.rb | 6 | ||||
-rw-r--r-- | lib/banzai/filter/reference_filter.rb | 6 | ||||
-rw-r--r-- | lib/banzai/filter/reference_gatherer_filter.rb | 8 | ||||
-rw-r--r-- | lib/banzai/filter/user_reference_filter.rb | 14 | ||||
-rw-r--r-- | lib/gitlab/reference_extractor.rb | 19 | ||||
-rw-r--r-- | spec/lib/banzai/filter/user_reference_filter_spec.rb | 19 | ||||
-rw-r--r-- | spec/models/concerns/mentionable_spec.rb | 4 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 3 |
11 files changed, 66 insertions, 18 deletions
diff --git a/CHANGELOG b/CHANGELOG index a20c3978a11..8135a5d5180 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.4.0 (unreleased) - Implement search inside emoji picker - Add API support for looking up a user by username (Stan Hu) - Add project permissions to all project API endpoints (Stan Hu) + - Only allow group/project members to mention `@all` - Expose Git's version in the admin area - Add "Frequently used" category to emoji picker - Add CAS support (tduehr) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 2dab04f2a7c..3004722bce0 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -178,7 +178,7 @@ class ProjectsController < ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) ext.analyze(text) render json: { diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 1fdcda97520..6316ee208b5 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -44,7 +44,7 @@ module Mentionable end def all_references(current_user = self.author, text = nil) - ext = Gitlab::ReferenceExtractor.new(self.project, current_user) + ext = Gitlab::ReferenceExtractor.new(self.project, current_user, self.author) if text ext.analyze(text) diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb index 89e7a79789a..f01a32b5ae5 100644 --- a/lib/banzai/filter/redactor_filter.rb +++ b/lib/banzai/filter/redactor_filter.rb @@ -11,7 +11,7 @@ module Banzai class RedactorFilter < HTML::Pipeline::Filter def call doc.css('a.gfm').each do |node| - unless user_can_reference?(node) + unless user_can_see_reference?(node) # The reference should be replaced by the original text, # which is not always the same as the rendered text. text = node.attr('data-original') || node.text @@ -24,12 +24,12 @@ module Banzai private - def user_can_reference?(node) + def user_can_see_reference?(node) if node.has_attribute?('data-reference-filter') reference_type = node.attr('data-reference-filter') reference_filter = Banzai::Filter.const_get(reference_type) - reference_filter.user_can_reference?(current_user, node, context) + reference_filter.user_can_see_reference?(current_user, node, context) else true end diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index a22a7a7afd3..8ca05ace88c 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -12,7 +12,7 @@ module Banzai # :project (required) - Current project, ignored if reference is cross-project. # :only_path - Generate path-only links. class ReferenceFilter < HTML::Pipeline::Filter - def self.user_can_reference?(user, node, context) + def self.user_can_see_reference?(user, node, context) if node.has_attribute?('data-project') project_id = node.attr('data-project').to_i return true if project_id == context[:project].try(:id) @@ -24,6 +24,10 @@ module Banzai end end + def self.user_can_reference?(user, node, context) + true + end + def self.referenced_by(node) raise NotImplementedError, "#{self} does not implement #{__method__}" end diff --git a/lib/banzai/filter/reference_gatherer_filter.rb b/lib/banzai/filter/reference_gatherer_filter.rb index 855f238ac1e..12412ff7ea9 100644 --- a/lib/banzai/filter/reference_gatherer_filter.rb +++ b/lib/banzai/filter/reference_gatherer_filter.rb @@ -35,7 +35,9 @@ module Banzai return if context[:reference_filter] && reference_filter != context[:reference_filter] - return unless reference_filter.user_can_reference?(current_user, node, context) + return if author && !reference_filter.user_can_reference?(author, node, context) + + return unless reference_filter.user_can_see_reference?(current_user, node, context) references = reference_filter.referenced_by(node) return unless references @@ -57,6 +59,10 @@ module Banzai def current_user context[:current_user] end + + def author + context[:author] + end end end end diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb index 7f302d51dd7..964ab60f614 100644 --- a/lib/banzai/filter/user_reference_filter.rb +++ b/lib/banzai/filter/user_reference_filter.rb @@ -39,7 +39,7 @@ module Banzai end end - def self.user_can_reference?(user, node, context) + def self.user_can_see_reference?(user, node, context) if node.has_attribute?('data-group') group = Group.find(node.attr('data-group')) rescue nil Ability.abilities.allowed?(user, :read_group, group) @@ -48,6 +48,18 @@ module Banzai end end + def self.user_can_reference?(user, node, context) + # Only team members can reference `@all` + if node.has_attribute?('data-project') + project = Project.find(node.attr('data-project')) rescue nil + return false unless project + + user && project.team.member?(user) + else + super + end + end + def call replace_text_nodes_matching(User.reference_pattern) do |content| user_link_filter(content) diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 0a70d21b1ce..be795649e59 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -3,11 +3,12 @@ require 'banzai' module Gitlab # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor < Banzai::ReferenceExtractor - attr_accessor :project, :current_user + attr_accessor :project, :current_user, :author - def initialize(project, current_user = nil) + def initialize(project, current_user = nil, author = nil) @project = project @current_user = current_user + @author = author @references = {} @@ -20,18 +21,22 @@ module Gitlab %i(user label merge_request snippet commit commit_range).each do |type| define_method("#{type}s") do - @references[type] ||= references(type, project: project, current_user: current_user) + @references[type] ||= references(type, reference_context) end end def issues - options = { project: project, current_user: current_user } - if project && project.jira_tracker? - @references[:external_issue] ||= references(:external_issue, options) + @references[:external_issue] ||= references(:external_issue, reference_context) else - @references[:issue] ||= references(:issue, options) + @references[:issue] ||= references(:issue, reference_context) end end + + private + + def reference_context + { project: project, current_user: current_user, author: author } + end end end diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb index 3534bf97784..8bdebae1841 100644 --- a/spec/lib/banzai/filter/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb @@ -37,9 +37,22 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do .to eq urls.namespace_project_url(project.namespace, project) end - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [project.creator] + context "when the author is a member of the project" do + + it 'adds to the results hash' do + result = reference_pipeline_result("Hey #{reference}", author: project.creator) + expect(result[:references][:user]).to eq [project.creator] + end + end + + context "when the author is not a member of the project" do + + let(:other_user) { create(:user) } + + it "doesn't add to the results hash" do + result = reference_pipeline_result("Hey #{reference}", author: other_user) + expect(result[:references][:user]).to eq [] + end end end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 6653621a83e..20f0c561e44 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' describe Mentionable do include Mentionable + def author + nil + end + describe :references do let(:project) { create(:project) } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index d7a898e85ff..c103752198d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -115,6 +115,7 @@ describe NotificationService, services: true do before do build_team(note.project) + note.project.team << [note.author, :master] ActionMailer::Base.deliveries.clear end @@ -126,6 +127,8 @@ describe NotificationService, services: true do note.project.team.members.each do |member| # User with disabled notification should not be notified next if member.id == @u_disabled.id + # Author should not be notified + next if member.id == note.author.id should_email(member) end |