Merge pull request #1100 from patthoyts/pt/ldap-no-email

Improve handling of misconfigured LDAP accounts.

2 files changed, 15 insertions, 1 deletions

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 629b6819fb1..fb759c371c4 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -1,4 +1,17 @@
 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
+
+  # Extend the standard message generation to accept our custom exception
+  def failure_message
+    exception = env["omniauth.error"]
+    if exception.class == OmniAuth::Error
+      error = exception.message
+    else
+      error   = exception.error_reason if exception.respond_to?(:error_reason)
+      error ||= exception.error        if exception.respond_to?(:error)
+      error ||= env["omniauth.error.type"].to_s
+    end
+    error.to_s.humanize if error
+  end
  
   def ldap
     # We only find ourselves here if the authentication to LDAP was successful.
diff --git a/app/models/user.rb b/app/models/user.rb
index b87e149ca0f..a3e08fa7d0b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -80,7 +80,8 @@ class User < ActiveRecord::Base
 
   def self.find_for_ldap_auth(omniauth_info)
     name = omniauth_info.name.force_encoding("utf-8")
-    email = omniauth_info.email.downcase
+    email = omniauth_info.email.downcase unless omniauth_info.email.nil?
+    raise OmniAuth::Error, "LDAP accounts must provide an email address" if email.nil?
 
     if @user = User.find_by_email(email)
       @user