Commit 3ed27553 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'ce-jej/sso-enforcement-redirect' into 'master'

Refactor RoutableActions to allow additional not_found checks

See merge request gitlab-org/gitlab-ce!28243
parents 373223e0 bedb9a3e
# frozen_string_literal: true
module ProjectUnauthorized
def project_unauthorized_proc
lambda do |project|
if project
label = project.external_authorization_classification_label
module ControllerActions
def self.on_routable_not_found
lambda do |routable|
return unless routable.is_a?(Project)
label = routable.external_authorization_classification_label
rejection_reason = nil
unless ::Gitlab::ExternalAuthorization.access_allowed?(current_user, label)
......@@ -12,9 +14,7 @@ module ProjectUnauthorized
rejection_reason ||= _('External authorization denied access to this project')
end
if rejection_reason
access_denied!(rejection_reason)
end
access_denied!(rejection_reason) if rejection_reason
end
end
end
......
......@@ -3,15 +3,13 @@
module RoutableActions
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?)
if routable_authorized?(routable, extra_authorization_proc)
ensure_canonical_path(routable, requested_full_path)
routable
else
if not_found_or_authorized_proc
not_found_or_authorized_proc.call(routable)
end
perform_not_found_actions(routable, not_found_actions)
route_not_found unless performed?
......@@ -19,6 +17,18 @@ module RoutableActions
end
end
def not_found_actions
[ProjectUnauthorized::ControllerActions.on_routable_not_found]
end
def perform_not_found_actions(routable, actions)
actions.each do |action|
break if performed?
instance_exec(routable, &action)
end
end
def routable_authorized?(routable, extra_authorization_proc)
return false unless routable
......
......@@ -3,7 +3,6 @@
class Projects::ApplicationController < ApplicationController
include CookiesHelper
include RoutableActions
include ProjectUnauthorized
include ChecksCollaboration
skip_before_action :authenticate_user!
......@@ -22,7 +21,7 @@ class Projects::ApplicationController < ApplicationController
path = File.join(params[:namespace_id], params[:project_id] || params[:id])
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
def build_canonical_path(project)
......
# frozen_string_literal: true
class Projects::Clusters::ApplicationsController < Clusters::ApplicationsController
include ProjectUnauthorized
prepend_before_action :project
private
......@@ -12,6 +10,6 @@ class Projects::Clusters::ApplicationsController < Clusters::ApplicationsControl
end
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
# frozen_string_literal: true
class Projects::ClustersController < Clusters::ClustersController
include ProjectUnauthorized
prepend_before_action :project
before_action :repository
......@@ -15,7 +13,7 @@ class Projects::ClustersController < Clusters::ClustersController
end
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
def repository
......
......@@ -3,8 +3,6 @@
module Projects
module Serverless
class FunctionsController < Projects::ApplicationController
include ProjectUnauthorized
before_action :authorize_read_cluster!
def index
......
......@@ -12,7 +12,7 @@ describe ProjectUnauthorized do
render_views
describe '#project_unauthorized_proc' do
describe '.on_routable_not_found' do
controller(::Projects::ApplicationController) do
def show
head :ok
......
# frozen_string_literal: true
require 'spec_helper'
describe RoutableActions do
controller(::ApplicationController) do
include RoutableActions # rubocop:disable RSpec/DescribedClass
before_action :routable
def routable
@klass = params[:type].constantize
@routable = find_routable!(params[:type].constantize, params[:id])
end
def show
head :ok
end
end
def get_routable(routable)
get :show, params: { id: routable.full_path, type: routable.class }
end
describe '#find_routable!' do
context 'when signed in' do
let(:user) { create(:user) }
before do
sign_in(user)
end
context 'with a project' do
let(:routable) { create(:project) }
context 'when authorized' do
before do
routable.add_guest(user)
end
it 'returns the project' do
get_routable(routable)
expect(assigns[:routable]).to be_a(Project)
end
it 'allows access' do
get_routable(routable)
expect(response).to have_gitlab_http_status(200)
end
end
it 'prevents access when not authorized' do
get_routable(routable)
expect(response).to have_gitlab_http_status(404)
end
end
context 'with a group' do
let(:routable) { create(:group, :private) }
context 'when authorized' do
before do
routable.add_guest(user)
end
it 'returns the group' do
get_routable(routable)
expect(assigns[:routable]).to be_a(Group)
end
it 'allows access' do
get_routable(routable)
expect(response).to have_gitlab_http_status(200)
end
end
it 'prevents access when not authorized' do
get_routable(routable)
expect(response).to have_gitlab_http_status(404)
end
end
context 'with a user' do
let(:routable) { user }
it 'allows access when authorized' do
get_routable(routable)
expect(response).to have_gitlab_http_status(200)
end
it 'prevents access when unauthorized' do
allow(subject).to receive(:can?).and_return(false)
get_routable(user)
expect(response).to have_gitlab_http_status(404)
end
end
end
context 'when not signed in' do
it 'redirects to sign in for private resouces' do
routable = create(:project, :private)
get_routable(routable)
expect(response).to have_gitlab_http_status(302)
expect(response.location).to end_with('/users/sign_in')
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
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