1638 reject empty password ldap auth (#14136)

* Do not allow empty password in LDAP logins

NEWS.md

@@ -723,6 +723,7 @@ ion for time-series (#12670)
 * Add rake to remove duplicate legends in layer
 * Fix private visualization imports when user has no private tables permission (https://github.com/CartoDB/cartodb/issues/14052)
 * Export and import `user`'s `client_application` and `oauth_tokens` (https://github.com/CartoDB/cartodb/pull/14060)
+* Do not allow empty password in LDAP logins
 * Disable syncs for locked users (https://github.com/CartoDB/cartodb/issues/13832)
 * Fix bugs in legends (https://github.com/CartoDB/support/issues/1339, )
 

app/models/carto/ldap/configuration.rb

@@ -66,6 +66,7 @@ class Carto::Ldap::Configuration < ActiveRecord::Base
   # @param String username. No full CN, just the username, e.g. 'administrator1'
   # @param String password
   def authenticate(username, password)
+    return false if username.blank? || password.blank?
     ldap_connection = Net::LDAP.new(connect_timeout: CONNECTION_TIMEOUT)
     ldap_connection.host = self.host
     ldap_connection.port = self.port

config/initializers/warden.rb

@@ -127,7 +127,7 @@ Warden::Strategies.add(:ldap) do
   end
 
   def authenticate!
-    (fail! and return) unless (params[:email] && params[:password])
+    (fail! and return) if (params[:email].blank? || params[:password].blank?)
 
     user = nil
     begin

spec/models/carto/ldap/configuration_spec.rb

@@ -26,6 +26,16 @@ describe Carto::Ldap::Configuration do
     FakeNetLdap.clear_query_registrations
   end
 
+  it 'returns right away if empty user' do
+    ldap = Carto::Ldap::Configuration.new
+    ldap.authenticate('', 'something').should eq false
+  end
+
+  it 'returns right away if empty password' do
+    ldap = Carto::Ldap::Configuration.new
+    ldap.authenticate('some@one.es', '').should eq false
+  end
+
   it 'tests basic authentication' do
     auth_username = 'admin'
     auth_cn = "cn=#{auth_username},#{@domain_bases[0]}"