From 53af3e6b9e6fd221f2b6da1f6029017cf4a23831 Mon Sep 17 00:00:00 2001 From: Kartikey Tanna Date: Tue, 18 Jun 2019 16:18:14 +0000 Subject: [PATCH] #57815 Password authentication disabled for UltraAuth users Disabled password authentication for the users registered using omniauth-ultraauth strategy --- .../enforces_two_factor_authentication.rb | 3 +- app/models/user.rb | 12 +++++-- changelogs/unreleased/57815.yml | 5 +++ doc/integration/ultra_auth.md | 4 +-- lib/gitlab/auth/o_auth/provider.rb | 4 +++ .../application_controller_spec.rb | 7 ++++ spec/models/user_spec.rb | 32 +++++++++++++++++++ 7 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/57815.yml diff --git a/app/controllers/concerns/enforces_two_factor_authentication.rb b/app/controllers/concerns/enforces_two_factor_authentication.rb index 0fddf15d197..825181568ad 100644 --- a/app/controllers/concerns/enforces_two_factor_authentication.rb +++ b/app/controllers/concerns/enforces_two_factor_authentication.rb @@ -23,7 +23,8 @@ module EnforcesTwoFactorAuthentication def two_factor_authentication_required? Gitlab::CurrentSettings.require_two_factor_authentication? || - current_user.try(:require_two_factor_authentication_from_group?) + current_user.try(:require_two_factor_authentication_from_group?) || + current_user.try(:ultraauth_user?) end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/models/user.rb b/app/models/user.rb index 2eb5c63a4cc..38cb4d1a6e8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -835,11 +835,11 @@ class User < ApplicationRecord end def allow_password_authentication_for_web? - Gitlab::CurrentSettings.password_authentication_enabled_for_web? && !ldap_user? + Gitlab::CurrentSettings.password_authentication_enabled_for_web? && !ldap_user? && !ultraauth_user? end def allow_password_authentication_for_git? - Gitlab::CurrentSettings.password_authentication_enabled_for_git? && !ldap_user? + Gitlab::CurrentSettings.password_authentication_enabled_for_git? && !ldap_user? && !ultraauth_user? end def can_change_username? @@ -919,6 +919,14 @@ class User < ApplicationRecord end end + def ultraauth_user? + if identities.loaded? + identities.find { |identity| Gitlab::Auth::OAuth::Provider.ultraauth_provider?(identity.provider) && !identity.extern_uid.nil? } + else + identities.exists?(["provider = ? AND extern_uid IS NOT NULL", "ultraauth"]) + end + end + def ldap_identity @ldap_identity ||= identities.find_by(["provider LIKE ?", "ldap%"]) end diff --git a/changelogs/unreleased/57815.yml b/changelogs/unreleased/57815.yml new file mode 100644 index 00000000000..ae8ec7036ce --- /dev/null +++ b/changelogs/unreleased/57815.yml @@ -0,0 +1,5 @@ +--- +title: Enforced requirements for UltraAuth users +merge_request: 28941 +author: Kartikey Tanna +type: changed diff --git a/doc/integration/ultra_auth.md b/doc/integration/ultra_auth.md index 139cca456aa..69b2a75050d 100644 --- a/doc/integration/ultra_auth.md +++ b/doc/integration/ultra_auth.md @@ -71,8 +71,8 @@ To get the credentials (a pair of Client ID and Client Secret), you must registe 1. [Reconfigure GitLab]( ../administration/restart_gitlab.md#omnibus-gitlab-reconfigure ) or [restart GitLab]( ../administration/restart_gitlab.md#installations-from-source ) for the changes to take effect if you installed GitLab via Omnibus or from source respectively. -On the sign in page, there should now be a UltraAuth icon below the regular sign in form. +On the sign in page, there should now be an UltraAuth icon below the regular sign in form. Click the icon to begin the authentication process. UltraAuth will ask the user to sign in and authorize the GitLab application. If everything goes well, the user will be returned to GitLab and will be signed in. -**Note:** GitLab requires the email address of each new user. Once the user is logged in using UltraAuth, GitLab will redirect the user to the profile page where they will have to provide the email and verify the email. +GitLab requires the email address of each new user. Once the user is logged in using UltraAuth, GitLab will redirect the user to the profile page where they will have to provide the email and verify the email. Password authentication will be disabled for UltraAuth users and two-factor authentication (2FA) will be enforced. diff --git a/lib/gitlab/auth/o_auth/provider.rb b/lib/gitlab/auth/o_auth/provider.rb index 9fdf3324db3..3d44c83736a 100644 --- a/lib/gitlab/auth/o_auth/provider.rb +++ b/lib/gitlab/auth/o_auth/provider.rb @@ -40,6 +40,10 @@ module Gitlab name.to_s.start_with?('ldap') end + def self.ultraauth_provider?(name) + name.to_s.eql?('ultraauth') + end + def self.sync_profile_from_provider?(provider) return true if ldap_provider?(provider) diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 40669ec5451..447a12b2fac 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -289,6 +289,13 @@ describe ApplicationController do expect(subject).to be_truthy end + + it 'returns true if user has signed up using omniauth-ultraauth' do + user = create(:omniauth_user, provider: 'ultraauth') + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to be_truthy + end end describe '#two_factor_grace_period' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c95bbb0b3f5..b098fe3c9f4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1769,6 +1769,26 @@ describe User do end end + describe '#ultraauth_user?' do + it 'is true if provider is ultraauth' do + user = create(:omniauth_user, provider: 'ultraauth') + + expect(user.ultraauth_user?).to be_truthy + end + + it 'is false with othe provider' do + user = create(:omniauth_user, provider: 'not-ultraauth') + + expect(user.ultraauth_user?).to be_falsey + end + + it 'is false if no extern_uid is provided' do + user = create(:omniauth_user, extern_uid: nil) + + expect(user.ldap_user?).to be_falsey + end + end + describe '#full_website_url' do let(:user) { create(:user) } @@ -2807,6 +2827,12 @@ describe User do expect(user.allow_password_authentication_for_web?).to be_falsey end + + it 'returns false for ultraauth user' do + user = create(:omniauth_user, provider: 'ultraauth') + + expect(user.allow_password_authentication_for_web?).to be_falsey + end end describe '#allow_password_authentication_for_git?' do @@ -2829,6 +2855,12 @@ describe User do expect(user.allow_password_authentication_for_git?).to be_falsey end + + it 'returns false for ultraauth user' do + user = create(:omniauth_user, provider: 'ultraauth') + + expect(user.allow_password_authentication_for_git?).to be_falsey + end end describe '#assigned_open_merge_requests_count' do -- GitLab