171 lines
5.1 KiB
Diff
171 lines
5.1 KiB
Diff
From d2536bf5a683098f4077cf644e8344cc7ea8e13a Mon Sep 17 00:00:00 2001
|
|
From: Robert Speicher <robert@gitlab.com>
|
|
Date: Tue, 9 Jan 2018 16:47:31 +0000
|
|
Subject: [PATCH] Merge branch 'jej/fix-disabled-oauth-access-10-3' into 'security-10-3'
|
|
|
|
[10.3] Prevent login with disabled OAuth providers
|
|
|
|
See merge request gitlab/gitlabhq!2296
|
|
|
|
(cherry picked from commit 4936650427ffc88e6ee927aedbb2c724d24b094c)
|
|
|
|
a0f9d222 Prevents login with disabled OAuth providers
|
|
---
|
|
app/controllers/omniauth_callbacks_controller.rb | 9 +++++++++
|
|
changelogs/unreleased/jej-fix-disabled-oauth-access.yml | 5 +++++
|
|
lib/gitlab/o_auth.rb | 6 ++++++
|
|
lib/gitlab/o_auth/user.rb | 11 ++++++-----
|
|
spec/controllers/omniauth_callbacks_controller_spec.rb | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
|
|
spec/features/oauth_login_spec.rb | 3 +--
|
|
spec/support/devise_helpers.rb | 15 +++++++++------
|
|
spec/support/login_helpers.rb | 7 +++++++
|
|
8 files changed, 118 insertions(+), 13 deletions(-)
|
|
create mode 100644 changelogs/unreleased/jej-fix-disabled-oauth-access.yml
|
|
create mode 100644 lib/gitlab/o_auth.rb
|
|
create mode 100644 spec/controllers/omniauth_callbacks_controller_spec.rb
|
|
|
|
--- a/app/controllers/omniauth_callbacks_controller.rb
|
|
+++ b/app/controllers/omniauth_callbacks_controller.rb
|
|
@@ -93,6 +93,8 @@
|
|
|
|
continue_login_process
|
|
end
|
|
+ rescue Gitlab::OAuth::SigninDisabledForProviderError
|
|
+ handle_disabled_provider
|
|
rescue Gitlab::OAuth::SignupDisabledError
|
|
handle_signup_error
|
|
end
|
|
@@ -136,6 +138,13 @@
|
|
@oauth ||= request.env['omniauth.auth']
|
|
end
|
|
|
|
+ def handle_disabled_provider
|
|
+ label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
|
|
+ flash[:alert] = "Signing in using #{label} has been disabled"
|
|
+
|
|
+ redirect_to new_user_session_path
|
|
+ end
|
|
+
|
|
def log_audit_event(user, options = {})
|
|
AuditEventService.new(user, user, options).
|
|
for_authentication.security_event
|
|
--- /dev/null
|
|
+++ b/changelogs/unreleased/jej-fix-disabled-oauth-access.yml
|
|
@@ -0,0 +1,5 @@
|
|
+---
|
|
+title: Prevent OAuth login POST requests when a provider has been disabled
|
|
+merge_request:
|
|
+author:
|
|
+type: security
|
|
--- /dev/null
|
|
+++ b/lib/gitlab/o_auth.rb
|
|
@@ -0,0 +1,6 @@
|
|
+module Gitlab
|
|
+ module OAuth
|
|
+ SignupDisabledError = Class.new(StandardError)
|
|
+ SigninDisabledForProviderError = Class.new(StandardError)
|
|
+ end
|
|
+end
|
|
--- a/lib/gitlab/o_auth/user.rb
|
|
+++ b/lib/gitlab/o_auth/user.rb
|
|
@@ -27,7 +27,8 @@
|
|
end
|
|
|
|
def save(provider = 'OAuth')
|
|
- unauthorized_to_create unless gl_user
|
|
+ raise SigninDisabledForProviderError if oauth_provider_disabled?
|
|
+ raise SignupDisabledError unless gl_user
|
|
|
|
if needs_blocking?
|
|
gl_user.save!
|
|
@@ -181,8 +182,10 @@
|
|
Gitlab::AppLogger
|
|
end
|
|
|
|
- def unauthorized_to_create
|
|
- raise SignupDisabledError
|
|
+ def oauth_provider_disabled?
|
|
+ Gitlab::CurrentSettings.current_application_settings
|
|
+ .disabled_oauth_sign_in_sources
|
|
+ .include?(auth_hash.provider)
|
|
end
|
|
end
|
|
end
|
|
--- /dev/null
|
|
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
|
|
@@ -0,0 +1,75 @@
|
|
+require 'spec_helper'
|
|
+
|
|
+describe OmniauthCallbacksController do
|
|
+ include LoginHelpers
|
|
+
|
|
+ let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: provider) }
|
|
+ let(:provider) { :github }
|
|
+
|
|
+ before do
|
|
+ mock_auth_hash(provider.to_s, 'my-uid', user.email)
|
|
+ stub_omniauth_provider(provider, context: request)
|
|
+ end
|
|
+
|
|
+ it 'allows sign in' do
|
|
+ post provider
|
|
+
|
|
+ expect(request.env['warden']).to be_authenticated
|
|
+ end
|
|
+
|
|
+ shared_context 'sign_up' do
|
|
+ let(:user) { double(email: 'new@example.com') }
|
|
+
|
|
+ before do
|
|
+ stub_omniauth_setting(block_auto_created_users: false)
|
|
+ end
|
|
+ end
|
|
+
|
|
+ context 'sign up' do
|
|
+ include_context 'sign_up'
|
|
+
|
|
+ it 'is allowed' do
|
|
+ post provider
|
|
+
|
|
+ expect(request.env['warden']).to be_authenticated
|
|
+ end
|
|
+ end
|
|
+
|
|
+ context 'when OAuth is disabled' do
|
|
+ before do
|
|
+ stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
|
|
+ settings = Gitlab::CurrentSettings.current_application_settings
|
|
+ settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
|
|
+ end
|
|
+
|
|
+ it 'prevents login via POST' do
|
|
+ post provider
|
|
+
|
|
+ expect(request.env['warden']).not_to be_authenticated
|
|
+ end
|
|
+
|
|
+ it 'shows warning when attempting login' do
|
|
+ post provider
|
|
+
|
|
+ expect(response).to redirect_to new_user_session_path
|
|
+ expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
|
|
+ end
|
|
+
|
|
+ it 'allows linking the disabled provider' do
|
|
+ user.identities.destroy_all
|
|
+ sign_in(user)
|
|
+
|
|
+ expect { post provider }.to change { user.reload.identities.count }.by(1)
|
|
+ end
|
|
+
|
|
+ context 'sign up' do
|
|
+ include_context 'sign_up'
|
|
+
|
|
+ it 'is prevented' do
|
|
+ post provider
|
|
+
|
|
+ expect(request.env['warden']).not_to be_authenticated
|
|
+ end
|
|
+ end
|
|
+ end
|
|
+end
|