Commit 148816cd authored by Bob Van Landuyt's avatar Bob Van Landuyt 👶

Port `read_cross_project` ability from EE

parent b5306075
...@@ -126,10 +126,15 @@ class ApplicationController < ActionController::Base ...@@ -126,10 +126,15 @@ class ApplicationController < ActionController::Base
Ability.allowed?(object, action, subject) Ability.allowed?(object, action, subject)
end end
def access_denied! def access_denied!(message = nil)
respond_to do |format| respond_to do |format|
format.json { head :not_found } format.any { head :not_found }
format.any { render "errors/access_denied", layout: "errors", status: 404 } format.html do
render "errors/access_denied",
layout: "errors",
status: 404,
locals: { message: message }
end
end end
end end
......
...@@ -55,7 +55,7 @@ module Boards ...@@ -55,7 +55,7 @@ module Boards
end end
def issue def issue
@issue ||= issues_finder.execute.find(params[:id]) @issue ||= issues_finder.find(params[:id])
end end
def filter_params def filter_params
......
module ControllerWithCrossProjectAccessCheck
extend ActiveSupport::Concern
included do
extend Gitlab::CrossProjectAccess::ClassMethods
before_action :cross_project_check
end
def cross_project_check
if Gitlab::CrossProjectAccess.find_check(self)&.should_run?(self)
authorize_cross_project_page!
end
end
def authorize_cross_project_page!
return if can?(current_user, :read_cross_project)
rejection_message = _(
"This page is unavailable because you are not allowed to read information "\
"across multiple projects."
)
access_denied!(rejection_message)
end
end
...@@ -3,16 +3,20 @@ module RoutableActions ...@@ -3,16 +3,20 @@ module RoutableActions
def find_routable!(routable_klass, requested_full_path, extra_authorization_proc: nil) def find_routable!(routable_klass, requested_full_path, extra_authorization_proc: nil)
routable = routable_klass.find_by_full_path(requested_full_path, follow_redirects: request.get?) routable = routable_klass.find_by_full_path(requested_full_path, follow_redirects: request.get?)
if routable_authorized?(routable, extra_authorization_proc) if routable_authorized?(routable, extra_authorization_proc)
ensure_canonical_path(routable, requested_full_path) ensure_canonical_path(routable, requested_full_path)
routable routable
else else
route_not_found handle_not_found_or_authorized(routable)
nil nil
end end
end end
# This is overridden in gitlab-ee.
def handle_not_found_or_authorized(_routable)
route_not_found
end
def routable_authorized?(routable, extra_authorization_proc) def routable_authorized?(routable, extra_authorization_proc)
action = :"read_#{routable.class.to_s.underscore}" action = :"read_#{routable.class.to_s.underscore}"
return false unless can?(current_user, action, routable) return false unless can?(current_user, action, routable)
......
class Dashboard::ApplicationController < ApplicationController class Dashboard::ApplicationController < ApplicationController
include ControllerWithCrossProjectAccessCheck
layout 'dashboard' layout 'dashboard'
requires_cross_project_access
private private
def projects def projects
......
class Dashboard::GroupsController < Dashboard::ApplicationController class Dashboard::GroupsController < Dashboard::ApplicationController
include GroupTree include GroupTree
skip_cross_project_access_check :index
def index def index
groups = GroupsFinder.new(current_user, all_available: false).execute groups = GroupsFinder.new(current_user, all_available: false).execute
render_group_tree(groups) render_group_tree(groups)
......
...@@ -4,6 +4,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -4,6 +4,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
before_action :set_non_archived_param before_action :set_non_archived_param
before_action :default_sorting before_action :default_sorting
skip_cross_project_access_check :index, :starred
def index def index
@projects = load_projects(params.merge(non_public: true)).page(params[:page]) @projects = load_projects(params.merge(non_public: true)).page(params[:page])
......
class Dashboard::SnippetsController < Dashboard::ApplicationController class Dashboard::SnippetsController < Dashboard::ApplicationController
skip_cross_project_access_check :index
def index def index
@snippets = SnippetsFinder.new( @snippets = SnippetsFinder.new(
current_user, current_user,
......
class Groups::ApplicationController < ApplicationController class Groups::ApplicationController < ApplicationController
include RoutableActions include RoutableActions
include ControllerWithCrossProjectAccessCheck
layout 'group' layout 'group'
skip_before_action :authenticate_user! skip_before_action :authenticate_user!
before_action :group before_action :group
requires_cross_project_access
private private
......
class Groups::AvatarsController < Groups::ApplicationController class Groups::AvatarsController < Groups::ApplicationController
before_action :authorize_admin_group! before_action :authorize_admin_group!
skip_cross_project_access_check :destroy
def destroy def destroy
@group.remove_avatar! @group.remove_avatar!
@group.save @group.save
......
module Groups module Groups
class ChildrenController < Groups::ApplicationController class ChildrenController < Groups::ApplicationController
before_action :group before_action :group
skip_cross_project_access_check :index
def index def index
parent = if params[:parent_id].present? parent = if params[:parent_id].present?
......
...@@ -6,6 +6,10 @@ class Groups::GroupMembersController < Groups::ApplicationController ...@@ -6,6 +6,10 @@ class Groups::GroupMembersController < Groups::ApplicationController
# Authorize # Authorize
before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access] before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access]
skip_cross_project_access_check :index, :create, :update, :destroy, :request_access,
:approve_access_request, :leave, :resend_invite,
:override
def index def index
@sort = params[:sort].presence || sort_value_name @sort = params[:sort].presence || sort_value_name
@project = @group.projects.find(params[:project_id]) if params[:project_id] @project = @group.projects.find(params[:project_id]) if params[:project_id]
......
module Groups module Groups
module Settings module Settings
class CiCdController < Groups::ApplicationController class CiCdController < Groups::ApplicationController
skip_cross_project_access_check :show
before_action :authorize_admin_pipeline! before_action :authorize_admin_pipeline!
def show def show
......
...@@ -2,6 +2,8 @@ module Groups ...@@ -2,6 +2,8 @@ module Groups
class VariablesController < Groups::ApplicationController class VariablesController < Groups::ApplicationController
before_action :authorize_admin_build! before_action :authorize_admin_build!
skip_cross_project_access_check :show, :update
def show def show
respond_to do |format| respond_to do |format|
format.json do format.json do
......
...@@ -19,6 +19,12 @@ class GroupsController < Groups::ApplicationController ...@@ -19,6 +19,12 @@ class GroupsController < Groups::ApplicationController
before_action :user_actions, only: [:show, :subgroups] before_action :user_actions, only: [:show, :subgroups]
skip_cross_project_access_check :index, :new, :create, :edit, :update,
:destroy, :projects
# When loading show as an atom feed, we render events that could leak cross
# project information
skip_cross_project_access_check :show, if: -> { request.format.html? }
layout :determine_layout layout :determine_layout
def index def index
......
class Oauth::ApplicationsController < Doorkeeper::ApplicationsController class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
include Gitlab::GonHelper include Gitlab::GonHelper
include Gitlab::Allowable
include PageLayoutHelper include PageLayoutHelper
include OauthApplications include OauthApplications
...@@ -8,6 +9,8 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController ...@@ -8,6 +9,8 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
before_action :add_gon_variables before_action :add_gon_variables
before_action :load_scopes, only: [:index, :create, :edit] before_action :load_scopes, only: [:index, :create, :edit]
helper_method :can?
layout 'profile' layout 'profile'
def index def index
......
...@@ -34,9 +34,9 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController ...@@ -34,9 +34,9 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController
def target def target
case params[:type]&.downcase case params[:type]&.downcase
when 'issue' when 'issue'
IssuesFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:type_id]) IssuesFinder.new(current_user, project_id: @project.id).find_by(iid: params[:type_id])
when 'mergerequest' when 'mergerequest'
MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:type_id]) MergeRequestsFinder.new(current_user, project_id: @project.id).find_by(iid: params[:type_id])
when 'commit' when 'commit'
@project.commit(params[:type_id]) @project.commit(params[:type_id])
end end
......
...@@ -133,7 +133,7 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -133,7 +133,7 @@ class Projects::BlobController < Projects::ApplicationController
end end
def after_edit_path def after_edit_path
from_merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:from_merge_request_iid]) from_merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).find_by(iid: params[:from_merge_request_iid])
if from_merge_request && @branch_name == @ref if from_merge_request && @branch_name == @ref
diffs_project_merge_request_path(from_merge_request.target_project, from_merge_request) + diffs_project_merge_request_path(from_merge_request.target_project, from_merge_request) +
"##{hexdigest(@path)}" "##{hexdigest(@path)}"
......
...@@ -75,7 +75,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap ...@@ -75,7 +75,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
def branch_to def branch_to
@target_project = selected_target_project @target_project = selected_target_project
if params[:ref].present? if @target_project && params[:ref].present?
@ref = params[:ref] @ref = params[:ref]
@commit = @target_project.commit(Gitlab::Git::BRANCH_REF_PREFIX + @ref) @commit = @target_project.commit(Gitlab::Git::BRANCH_REF_PREFIX + @ref)
end end
...@@ -85,7 +85,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap ...@@ -85,7 +85,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
def update_branches def update_branches
@target_project = selected_target_project @target_project = selected_target_project
@target_branches = @target_project.repository.branch_names @target_branches = @target_project ? @target_project.repository.branch_names : []
render layout: false render layout: false
end end
...@@ -121,7 +121,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap ...@@ -121,7 +121,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
@project @project
elsif params[:target_project_id].present? elsif params[:target_project_id].present?
MergeRequestTargetProjectFinder.new(current_user: current_user, source_project: @project) MergeRequestTargetProjectFinder.new(current_user: current_user, source_project: @project)
.execute.find(params[:target_project_id]) .find_by(id: params[:target_project_id])
else else
@project.forked_from_project @project.forked_from_project
end end
......
class SearchController < ApplicationController class SearchController < ApplicationController
skip_before_action :authenticate_user! include ControllerWithCrossProjectAccessCheck
include SearchHelper include SearchHelper
include RendersCommits include RendersCommits
skip_before_action :authenticate_user!
requires_cross_project_access if: -> do
search_term_present = params[:search].present? || params[:term].present?
search_term_present && !params[:project_id].present?
end
layout 'search' layout 'search'
def show def show
......
class UsersController < ApplicationController class UsersController < ApplicationController
include RoutableActions include RoutableActions
include RendersMemberAccess include RendersMemberAccess
include ControllerWithCrossProjectAccessCheck
requires_cross_project_access show: false,
groups: false,
projects: false,
contributed: false,
snippets: true,
calendar: false,
calendar_activities: true
skip_before_action :authenticate_user! skip_before_action :authenticate_user!
before_action :user, except: [:exists] before_action :user, except: [:exists]
...@@ -103,12 +112,7 @@ class UsersController < ApplicationController ...@@ -103,12 +112,7 @@ class UsersController < ApplicationController
end end
def load_events def load_events
# Get user activity feed for projects common for both users @events = UserRecentEventsFinder.new(current_user, user, params).execute
@events = user.recent_events
.merge(projects_for_current_user)
.references(:project)
.with_associations
.limit_recent(20, params[:offset])
Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?)
end end
...@@ -141,10 +145,6 @@ class UsersController < ApplicationController ...@@ -141,10 +145,6 @@ class UsersController < ApplicationController
).execute.page(params[:page]) ).execute.page(params[:page])
end end
def projects_for_current_user
ProjectsFinder.new(current_user: current_user).execute
end
def build_canonical_path(user) def build_canonical_path(user)
url_for(params.merge(username: user.to_param)) url_for(params.merge(username: user.to_param))
end end
......
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
# 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
class EventsFinder class EventsFinder
prepend FinderMethods
prepend FinderWithCrossProjectAccess
attr_reader :source, :params, :current_user attr_reader :source, :params, :current_user
requires_cross_project_access unless: -> { source.is_a?(Project) }
# Used to filter Events # Used to filter Events
# #
# Arguments: # Arguments:
......
...@@ -21,8 +21,12 @@ ...@@ -21,8 +21,12 @@
# my_reaction_emoji: string # my_reaction_emoji: string
# #
class IssuableFinder class IssuableFinder
prepend FinderWithCrossProjectAccess
include FinderMethods
include CreatedAtFilter include CreatedAtFilter
requires_cross_project_access unless: -> { project? }
NONE = '0'.freeze NONE = '0'.freeze
attr_accessor :current_user, :params attr_accessor :current_user, :params
...@@ -87,14 +91,6 @@ class IssuableFinder ...@@ -87,14 +91,6 @@ class IssuableFinder
by_my_reaction_emoji(items) by_my_reaction_emoji(items)
end end
def find(*params)
execute.find(*params)
end
def find_by(*params)
execute.find_by(*params)
end
def row_count def row_count
Gitlab::IssuablesCountForState.new(self).for_state_or_opened(params[:state]) Gitlab::IssuablesCountForState.new(self).for_state_or_opened(params[:state])
end end
...@@ -124,10 +120,6 @@ class IssuableFinder ...@@ -124,10 +120,6 @@ class IssuableFinder
counts counts
end end
def find_by!(*params)
execute.find_by!(*params)
end
def group def group
return @group if defined?(@group) return @group if defined?(@group)
......
class LabelsFinder < UnionFinder class LabelsFinder < UnionFinder
prepend FinderWithCrossProjectAccess
include FinderMethods
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
requires_cross_project_access unless: -> { project? }
def initialize(current_user, params = {}) def initialize(current_user, params = {})
@current_user = current_user @current_user = current_user
@params = params @params = params
......
class MergeRequestTargetProjectFinder class MergeRequestTargetProjectFinder
include FinderMethods
attr_reader :current_user, :source_project attr_reader :current_user, :source_project
def initialize(current_user: nil, source_project:) def initialize(current_user: nil, source_project:)
......
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
# state - filters by state. # state - filters by state.
class MilestonesFinder class MilestonesFinder
include FinderMethods