Commit ee925603 authored by Catalin Irimie's avatar Catalin Irimie

Update Devise sign_in path for Geo secondaries

In order to support proxying from a secondary to a primary,
while the authentication can still happen on both sites,
we need to have two different paths so that the secondary-specific
path can be excluded from proxying.

Changelog: changed
EE: true
parent 446c75a1
...@@ -21,11 +21,35 @@ if Gitlab::Auth::Ldap::Config.sign_in_enabled? ...@@ -21,11 +21,35 @@ if Gitlab::Auth::Ldap::Config.sign_in_enabled?
end end
end end
devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, devise_controllers = { omniauth_callbacks: :omniauth_callbacks,
registrations: :registrations, registrations: :registrations,
passwords: :passwords, passwords: :passwords,
sessions: :sessions, sessions: :sessions,
confirmations: :confirmations } confirmations: :confirmations }
if ::Gitlab.ee? && ::Gitlab::Geo.connected? && ::Gitlab::Geo.secondary?
devise_for :users, controllers: devise_controllers, path_names: { sign_in: 'auth/geo/sign_in',
sign_out: 'auth/geo/sign_out' }
# When using Geo, the other type of routes should be present as well, as browsers
# cache 302 redirects locally, and events like primary going offline or a failover
# can result in browsers requesting the other paths because of it.
as :user do
get '/users/sign_in', to: 'sessions#new'
post '/users/sign_in', to: 'sessions#create'
post '/users/sign_out', to: 'sessions#destroy'
end
else
devise_for :users, controllers: devise_controllers
# We avoid drawing Geo routes for FOSS, but keep them in for EE
Gitlab.ee do
as :user do
get '/users/auth/geo/sign_in', to: 'sessions#new'
post '/users/auth/geo/sign_in', to: 'sessions#create'
post '/users/auth/geo/sign_out', to: 'sessions#destroy'
end
end
end
devise_scope :user do devise_scope :user do
get '/users/almost_there' => 'confirmations#almost_there' get '/users/almost_there' => 'confirmations#almost_there'
......
...@@ -24,6 +24,10 @@ module EE ...@@ -24,6 +24,10 @@ module EE
'repositories/lfs_locks_api' => %w{verify create unlock} 'repositories/lfs_locks_api' => %w{verify create unlock}
}.freeze }.freeze
ALLOWLISTED_SIGN_OUT_ROUTES = {
'sessions' => %w{destroy}
}.freeze
ALLOWLISTED_SIGN_IN_ROUTES = { ALLOWLISTED_SIGN_IN_ROUTES = {
'sessions' => %w{create}, 'sessions' => %w{create},
'oauth/tokens' => %w{create} 'oauth/tokens' => %w{create}
...@@ -33,12 +37,11 @@ module EE ...@@ -33,12 +37,11 @@ module EE
# In addition to routes allowed in FOSS, allow geo node update route # In addition to routes allowed in FOSS, allow geo node update route
# and geo api route, on both Geo primary and secondary. # and geo api route, on both Geo primary and secondary.
# If this is on a Geo secondary, also allow git write routes. # If this is on a Geo secondary, also allow git write routes and custom logout routes.
# If in maintenance mode, don't allow git write routes on Geo # If in maintenance mode, don't allow git write routes on Geo secondary either
# secondary either
override :allowlisted_routes override :allowlisted_routes
def allowlisted_routes def allowlisted_routes
allowed = super || geo_node_update_route? || geo_api_route? || admin_settings_update? allowed = super || geo_node_update_route? || geo_api_route? || geo_sign_out_route? || admin_settings_update?
return true if allowed return true if allowed
return sign_in_route? if ::Gitlab.maintenance_mode? return sign_in_route? if ::Gitlab.maintenance_mode?
...@@ -90,8 +93,15 @@ module EE ...@@ -90,8 +93,15 @@ module EE
end end
end end
def geo_sign_out_route?
return unless request_path.start_with?('/users/auth/geo/sign_out')
ALLOWLISTED_SIGN_OUT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
def sign_in_route? def sign_in_route?
return unless request.post? && request.path.start_with?('/users/sign_in', '/oauth/token') return unless request.post? && request.path.start_with?('/users/sign_in', '/oauth/token',
'/users/auth/geo/sign_in')
ALLOWLISTED_SIGN_IN_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) ALLOWLISTED_SIGN_IN_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'EE-specific user routing' do
describe 'devise_for users scope' do
it 'defines regular and Geo routes' do
[
['/users/sign_in', 'GET', 'new'],
['/users/auth/geo/sign_in', 'GET', 'new'],
['/users/sign_in', 'POST', 'create'],
['/users/auth/geo/sign_in', 'POST', 'create'],
['/users/sign_out', 'POST', 'destroy'],
['/users/auth/geo/sign_out', 'POST', 'destroy']
].each do |path, method, action|
expect(Rails.application.routes.recognize_path(path, { method: method })).to include(
{ controller: 'sessions', action: action }
)
end
end
shared_examples 'routes session paths' do |route_type|
it "handles #{route_type} named route helpers" do
Rails.application.reload_routes!
sign_in_path, sign_out_path = case route_type
when :regular then
['/users/sign_in', '/users/sign_out']
when :geo then
['/users/auth/geo/sign_in', '/users/auth/geo/sign_out']
end
expect(Gitlab::Routing.url_helpers.new_user_session_path).to eq(sign_in_path)
expect(Gitlab::Routing.url_helpers.destroy_user_session_path).to eq(sign_out_path)
end
end
context 'no database connection' do
before do
allow(Gitlab::Geo).to receive(:connected?).and_return(false)
end
it_behaves_like 'routes session paths', :regular
end
context 'Geo is disabled' do
before do
allow(Gitlab::Geo).to receive(:enabled?).and_return(false)
end
it_behaves_like 'routes session paths', :regular
end
context 'current node is a Geo primary' do
before do
allow(Gitlab::Geo).to receive(:secondary?).and_return(false)
end
it_behaves_like 'routes session paths', :regular
end
context 'current node is a Geo secondary' do
before do
allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
end
it_behaves_like 'routes session paths', :geo
end
end
end
...@@ -37,6 +37,8 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main ...@@ -37,6 +37,8 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
where(:description, :path) do where(:description, :path) do
'LFS request to batch' | '/root/rouge.git/info/lfs/objects/batch' 'LFS request to batch' | '/root/rouge.git/info/lfs/objects/batch'
'to geo replication node api' | "/api/#{API::API.version}/geo_replication/designs/resync" 'to geo replication node api' | "/api/#{API::API.version}/geo_replication/designs/resync"
'Geo sign in' | '/users/auth/geo/sign_in'
'Geo sign out' | '/users/auth/geo/sign_out'
end end
with_them do with_them do
...@@ -116,6 +118,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main ...@@ -116,6 +118,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
where(:description, :path) do where(:description, :path) do
'sign in route' | '/users/sign_in' 'sign in route' | '/users/sign_in'
'sign out route' | '/users/sign_out'
'oauth token route' | '/oauth/token' 'oauth token route' | '/oauth/token'
end end
......
...@@ -359,7 +359,7 @@ func configureRoutes(u *upstream) { ...@@ -359,7 +359,7 @@ func configureRoutes(u *upstream) {
u.route("", "^/-/metrics$", defaultUpstream), u.route("", "^/-/metrics$", defaultUpstream),
// Authentication routes // Authentication routes
u.route("", "^/users/(sign_in|sign_out)$", defaultUpstream), u.route("", "^/users/auth/geo/(sign_in|sign_out)$", defaultUpstream),
u.route("", "^/oauth/geo/(auth|callback|logout)$", defaultUpstream), u.route("", "^/oauth/geo/(auth|callback|logout)$", defaultUpstream),
// Admin Area > Geo routes // Admin Area > Geo routes
......
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