Commit 85def2d62500afdab39956f0bd987340889894c4
Exists in
master
and in
4 other branches
Merge pull request #1451 from tsigo/omniauth_settings
Be more resilient in the case of missing omniauth settings
Showing
3 changed files
with
10 additions
and
8 deletions
Show diff stats
config/initializers/1_settings.rb
| @@ -121,19 +121,19 @@ class Settings < Settingslogic | @@ -121,19 +121,19 @@ class Settings < Settingslogic | ||
| 121 | end | 121 | end |
| 122 | 122 | ||
| 123 | def ldap_enabled? | 123 | def ldap_enabled? |
| 124 | - ldap['enabled'] | ||
| 125 | - rescue | 124 | + ldap && ldap['enabled'] |
| 125 | + rescue Settingslogic::MissingSetting | ||
| 126 | false | 126 | false |
| 127 | end | 127 | end |
| 128 | 128 | ||
| 129 | def omniauth_enabled? | 129 | def omniauth_enabled? |
| 130 | omniauth && omniauth['enabled'] | 130 | omniauth && omniauth['enabled'] |
| 131 | - rescue | 131 | + rescue Settingslogic::MissingSetting |
| 132 | false | 132 | false |
| 133 | end | 133 | end |
| 134 | 134 | ||
| 135 | def omniauth_providers | 135 | def omniauth_providers |
| 136 | - omniauth['providers'] || [] | 136 | + (omniauth_enabled? && omniauth['providers']) || [] |
| 137 | end | 137 | end |
| 138 | 138 | ||
| 139 | def disable_gravatar? | 139 | def disable_gravatar? |
lib/gitlab/auth.rb
| @@ -17,7 +17,7 @@ module Gitlab | @@ -17,7 +17,7 @@ module Gitlab | ||
| 17 | end | 17 | end |
| 18 | end | 18 | end |
| 19 | 19 | ||
| 20 | - def create_from_omniauth auth, ldap = false | 20 | + def create_from_omniauth(auth, ldap = false) |
| 21 | provider = auth.provider | 21 | provider = auth.provider |
| 22 | uid = auth.info.uid || auth.uid | 22 | uid = auth.info.uid || auth.uid |
| 23 | name = auth.info.name.force_encoding("utf-8") | 23 | name = auth.info.name.force_encoding("utf-8") |
| @@ -39,7 +39,7 @@ module Gitlab | @@ -39,7 +39,7 @@ module Gitlab | ||
| 39 | password_confirmation: password, | 39 | password_confirmation: password, |
| 40 | projects_limit: Gitlab.config.default_projects_limit, | 40 | projects_limit: Gitlab.config.default_projects_limit, |
| 41 | ) | 41 | ) |
| 42 | - if Gitlab.config.omniauth.block_auto_created_users && !ldap | 42 | + if Gitlab.config.omniauth['block_auto_created_users'] && !ldap |
| 43 | @user.blocked = true | 43 | @user.blocked = true |
| 44 | end | 44 | end |
| 45 | @user.save! | 45 | @user.save! |
| @@ -52,7 +52,7 @@ module Gitlab | @@ -52,7 +52,7 @@ module Gitlab | ||
| 52 | if @user = User.find_by_provider_and_extern_uid(provider, uid) | 52 | if @user = User.find_by_provider_and_extern_uid(provider, uid) |
| 53 | @user | 53 | @user |
| 54 | else | 54 | else |
| 55 | - if Gitlab.config.omniauth.allow_single_sign_on | 55 | + if Gitlab.config.omniauth['allow_single_sign_on'] |
| 56 | @user = create_from_omniauth(auth) | 56 | @user = create_from_omniauth(auth) |
| 57 | @user | 57 | @user |
| 58 | end | 58 | end |
spec/lib/auth_spec.rb
| @@ -4,6 +4,8 @@ describe Gitlab::Auth do | @@ -4,6 +4,8 @@ describe Gitlab::Auth do | ||
| 4 | let(:gl_auth) { Gitlab::Auth.new } | 4 | let(:gl_auth) { Gitlab::Auth.new } |
| 5 | 5 | ||
| 6 | before do | 6 | before do |
| 7 | + Gitlab.config.stub(omniauth: {}) | ||
| 8 | + | ||
| 7 | @info = mock( | 9 | @info = mock( |
| 8 | uid: '12djsak321', | 10 | uid: '12djsak321', |
| 9 | name: 'John', | 11 | name: 'John', |
| @@ -64,7 +66,7 @@ describe Gitlab::Auth do | @@ -64,7 +66,7 @@ describe Gitlab::Auth do | ||
| 64 | end | 66 | end |
| 65 | 67 | ||
| 66 | it "should create user if single_sing_on"do | 68 | it "should create user if single_sing_on"do |
| 67 | - Gitlab.config.omniauth.stub allow_single_sign_on: true | 69 | + Gitlab.config.omniauth['allow_single_sign_on'] = true |
| 68 | User.stub find_by_provider_and_extern_uid: nil | 70 | User.stub find_by_provider_and_extern_uid: nil |
| 69 | gl_auth.should_receive :create_from_omniauth | 71 | gl_auth.should_receive :create_from_omniauth |
| 70 | gl_auth.find_or_new_for_omniauth(@auth) | 72 | gl_auth.find_or_new_for_omniauth(@auth) |