diff --git a/NEWS.md b/NEWS.md index d53305a208..7ce3956e9b 100644 --- a/NEWS.md +++ b/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, ) diff --git a/app/models/carto/ldap/configuration.rb b/app/models/carto/ldap/configuration.rb index d827319ed0..3d685063ba 100644 --- a/app/models/carto/ldap/configuration.rb +++ b/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 diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index b159ff385e..04e44aaf11 100644 --- a/config/initializers/warden.rb +++ b/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 diff --git a/spec/models/carto/ldap/configuration_spec.rb b/spec/models/carto/ldap/configuration_spec.rb index 3b53d3b49e..e629257069 100644 --- a/spec/models/carto/ldap/configuration_spec.rb +++ b/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]}"