Commit f322975c506966e080e58dd3eb0c38b22183415a
1 parent
2a705c4f
Exists in
master
and in
4 other branches
Improve handling of misconfigured LDAP accounts.
Gitlab requires an email address for all user accounts as this is the default account id and is used for sending notifications. LDAP accounts may be missing email fields so handle this by showing a sensible error message before redirecting to the login screen again. Resolves github issue #899 Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
Showing
2 changed files
with
15 additions
and
1 deletions
Show diff stats
app/controllers/omniauth_callbacks_controller.rb
| 1 | class OmniauthCallbacksController < Devise::OmniauthCallbacksController | 1 | class OmniauthCallbacksController < Devise::OmniauthCallbacksController |
| 2 | + | ||
| 3 | + # Extend the standard message generation to accept our custom exception | ||
| 4 | + def failure_message | ||
| 5 | + exception = env["omniauth.error"] | ||
| 6 | + if exception.class == OmniAuth::Error | ||
| 7 | + error = exception.message | ||
| 8 | + else | ||
| 9 | + error = exception.error_reason if exception.respond_to?(:error_reason) | ||
| 10 | + error ||= exception.error if exception.respond_to?(:error) | ||
| 11 | + error ||= env["omniauth.error.type"].to_s | ||
| 12 | + end | ||
| 13 | + error.to_s.humanize if error | ||
| 14 | + end | ||
| 2 | 15 | ||
| 3 | def ldap | 16 | def ldap |
| 4 | # We only find ourselves here if the authentication to LDAP was successful. | 17 | # We only find ourselves here if the authentication to LDAP was successful. |
app/models/user.rb
| @@ -80,7 +80,8 @@ class User < ActiveRecord::Base | @@ -80,7 +80,8 @@ class User < ActiveRecord::Base | ||
| 80 | 80 | ||
| 81 | def self.find_for_ldap_auth(omniauth_info) | 81 | def self.find_for_ldap_auth(omniauth_info) |
| 82 | name = omniauth_info.name.force_encoding("utf-8") | 82 | name = omniauth_info.name.force_encoding("utf-8") |
| 83 | - email = omniauth_info.email.downcase | 83 | + email = omniauth_info.email.downcase unless omniauth_info.email.nil? |
| 84 | + raise OmniAuth::Error, "LDAP accounts must provide an email address" if email.nil? | ||
| 84 | 85 | ||
| 85 | if @user = User.find_by_email(email) | 86 | if @user = User.find_by_email(email) |
| 86 | @user | 87 | @user |