Patch #34071: Handle AuthSourceExceptions in User.try_to_login - Redmine

Actions

Copy link

Patch #34071

closed

Handle AuthSourceExceptions in User.try_to_login

Added by Jens Krämer over 3 years ago. Updated almost 3 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Go MAEDA

Category:

Accounts / authentication

Target version:

4.2.0

Start date:

Due date:

% Done:

Estimated time:

Description

this patch changes User.try_to_login to catch and log AuthSourceExceptions, and introduces User.try_to_login! which replicates the original behaviour. Before that change, any places outside of AccountController (which already handles the exceptions) that were using User.try_to_login threw an error i.e. in case of an unreachable LDAP server, instead of just treating the user as unauthenticated.

Files

0001-changes-User.try_to_login-to-catch-and-log-AuthSourc.patch (3.62 KB) 0001-changes-User.try_to_login-to-catch-and-log-AuthSourc.patch

Jens Krämer, 2020-10-06 09:45

Related issues

Actions

Copy link

Updated by Go MAEDA over 3 years ago

Category set to Accounts / authentication
Target version set to Candidate for next major release

Actions

Copy link

Updated by Go MAEDA over 3 years ago

The patch also fixes an issue that Redmine might return 500 error and HTML body against an API request.

$ curl -v --user dlopper:foo http://localhost:3000/issues/1.json
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
* Server auth using Basic with user 'dlopper'
> GET /issues/1.json HTTP/1.1
> Host: localhost:3000
> Authorization: Basic ZGxvcHBlcjpmb28=
> User-Agent: curl/7.64.1
> Accept: */*
>
< HTTP/1.1 500 Internal Server Error
< Content-Type: text/html; charset=utf-8
< X-Request-Id: 96e3f1a3-256f-4b0e-a570-b60a8a767ba6
< X-Runtime: 0.205900
< Content-Length: 93450
<
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8" />
  <title>Action Controller: Exception caught</title>
  <style>
    body {
      background-color: #FAFAFA;
.
.
.

Actions

Copy link

Updated by Go MAEDA over 3 years ago

Target version changed from Candidate for next major release to 4.2.0

Setting the target version to 4.2.0.

Actions

Copy link

Updated by Go MAEDA over 3 years ago

After applying the patch, deprecation warnings are displayed during a test.

$ ruby test/unit/user_test.rb 
Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
Skipping LDAP tests.
Run options: --seed 11608

# 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.
...................................................Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
..............................................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 9.026865s, 15.0661 runs/s, 34.6743 assertions/s.
136 runs, 313 assertions, 0 failures, 0 errors, 0 skips

Actions

Copy link

Updated by Go MAEDA over 3 years ago

The deprecation warning is displayed after executing source:branches/4.1-stable/app/models/auth_source_ldap.rb#L101.

    ldap_con.search(:base => self.base_dn,
                    :filter => search_filter,
                    :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)
      results << attrs
    end

[1] pry(main)> AuthSourceLdap.first.authenticate('admin', 'admin')
  AuthSourceLdap Load (0.2ms)  SELECT  "auth_sources".* FROM "auth_sources" WHERE "auth_sources"."type" IN ('AuthSourceLdap') ORDER BY "auth_sources"."id" ASC LIMIT ?  [["LIMIT", 1]]
Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.
AuthSourceException: Connection refused - connect(2) for 0.0.0.0:389
from /path/to/redmine/app/models/auth_source_ldap.rb:66:in `rescue in authenticate'
Caused by Net::LDAP::ConnectionRefusedError: Connection refused - connect(2) for 0.0.0.0:389
from /path/to/gems/ruby/2.7.0/gems/net-ldap-0.16.3/lib/net/ldap/connection.rb:72:in `open_connection'

Actions

Copy link

Updated by Go MAEDA over 3 years ago

Target version changed from 4.2.0 to Candidate for next major release

I have confirmed that using the master branch of net-ldap fixes the deprecation warning. Probably it will be fixed in the feature version of net-ldap.

I will await new releases of net-ldap.

diff --git a/Gemfile b/Gemfile
index c6edcf370..193dfb0a8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -28,7 +28,7 @@ gem 'rqrcode'

 # Optional gem for LDAP authentication
 group :ldap do
-  gem "net-ldap", "~> 0.16.0" 
+  gem "net-ldap", git: 'https://github.com/ruby-ldap/ruby-net-ldap'
 end

 # Optional gem for OpenID authentication

Actions

Copy link