Skip to content
This repository

Fix: can't login when login or dn is in UTF-8 #87

Closed
wants to merge 3 commits into from

4 participants

Aldis Berjoza Steven Xu Quentin Rousseau Curtis Schiewek
Aldis Berjoza

I wrote simple patch to fix issue, when some data (login or dn) is in UTF-8.
Without this patch you won't be able to login in this case.

For example I had situation when devise_ldap_authenticatable tried and failed with cn=graudējs,dc=example,dc=com or cn=graudeejs,ou=piemērs,dc=example,dc=com

To avoid problems you need to force BINARY encoding for data that you send to ldap

Steven Xu
Collaborator

Thanks for the patch! A couple questions.

  1. have you considered 1.8 compatibility? afaik, String#force_encoding is 1.9 only.
  2. have you identified the fundamental flaw that prevents you from sending strings encoded in UTF-8 over the wire? I get the impression that, if this is indeed a library issue, it may well take place at the net/ldap layer instead of at ours. That's not to rule out the appropriateness of a fix here, but I'd just like to understand if it's a flaw in our code that's causing this.
  3. why are you force encoding rather than transcoding via #encode?

Thanks for the submitting the patch. I look forward to merging it so others can enjoy it once we resolve the compatibility issue and once I educate myself a bit more over the methodology and rationale.

Aldis Berjoza

1) No. Sorry didn't even thought about it. We use 1.9 only.
2) Actually you're right, the flaw is more like in net/ldap, I have Class which is syncing users from/to ldap and all fields that have UTF-8 need to be forced to be BINARY before sending, and to UTF-8 when they are received. Perhaps net/ldap should be patched instead.
3) I'm pretty new to Ruby. Google told me about force_encoding, didn't know about #encode.

Quentin Rousseau
kwent commented

Pull request very helpful. I clone @graudeejs repository and users login with non latin characters works now !

You have to implements this as soon as possible in the official repo...

Curtis Schiewek cschiewek closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 26 additions and 9 deletions. Show diff stats Hide diff stats

  1. +26 9 lib/devise_ldap_authenticatable/ldap_adapter.rb
35 lib/devise_ldap_authenticatable/ldap_adapter.rb
@@ -5,7 +5,9 @@ module Devise
5 5 module LdapAdapter
6 6
7 7 def self.valid_credentials?(login, password_plaintext)
8   - options = {:login => login,
  8 + bin_login = login.clone
  9 + bin_login.force_encoding('BINARY')
  10 + options = {:login => bin_login,
9 11 :password => password_plaintext,
10 12 :ldap_auth_username_builder => ::Devise.ldap_auth_username_builder,
11 13 :admin => ::Devise.ldap_use_admin_to_bind}
@@ -15,8 +17,14 @@ def self.valid_credentials?(login, password_plaintext)
15 17 end
16 18
17 19 def self.update_password(login, new_password)
18   - options = {:login => login,
19   - :new_password => new_password,
  20 + bin_login = login.clone
  21 + bin_login.force_encoding('BINARY')
  22 +
  23 + bin_new_password = new_password.clone
  24 + bin_new_password.force_encoding('BINARY')
  25 +
  26 + options = {:login => bin_login,
  27 + :new_password => bin_new_password,
20 28 :ldap_auth_username_builder => ::Devise.ldap_auth_username_builder,
21 29 :admin => ::Devise.ldap_use_admin_to_bind}
22 30
@@ -29,7 +37,9 @@ def self.update_own_password(login, new_password, current_password)
29 37 end
30 38
31 39 def self.ldap_connect(login)
32   - options = {:login => login,
  40 + bin_login = login.clone
  41 + bin_login.force_encoding('BINARY')
  42 + options = {:login => bin_login,
33 43 :ldap_auth_username_builder => ::Devise.ldap_auth_username_builder,
34 44 :admin => ::Devise.ldap_use_admin_to_bind}
35 45
@@ -49,7 +59,9 @@ def self.get_dn(login)
49 59 end
50 60
51 61 def self.set_ldap_param(login, param, new_value, password = nil)
52   - options = { :login => login,
  62 + bin_login = login.clone
  63 + bin_login.force_encoding('BINARY')
  64 + options = { :login => bin_login,
53 65 :ldap_auth_username_builder => ::Devise.ldap_auth_username_builder,
54 66 :password => password }
55 67
@@ -101,15 +113,18 @@ def set_param(param, new_value)
101 113 def dn
102 114 DeviseLdapAuthenticatable::Logger.send("LDAP dn lookup: #{@attribute}=#{@login}")
103 115 ldap_entry = search_for_login
104   - if ldap_entry.nil?
  116 + bin_dn = if ldap_entry.nil?
105 117 @ldap_auth_username_builder.call(@attribute,@login,@ldap)
106 118 else
107 119 ldap_entry.dn
108 120 end
  121 + bin_dn.force_encoding('BINARY')
109 122 end
110 123
111 124 def ldap_param_value(param)
112   - filter = Net::LDAP::Filter.eq(@attribute.to_s, @login.to_s)
  125 + bin_login = @login.to_s.clone
  126 + bin_login.force_encoding('BINARY')
  127 + filter = Net::LDAP::Filter.eq(@attribute.to_s, bin_login)
113 128 ldap_entry = nil
114 129 @ldap.search(:filter => filter) {|entry| ldap_entry = entry}
115 130
@@ -219,8 +234,10 @@ def valid_login?
219 234 #
220 235 # @return [Object] the LDAP entry found; nil if not found
221 236 def search_for_login
222   - DeviseLdapAuthenticatable::Logger.send("LDAP search for login: #{@attribute}=#{@login}")
223   - filter = Net::LDAP::Filter.eq(@attribute.to_s, @login.to_s)
  237 + bin_login = @login.to_s.clone
  238 + bin_login.force_encoding('BINARY')
  239 + DeviseLdapAuthenticatable::Logger.send("LDAP search for login: #{@attribute}=#{bin_login}")
  240 + filter = Net::LDAP::Filter.eq(@attribute.to_s, bin_login)
224 241 ldap_entry = nil
225 242 @ldap.search(:filter => filter) {|entry| ldap_entry = entry}
226 243 ldap_entry

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.