Hi.

After some testing and reading, I came to the conclusion that when I use objectSid or objectGUID as Persistent and Unique User ID Attribute, this module can't convert it to string, even using or not using "Does PUID hold a binary value?" checkbox.

When testing ldap server configuration I get "Warning: htmlspecialchars(): Invalid multibyte sequence in argument em check_plain()", because check_plain can't convert that binary to UTF8.

When using this module to authenticate and provision drupal users, I get a fatal "PDOException: em field_sql_storage_field_storage_write()" because that binary value is trying to be commited on database.

tia

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

trumanru’s picture

Same problem in last dev version 7.x-2.0-beta3+8-dev (2012-Dec-12).
I've excluded objectGUID and now i use sAMAccountName LDAP field as PUID. It's not so good choice cause we have been losing link to LDAP after lastname changed.

johnbarclay’s picture

Version: 7.x-2.0-beta3 » 7.x-2.x-dev
jdurfer’s picture

Priority: Normal » Minor
Status: Active » Needs review

We were getting these results too. It appears to be related to improper handling of the encoded value regardless of the binary checkbox.

I edited the bootstrap.inc file, line 1545 and Commented this line:
return htmlspecialchars($text, ENT_QUOTES, 'UTF-8');
And inserted next:
if (drupal_validate_utf8($text)) return htmlspecialchars($text, ENT_QUOTES, 'UTF-8');
return htmlspecialchars($text, ENT_QUOTES);

And the errors went away. Found this solution here: http://drupal.org/node/891800.

goodydc’s picture

I applied the solution proposed in comment #3 on a fresh install, using the latest code for core and the ldap module pulled from git. It did not work for me. It did remove the htmlspecialchars warnings, but I still get the following PDOException:

PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect string value: '\x93\x82\xF7\xE5\x9Fk...' for column 'ldap_user_puid_value' at row 1: INSERT INTO {field_data_ldap_user_puid} (entity_type, entity_id, revision_id, bundle, delta, language, ldap_user_puid_value, ldap_user_puid_format) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7); Array ( [:db_insert_placeholder_0] => user [:db_insert_placeholder_1] => 4 [:db_insert_placeholder_2] => 4 [:db_insert_placeholder_3] => user [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => ?????kF???x?3?? [:db_insert_placeholder_7] => ) in field_sql_storage_field_storage_write() (line 451 of C:\OCIO\inetpub\wwwroot\drupal-7.x\modules\field\modules\field_sql_storage\field_sql_storage.module).

This was a fresh install, with LDAP configured to provision new users. The user I attempted to log in as is in Active Directory, but not Drupal, so the account should have been created.

goodydc’s picture

Priority: Minor » Normal
FileSize
3.42 KB

Here is a patch that resolves the issue for me. It includes four updates:

  1. LdapUserConf.class.php: Used already provided ldap_servers_binary() function to convert binary puid to hex string for storage in database.
  2. ldap_servers.module: Added ldap_servers_binary_filter() function to create "binary safe" string for use in LDAP filters (based on a similar function by cgmonroe in Drupal 6 LDAP integration module)
  3. ldap_user.cron.inc: Converted hex puid into "binary safe" string when building query to check for orphaned users
  4. ldap_user.cron.inc: Converted binary puid to hex string to create valid array key

Please review.

johnbarclay’s picture

Thanks. Excellent patch. I reworked it a bit and committed it to 7.x-2.x-dev. Do you mind checking the tweaked patch over?

Here's the reworking:

http://drupalcode.org/project/ldap.git/commitdiff/6b8534a998e67cbdb0c2e5...

johnbarclay’s picture

Component: User interface » Code
johnbarclay’s picture

#6 ended up going in 3 different commits separated by some other commits. Kindof a mess to look at.

goodydc’s picture

I like your rework. I looked it over and found a couple of issues, one of which was an oversight with my original patch; a documentation bug; and finally, a suggested change in terminology. Here is a patch that updates the following:

  1. ldap_user.cron.inc: Added a check if puid is binary before calling ldap_servers_binary_filter(). It was my oversight that I neglected the check in the last patch.
  2. LdapServer.class.php: Removed call to ldap_servers_binary() inside userUserEntityFromPuid() since the puid is already converted at this point.
  3. LdapServer.class.php: Updated comment for userPuidFromLdapEntry() to indicate puid is converted from binary, if applicable
  4. Various: Renamed userHexPuidFromLdapEntry() back to userPuidFromLdapEntry() since the value returned is only hex if the puid is binary. If it's not binary, the puid is returned without conversion. Keeping the original name seems a bit more accurate to me.

Please look it over and let me know what you think.

johnbarclay’s picture

Thanks. Appreciate the thoroughness and followup. I committed this. See http://drupalcode.org/project/ldap.git/commitdiff/9af11caaf2efa9ab4fd6c5...

goodydc’s picture

My pleasure. Thanks for all your great work on this module. I noticed a couple of lines from my last patch didn't make it into the last commit. Here's a small patch to reapply the missing updates:

  1. ldap_user.cron.inc: Removed duplicate line adding puid to filter outside of conditional
  2. ldap_user.cron.inc: Updated call using undefined $ldap_server object to use $ldap_servers[$sid] instead.
johnbarclay’s picture

This is committed. thanks.

toralpatelin’s picture

I am getting below error:

SQLSTATE[HY000]: General error: 1366 Incorrect string value: '\xBF\xC8\xB3|&\xA6...' for column 'ldap_user_puid_value' at row 1

I need to get this changes you guys have made. Where do I get it. Is this changes are there in 7.x-2.0-beta3?

Please help on this.

Thanks.

johnbarclay’s picture

Yes 7.x-2.x-dev.

johnbarclay’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

bahuma20’s picture

Version: 7.x-2.x-dev » 8.x-3.x-dev
Issue summary: View changes

Maybe again exists in D8?
https://www.drupal.org/node/2891193

bahuma20’s picture

Version: 8.x-3.x-dev » 7.x-2.x-dev

Created separate issue instead of changing version of this issue