Merge branch 'reference-access-control' into 'master'

Only allow users to reference groups, projects, issues, MRs, commits they have access to.

Addresses https://dev.gitlab.org/gitlab/gitlabhq/issues/2183.

See merge request !1742

14 files changed, 87 insertions, 79 deletions

diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 5aae697e2f0..d13d80be293 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -134,12 +134,13 @@ module CommitsHelper
   #  avatar: true will prepend the avatar image
   #  size:   size of the avatar image in px
   def commit_person_link(commit, options = {})
+    user = commit.send(options[:source])
+    
     source_name = clean(commit.send "#{options[:source]}_name".to_sym)
     source_email = clean(commit.send "#{options[:source]}_email".to_sym)
 
-    user = User.find_for_commit(source_email, source_name)
-    person_name = user.nil? ? source_name : user.name
-    person_email = user.nil? ? source_email : user.email
+    person_name = user.try(:name) || source_name
+    person_email = user.try(:email) || source_email
 
     text =
       if options[:avatar]
diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb
index 26f43bf955e..626eb593d51 100644
--- a/app/mailers/emails/groups.rb
+++ b/app/mailers/emails/groups.rb
@@ -4,6 +4,7 @@ module Emails
       @group_member = GroupMember.find(group_member_id)
       @group = @group_member.group
       @target_url = group_url(@group)
+      @current_user = @group_member.user
       mail(to: @group_member.user.email,
            subject: subject("Access to group was granted"))
     end
diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb
index ab5b0765352..3a83b083109 100644
--- a/app/mailers/emails/profile.rb
+++ b/app/mailers/emails/profile.rb
@@ -1,7 +1,7 @@
 module Emails
   module Profile
     def new_user_email(user_id, token = nil)
-      @user = User.find(user_id)
+      @current_user = @user = User.find(user_id)
       @target_url = user_url(@user)
       @token = token
       mail(to: @user.notification_email, subject: subject("Account was created for you"))
@@ -9,13 +9,13 @@ module Emails
 
     def new_email_email(email_id)
       @email = Email.find(email_id)
-      @user = @email.user
+      @current_user = @user = @email.user
       mail(to: @user.notification_email, subject: subject("Email was added to your account"))
     end
 
     def new_ssh_key_email(key_id)
       @key = Key.find(key_id)
-      @user = @key.user
+      @current_user = @user = @key.user
       @target_url = user_url(@user)
       mail(to: @user.notification_email, subject: subject("SSH key was added to your account"))
     end
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index 3cd812825e2..20a863c3742 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -4,12 +4,13 @@ module Emails
       @project_member = ProjectMember.find user_project_id
       @project = @project_member.project
       @target_url = namespace_project_url(@project.namespace, @project)
+      @current_user = @project_member.user
       mail(to: @project_member.user.email,
            subject: subject("Access to project was granted"))
     end
 
     def project_was_moved_email(project_id, user_id)
