On a client project we needed the ability to prevent administrators from manually setting the bind password for non-anonymous searches in the Binding Method section of the LDAP Server Configuration tab. Instead, the preference was for this to be configurable through Drush instead, making it easily configurable during the build process without actually storing any password-related information in the code repository for the site via an LDAP configuration feature.

To control this, I added a checkbox that is used to remove the "Password for non-anonymous search" and "Clear existing password from the database" fields from the UI where they appear on the main LDAP Settings tab. I also added a "drush lssd" (drush ldap-servers-set-password) command for setting the password through Drush.

The attached patch contains these new features.

CommentFileSizeAuthor
#16 ldap_servers-drush-password-2400177-15.patch3.76 KBgrahl
#10 ldap_servers-drush-password-2400177-10.patch6.54 KBNWOM
#9 ldap_servers-drush-password-2400177-9.patch6.54 KBNWOM
#6 ldap_servers-drush-password-2400177-6.patch6.51 KBrklawson
#5 ldap_servers-drush-password-2400177-5.patch6.63 KBrklawson
#4 ldap_servers-drush-password-2400177-4.patch6.72 KBrklawson
#3 ldap_servers-drush-password-2400177-2.patch6.6 KBrklawson
#1 ldap-servers-drush-password-2400177-1.patch6.53 KBrklawson
ldap_module_drush_password.patch6.53 KBrklawson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rklawson’s picture

rklawson CreditAttribution: rklawson commented
FileSize
ldap-servers-drush-password-2400177-1.patch6.53 KB
rklawson’s picture

rklawson CreditAttribution: rklawson commented

Reformatted the title of the patch file.

rklawson’s picture

rklawson CreditAttribution: rklawson commented
FileSize
ldap_servers-drush-password-2400177-2.patch6.6 KB

Fixed some help text and an issue with the way that the 'bindpw' and 'clear_bindpw' were being unset in terms of the bind process.

rklawson’s picture

rklawson CreditAttribution: rklawson commented
FileSize
ldap_servers-drush-password-2400177-4.patch6.72 KB

Re-rolled the patch due to failures in Drush make when downloading from here at Drupal.org as a result of line endings.

rklawson’s picture

rklawson CreditAttribution: rklawson commented
FileSize
ldap_servers-drush-password-2400177-5.patch6.63 KB

Re-rolled the patch, replacing the database queries to save the password in the Drush command with the save method of the LdapServerAdmin class.

Not only is this much better from a technical perspective, it was necessary because the defaults that set up the CTools export for the server config are not initially saved to the database without a save via the UI. This prevented the previous version of the Drush command from working during a build, since it was unable to find any configurations in the ldap_servers table.

rklawson’s picture

rklawson CreditAttribution: rklawson commented
FileSize
ldap_servers-drush-password-2400177-6.patch6.51 KB

Re-rolled the patch to remove a call to ldap_servers_encrypt() in the drush command, since this is already handled by the class. The password was being encrypted and then that hash was being encrypted within the class.

rklawson’s picture

rklawson CreditAttribution: rklawson commented
Version: 7.x-1.0-beta12 » 7.x-2.0-beta7
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 7.x-2.0-beta7 » 7.x-2.x-dev
NWOM’s picture

NWOM CreditAttribution: NWOM commented
FileSize
ldap_servers-drush-password-2400177-9.patch6.54 KB

This patch worked perfectly and applied cleanly to the newest dev. Thank you very much! I however found one small typo:

Line 122:

+ the service account through the user interface, so use the "drush lssd" command to set the password instead.'),

Should instead be:

+ the service account through the user interface, so use the "drush lssp" command to set the password instead.'),

However, I noticed that it wasn't clear from the description how the drush command should be formatted, so I changed it further to this:

+ the service account through the user interface, so use the "drush lssp SERVERID --password="PASSWORD"" command to set the password instead.'),

Please review the attached patch. Thanks!

NWOM’s picture

NWOM CreditAttribution: NWOM commented
FileSize
ldap_servers-drush-password-2400177-10.patch6.54 KB

Please disregard the last patch. Here it is again.

NWOM’s picture

NWOM CreditAttribution: NWOM commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: ldap_servers-drush-password-2400177-10.patch, failed testing.

NWOM’s picture

NWOM CreditAttribution: NWOM commented

This sadly no longer works on the newest dev (it applied cleanly however). It now always shows "Failed to Bind" when attempting to login, even though the Status Report shows that it is binded correctly.

NWOM’s picture

NWOM CreditAttribution: NWOM commented
Status: Needs work » Needs review

Nevermind, #10 applies cleanly and works on the newest dev. Please review.

  • grahl committed 184e62d on 7.x-2.x authored by rklawson 
    Issue #2400177 by rklawson, NWOM: New Admin Config Setting and Drush...
grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl as a volunteer commented
Priority: Normal » Minor
Status: Needs review » Postponed
FileSize
ldap_servers-drush-password-2400177-15.patch3.76 KB

Hi

Thanks for following up with this NWOM.

I've added the drush command since it works fine, has no side-effects I can see and serves a genuine need to automate setting the service account in 7 outside of a feature module.

The remainder I've put into a separate patch since I don't see disabling the UI for setting the password as essential. If there is significant need for this, please speak up or I at least will leave this issue as postponed indefinitely.

I won't add this to 8 since configuration overrides can achieve the same thing there and are documented in the installation instructions.

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl as a volunteer commented
Status: Postponed » Closed (outdated)

Since configuration overrides have been added to 7.x, I'm closing this issue. Users are free to use this approach but I won't be committing it.