From ca58e369c9f2a72402cfcf4d86d29c115b1b909c Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 27 Mar 2015 11:38:22 +0100 Subject: [PATCH] Only allow user to reference objects they have access to. --- app/helpers/gitlab_markdown_helper.rb | 2 +- lib/gitlab/markdown.rb | 38 ++++++++++++++------------- lib/gitlab/reference_extractor.rb | 14 +++++++--- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 7ca3f058636..a4157d62533 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -19,7 +19,7 @@ module GitlabMarkdownHelper escape_once(body) end - gfm_body = gfm(escaped_body, @project, html_options) + gfm_body = gfm(escaped_body, @project, current_user, html_options) gfm_body.gsub!(%r{.*?}m) do |match| "#{match}#{link_to("", url, html_options)[0..-5]}" # "".length +1 diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 1dbea48ac14..7a3c8823e4d 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -39,7 +39,7 @@ module Gitlab # text - the source text # project - the project # html_options - extra options for the reference links as given to link_to - def gfm(text, project = @project, html_options = {}) + def gfm(text, project = @project, user = current_user, html_options = {}) gfm_with_options(text, {}, project, html_options) end @@ -51,7 +51,7 @@ module Gitlab # - reference_only_path - Use relative path for reference links # project - the project # html_options - extra options for the reference links as given to link_to - def gfm_with_options(text, options = {}, project = @project, html_options = {}) + def gfm_with_options(text, options = {}, project = @project, user = current_user, html_options = {}) return text if text.nil? # Duplicate the string so we don't alter the original, then call to_str @@ -78,7 +78,7 @@ module Gitlab # TODO: add popups with additional information - text = parse(text, project) + text = parse(text, project, user) # Insert pre block extractions text.gsub!(/\{gfm-extraction-(\h{32})\}/) do @@ -155,8 +155,8 @@ module Gitlab # text - Text to parse # # Returns parsed text - def parse(text, project = @project) - parse_references(text, project) if project + def parse(text, project = @project, user = current_user) + parse_references(text, project, user) if project text end @@ -182,7 +182,7 @@ module Gitlab TYPES = [:user, :issue, :label, :merge_request, :snippet, :commit, :commit_range].freeze - def parse_references(text, project = @project) + def parse_references(text, project = @project, user = current_user) # parse reference links text.gsub!(REFERENCE_PATTERN) do |match| type = TYPES.select{|t| !$~[t].nil?}.first @@ -192,11 +192,12 @@ module Gitlab project_path = $LAST_MATCH_INFO[:project] if project_path actual_project = ::Project.find_with_namespace(project_path) + actual_project ||= nil unless can?(user, :read_project, actual_project) project_prefix = project_path end parse_result($LAST_MATCH_INFO, type, - actual_project, project_prefix) || match + actual_project, user, project_prefix) || match end end @@ -204,7 +205,7 @@ module Gitlab # link. Returns nil if +type+ is nil, if the match string is an HTML # entity, if the reference is invalid, or if the matched text includes an # invalid project path. - def parse_result(match_info, type, project, project_prefix) + def parse_result(match_info, type, project, user, project_prefix) prefix = match_info[:prefix] suffix = match_info[:suffix] @@ -212,7 +213,7 @@ module Gitlab return nil if project.nil? && !project_prefix.nil? identifier = match_info[type] - ref_link = reference_link(type, identifier, project, project_prefix) + ref_link = reference_link(type, identifier, project, user, project_prefix) if ref_link "#{prefix}#{ref_link}#{suffix}" @@ -233,11 +234,11 @@ module Gitlab # identifier - Object identifier (Issue ID, SHA hash, etc.) # # Returns string rendered by the processing method - def reference_link(type, identifier, project = @project, prefix_text = nil) + def reference_link(type, identifier, project = @project, user = current_user, prefix_text = nil) send("reference_#{type}", identifier, project, prefix_text) end - def reference_user(identifier, project = @project, _ = nil) + def reference_user(identifier, project = @project, user = current_user, _ = nil) link_options = html_options.merge( class: "gfm gfm-project_member #{html_options[:class]}" ) @@ -251,6 +252,7 @@ module Gitlab elsif namespace = Namespace.find_by(path: identifier) url = if namespace.is_a?(Group) + return nil unless can?(user, :read_group, namespace) group_url(identifier, only_path: options[:reference_only_path]) else user_url(identifier, only_path: options[:reference_only_path]) @@ -260,7 +262,7 @@ module Gitlab end end - def reference_label(identifier, project = @project, _ = nil) + def reference_label(identifier, project = @project, user = current_user, _ = nil) if label = project.labels.find_by(id: identifier) link_options = html_options.merge( class: "gfm gfm-label #{html_options[:class]}" @@ -273,7 +275,7 @@ module Gitlab end end - def reference_issue(identifier, project = @project, prefix_text = nil) + def reference_issue(identifier, project = @project, user = current_user, prefix_text = nil) if project.default_issues_tracker? if project.issue_exists? identifier url = url_for_issue(identifier, project, only_path: options[:reference_only_path]) @@ -293,7 +295,7 @@ module Gitlab end end - def reference_merge_request(identifier, project = @project, prefix_text = nil) + def reference_merge_request(identifier, project = @project, user = current_user, prefix_text = nil) if merge_request = project.merge_requests.find_by(iid: identifier) link_options = html_options.merge( title: "Merge Request: #{merge_request.title}", @@ -306,7 +308,7 @@ module Gitlab end end - def reference_snippet(identifier, project = @project, _ = nil) + def reference_snippet(identifier, project = @project, user = current_user, _ = nil) if snippet = project.snippets.find_by(id: identifier) link_options = html_options.merge( title: "Snippet: #{snippet.title}", @@ -321,7 +323,7 @@ module Gitlab end end - def reference_commit(identifier, project = @project, prefix_text = nil) + def reference_commit(identifier, project = @project, user = current_user, prefix_text = nil) if project.valid_repo? && commit = project.repository.commit(identifier) link_options = html_options.merge( title: commit.link_title, @@ -337,7 +339,7 @@ module Gitlab end end - def reference_commit_range(identifier, project = @project, prefix_text = nil) + def reference_commit_range(identifier, project = @project, user = current_user, prefix_text = nil) from_id, to_id = identifier.split(/\.{2,3}/, 2) inclusive = identifier !~ /\.{3}/ @@ -363,7 +365,7 @@ module Gitlab end end - def reference_external_issue(identifier, project = @project, prefix_text = nil) + def reference_external_issue(identifier, project = @project, user = current_user, prefix_text = nil) url = url_for_issue(identifier, project, only_path: options[:reference_only_path]) title = project.external_issue_tracker.title diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 90e04818719..3bf6f14a7bc 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -1,16 +1,24 @@ module Gitlab # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor - attr_accessor :project, :references + attr_accessor :project, :current_user, :references include Markdown - def initialize(project) + def initialize(project, current_user = nil) @project = project + @current_user = user @references = Hash.new { [] } end + def can?(user, action, subject) + # When extracting references, no user means access to everything. + return true if user.nil? + + Ability.abilities.allowed?(user, action, subject) + end + def analyze(text) text = text.dup @@ -79,7 +87,7 @@ module Gitlab private - def reference_link(type, identifier, project, _) + def reference_link(type, identifier, project, user, _) references[type] << { project: project, id: identifier } end