Commit bedb9a3e authored by James Edwards-Jones's avatar James Edwards-Jones

Avoid passing not_found_or_authorized_proc around

Since this needs to be called on every find_routable!(Project, ...
we can instead move it to a RoutableActions check.
parent 224c0103
# frozen_string_literal: true # frozen_string_literal: true
module ProjectUnauthorized module ProjectUnauthorized
def project_unauthorized_proc module ControllerActions
lambda do |project| def self.on_routable_not_found
if project lambda do |routable|
label = project.external_authorization_classification_label return unless routable.is_a?(Project)
label = routable.external_authorization_classification_label
rejection_reason = nil rejection_reason = nil
unless ::Gitlab::ExternalAuthorization.access_allowed?(current_user, label) unless ::Gitlab::ExternalAuthorization.access_allowed?(current_user, label)
...@@ -12,9 +14,7 @@ module ProjectUnauthorized ...@@ -12,9 +14,7 @@ module ProjectUnauthorized
rejection_reason ||= _('External authorization denied access to this project') rejection_reason ||= _('External authorization denied access to this project')
end end
if rejection_reason access_denied!(rejection_reason) if rejection_reason
access_denied!(rejection_reason)
end
end end
end end
end end
......
...@@ -3,13 +3,13 @@ ...@@ -3,13 +3,13 @@
module RoutableActions module RoutableActions
extend ActiveSupport::Concern extend ActiveSupport::Concern
def find_routable!(routable_klass, requested_full_path, extra_authorization_proc: nil, not_found_or_authorized_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
perform_not_found_actions(routable, [not_found_or_authorized_proc]) perform_not_found_actions(routable, not_found_actions)
route_not_found unless performed? route_not_found unless performed?
...@@ -17,11 +17,15 @@ module RoutableActions ...@@ -17,11 +17,15 @@ module RoutableActions
end end
end end
def not_found_actions
[ProjectUnauthorized::ControllerActions.on_routable_not_found]
end
def perform_not_found_actions(routable, actions) def perform_not_found_actions(routable, actions)
actions.compact.each do |action| actions.each do |action|
break if performed? break if performed?
action.call(routable) instance_exec(routable, &action)
end end
end end
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
class Projects::ApplicationController < ApplicationController class Projects::ApplicationController < ApplicationController
include CookiesHelper include CookiesHelper
include RoutableActions include RoutableActions
include ProjectUnauthorized
include ChecksCollaboration include ChecksCollaboration
skip_before_action :authenticate_user! skip_before_action :authenticate_user!
...@@ -22,7 +21,7 @@ class Projects::ApplicationController < ApplicationController ...@@ -22,7 +21,7 @@ class Projects::ApplicationController < ApplicationController
path = File.join(params[:namespace_id], params[:project_id] || params[:id]) path = File.join(params[:namespace_id], params[:project_id] || params[:id])
auth_proc = ->(project) { !project.pending_delete? } auth_proc = ->(project) { !project.pending_delete? }
@project = find_routable!(Project, path, extra_authorization_proc: auth_proc, not_found_or_authorized_proc: project_unauthorized_proc) @project = find_routable!(Project, path, extra_authorization_proc: auth_proc)
end end
def build_canonical_path(project) def build_canonical_path(project)
......
# frozen_string_literal: true # frozen_string_literal: true
class Projects::Clusters::ApplicationsController < Clusters::ApplicationsController class Projects::Clusters::ApplicationsController < Clusters::ApplicationsController
include ProjectUnauthorized
prepend_before_action :project prepend_before_action :project
private private
...@@ -12,6 +10,6 @@ class Projects::Clusters::ApplicationsController < Clusters::ApplicationsControl ...@@ -12,6 +10,6 @@ class Projects::Clusters::ApplicationsController < Clusters::ApplicationsControl
end end
def project def project
@project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc) @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]))
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
class Projects::ClustersController < Clusters::ClustersController class Projects::ClustersController < Clusters::ClustersController
include ProjectUnauthorized
prepend_before_action :project prepend_before_action :project
before_action :repository before_action :repository
...@@ -15,7 +13,7 @@ class Projects::ClustersController < Clusters::ClustersController ...@@ -15,7 +13,7 @@ class Projects::ClustersController < Clusters::ClustersController
end end
def project def project
@project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc) @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]))
end end
def repository def repository
......
...@@ -3,8 +3,6 @@ ...@@ -3,8 +3,6 @@
module Projects module Projects
module Serverless module Serverless
class FunctionsController < Projects::ApplicationController class FunctionsController < Projects::ApplicationController
include ProjectUnauthorized
before_action :authorize_read_cluster! before_action :authorize_read_cluster!
def index def index
......
...@@ -12,7 +12,7 @@ describe ProjectUnauthorized do ...@@ -12,7 +12,7 @@ describe ProjectUnauthorized do
render_views render_views
describe '#project_unauthorized_proc' do describe '.on_routable_not_found' do
controller(::Projects::ApplicationController) do controller(::Projects::ApplicationController) do
def show def show
head :ok head :ok
......
...@@ -116,4 +116,41 @@ describe RoutableActions do ...@@ -116,4 +116,41 @@ describe RoutableActions do
end end
end end
end end
describe '#perform_not_found_actions' do
let(:routable) { create(:project) }
before do
sign_in(create(:user))
end
it 'performs multiple checks' do
last_check_called = false
checks = [proc {}, proc { last_check_called = true }]
allow(subject).to receive(:not_found_actions).and_return(checks)
get_routable(routable)
expect(last_check_called).to eq(true)
end
it 'performs checks in the context of the controller' do
check = lambda { |routable| redirect_to routable }
allow(subject).to receive(:not_found_actions).and_return([check])
get_routable(routable)
expect(response.location).to end_with(routable.to_param)
end
it 'skips checks once one has resulted in a render/redirect' do
first_check = proc { render plain: 'first' }
second_check = proc { render plain: 'second' }
allow(subject).to receive(:not_found_actions).and_return([first_check, second_check])
get_routable(routable)
expect(response.body).to eq('first')
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