diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2017-12-11 15:21:06 +0100 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-02-22 17:11:36 +0100 |
commit | 148816cd67a314f17e79c107270cc708501bdd39 (patch) | |
tree | eba07d109322392bb5862b715adc066a0ebbdf95 /app/finders | |
parent | b5306075c21f5546d1447052558da6227629c15e (diff) | |
download | gitlab-ce-148816cd67a314f17e79c107270cc708501bdd39.tar.gz |
Port `read_cross_project` ability from EE
Diffstat (limited to 'app/finders')
-rw-r--r-- | app/finders/concerns/finder_methods.rb | 51 | ||||
-rw-r--r-- | app/finders/concerns/finder_with_cross_project_access.rb | 70 | ||||
-rw-r--r-- | app/finders/events_finder.rb | 4 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 16 | ||||
-rw-r--r-- | app/finders/labels_finder.rb | 4 | ||||
-rw-r--r-- | app/finders/merge_request_target_project_finder.rb | 2 | ||||
-rw-r--r-- | app/finders/milestones_finder.rb | 2 | ||||
-rw-r--r-- | app/finders/snippets_finder.rb | 10 | ||||
-rw-r--r-- | app/finders/todos_finder.rb | 5 | ||||
-rw-r--r-- | app/finders/user_recent_events_finder.rb | 33 |
10 files changed, 183 insertions, 14 deletions
diff --git a/app/finders/concerns/finder_methods.rb b/app/finders/concerns/finder_methods.rb new file mode 100644 index 00000000000..2e905fa5750 --- /dev/null +++ b/app/finders/concerns/finder_methods.rb @@ -0,0 +1,51 @@ +module FinderMethods + def find_by!(*args) + raise_not_found_unless_authorized execute.find_by!(*args) + end + + def find_by(*args) + if_authorized execute.find_by(*args) + end + + def find(*args) + raise_not_found_unless_authorized model.find(*args) + end + + private + + def raise_not_found_unless_authorized(result) + result = if_authorized(result) + + raise ActiveRecord::RecordNotFound.new("Couldn't find #{model}") unless result + + result + end + + def if_authorized(result) + # Return the result if the finder does not perform authorization checks. + # this is currently the case in the `MilestoneFinder` + return result unless respond_to?(:current_user) + + if can_read_object?(result) + result + else + nil + end + end + + def can_read_object?(object) + # When there's no policy, we'll allow the read, this is for example the case + # for Todos + return true unless DeclarativePolicy.has_policy?(object) + + model_name = object&.model_name || model.model_name + + Ability.allowed?(current_user, :"read_#{model_name.singular}", object) + end + + # This fetches the model from the `ActiveRecord::Relation` but does not + # actually execute the query. + def model + execute.model + end +end diff --git a/app/finders/concerns/finder_with_cross_project_access.rb b/app/finders/concerns/finder_with_cross_project_access.rb new file mode 100644 index 00000000000..92bf98d7cd2 --- /dev/null +++ b/app/finders/concerns/finder_with_cross_project_access.rb @@ -0,0 +1,70 @@ +# Module to prepend into finders to specify wether or not the finder requires +# cross project access +# +# This module depends on the finder implementing the following methods: +# +# - `#execute` should return an `ActiveRecord::Relation` +# - `#current_user` the user that requires access (or nil) +module FinderWithCrossProjectAccess + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + prepended do + extend Gitlab::CrossProjectAccess::ClassMethods + end + + override :execute + def execute(*args) + check = Gitlab::CrossProjectAccess.find_check(self) + original = super + + return original unless check + return original if should_skip_cross_project_check || can_read_cross_project? + + if check.should_run?(self) + original.model.none + else + original + end + end + + # We can skip the cross project check for finding indivitual records. + # this would be handled by the `can?(:read_*, result)` call in `FinderMethods` + # itself. + override :find_by! + def find_by!(*args) + skip_cross_project_check { super } + end + + override :find_by + def find_by(*args) + skip_cross_project_check { super } + end + + override :find + def find(*args) + skip_cross_project_check { super } + end + + private + + attr_accessor :should_skip_cross_project_check + + def skip_cross_project_check + self.should_skip_cross_project_check = true + + yield + ensure + # The find could raise an `ActiveRecord::RecordNotFound`, after which we + # still want to re-enable the check. + self.should_skip_cross_project_check = false + end + + def can_read_cross_project? + Ability.allowed?(current_user, :read_cross_project) + end + + def can_read_project?(project) + Ability.allowed?(current_user, :read_project, project) + end +end diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb index 46ecbaba73a..8676925a540 100644 --- a/app/finders/events_finder.rb +++ b/app/finders/events_finder.rb @@ -1,6 +1,10 @@ class EventsFinder + prepend FinderMethods + prepend FinderWithCrossProjectAccess attr_reader :source, :params, :current_user + requires_cross_project_access unless: -> { source.is_a?(Project) } + # Used to filter Events # # Arguments: diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 384a336e2bb..9dd6634b38f 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -21,8 +21,12 @@ # my_reaction_emoji: string # class IssuableFinder + prepend FinderWithCrossProjectAccess + include FinderMethods include CreatedAtFilter + requires_cross_project_access unless: -> { project? } + NONE = '0'.freeze attr_accessor :current_user, :params @@ -87,14 +91,6 @@ class IssuableFinder by_my_reaction_emoji(items) end - def find(*params) - execute.find(*params) - end - - def find_by(*params) - execute.find_by(*params) - end - def row_count Gitlab::IssuablesCountForState.new(self).for_state_or_opened(params[:state]) end @@ -124,10 +120,6 @@ class IssuableFinder counts end - def find_by!(*params) - execute.find_by!(*params) - end - def group return @group if defined?(@group) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 1427cdaa382..f013e177c5b 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -1,6 +1,10 @@ class LabelsFinder < UnionFinder + prepend FinderWithCrossProjectAccess + include FinderMethods include Gitlab::Utils::StrongMemoize + requires_cross_project_access unless: -> { project? } + def initialize(current_user, params = {}) @current_user = current_user @params = params diff --git a/app/finders/merge_request_target_project_finder.rb b/app/finders/merge_request_target_project_finder.rb index 189eb3847eb..f358938344e 100644 --- a/app/finders/merge_request_target_project_finder.rb +++ b/app/finders/merge_request_target_project_finder.rb @@ -1,4 +1,6 @@ class MergeRequestTargetProjectFinder + include FinderMethods + attr_reader :current_user, :source_project def initialize(current_user: nil, source_project:) diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb index b4605fca193..f5d2b9f253a 100644 --- a/app/finders/milestones_finder.rb +++ b/app/finders/milestones_finder.rb @@ -8,6 +8,8 @@ # state - filters by state. class MilestonesFinder + include FinderMethods + attr_reader :params, :project_ids, :group_ids def initialize(params = {}) diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index ec61fe1892e..a73c573736e 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -13,7 +13,9 @@ # params are optional class SnippetsFinder < UnionFinder include Gitlab::Allowable - attr_accessor :current_user, :params, :project + include FinderMethods + + attr_accessor :current_user, :project, :params def initialize(current_user, params = {}) @current_user = current_user @@ -52,10 +54,14 @@ class SnippetsFinder < UnionFinder end def authorized_snippets - Snippet.where(feature_available_projects.or(not_project_related)).public_or_visible_to_user(current_user) + Snippet.where(feature_available_projects.or(not_project_related)) + .public_or_visible_to_user(current_user) end def feature_available_projects + # Don't return any project related snippets if the user cannot read cross project + return table[:id].eq(nil) unless Ability.allowed?(current_user, :read_cross_project) + projects = Project.public_or_visible_to_user(current_user, use_where_in: false) do |part| part.with_feature_available_for_user(:snippets, current_user) end.select(:id) diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 3502bf08971..edb17843002 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -13,6 +13,11 @@ # class TodosFinder + prepend FinderWithCrossProjectAccess + include FinderMethods + + requires_cross_project_access unless: -> { project? } + NONE = '0'.freeze attr_accessor :current_user, :params diff --git a/app/finders/user_recent_events_finder.rb b/app/finders/user_recent_events_finder.rb new file mode 100644 index 00000000000..6f7f7c30d92 --- /dev/null +++ b/app/finders/user_recent_events_finder.rb @@ -0,0 +1,33 @@ +# Get user activity feed for projects common for a user and a logged in user +# +# - current_user: The user viewing the events +# - user: The user for which to load the events +# - params: +# - offset: The page of events to return +class UserRecentEventsFinder + prepend FinderWithCrossProjectAccess + include FinderMethods + + requires_cross_project_access + + attr_reader :current_user, :target_user, :params + + def initialize(current_user, target_user, params = {}) + @current_user = current_user + @target_user = target_user + @params = params + end + + def execute + target_user + .recent_events + .merge(projects_for_current_user) + .references(:project) + .with_associations + .limit_recent(20, params[:offset]) + end + + def projects_for_current_user + ProjectsFinder.new(current_user: current_user).execute + end +end |