View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0015361 | mantisbt | ldap | public | 2013-01-10 13:42 | 2022-04-18 20:17 |
Reporter | illmnec | Assigned To | community | ||
Priority | normal | Severity | feature | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.2.12 | ||||
Target Version | 2.25.0 | Fixed in Version | 2.25.0 | ||
Summary | 0015361: Add STARTTLS Support to LDAP | ||||
Description | It would be great if Mantis supported STARTTLS as this is the officially supported method for encrypted LDAP connections (LDAPS is being deprecated). This can be done very simply by adding Obviously, the proper method would require adding a new variable to the configuraton ($g_ldap_starttls) and then checking if the connection is really encrypted before continuing. | ||||
Tags | patch | ||||
Attached Files | mantis_patch_ldap_starttls.patch (2,079 bytes)
diff -rupN mantisbt-1.2.17/config_defaults_inc.php mantisbt-1.2.17-ldap-starttls/config_defaults_inc.php --- mantisbt-1.2.17/config_defaults_inc.php 2014-03-03 14:19:50.000000000 -0500 +++ mantisbt-1.2.17-ldap-starttls/config_defaults_inc.php 2014-09-18 14:37:37.000000000 -0400 @@ -1765,6 +1765,13 @@ $g_ldap_bind_passwd = ''; /** + * Should the connection use STARTTLS (use ldap:// url for server address) + * + * @global string $g_ldap_starttls + */ + $g_ldap_starttls = FALSE; + + /** * Should we send to the LDAP email address or what MySql tells us * @global int $g_use_ldap_email */ diff -rupN mantisbt-1.2.17/core/constant_inc.php mantisbt-1.2.17-ldap-starttls/core/constant_inc.php --- mantisbt-1.2.17/core/constant_inc.php 2014-03-03 14:19:50.000000000 -0500 +++ mantisbt-1.2.17-ldap-starttls/core/constant_inc.php 2014-09-18 14:37:37.000000000 -0400 @@ -312,6 +312,7 @@ define( 'ERROR_LDAP_SERVER_CONNECT_FAILE define( 'ERROR_LDAP_UPDATE_FAILED', 1402 ); define( 'ERROR_LDAP_USER_NOT_FOUND', 1403 ); define( 'ERROR_LDAP_EXTENSION_NOT_LOADED', 1404 ); +define( 'ERROR_LDAP_UNABLE_TO_STARTTLS', 1405 ); # ERROR_CATEGORY_* define( 'ERROR_CATEGORY_DUPLICATE', 1500 ); diff -rupN mantisbt-1.2.17/core/ldap_api.php mantisbt-1.2.17-ldap-starttls/core/ldap_api.php --- mantisbt-1.2.17/core/ldap_api.php 2014-03-03 14:19:50.000000000 -0500 +++ mantisbt-1.2.17-ldap-starttls/core/ldap_api.php 2014-09-18 14:37:37.000000000 -0400 @@ -50,6 +50,13 @@ function ldap_connect_bind( $p_binddn = log_event( LOG_LDAP, "Attempting connection to LDAP URI '{$t_ldap_server}'." ); $t_ds = @ldap_connect( $t_ldap_server ); + $t_ldap_starttls = config_get( 'ldap_starttls'); + if ($t_ldap_starttls) { + if (! @ldap_start_tls($t_ds)){ + log_event( LOG_LDAP, "Error: Cannot initiate STARTTLS on LDAP Server" ); + trigger_error( ERROR_LDAP_UNABLE_TO_STARTTLS, ERROR ); + } + } if ( $t_ds !== false && $t_ds > 0 ) { log_event( LOG_LDAP, "Connection accepted by LDAP server" ); $t_protocol_version = config_get( 'ldap_protocol_version' ); | ||||
Added a patch that adds the functionality to mantisbt 1.2.17. |
|
Bump. Can someone please take 5 minutes and integrate this? It's pretty damn trivial (just got it working myself). The request dates back six years, the code has been there since late 2014, and the effort required is nil. |
|
@tvleavitt would you mind submitting a Pull Request on our Github repository [1] with the changes, making sure that your submissions adhere to our Coding Guidelines [2] ? Thanks ! Note that the provided patch is not complete - an error message for ERROR_LDAP_UNABLE_TO_STARTTLS should be defined in strings_english.txt, and the Admin Guide should document the new config as well. [1] https://github.com/mantisbt/mantisbt |
|
I'll do my best to get to it this week. I'm not overly familiar with Git and pull requests, etc. but I'm sure I can figure it out. How do I edit / update the Admin Guide? Adhering to the Coding Guidelines should be pretty trivial, the patches are tiny and basically exact duplicates of similar functions. I presume that the lang/strings_english.txt bit would map to the item specified in core/constant_inc.php in the patch. |
|
Awesome, thanks
If you need help just let me know. Feel free to ping me on Gitter
That's XML files (docbook format)
Correct, just copy paste the line for another error message and update it as needed |
|
Was this patch ever integrated? |
|
@rogueresearch I don't believe @tvleavitt ever submitted a pull request for this (or if he did, I missed it). Feel free to finalize the patch per my indications in 0015361:0062061 and submit it yourself if you want; I'll gladly review and merge it. |
|
OK, I've started work on it... |
|
Thanks, let me know if you need any help. |
|
@dregad see draft at: https://github.com/mantisbt/mantisbt/pull/1727 Note that I haven't tested it yet. Also, though I'm a professional programmer, I don't know PHP or web development at all. |
|
Many thanks @rogueresearch for taking care of this, it always feel good to be able to close long-standing issues like this :-) |
|
MantisBT: master 94462f8c 2021-01-04 07:29 Sean McBride Committer: dregad Details Diff |
Review of LDAP code; added StartTLS support - added StartTLS support for LDAP, based on illmnec's patch (fixes 0015361). - added new ldap_tls_protocol_min option to specify minimun TLS version. - changed default $g_ldap_protocol_version from 0 to 3 (fixes 0027848). - improved Admin Guide and config_defaults_inc.php PHPDoc comments - corrected log output for ldap_connect, which, despite its name, doesn't actually perform a network connection, according to its docs. - added an Admin Check to ensure that ldap_server config option is in URI form (fixes 0027849). Signed-off-by: Damien Regad <dregad@mantisbt.org> PR https://github.com/mantisbt/mantisbt/pull/1727 |
Affected Issues 0015361, 0027848, 0027849 |
|
mod - admin/check/check_config_inc.php | Diff File | ||
mod - config_defaults_inc.php | Diff File | ||
mod - core/constant_inc.php | Diff File | ||
mod - core/ldap_api.php | Diff File | ||
mod - docbook/Admin_Guide/en-US/config/auth.xml | Diff File | ||
mod - lang/strings_english.txt | Diff File |