-      @user = User.find user_id
+      @current_user = @user = User.find user_id
       @project = Project.find project_id
       @target_url = namespace_project_url(@project.namespace, @project)
       mail(to: @user.notification_email,
@@ -28,7 +29,7 @@ module Emails
       end
 
       @project = Project.find(project_id)
-      @author  = User.find(author_id)
+      @current_user = @author  = User.find(author_id)
       @reverse_compare = reverse_compare
       @compare = compare
       @ref_name  = Gitlab::Git.ref_name(ref)
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 0c186ab5866..7c8b37029d1 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -13,6 +13,9 @@ class Notify < ActionMailer::Base
   add_template_helper MergeRequestsHelper
   add_template_helper EmailsHelper
 
+  attr_accessor :current_user
+  helper_method :current_user, :can?
+
   default_url_options[:host]     = Gitlab.config.gitlab.host
   default_url_options[:protocol] = Gitlab.config.gitlab.protocol
   default_url_options[:port]     = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port?
@@ -79,9 +82,8 @@ class Notify < ActionMailer::Base
   #
   # Returns a String containing the User's email address.
   def recipient(recipient_id)
-    if recipient = User.find(recipient_id)
-      recipient.notification_email
-    end
+    @current_user = User.find(recipient_id)
+    @current_user.notification_email
   end
 
   # Set the References header field
@@ -154,4 +156,8 @@ class Notify < ActionMailer::Base
 
     mail(headers, &block)
   end
+
+  def can?
+    Ability.abilities.allowed?(user, action, subject)
+  end
 end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index e0461809e10..7a0ad137650 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -117,8 +117,8 @@ class Commit
 
   # Discover issues should be closed when this commit is pushed to a project's
   # default branch.
-  def closes_issues(project)
-    Gitlab::ClosingIssueExtractor.closed_by_message_in_project(safe_message, project)
+  def closes_issues(project, current_user = self.committer)
+    Gitlab::ClosingIssueExtractor.new(project, current_user).closed_by_message(safe_message)
   end
 
   # Mentionable override.
@@ -126,6 +126,14 @@ class Commit
     "commit #{id}"
   end
 
+  def author
+    User.find_for_commit(author_email, author_name)
+  end
+
+  def committer
+    User.find_for_commit(committer_email, committer_name)
+  end
+
   def method_missing(m, *args, &block)
     @raw.send(m, *args, &block)
   end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 88ac83744df..478134dff68 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -118,16 +118,16 @@ module Issuable
   end
 
   # Return all users participating on the discussion
-  def participants
+  def participants(current_user = self.author)
     users = []
     users << author
     users << assignee if is_assigned?
     mentions = []
-    mentions << self.mentioned_users
+    mentions << self.mentioned_users(current_user)
 
     notes.each do |note|
       users << note.author
-      mentions << note.mentioned_users
+      mentions << note.mentioned_users(current_user)
     end
 
     users.concat(mentions.reduce([], :|)).uniq
@@ -140,7 +140,7 @@ module Issuable
       return subscription.subscribed
     end
 
-    participants.include?(user)
+    participants(user).include?(user)
   end
 
   def toggle_subscription(user)
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index d96e07034ec..b7882a2bb16 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -42,35 +42,22 @@ module Mentionable
     Note.cross_reference_exists?(target, local_reference)
   end
 
-  def mentioned_users
-    users = []
-    return users if mentionable_text.blank?
-    has_project = self.respond_to? :project
-    matches = mentionable_text.scan(/@[a-zA-Z][a-zA-Z0-9_\-\.]*/)
-    matches.each do |match|
-      identifier = match.delete "@"
-      if identifier == "all"
-        users.push(*project.team.members.flatten)
-      elsif namespace = Namespace.find_by(path: identifier)
-        if namespace.is_a?(Group)
-          users.push(*namespace.users)
-        else
-          users << namespace.owner
-        end
-      end
-    end
-    users.uniq
+  def mentioned_users(current_user = nil)
+    return [] if mentionable_text.blank?
+
+    ext = Gitlab::ReferenceExtractor.new(self.project, current_user)
+    ext.analyze(mentionable_text)
+    ext.users.uniq
   end
 
   # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
-  def references(p = project, text = mentionable_text)
+  def references(p = project, current_user = self.author, text = mentionable_text)
     return [] if text.blank?
-    ext = Gitlab::ReferenceExtractor.new
-    ext.analyze(text, p)
 
-    (ext.issues_for(p)  +
-     ext.merge_requests_for(p) +
-     ext.commits_for(p)).uniq - [local_reference]
+    ext = Gitlab::ReferenceExtractor.new(p, current_user)
+    ext.analyze(text)
+
+    (ext.issues + ext.merge_requests + ext.commits).uniq - [local_reference]
   end
 
   # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.
@@ -96,7 +83,7 @@ module Mentionable
     # Only proceed if the saved changes actually include a chance to an attr_mentionable field.
     return unless mentionable_changed
 
-    preexisting = references(p, original)
+    preexisting = references(p, self.author, original)
     create_cross_references!(p, a, preexisting)
   end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 5634f9a686e..35cb920d8bc 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -257,11 +257,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   # Return the set of issues that will be closed if this merge request is accepted.
-  def closes_issues
+  def closes_issues(current_user = self.author)
     if target_branch == project.default_branch
-      issues = commits.flat_map { |c| c.closes_issues(project) }
-      issues.push(*Gitlab::ClosingIssueExtractor.
-                  closed_by_message_in_project(description, project))
+      issues = commits.flat_map { |c| c.closes_issues(project, current_user) }
+      issues.push(*Gitlab::ClosingIssueExtractor.new(project, current_user).
+                  closed_by_message(description))
       issues.uniq.sort_by(&:id)
     else
       []
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 1f0b29dff5e..31e0167d247 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -70,7 +70,7 @@ class GitPushService
       # Close issues if these commits were pushed to the project's default branch and the commit message matches the
       # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
       # a different branch.
-      issues_to_close = commit.closes_issues(project)
+      issues_to_close = commit.closes_issues(project, user)
 
       # Load commit author only if needed.
       # For push with 1k commits it prevents 900+ requests in database
@@ -87,7 +87,7 @@ class GitPushService
       # Create cross-reference notes for any other references. Omit any issues that were referenced in an
       # issue-closing phrase, or have already been mentioned from this commit (probably from this commit
       # being pushed to a different branch).
-      refs = commit.references(project) - issues_to_close
+      refs = commit.references(project, user) - issues_to_close
       refs.reject! { |r| commit.has_mentioned?(r) }
 
       if refs.present?
@@ -127,6 +127,6 @@ class GitPushService
   end
 
   def commit_user(commit)
-    User.find_for_commit(commit.author_email, commit.author_name) || user
+    commit.author || user
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index cc5853144c5..42547f6f481 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -123,32 +123,29 @@ class NotificationService
     return true if note.note.start_with?('Status changed to closed')
     return true if note.cross_reference? && note.system == true
 
-    opts = { noteable_type: note.noteable_type, project_id: note.project_id }
-
     target = note.noteable
 
-    if target.respond_to?(:participants)
-      recipients = target.participants
-    else
-      recipients = note.mentioned_users
-    end
+    recipients = []
 
     if note.commit_id.present?
-      opts.merge!(commit_id: note.commit_id)
       recipients << note.commit_author
-    else
-      opts.merge!(noteable_id: note.noteable_id)
     end
 
     # Get users who left comment in thread
-    recipients = recipients.concat(User.where(id: Note.where(opts).pluck(:author_id)))
+    recipients = recipients.concat(noteable_commenters(note))
 
     # Merge project watchers
     recipients = recipients.concat(project_watchers(note.project)).compact.uniq
 
-    # Reject mention users unless mentioned in comment
-    recipients = reject_mention_users(recipients - note.mentioned_users, note.project)
-    recipients = recipients + note.mentioned_users
+    # Reject users with Mention notification level
+    recipients = reject_mention_users(recipients, note.project)
+
+    # Add explicitly mentioned users
+    if target.respond_to?(:participants)
+      recipients = recipients.concat(target.participants)
+    else
+      recipients = recipients.concat(note.mentioned_users)
+    end
 
     # Reject mutes users
     recipients = reject_muted_users(recipients, note.project)
@@ -195,6 +192,18 @@ class NotificationService
 
   protected
 
+  def noteable_commenters(note)
+    opts = { noteable_type: note.noteable_type, project_id: note.project_id }
+
+    if note.commit_id.present?
+      opts.merge!(commit_id: note.commit_id)
+    else
+      opts.merge!(noteable_id: note.noteable_id)
+    end
+
+    User.where(id: Note.where(opts).pluck(:author_id))
+  end
+
   # Get project users with WATCH notification level
   def project_watchers(project)
     project_members = project_member_notification(project)
diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb
index bcbacbff562..ae6260bcdab 100644
--- a/app/services/projects/participants_service.rb
+++ b/app/services/projects/participants_service.rb
@@ -1,10 +1,5 @@
 module Projects
   class ParticipantsService < BaseService
-    def initialize(project, user)
-      @project  = project
-      @user     = user
-    end
-
     def execute(note_type, note_id)
       participating =
         if note_type && note_id
@@ -12,7 +7,7 @@ module Projects
         else
           []
         end
-      project_members = sorted(@project.team.members)
+      project_members = sorted(project.team.members)
       participants = all_members + groups + project_members + participating
       participants.uniq
     end
@@ -20,11 +15,11 @@ module Projects
     def participants_in(type, id)
       users = case type
               when "Issue"
-                issue = @project.issues.find_by_iid(id)
-                issue ? issue.participants : []
+                issue = project.issues.find_by_iid(id)
+                issue ? issue.participants(current_user) : []
               when "MergeRequest"
-                merge_request = @project.merge_requests.find_by_iid(id)
-                merge_request ? merge_request.participants : []
+                merge_request = project.merge_requests.find_by_iid(id)
+                merge_request ? merge_request.participants(current_user) : []
               when "Commit"
                 author_ids = Note.for_commit_id(id).pluck(:author_id).uniq
                 User.where(id: author_ids)
@@ -41,14 +36,14 @@ module Projects
     end
 
     def groups
-      @user.authorized_groups.sort_by(&:path).map do |group| 
+      current_user.authorized_groups.sort_by(&:path).map do |group| 
         count = group.users.count
         { username: group.path, name: "#{group.name} (#{count})" }
       end
     end
 
     def all_members
-      count = @project.team.members.flatten.count
+      count = project.team.members.flatten.count
       [{ username: "all", name: "All Project and Group Members (#{count})" }]
     end
   end
diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml
index 0d3028d50b4..288b48f4583 100644
--- a/app/views/projects/issues/_discussion.html.haml
+++ b/app/views/projects/issues/_discussion.html.haml
@@ -9,8 +9,8 @@
     .votes-holder.pull-right
       #votes= render 'votes/votes_block', votable: @issue
     .participants
-      %span= pluralize(@issue.participants.count, 'participant')
-      - @issue.participants.each do |participant|
+      %span= pluralize(@issue.participants(current_user).count, 'participant')
+      - @issue.participants(current_user).each do |participant|
         = link_to_member(@project, participant, name: false, size: 24)
     .voting_notes#notes= render "projects/notes/notes_with_form"
   %aside.col-md-3
diff --git a/app/views/projects/merge_requests/show/_participants.html.haml b/app/views/projects/merge_requests/show/_participants.html.haml
index 4f34af1737d..9c93fa55fe6 100644
--- a/app/views/projects/merge_requests/show/_participants.html.haml
+++ b/app/views/projects/merge_requests/show/_participants.html.haml
@@ -1,4 +1,4 @@
 .participants
-  %span #{@merge_request.participants.count} participants
-  - @merge_request.participants.each do |participant|
+  %span #{@merge_request.participants(current_user).count} participants
+  - @merge_request.participants(current_user).each do |participant|
     = link_to_member(@project, participant, name: false, size: 24)