Commit ecb58dac authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

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
parents 8cf1a6f0 16e1076e
...@@ -81,6 +81,7 @@ v 7.9.3 ...@@ -81,6 +81,7 @@ v 7.9.3
- Warn when gitlab-shell version doesn't match requirement. - Warn when gitlab-shell version doesn't match requirement.
- Skip email confirmation when set by admin or via LDAP. - Skip email confirmation when set by admin or via LDAP.
- Only allow users to reference groups, projects, issues, MRs, commits they have access to.
v 7.9.2 v 7.9.2
- Contains no changes - Contains no changes
......
...@@ -208,7 +208,6 @@ group :development do ...@@ -208,7 +208,6 @@ group :development do
gem "letter_opener" gem "letter_opener"
gem 'quiet_assets', '~> 1.0.1' gem 'quiet_assets', '~> 1.0.1'
gem 'rack-mini-profiler', require: false gem 'rack-mini-profiler', require: false
gem "byebug"
# Better errors handler # Better errors handler
gem 'better_errors' gem 'better_errors'
...@@ -257,6 +256,8 @@ group :development, :test do ...@@ -257,6 +256,8 @@ group :development, :test do
gem "spring", '~> 1.3.1' gem "spring", '~> 1.3.1'
gem "spring-commands-rspec", '1.0.4' gem "spring-commands-rspec", '1.0.4'
gem "spring-commands-spinach", '1.0.0' gem "spring-commands-spinach", '1.0.0'
gem "byebug"
end end
group :test do group :test do
......
...@@ -134,12 +134,13 @@ module CommitsHelper ...@@ -134,12 +134,13 @@ module CommitsHelper
# avatar: true will prepend the avatar image # avatar: true will prepend the avatar image
# size: size of the avatar image in px # size: size of the avatar image in px
def commit_person_link(commit, options = {}) def commit_person_link(commit, options = {})
user = commit.send(options[:source])
source_name = clean(commit.send "#{options[:source]}_name".to_sym) source_name = clean(commit.send "#{options[:source]}_name".to_sym)
source_email = clean(commit.send "#{options[:source]}_email".to_sym) source_email = clean(commit.send "#{options[:source]}_email".to_sym)
user = User.find_for_commit(source_email, source_name) person_name = user.try(:name) || source_name
person_name = user.nil? ? source_name : user.name person_email = user.try(:email) || source_email
person_email = user.nil? ? source_email : user.email
text = text =
if options[:avatar] if options[:avatar]
......
...@@ -4,6 +4,7 @@ module Emails ...@@ -4,6 +4,7 @@ module Emails
@group_member = GroupMember.find(group_member_id) @group_member = GroupMember.find(group_member_id)
@group = @group_member.group @group = @group_member.group
@target_url = group_url(@group) @target_url = group_url(@group)
@current_user = @group_member.user
mail(to: @group_member.user.email, mail(to: @group_member.user.email,
subject: subject("Access to group was granted")) subject: subject("Access to group was granted"))
end end
......
module Emails module Emails
module Profile module Profile
def new_user_email(user_id, token = nil) 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) @target_url = user_url(@user)
@token = token @token = token
mail(to: @user.notification_email, subject: subject("Account was created for you")) mail(to: @user.notification_email, subject: subject("Account was created for you"))
...@@ -9,13 +9,13 @@ module Emails ...@@ -9,13 +9,13 @@ module Emails
def new_email_email(email_id) def new_email_email(email_id)
@email = Email.find(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")) mail(to: @user.notification_email, subject: subject("Email was added to your account"))
end end
def new_ssh_key_email(key_id) def new_ssh_key_email(key_id)
@key = Key.find(key_id) @key = Key.find(key_id)
@user = @key.user @current_user = @user = @key.user
@target_url = user_url(@user) @target_url = user_url(@user)
mail(to: @user.notification_email, subject: subject("SSH key was added to your account")) mail(to: @user.notification_email, subject: subject("SSH key was added to your account"))
end end
......
...@@ -4,12 +4,13 @@ module Emails ...@@ -4,12 +4,13 @@ module Emails
@project_member = ProjectMember.find user_project_id @project_member = ProjectMember.find user_project_id
@project = @project_member.project @project = @project_member.project
@target_url = namespace_project_url(@project.namespace, @project) @target_url = namespace_project_url(@project.namespace, @project)
@current_user = @project_member.user
mail(to: @project_member.user.email, mail(to: @project_member.user.email,
subject: subject("Access to project was granted")) subject: subject("Access to project was granted"))
end end
def project_was_moved_email(project_id, user_id) 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 @project = Project.find project_id
@target_url = namespace_project_url(@project.namespace, @project) @target_url = namespace_project_url(@project.namespace, @project)
mail(to: @user.notification_email, mail(to: @user.notification_email,
...@@ -28,7 +29,7 @@ module Emails ...@@ -28,7 +29,7 @@ module Emails
end end
@project = Project.find(project_id) @project = Project.find(project_id)
@author = User.find(author_id) @current_user = @author = User.find(author_id)
@reverse_compare = reverse_compare @reverse_compare = reverse_compare
@compare = compare @compare = compare
@ref_name = Gitlab::Git.ref_name(ref) @ref_name = Gitlab::Git.ref_name(ref)
......
...@@ -13,6 +13,9 @@ class Notify < ActionMailer::Base ...@@ -13,6 +13,9 @@ class Notify < ActionMailer::Base
add_template_helper MergeRequestsHelper add_template_helper MergeRequestsHelper
add_template_helper EmailsHelper add_template_helper EmailsHelper
attr_accessor :current_user
helper_method :current_user, :can?
default_url_options[:host] = Gitlab.config.gitlab.host default_url_options[:host] = Gitlab.config.gitlab.host
default_url_options[:protocol] = Gitlab.config.gitlab.protocol default_url_options[:protocol] = Gitlab.config.gitlab.protocol
default_url_options[:port] = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port? default_url_options[:port] = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port?
...@@ -79,9 +82,8 @@ class Notify < ActionMailer::Base ...@@ -79,9 +82,8 @@ class Notify < ActionMailer::Base
# #
# Returns a String containing the User's email address. # Returns a String containing the User's email address.
def recipient(recipient_id) def recipient(recipient_id)
if recipient = User.find(recipient_id) @current_user = User.find(recipient_id)
recipient.notification_email @current_user.notification_email
end
end end
# Set the References header field # Set the References header field
...@@ -154,4 +156,8 @@ class Notify < ActionMailer::Base ...@@ -154,4 +156,8 @@ class Notify < ActionMailer::Base
mail(headers, &block) mail(headers, &block)
end end
def can?
Ability.abilities.allowed?(user, action, subject)
end
end end
...@@ -117,8 +117,8 @@ class Commit ...@@ -117,8 +117,8 @@ class Commit
# Discover issues should be closed when this commit is pushed to a project's # Discover issues should be closed when this commit is pushed to a project's
# default branch. # default branch.
def closes_issues(project) def closes_issues(project, current_user = self.committer)
Gitlab::ClosingIssueExtractor.closed_by_message_in_project(safe_message, project) Gitlab::ClosingIssueExtractor.new(project, current_user).closed_by_message(safe_message)
end end
# Mentionable override. # Mentionable override.
...@@ -126,6 +126,14 @@ class Commit ...@@ -126,6 +126,14 @@ class Commit
"commit #{id}" "commit #{id}"
end 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) def method_missing(m, *args, &block)
@raw.send(m, *args, &block) @raw.send(m, *args, &block)
end end
......
...@@ -118,16 +118,16 @@ module Issuable ...@@ -118,16 +118,16 @@ module Issuable
end end
# Return all users participating on the discussion # Return all users participating on the discussion
def participants def participants(current_user = self.author)
users = [] users = []
users << author users << author
users << assignee if is_assigned? users << assignee if is_assigned?
mentions = [] mentions = []
mentions << self.mentioned_users mentions << self.mentioned_users(current_user)
notes.each do |note| notes.each do |note|
users << note.author users << note.author
mentions << note.mentioned_users mentions << note.mentioned_users(current_user)
end end
users.concat(mentions.reduce([], :|)).uniq users.concat(mentions.reduce([], :|)).uniq
...@@ -140,7 +140,7 @@ module Issuable ...@@ -140,7 +140,7 @@ module Issuable
return subscription.subscribed return subscription.subscribed
end end
participants.include?(user) participants(user).include?(user)
end end
def toggle_subscription(user) def toggle_subscription(user)
......
...@@ -42,35 +42,22 @@ module Mentionable ...@@ -42,35 +42,22 @@ module Mentionable
Note.cross_reference_exists?(target, local_reference) Note.cross_reference_exists?(target, local_reference)
end end
def mentioned_users def mentioned_users(current_user = nil)
users = [] return [] if mentionable_text.blank?
return users if mentionable_text.blank?
has_project = self.respond_to? :project ext = Gitlab::ReferenceExtractor.new(self.project, current_user)
matches = mentionable_text.scan(/@[a-zA-Z][a-zA-Z0-9_\-\.]*/) ext.analyze(mentionable_text)
matches.each do |match| ext.users.uniq
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
end end
# Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. # 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? return [] if text.blank?
ext = Gitlab::ReferenceExtractor.new
ext.analyze(text, p)
(ext.issues_for(p) + ext = Gitlab::ReferenceExtractor.new(p, current_user)
ext.merge_requests_for(p) + ext.analyze(text)
ext.commits_for(p)).uniq - [local_reference]
(ext.issues + ext.merge_requests + ext.commits).uniq - [local_reference]
end end
# Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+. # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.
...@@ -96,7 +83,7 @@ module Mentionable ...@@ -96,7 +83,7 @@ module Mentionable
# Only proceed if the saved changes actually include a chance to an attr_mentionable field. # Only proceed if the saved changes actually include a chance to an attr_mentionable field.
return unless mentionable_changed return unless mentionable_changed
preexisting = references(p, original) preexisting = references(p, self.author, original)
create_cross_references!(p, a, preexisting) create_cross_references!(p, a, preexisting)
end end
end end
...@@ -257,11 +257,11 @@ class MergeRequest < ActiveRecord::Base ...@@ -257,11 +257,11 @@ class MergeRequest < ActiveRecord::Base
end end
# Return the set of issues that will be closed if this merge request is accepted. # 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 if target_branch == project.default_branch
issues = commits.flat_map { |c| c.closes_issues(project) } issues = commits.flat_map { |c| c.closes_issues(project, current_user) }
issues.push(*Gitlab::ClosingIssueExtractor. issues.push(*Gitlab::ClosingIssueExtractor.new(project, current_user).
closed_by_message_in_project(description, project)) closed_by_message(description))
issues.uniq.sort_by(&:id) issues.uniq.sort_by(&:id)
else else
[] []
......
...@@ -70,7 +70,7 @@ class GitPushService ...@@ -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 # 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 # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
# a different branch. # a different branch.
issues_to_close = commit.closes_issues(project) issues_to_close = commit.closes_issues(project, user)
# Load commit author only if needed. # Load commit author only if needed.
# For push with 1k commits it prevents 900+ requests in database # For push with 1k commits it prevents 900+ requests in database
...@@ -87,7 +87,7 @@ class GitPushService ...@@ -87,7 +87,7 @@ class GitPushService
# Create cross-reference notes for any other references. Omit any issues that were referenced in an # 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 # issue-closing phrase, or have already been mentioned from this commit (probably from this commit
# being pushed to a different branch). # 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) } refs.reject! { |r| commit.has_mentioned?(r) }
if refs.present? if refs.present?
...@@ -127,6 +127,6 @@ class GitPushService ...@@ -127,6 +127,6 @@ class GitPushService
end end
def commit_user(commit) def commit_user(commit)
User.find_for_commit(commit.author_email, commit.author_name) || user commit.author || user
end end
end end
...@@ -123,32 +123,29 @@ class NotificationService ...@@ -123,32 +123,29 @@ class NotificationService
return true if note.note.start_with?('Status changed to closed') return true if note.note.start_with?('Status changed to closed')
return true if note.cross_reference? && note.system == true return true if note.cross_reference? && note.system == true
opts = { noteable_type: note.noteable_type, project_id: note.project_id }
target = note.noteable target = note.noteable
if target.respond_to?(:participants) recipients = []
recipients = target.participants
else
recipients = note.mentioned_users
end
if note.commit_id.present? if note.commit_id.present?
opts.merge!(commit_id: note.commit_id)
recipients << note.commit_author recipients << note.commit_author
else
opts.merge!(noteable_id: note.noteable_id)
end end
# Get users who left comment in thread # 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 # Merge project watchers
recipients = recipients.concat(project_watchers(note.project)).compact.uniq recipients = recipients.concat(project_watchers(note.project)).compact.uniq
# Reject mention users unless mentioned in comment # Reject users with Mention notification level
recipients = reject_mention_users(recipients - note.mentioned_users, note.project) recipients = reject_mention_users(recipients, note.project)
recipients = recipients + note.mentioned_users
# 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 # Reject mutes users
recipients = reject_muted_users(recipients, note.project) recipients = reject_muted_users(recipients, note.project)
...@@ -195,6 +192,18 @@ class NotificationService ...@@ -195,6 +192,18 @@ class NotificationService
protected 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 # Get project users with WATCH notification level
def project_watchers(project) def project_watchers(project)
project_members = project_member_notification(project) project_members = project_member_notification(project)
......
module Projects module Projects
class ParticipantsService < BaseService class ParticipantsService < BaseService
def initialize(project, user)
@project = project
@user = user
end
def execute(note_type, note_id) def execute(note_type, note_id)
participating = participating =
if note_type && note_id if note_type && note_id
...@@ -12,7 +7,7 @@ module Projects ...@@ -12,7 +7,7 @@ module Projects
else else
[] []
end end
project_members = sorted(@project.team.members) project_members = sorted(project.team.members)
participants = all_members + groups + project_members + participating participants = all_members + groups + project_members + participating
participants.uniq participants.uniq
end end
...@@ -20,11 +15,11 @@ module Projects ...@@ -20,11 +15,11 @@ module Projects
def participants_in(type, id) def participants_in(type, id)
users = case type users = case type
when "Issue" when "Issue"
issue = @project.issues.find_by_iid(id) issue = project.issues.find_by_iid(id)
issue ? issue.participants : [] issue ? issue.participants(current_user) : []
when "MergeRequest" when "MergeRequest"
merge_request = @project.merge_requests.find_by_iid(id) merge_request = project.merge_requests.find_by_iid(id)
merge_request ? merge_request.participants : [] merge_request ? merge_request.participants(current_user) : []
when "Commit" when "Commit"
author_ids = Note.for_commit_id(id).pluck(:author_id).uniq author_ids = Note.for_commit_id(id).pluck(:author_id).uniq
User.where(id: author_ids) User.where(id: author_ids)
...@@ -41,14 +36,14 @@ module Projects ...@@ -41,14 +36,14 @@ module Projects
end end
def groups 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 count = group.users.count
{ username: group.path, name: "#{group.name} (#{count})" } { username: group.path, name: "#{group.name} (#{count})" }
end end
end end
def all_members def all_members
count = @project.team.members.flatten.count count = project.team.members.flatten.count
[{ username: "all", name: "All Project and Group Members (#{count})" }] [{ username: "all", name: "All Project and Group Members (#{count})" }]
end end
end end
......
...@@ -9,8 +9,8 @@ ...@@ -9,8 +9,8 @@
.votes-holder.pull-right .votes-holder.pull-right
#votes= render 'votes/votes_block', votable: @issue #votes= render 'votes/votes_block', votable: @issue
.participants .participants
%span= pluralize(@issue.participants.count, 'participant') %span= pluralize(@issue.participants(current_user).count, 'participant')
- @issue.participants.each do |participant| - @issue.participants(current_user).each do |participant|
= link_to_member(@project, participant, name: false, size: 24) = link_to_member(@project, participant, name: false, size: 24)
.voting_notes#notes= render "projects/notes/notes_with_form" .voting_notes#notes= render "projects/notes/notes_with_form"
%aside.col-md-3 %aside.col-md-3
......
.participants .participants
%span #{@merge_request.participants.count} participants %span #{@merge_request.participants(current_user).count} participants
- @merge_request.participants.each do |participant| - @merge_request.participants(current_user).each do |participant|
= link_to_member(@project, participant, name: false, size: 24) = link_to_member(@project, participant, name: false, size: 24)
module Gitlab module Gitlab
module ClosingIssueExtractor class ClosingIssueExtractor
ISSUE_CLOSING_REGEX = Regexp.new(Gitlab.config.gitlab.issue_closing_pattern) ISSUE_CLOSING_REGEX = Regexp.new(Gitlab.config.gitlab.issue_closing_pattern)
def self.closed_by_message_in_project(message, project) def initialize(project, current_user = nil)
issues = [] @extractor = Gitlab::ReferenceExtractor.new(project, current_user)
end
unless message.nil? def closed_by_message(message)
md = message.scan(ISSUE_CLOSING_REGEX) return [] if message.nil?
md.each do |ref| closing_statements = message.scan(ISSUE_CLOSING_REGEX).
extractor = Gitlab::ReferenceExtractor.new map { |ref| ref[0] }.join(" ")
extractor.analyze(ref[0], project)
issues += extractor.issues_for(project) @extractor.analyze(closing_statements)
end
end
issues.uniq @extractor.issues
end end
end end
end end
...@@ -192,6 +192,7 @@ module Gitlab ...@@ -192,6 +192,7 @@ module Gitlab
project_path = $LAST_MATCH_INFO[:project] project_path = $LAST_MATCH_INFO[:project]
if project_path if project_path
actual_project = ::Project.find_with_namespace(project_path) actual_project = ::Project.find_with_namespace(project_path)
actual_project = nil unless can?(current_user, :read_project, actual_project)
project_prefix = project_path project_prefix = project_path
end end
...@@ -251,6 +252,7 @@ module Gitlab ...@@ -251,6 +252,7 @@ module Gitlab
elsif namespace = Namespace.find_by(path: identifier) elsif namespace = Namespace.find_by(path: identifier)
url = url =
if namespace.is_a?(Group) if namespace.is_a?(Group)
return nil unless can?(current_user, :read_group, namespace)
group_url(identifier, only_path: options[:reference_only_path]) group_url(identifier, only_path: options[:reference_only_path])
else else
user_url(identifier, only_path: options[:reference_only_path]) user_url(identifier, only_path: options[:reference_only_path])
......
module Gitlab module Gitlab
# Extract possible GFM references from an arbitrary String for further processing. # Extract possible GFM references from an arbitrary String for further processing.
class ReferenceExtractor class ReferenceExtractor
attr_accessor :users, :labels, :issues, :merge_requests, :snippets, :commits, :commit_ranges attr_accessor :project, :current_user, :references
include ::Gitlab::Markdown include ::Gitlab::Markdown
def initialize def initialize(project, current_user = nil)
@users, @labels, @issues, @merge_requests, @snippets, @commits, @commit_ranges = @project = project
[], [], [], [], [], [], [] @current_user = current_user
end end
def analyze(string, project) def can?(user, action, subject)
text = string.dup Ability.abilities.allowed?(user, action, subject)
end
def analyze(text)
text = text.dup
# Remove preformatted/code blocks so that references are not included # Remove preformatted/code blocks so that references are not included
text.gsub!(%r{<pre>.*?</pre>|<code>.*?</code>}m) { |match| '' } text.gsub!(%r{<pre>.*?</pre>|<code>.*?</code>}m) { |match| '' }
text.gsub!(%r{^```.*?^```}m) { |match| '' } text.gsub!(%r{^```.*?^```}m) { |match| '' }
parse_references(text, project) @references = Hash.new { |hash, type| hash[type] = [] }
parse_references(text)
end end
# Given a valid project, resolve the extracted identifiers of the requested type to # Given a valid project, resolve the extracted identifiers of the requested type to
# model objects. # model objects.
def users_for(project) def users
users.map do |entry| references[:user].uniq.map do |project, identifier|
project.users.where(username: entry[:id]).first if identifier == "all"
end.reject(&:nil?) project.team.members.flatten
elsif namespace = Namespace.find_by(path: identifier)
if namespace.is_a?(Group)
namespace.users
else
namespace.owner
end end
def labels_for(project = nil)
labels.map do |entry|
project.labels.where(id: entry[:id]).first
end.reject(&:nil?)
end end
end.flatten.compact.uniq
def issues_for(project = nil)
issues.map do |entry|
if should_lookup?(project, entry[:project])
entry[:project].issues.where(iid: entry[:id]).first
end end
end.reject(&:nil?)
def labels
references[:label].uniq.map do |project, identifier|
project.labels.where(id: identifier).first
end.compact.uniq
end end
def merge_requests_for(project = nil) def issues
merge_requests.map do |entry| references[:issue].uniq.map do |project, identifier|
if should_lookup?(project, entry[:project]) if project.default_issues_tracker?
entry[:project].merge_requests.where(iid: entry[:id]).first project.issues.where(iid: identifier).first
end end
end.reject(&:nil?) end.compact.uniq
end end
def snippets_for(project) def merge_requests
snippets.map do |entry| references[:merge_request].uniq.map do |project, identifier|
project.snippets.where(id: entry[:id]).first project.merge_requests.where(iid: identifier).first
end.reject(&:nil?) end.compact.uniq
end end
def commits_for(project = nil) def snippets
commits.map do |entry| references[:snippet].uniq.map do |project, identifier|
repo = entry[:project].repository if entry[:project] project.snippets.where(id: identifier).first
if should_lookup?(project, entry[:project]) end.compact.uniq
repo.commit(entry[:id]) if repo
end end
end.reject(&:nil?)
def commits
references[:commit].uniq.map do |project, identifier|
repo = project.repository
repo.commit(identifier) if repo
end.compact.uniq
end end
def commit_ranges_for(project = nil) def commit_ranges
commit_ranges.map do |entry| references[:commit_range].uniq.map do |project, identifier|
repo = entry[:project].repository if entry[:project] repo = project.repository
if repo && should_lookup?(project, entry[:project]) if repo
from_id, to_id = entry[:id].split(/\.{2,3}/, 2) from_id, to_id = identifier.split(/\.{2,3}/, 2)
[repo.commit(from_id), repo.commit(to_id)] [repo.commit(from_id), repo.commit(to_id)]
end end
end.reject(&:nil?) end.compact.uniq
end end
private private
def reference_link(type, identifier, project, _) def reference_link(type, identifier, project, _)
# Append identifier to the appropriate collection. references[type] << [project, identifier]
send("#{type}s") << { project: project, id: identifier }
end
def should_lookup?(project, entry_project)
if entry_project.nil?
false
else
project.nil? || entry_project.default_issues_tracker?
end
end end
end end
end end
...@@ -4,6 +4,11 @@ describe GitlabMarkdownHelper do ...@@ -4,6 +4,11 @@ describe GitlabMarkdownHelper do
include ApplicationHelper include ApplicationHelper
include IssuesHelper include IssuesHelper
# TODO: Properly test this
def can?(*)
true
end
let!(:project) { create(:project) } let!(:project) { create(:project) }
let(:empty_project) { create(:empty_project) } let(:empty_project) { create(:empty_project) }
...@@ -15,6 +20,9 @@ describe GitlabMarkdownHelper do ...@@ -15,6 +20,9 @@ describe GitlabMarkdownHelper do
let(:snippet) { create(:project_snippet, project: project) } let(:snippet) { create(:project_snippet, project: project) }
let(:member) { project.project_members.where(user_id: user).first } let(:member) { project.project_members.where(user_id: user).first }
# Helper expects a current_user method.
let(:current_user) { user }
def url_helper(image_name) def url_helper(image_name)
File.join(root_url, 'assets', image_name) File.join(root_url, 'assets', image_name)
end end
......
...@@ -5,126 +5,128 @@ describe Gitlab::ClosingIssueExtractor do ...@@ -5,126 +5,128 @@ describe Gitlab::ClosingIssueExtractor do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:iid1) { issue.iid } let(:iid1) { issue.iid }
describe :closed_by_message_in_project do subject { described_class.new(project, project.creator) }
describe "#closed_by_message" do
context 'with a single reference' do context 'with a single reference' do
it do it do
message = "Awesome commit (Closes ##{iid1})" message = "Awesome commit (Closes ##{iid1})"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Awesome commit (closes ##{iid1})" message = "Awesome commit (closes ##{iid1})"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Closed ##{iid1}" message = "Closed ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "closed ##{iid1}" message = "closed ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Closing ##{iid1}" message = "Closing ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "closing ##{iid1}" message = "closing ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Close ##{iid1}" message = "Close ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "close ##{iid1}" message = "close ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Awesome commit (Fixes ##{iid1})" message = "Awesome commit (Fixes ##{iid1})"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Awesome commit (fixes ##{iid1})" message = "Awesome commit (fixes ##{iid1})"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Fixed ##{iid1}" message = "Fixed ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "fixed ##{iid1}" message = "fixed ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Fixing ##{iid1}" message = "Fixing ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "fixing ##{iid1}" message = "fixing ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Fix ##{iid1}" message = "Fix ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "fix ##{iid1}" message = "fix ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Awesome commit (Resolves ##{iid1})" message = "Awesome commit (Resolves ##{iid1})"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Awesome commit (resolves ##{iid1})" message = "Awesome commit (resolves ##{iid1})"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Resolved ##{iid1}" message = "Resolved ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "resolved ##{iid1}" message = "resolved ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Resolving ##{iid1}" message = "Resolving ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "resolving ##{iid1}" message = "resolving ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "Resolve ##{iid1}" message = "Resolve ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
it do it do
message = "resolve ##{iid1}" message = "resolve ##{iid1}"
expect(subject.closed_by_message_in_project(message, project)).to eq([issue]) expect(subject.closed_by_message(message)).to eq([issue])
end end
end end
...@@ -137,28 +139,28 @@ describe Gitlab::ClosingIssueExtractor do ...@@ -137,28 +139,28 @@ describe Gitlab::ClosingIssueExtractor do
it 'fetches issues in single line message' do it 'fetches issues in single line message' do
message = "Closes ##{iid1} and fix ##{iid2}" message = "Closes ##{iid1} and fix ##{iid2}"
expect(subject.closed_by_message_in_project(message, project)). expect(subject.closed_by_message(message)).
to eq([issue, other_issue]) to eq([issue, other_issue])
end end
it 'fetches comma-separated issues references in single line message' do it 'fetches comma-separated issues references in single line message' do
message = "Closes ##{iid1}, closes ##{iid2}" message = "Closes ##{iid1}, closes ##{iid2}"
expect(subject.closed_by_message_in_project(message, project)). expect(subject.closed_by_message(message)).
to eq([issue, other_issue]) to eq([issue, other_issue])
end end
it 'fetches comma-separated issues numbers in single line message' do it 'fetches comma-separated issues numbers in single line message' do
message = "Closes ##{iid1}, ##{iid2} and ##{iid3}" message = "Closes ##{iid1}, ##{iid2} and ##{iid3}"
expect(subject.closed_by_message_in_project(message, project)). expect(subject.closed_by_message(message)).
to eq([issue, other_issue, third_issue]) to eq([issue, other_issue, third_issue])
end end
it 'fetches issues in multi-line message' do it 'fetches issues in multi-line message' do
message = "Awesome commit (closes ##{iid1})\nAlso fixes ##{iid2}" message = "Awesome commit (closes ##{iid1})\nAlso fixes ##{iid2}"
expect(subject.closed_by_message_in_project(message, project)). expect(subject.closed_by_message(message)).
to eq([issue, other_issue]) to eq([issue, other_issue])
end end
...@@ -166,7 +168,7 @@ describe Gitlab::ClosingIssueExtractor do ...@@ -166,7 +168,7 @@ describe Gitlab::ClosingIssueExtractor do
message = "Awesome commit (closes ##{iid1})\n"\ message = "Awesome commit (closes ##{iid1})\n"\
"Also fixing issues ##{iid2}, ##{iid3} and #4" "Also fixing issues ##{iid2}, ##{iid3} and #4"
expect(subject.closed_by_message_in_project(message, project)). expect(subject.closed_by_message(message)).
to eq([issue, other_issue, third_issue]) to eq([issue, other_issue, third_issue])
end end
end end
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::ReferenceExtractor do describe Gitlab::ReferenceExtractor do
let(:project) { create(:project) }
subject { Gitlab::ReferenceExtractor.new(project, project.creator) }
it 'extracts username references' do it 'extracts username references' do
subject.analyze('this contains a @user reference', nil) subject.analyze('this contains a @user reference')
expect(subject.users).to eq([{ project: nil, id: 'user' }]) expect(subject.references[:user]).to eq([[project, 'user']])
end end
it 'extracts issue references' do it 'extracts issue references' do
subject.analyze('this one talks about issue #1234', nil) subject.analyze('this one talks about issue #1234')
expect(subject.issues).to eq([{ project: nil, id: '1234' }]) expect(subject.references[:issue]).to eq([[project, '1234']])
end end
it 'extracts JIRA issue references' do it 'extracts JIRA issue references' do
subject.analyze('this one talks about issue JIRA-1234', nil) subject.analyze('this one talks about issue JIRA-1234')
expect(subject.issues).to eq([{ project: nil, id: 'JIRA-1234' }]) expect(subject.references[:issue]).to eq([[project, 'JIRA-1234']])
end end
it 'extracts merge request references' do it 'extracts merge request references' do
subject.analyze("and here's !43, a merge request", nil) subject.analyze("and here's !43, a merge request")
expect(subject.merge_requests).to eq([{ project: nil, id: '43' }]) expect(subject.references[:merge_request]).to eq([[project, '43']])
end end
it 'extracts snippet ids' do it 'extracts snippet ids' do
subject.analyze('snippets like $12 get extracted as well', nil) subject.analyze('snippets like $12 get extracted as well')
expect(subject.snippets).to eq([{ project: nil, id: '12' }]) expect(subject.references[:snippet]).to eq([[project, '12']])
end end
it 'extracts commit shas' do it 'extracts commit shas' do
subject.analyze('commit shas 98cf0ae3 are pulled out as Strings', nil) subject.analyze('commit shas 98cf0ae3 are pulled out as Strings')
expect(subject.commits).to eq([{ project: nil, id: '98cf0ae3' }]) expect(subject.references[:commit]).to eq([[project, '98cf0ae3']])
end end
it 'extracts commit ranges' do it 'extracts commit ranges' do
subject.analyze('here you go, a commit range: 98cf0ae3...98cf0ae4', nil) subject.analyze('here you go, a commit range: 98cf0ae3...98cf0ae4')
expect(subject.commit_ranges).to eq([{ project: nil, id: '98cf0ae3...98cf0ae4' }]) expect(subject.references[:commit_range]).to eq([[project, '98cf0ae3...98cf0ae4']])
end end
it 'extracts multiple references and preserves their order' do it 'extracts multiple references and preserves their order' do
subject.analyze('@me and @you both care about this', nil) subject.analyze('@me and @you both care about this')
expect(subject.users).to eq([ expect(subject.references[:user]).to eq([
{ project: nil, id: 'me' }, [project, 'me'],
{ project: nil, id: 'you' } [project, 'you']
]) ])
end end
it 'leaves the original note unmodified' do it 'leaves the original note unmodified' do
text = 'issue #123 is just the worst, @user' text = 'issue #123 is just the worst, @user'
subject.analyze(text, nil) subject.analyze(text)
expect(text).to eq('issue #123 is just the worst, @user') expect(text).to eq('issue #123 is just the worst, @user')
end end
it 'extracts no references for <pre>..</pre> blocks' do it 'extracts no references for <pre>..</pre> blocks' do
subject.analyze("<pre>def puts '#1 issue'\nend\n</pre>```", nil) subject.analyze("<pre>def puts '#1 issue'\nend\n</pre>```")
expect(subject.issues).to be_blank expect(subject.issues).to be_blank
end end
it 'extracts no references for <code>..</code> blocks' do it 'extracts no references for <code>..</code> blocks' do
subject.analyze("<code>def puts '!1 request'\nend\n</code>```", nil) subject.analyze("<code>def puts '!1 request'\nend\n</code>```")
expect(subject.merge_requests).to be_blank expect(subject.merge_requests).to be_blank
end end
it 'extracts no references for code blocks with language' do it 'extracts no references for code blocks with language' do
subject.analyze("this code:\n```ruby\ndef puts '#1 issue'\nend\n```", nil) subject.analyze("this code:\n```ruby\ndef puts '#1 issue'\nend\n```")
expect(subject.issues).to be_blank expect(subject.issues).to be_blank
end end
it 'extracts issue references for invalid code blocks' do it 'extracts issue references for invalid code blocks' do
subject.analyze('test: ```this one talks about issue #1234```', nil) subject.analyze('test: ```this one talks about issue #1234```')
expect(subject.issues).to eq([{ project: nil, id: '1234' }]) expect(subject.references[:issue]).to eq([[project, '1234']])
end end
it 'handles all possible kinds of references' do it 'handles all possible kinds of references' do
...@@ -75,35 +78,32 @@ describe Gitlab::ReferenceExtractor do ...@@ -75,35 +78,32 @@ describe Gitlab::ReferenceExtractor do
expect(subject).to respond_to(*accessors) expect(subject).to respond_to(*accessors)
end end
context 'with a project' do it 'accesses valid user objects' do
let(:project) { create(:project) }
it 'accesses valid user objects on the project team' do
@u_foo = create(:user, username: 'foo') @u_foo = create(:user, username: 'foo')
@u_bar = create(:user, username: 'bar') @u_bar = create(:user, username: 'bar')
create(:user, username: 'offteam') @u_offteam = create(:user, username: 'offteam')
project.team << [@u_foo, :reporter] project.team << [@u_foo, :reporter]
project.team << [@u_bar, :guest] project.team << [@u_bar, :guest]
subject.analyze('@foo, @baduser, @bar, and @offteam', project) subject.analyze('@foo, @baduser, @bar, and @offteam')
expect(subject.users_for(project)).to eq([@u_foo, @u_bar]) expect(subject.users).to eq([@u_foo, @u_bar, @u_offteam])
end end
it 'accesses valid issue objects' do it 'accesses valid issue objects' do
@i0 = create(:issue, project: project) @i0 = create(:issue, project: project)
@i1 = create(:issue, project: project) @i1 = create(:issue, project: project)
subject.analyze("##{@i0.iid}, ##{@i1.iid}, and #999.", project) subject.analyze("##{@i0.iid}, ##{@i1.iid}, and #999.")
expect(subject.issues_for(project)).to eq([@i0, @i1]) expect(subject.issues).to eq([@i0, @i1])
end end
it 'accesses valid merge requests' do it 'accesses valid merge requests' do
@m0 = create(:merge_request, source_project: project, target_project: project, source_branch: 'aaa') @m0 = create(:merge_request, source_project: project, target_project: project, source_branch: 'aaa')
@m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'bbb') @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'bbb')
subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.", project) subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.")
expect(subject.merge_requests_for(project)).to eq([@m1, @m0]) expect(subject.merge_requests).to eq([@m1, @m0])
end end
it 'accesses valid snippets' do it 'accesses valid snippets' do
...@@ -111,16 +111,15 @@ describe Gitlab::ReferenceExtractor do ...@@ -111,16 +111,15 @@ describe Gitlab::ReferenceExtractor do
@s1 = create(:project_snippet, project: project) @s1 = create(:project_snippet, project: project)
@s2 = create(:project_snippet) @s2 = create(:project_snippet)
subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}", project) subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}")
expect(subject.snippets_for(project)).to eq([@s0, @s1]) expect(subject.snippets).to eq([@s0, @s1])
end end
it 'accesses valid commits' do it 'accesses valid commits' do
commit = project.repository.commit('master') commit = project.repository.commit('master')
subject.analyze("this references commits #{commit.sha[0..6]} and 012345", subject.analyze("this references commits #{commit.sha[0..6]} and 012345")
project) extracted = subject.commits
extracted = subject.commits_for(project)
expect(extracted.size).to eq(1) expect(extracted.size).to eq(1)
expect(extracted[0].sha).to eq(commit.sha) expect(extracted[0].sha).to eq(commit.sha)
expect(extracted[0].message).to eq(commit.message) expect(extracted[0].message).to eq(commit.message)
...@@ -130,28 +129,28 @@ describe Gitlab::ReferenceExtractor do ...@@ -130,28 +129,28 @@ describe Gitlab::ReferenceExtractor do
commit = project.repository.commit('master') commit = project.repository.commit('master')
earlier_commit = project.repository.commit('master~2') earlier_commit = project.repository.commit('master~2')
subject.analyze("this references commits #{earlier_commit.sha[0..6]}...#{commit.sha[0..6]}", subject.analyze("this references commits #{earlier_commit.sha[0..6]}...#{commit.sha[0..6]}")
project) extracted = subject.commit_ranges
extracted = subject.commit_ranges_for(project)
expect(extracted.size).to eq(1) expect(extracted.size).to eq(1)
expect(extracted[0][0].sha).to eq(earlier_commit.sha) expect(extracted[0][0].sha).to eq(earlier_commit.sha)
expect(extracted[0][0].message).to eq(earlier_commit.message) expect(extracted[0][0].message).to eq(earlier_commit.message)
expect(extracted[0][1].sha).to eq(commit.sha) expect(extracted[0][1].sha).to eq(commit.sha)
expect(extracted[0][1].message).to eq(commit.message) expect(extracted[0][1].message).to eq(commit.message)
end end
end
context 'with a project with an underscore' do context 'with a project with an underscore' do
let(:project) { create(:project, path: 'test_project') } let(:other_project) { create(:project, path: 'test_project') }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: other_project) }
before do
other_project.team << [project.creator, :developer]
end
it 'handles project issue references' do it 'handles project issue references' do
subject.analyze("this refers issue #{project.path_with_namespace}##{issue.iid}", subject.analyze("this refers issue #{other_project.path_with_namespace}##{issue.iid}")
project) extracted = subject.issues
extracted = subject.issues_for(project)
expect(extracted.size).to eq(1) expect(extracted.size).to eq(1)
expect(extracted).to eq([issue]) expect(extracted).to eq([issue])
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment