Redmine

Go MAEDA wrote:

r18692 changed AuthSourceLdap.get_attr method to private, but there are some codes that call AuthSourceLdap.get_attr. As a result, those codes raise NoMethodError and users cannot sign in with LDAP authentication.

[...]

I confirmed.

Reverting r18692 should fix this issue (I have not yet tested it).

I think you can revert, but the following patch confirmed that LDAP authentication works.

diff --git a/app/models/auth_source_ldap.rb b/app/models/auth_source_ldap.rb
index a1739c00e..546597e5d 100644
--- a/app/models/auth_source_ldap.rb
+++ b/app/models/auth_source_ldap.rb
@@ -103,7 +103,7 @@ class AuthSourceLdap < AuthSource
                     :attributes => ['dn', self.attr_login, self.attr_firstname, self.attr_lastname, self.attr_mail],
                     :size => 10) do |entry|
       attrs = get_user_attributes_from_ldap_entry(entry)
-      attrs[:login] = AuthSourceLdap.get_attr(entry, self.attr_login)
+      attrs[:login] = get_attr(entry, self.attr_login)
       results << attrs
     end
     results
@@ -197,9 +197,9 @@ class AuthSourceLdap < AuthSource
   def get_user_attributes_from_ldap_entry(entry)
     {
      :dn => entry.dn,
-     :firstname => AuthSourceLdap.get_attr(entry, self.attr_firstname),
-     :lastname => AuthSourceLdap.get_attr(entry, self.attr_lastname),
-     :mail => AuthSourceLdap.get_attr(entry, self.attr_mail),
+     :firstname => get_attr(entry, self.attr_firstname),
+     :lastname => get_attr(entry, self.attr_lastname),
+     :mail => get_attr(entry, self.attr_mail),
      :auth_source_id => self.id
     }
   end
@@ -244,11 +244,10 @@ class AuthSourceLdap < AuthSource
     attrs
   end

-  def self.get_attr(entry, attr_name)
+  def get_attr(entry, attr_name)
     if !attr_name.blank?
       value = entry[attr_name].is_a?(Array) ? entry[attr_name].first : entry[attr_name]
       value.to_s.force_encoding('UTF-8')
     end
   end
-  private_class_method :get_attr
 end

Yuichi HARADA wrote:

Reverting r18692 should fix this issue (I have not yet tested it).

I think you can revert, but the following patch confirmed that LDAP authentication works.

[...]

The following is the test execution result.

-- Before: trunk(r19072) --

$ RAILS_ENV=test bundle exec rake test TEST=test/unit/auth_source_ldap_test.rb
Run options: --seed 11589

# Running:

..E

Error:
AuthSourceLdapTest#test_search_should_return_matching_entries:
NoMethodError: private method `get_attr' called for #<Class:0x00007fe216323390>
    app/models/auth_source_ldap.rb:200:in `get_user_attributes_from_ldap_entry'
    app/models/auth_source_ldap.rb:105:in `block in search'
    app/models/auth_source_ldap.rb:101:in `search'
    app/models/auth_source.rb:80:in `block in search'
    app/models/auth_source.rb:77:in `search'
    test/unit/auth_source_ldap_test.rb:177:in `test_search_should_return_matching_entries'

bin/rails test test/unit/auth_source_ldap_test.rb:176

.E

Error:
AuthSourceLdapTest#test_#authenticate_with_a_valid_LDAP_user_should_return_the_user_attributes:
NoMethodError: private method `get_attr' called for #<Class:0x00007fe216323390>
    app/models/auth_source_ldap.rb:200:in `get_user_attributes_from_ldap_entry'
    app/models/auth_source_ldap.rb:238:in `block in get_user_dn'
    app/models/auth_source_ldap.rb:234:in `get_user_dn'
    app/models/auth_source_ldap.rb:59:in `block in authenticate'
    app/models/auth_source_ldap.rb:145:in `block in with_timeout'
    app/models/auth_source_ldap.rb:144:in `with_timeout'
    app/models/auth_source_ldap.rb:58:in `authenticate'
    test/unit/auth_source_ldap_test.rb:126:in `block in <class:AuthSourceLdapTest>'

bin/rails test test/unit/auth_source_ldap_test.rb:122

..Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
.................

Finished in 2.005034s, 11.9699 runs/s, 29.9247 assertions/s.
24 runs, 60 assertions, 0 failures, 2 errors, 0 skips

-- After: Apply #32470-2 patch --

$ RAILS_ENV=test bundle exec rake test TEST=test/unit/auth_source_ldap_test.rb
Run options: --seed 19832

# Running:

.......Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
.................

Finished in 2.031780s, 11.8123 runs/s, 37.4056 assertions/s.
24 runs, 76 assertions, 0 failures, 0 errors, 0 skips

I think get_attr is not necessary to be a private class method, so the patch in #32470#note-2 looks a nice solution.