diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-02-23 12:14:14 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-02-23 12:14:14 +0300 |
commit | f4bc6ec92e2af0b6cfd64f9ff0ca683bf62820d1 (patch) | |
tree | 9e34a9a071d0c0c5900c0ba37927de4590fa23f9 /lib | |
parent | 0a8aebcb550b705ec5987c6f905eaf5c5abb1cc1 (diff) | |
parent | 08266ba0a14ec296b51cda6b54d1648985a11adf (diff) |
Merge branch 'bvl-external-auth-port' into 'master'
Port `read_cross_project` ability from EE
See merge request gitlab-org/gitlab-ce!17208
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/helpers.rb | 2 | ||||
-rw-r--r-- | lib/api/settings.rb | 2 | ||||
-rw-r--r-- | lib/banzai/filter/issuable_state_filter.rb | 6 | ||||
-rw-r--r-- | lib/banzai/filter/milestone_reference_filter.rb | 2 | ||||
-rw-r--r-- | lib/banzai/redactor.rb | 21 | ||||
-rw-r--r-- | lib/banzai/reference_parser/issuable_parser.rb | 2 | ||||
-rw-r--r-- | lib/banzai/reference_parser/issue_parser.rb | 25 | ||||
-rw-r--r-- | lib/gitlab/contributions_calendar.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/cross_project_access.rb | 67 | ||||
-rw-r--r-- | lib/gitlab/cross_project_access/check_collection.rb | 47 | ||||
-rw-r--r-- | lib/gitlab/cross_project_access/check_info.rb | 66 | ||||
-rw-r--r-- | lib/gitlab/cross_project_access/class_methods.rb | 48 | ||||
-rw-r--r-- | lib/gitlab/user_access.rb | 2 |
13 files changed, 287 insertions, 9 deletions
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 6134ad2bfc7..e4fca77ab5d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -172,7 +172,7 @@ module API def find_project_snippet(id) finder_params = { project: user_project } - SnippetsFinder.new(current_user, finder_params).execute.find(id) + SnippetsFinder.new(current_user, finder_params).find(id) end def find_merge_request_with_access(iid, access_level = :read_merge_request) diff --git a/lib/api/settings.rb b/lib/api/settings.rb index cee4d309816..152df23a327 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -147,7 +147,7 @@ module API attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled) end - if current_settings.update_attributes(attrs) + if ApplicationSettings::UpdateService.new(current_settings, current_user, attrs).execute present current_settings, with: Entities::ApplicationSetting else render_validation_error!(current_settings) diff --git a/lib/banzai/filter/issuable_state_filter.rb b/lib/banzai/filter/issuable_state_filter.rb index 327ea9449a1..77299abe324 100644 --- a/lib/banzai/filter/issuable_state_filter.rb +++ b/lib/banzai/filter/issuable_state_filter.rb @@ -15,6 +15,8 @@ module Banzai issuables = extractor.extract([doc]) issuables.each do |node, issuable| + next if !can_read_cross_project? && issuable.project != project + if VISIBLE_STATES.include?(issuable.state) && node.inner_html == issuable.reference_link_text(project) node.content += " (#{issuable.state})" end @@ -25,6 +27,10 @@ module Banzai private + def can_read_cross_project? + Ability.allowed?(current_user, :read_cross_project) + end + def current_user context[:current_user] end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 2a6b0964ac5..8ec696ce5fc 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -64,7 +64,7 @@ module Banzai finder_params[:group_ids] = [project.group.id] end - MilestonesFinder.new(finder_params).execute.find_by(params) + MilestonesFinder.new(finder_params).find_by(params) end def url_for_object(milestone, project) diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb index de3ebe72720..827df7c08ae 100644 --- a/lib/banzai/redactor.rb +++ b/lib/banzai/redactor.rb @@ -19,8 +19,9 @@ module Banzai # # Returns the documents passed as the first argument. def redact(documents) - all_document_nodes = document_nodes(documents) + redact_cross_project_references(documents) unless can_read_cross_project? + all_document_nodes = document_nodes(documents) redact_document_nodes(all_document_nodes) end @@ -51,6 +52,18 @@ module Banzai metadata end + def redact_cross_project_references(documents) + extractor = Banzai::IssuableExtractor.new(project, user) + issuables = extractor.extract(documents) + + issuables.each do |node, issuable| + next if issuable.project == project + + node['class'] = node['class'].gsub('has-tooltip', '') + node['title'] = nil + end + end + # Returns the nodes visible to the current user. # # nodes - The input nodes to check. @@ -78,5 +91,11 @@ module Banzai { document: document, nodes: Querying.css(document, 'a.gfm[data-reference-type]') } end end + + private + + def can_read_cross_project? + Ability.allowed?(user, :read_cross_project) + end end end diff --git a/lib/banzai/reference_parser/issuable_parser.rb b/lib/banzai/reference_parser/issuable_parser.rb index 3953867eb83..fad127d7e5b 100644 --- a/lib/banzai/reference_parser/issuable_parser.rb +++ b/lib/banzai/reference_parser/issuable_parser.rb @@ -18,7 +18,7 @@ module Banzai end def can_read_reference?(user, issuable) - can?(user, "read_#{issuable.class.to_s.underscore}".to_sym, issuable) + can?(user, "read_#{issuable.class.to_s.underscore}_iid".to_sym, issuable) end end end diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb index 38d4e3f3e44..230827129b6 100644 --- a/lib/banzai/reference_parser/issue_parser.rb +++ b/lib/banzai/reference_parser/issue_parser.rb @@ -5,12 +5,31 @@ module Banzai def nodes_visible_to_user(user, nodes) issues = records_for_nodes(nodes) + issues_to_check = issues.values - readable_issues = Ability - .issues_readable_by_user(issues.values, user).to_set + unless can?(user, :read_cross_project) + issues_to_check, cross_project_issues = issues_to_check.partition do |issue| + issue.project == project + end + end + + readable_issues = Ability.issues_readable_by_user(issues_to_check, user).to_set nodes.select do |node| - readable_issues.include?(issues[node]) + issue_in_node = issues[node] + + # We check the inclusion of readable issues first because it's faster. + # + # But we need to fall back to `read_issue_iid` if the user cannot read + # cross project, since it might be possible the user can see the IID + # but not the issue. + if readable_issues.include?(issue_in_node) + true + elsif cross_project_issues&.include?(issue_in_node) + can_read_reference?(user, issue_in_node) + else + false + end end end diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index 0735243e021..9576d5a3fd8 100644 --- a/lib/gitlab/contributions_calendar.rb +++ b/lib/gitlab/contributions_calendar.rb @@ -34,6 +34,8 @@ module Gitlab end def events_by_date(date) + return Event.none unless can_read_cross_project? + events = Event.contributions.where(author_id: contributor.id) .where(created_at: date.beginning_of_day..date.end_of_day) .where(project_id: projects) @@ -53,6 +55,10 @@ module Gitlab private + def can_read_cross_project? + Ability.allowed?(current_user, :read_cross_project) + end + def event_counts(date_from, feature) t = Event.arel_table diff --git a/lib/gitlab/cross_project_access.rb b/lib/gitlab/cross_project_access.rb new file mode 100644 index 00000000000..6eaed51b64c --- /dev/null +++ b/lib/gitlab/cross_project_access.rb @@ -0,0 +1,67 @@ +module Gitlab + class CrossProjectAccess + class << self + delegate :add_check, :find_check, :checks, + to: :instance + end + + def self.instance + @instance ||= new + end + + attr_reader :checks + + def initialize + @checks = {} + end + + def add_check( + klass, + actions: {}, + positive_condition: nil, + negative_condition: nil, + skip: false) + + new_check = CheckInfo.new(actions, + positive_condition, + negative_condition, + skip + ) + + @checks[klass] ||= Gitlab::CrossProjectAccess::CheckCollection.new + @checks[klass].add_check(new_check) + recalculate_checks_for_class(klass) + + @checks[klass] + end + + def find_check(object) + @cached_checks ||= Hash.new do |cache, new_class| + parent_classes = @checks.keys.select { |existing_class| new_class <= existing_class } + closest_class = closest_parent(parent_classes, new_class) + cache[new_class] = @checks[closest_class] + end + + @cached_checks[object.class] + end + + private + + def recalculate_checks_for_class(klass) + new_collection = @checks[klass] + + @checks.each do |existing_class, existing_check_collection| + if existing_class < klass + existing_check_collection.add_collection(new_collection) + elsif klass < existing_class + new_collection.add_collection(existing_check_collection) + end + end + end + + def closest_parent(classes, subject) + relevant_ancestors = subject.ancestors & classes + relevant_ancestors.first + end + end +end diff --git a/lib/gitlab/cross_project_access/check_collection.rb b/lib/gitlab/cross_project_access/check_collection.rb new file mode 100644 index 00000000000..88376232065 --- /dev/null +++ b/lib/gitlab/cross_project_access/check_collection.rb @@ -0,0 +1,47 @@ +module Gitlab + class CrossProjectAccess + class CheckCollection + attr_reader :checks + + def initialize + @checks = [] + end + + def add_collection(collection) + @checks |= collection.checks + end + + def add_check(check) + @checks << check + end + + def should_run?(object) + skips, runs = arranged_checks + + # If one rule tells us to skip, we skip the cross project check + return false if skips.any? { |check| check.should_skip?(object) } + + # If the rule isn't skipped, we run it if any of the checks says we + # should run + runs.any? { |check| check.should_run?(object) } + end + + def arranged_checks + return [@skips, @runs] if @skips && @runs + + @skips = [] + @runs = [] + + @checks.each do |check| + if check.skip + @skips << check + else + @runs << check + end + end + + [@skips, @runs] + end + end + end +end diff --git a/lib/gitlab/cross_project_access/check_info.rb b/lib/gitlab/cross_project_access/check_info.rb new file mode 100644 index 00000000000..e8a845c7f1e --- /dev/null +++ b/lib/gitlab/cross_project_access/check_info.rb @@ -0,0 +1,66 @@ +module Gitlab + class CrossProjectAccess + class CheckInfo + attr_accessor :actions, :positive_condition, :negative_condition, :skip + + def initialize(actions, positive_condition, negative_condition, skip) + @actions = actions + @positive_condition = positive_condition + @negative_condition = negative_condition + @skip = skip + end + + def should_skip?(object) + return !should_run?(object) unless @skip + + skip_for_action = @actions[current_action(object)] + skip_for_action = false if @actions[current_action(object)].nil? + + # We need to do the opposite of what was defined in the following cases: + # - skip_cross_project_access_check index: true, if: -> { false } + # - skip_cross_project_access_check index: true, unless: -> { true } + if positive_condition_is_false?(object) + skip_for_action = !skip_for_action + end + + if negative_condition_is_true?(object) + skip_for_action = !skip_for_action + end + + skip_for_action + end + + def should_run?(object) + return !should_skip?(object) if @skip + + run_for_action = @actions[current_action(object)] + run_for_action = true if @actions[current_action(object)].nil? + + # We need to do the opposite of what was defined in the following cases: + # - requires_cross_project_access index: true, if: -> { false } + # - requires_cross_project_access index: true, unless: -> { true } + if positive_condition_is_false?(object) + run_for_action = !run_for_action + end + + if negative_condition_is_true?(object) + run_for_action = !run_for_action + end + + run_for_action + end + + def positive_condition_is_false?(object) + @positive_condition && !object.instance_exec(&@positive_condition) + end + + def negative_condition_is_true?(object) + @negative_condition && object.instance_exec(&@negative_condition) + end + + def current_action(object) + object.respond_to?(:action_name) ? object.action_name.to_sym : nil + end + end + end +end diff --git a/lib/gitlab/cross_project_access/class_methods.rb b/lib/gitlab/cross_project_access/class_methods.rb new file mode 100644 index 00000000000..90eac94800c --- /dev/null +++ b/lib/gitlab/cross_project_access/class_methods.rb @@ -0,0 +1,48 @@ +module Gitlab + class CrossProjectAccess + module ClassMethods + def requires_cross_project_access(*args) + positive_condition, negative_condition, actions = extract_params(args) + + Gitlab::CrossProjectAccess.add_check( + self, + actions: actions, + positive_condition: positive_condition, + negative_condition: negative_condition + ) + end + + def skip_cross_project_access_check(*args) + positive_condition, negative_condition, actions = extract_params(args) + + Gitlab::CrossProjectAccess.add_check( + self, + actions: actions, + positive_condition: positive_condition, + negative_condition: negative_condition, + skip: true + ) + end + + private + + def extract_params(args) + actions = {} + positive_condition = nil + negative_condition = nil + + args.each do |argument| + if argument.is_a?(Hash) + positive_condition = argument.delete(:if) + negative_condition = argument.delete(:unless) + actions.merge!(argument) + else + actions[argument] = true + end + end + + [positive_condition, negative_condition, actions] + end + end + end +end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 15eb1c41213..ff4dc29efea 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -65,7 +65,7 @@ module Gitlab return false unless can_access_git? if protected?(ProtectedBranch, project, ref) - return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) + return true if project.user_can_push_to_empty_repo?(user) protected_branch_accessible_to?(ref, action: :push) else |