Feature #33345
Include an authentication method name in LDAP connection error messages
| Status: | Closed | Start date: | ||
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  Go MAEDA | % Done: | 0% | |
| Category: | Accounts / authentication | |||
| Target version: | 5.0.0 | |||
| Resolution: |
Description
When LDAP cannot be connected for some reason, the following message is displayed.
The error message is displayed as it is, but I don't know if it is a message from LDAP. You can understand that it is an LDAP message by adding a fixed message to the beginning as shown below.
before.png (29.6 KB)
after.png (34.1 KB)
improvement-ldap-error-message.patch 
(2.68 KB)
33345-v2.patch
(2.23 KB)
Associated revisions
Include an authentication method name in LDAP connection error messages (#33345).
Contributed by Yuichi HARADA.
History
#1
Updated by Go MAEDA about 2 years ago
- Target version set to Candidate for next major release
#2
Updated by Go MAEDA almost 2 years ago
- Category set to Accounts / authentication
#3
Updated by Go MAEDA 10 months ago
I think that just simply adding the prefix "LDAP" to an error message is better.
LDAP: Connection refused - connect(2) for 192.0.2.1:389
The reasons are as follows:
- Adding the prefix "LDAP" is enough to understand that there is a problem with communication with the LDAP server
- We don't have to add a new string to the locales
- Redmine may support new auth sources other than LDAP in the future. The following way of using auth_method_name method can be applied for new auth sources
diff --git a/app/models/auth_source_ldap.rb b/app/models/auth_source_ldap.rb
index 8ed7ce27f..846c7d2f3 100644
--- a/app/models/auth_source_ldap.rb
+++ b/app/models/auth_source_ldap.rb
@@ -63,7 +63,7 @@ class AuthSourceLdap < AuthSource
end
end
rescue *NETWORK_EXCEPTIONS => e
- raise AuthSourceException.new(e.message)
+ raise AuthSourceException.new("#{auth_method_name}: #{e.message}")
end
# Test the connection to the LDAP
@@ -77,7 +77,7 @@ class AuthSourceLdap < AuthSource
end
end
rescue *NETWORK_EXCEPTIONS => e
- raise AuthSourceException.new(e.message)
+ raise AuthSourceException.new("#{auth_method_name}: #{e.message}")
end
def auth_method_name
@@ -107,7 +107,7 @@ class AuthSourceLdap < AuthSource
end
results
rescue *NETWORK_EXCEPTIONS => e
- raise AuthSourceException.new(e.message)
+ raise AuthSourceException.new("#{auth_method_name}: #{e.message}")
end
def ldap_mode
#4
Updated by Yuichi HARADA 10 months ago
Go MAEDA wrote:
I think that just simply adding the prefix "LDAP" to an error message is better.
[...]
The reasons are as follows:
- Adding the prefix "LDAP" is enough to understand that there is a problem with communication with the LDAP server
- We don't have to add a new string to the locales
- Redmine may support new auth sources other than LDAP in the future. The following way of using auth_method_name method can be applied for new auth sources
[...]
I agree. I think a patch will be simple.
#5
Updated by Go MAEDA 9 months ago
- File 33345-v2.patch added
Updated the patch using the code posted in #33345#note-3.