Commit 1d2429af authored by Patricio Cano's avatar Patricio Cano

Add missing proper nil and error handling to SAML login process.

parent 4361cc39
...@@ -60,6 +60,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -60,6 +60,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
continue_login_process continue_login_process
end end
rescue Gitlab::OAuth::SignupDisabledError
handle_signup_error
end end
def omniauth_error def omniauth_error
...@@ -92,16 +94,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -92,16 +94,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
continue_login_process continue_login_process
end end
rescue Gitlab::OAuth::SignupDisabledError rescue Gitlab::OAuth::SignupDisabledError
label = Gitlab::OAuth::Provider.label_for(oauth['provider']) handle_signup_error
message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed."
if current_application_settings.signup_enabled?
message << " Create a GitLab account first, and then connect it to your #{label} account."
end
flash[:notice] = message
redirect_to new_user_session_path
end end
def handle_service_ticket provider, ticket def handle_service_ticket provider, ticket
...@@ -122,6 +115,19 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -122,6 +115,19 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
end end
end end
def handle_signup_error
label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed."
if current_application_settings.signup_enabled?
message << " Create a GitLab account first, and then connect it to your #{label} account."
end
flash[:notice] = message
redirect_to new_user_session_path
end
def oauth def oauth
@oauth ||= request.env['omniauth.auth'] @oauth ||= request.env['omniauth.auth']
end end
......
...@@ -26,13 +26,15 @@ module Gitlab ...@@ -26,13 +26,15 @@ module Gitlab
@user ||= build_new_user @user ||= build_new_user
end end
if external_users_enabled? unless @user.nil?
# Check if there is overlap between the user's groups and the external groups if external_users_enabled?
# setting then set user as external or internal. # Check if there is overlap between the user's groups and the external groups
if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? # setting then set user as external or internal.
@user.external = false if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
else @user.external = false
@user.external = true else
@user.external = true
end
end end
end end
...@@ -48,7 +50,11 @@ module Gitlab ...@@ -48,7 +50,11 @@ module Gitlab
end end
def changed? def changed?
gl_user.changed? || gl_user.identities.any?(&:changed?) if gl_user
gl_user.changed? || gl_user.identities.any?(&:changed?)
else
true
end
end end
protected protected
......
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