LDAP User: Provisioning Data to Drupal Issues with images

I have the same issue, the image is uploaded correct and the db entry in file_usage is created, but the field picture in the users table is set to zero.
If I disable the provisioning everything works well.

7.x-2.x is unstable now. Keep an eye on #1115704: Drupal 7 Status Updates for when its ready for testing and then please test this again.

FYI, this issue is present in 7.x-2.0-beta1. Disabling user provisioning (but leaving everything else intact) causes a return to normal picture functionality.

Hi, the issue occurs because of the LDAP User module changes the category to 'ldap_user' on profile save and then stops the update values from saving.

A work around without changing the LDAP module is to implement hook_user_update in your own module, detect the ldap_user category, and manually save the user picture (using the same code from the user_user_update() function like so:

/**
 * Implements hook_users_update().
 * Saves user pictures on LDAP user category update.
 */
function CUSTOM_MODULE_user_update(&$user_edit, $account, $category) {
  if (is_object($account) && !$account->is_new && $category == 'ldap_user') {
    if (!empty($account->picture_upload)) {
      $account->picture = $account->picture_upload;
    }
    // Delete picture if requested, and if no replacement picture was given.
    elseif (!empty($account->picture_delete)) {
      $account->picture = NULL;
    }
    else {
        // No picture uploaded or deleted.
        return '';
    }

    // Process picture uploads.
    if (!empty($account->picture->fid) && (!isset($account->original->picture->fid) || $account->picture->fid != $account->original->picture->fid)) {
      $picture = $account->picture;
      // If the picture is a temporary file move it to its final location and
      // make it permanent.
      if (!$picture->status) {
        $info = image_get_info($picture->uri);
        $picture_directory =  file_default_scheme() . '://' . variable_get('user_picture_path', 'pictures');

        // Prepare the pictures directory.
        file_prepare_directory($picture_directory, FILE_CREATE_DIRECTORY);
        $destination = file_stream_wrapper_uri_normalize($picture_directory . '/picture-' . $account->uid . '-' . REQUEST_TIME . '.' . $info['extension']);

        // Move the temporary file into the final location.
        if ($picture = file_move($picture, $destination, FILE_EXISTS_RENAME)) {
          $picture->status = FILE_STATUS_PERMANENT;
          $account->picture = file_save($picture);
          file_usage_add($picture, 'user', 'user', $account->uid);
        }
      }
      // Delete the previous picture if it was deleted or replaced.
      if (!empty($account->original->picture->fid)) {
        file_usage_delete($account->original->picture, 'user', 'user', $account->uid);
        file_delete($account->original->picture);
      }
    }
    elseif (isset($edit['picture_delete']) && $edit['picture_delete']) {
      file_usage_delete($account->original->picture, 'user', 'user', $account->uid);
      file_delete($account->original->picture);
    }
    $account->picture = empty($account->picture->fid) ? 0 : $account->picture->fid;

    // Do not allow 'uid' to be changed.
    $account->uid = $account->original->uid;
    // Save changes to the user table.
    $success = drupal_write_record('users', $account, 'uid');
    if ($success === FALSE) {
      // The query failed - better to abort the save than risk further
      // data loss.
      return FALSE;
    }
	}
}

The only sad thing about the above is that it's duplicate code from the Drupal user module. Unfortunately saving user pictures isn't included in a separate function.

I think the ldap_user_user_update function in the ldap_user module could include an option (settable in the admin configuration) to save user pictures because I think having user pictures separate from LDAP isn't an edge case. If I have time in the new year I'll whip up a patch which will pretty much be the above code integrated into ldap_user.

I was looking into the same problem, have reopened since the above seems a sensible adjustment and it's not included as of 7.x-2.0-beta3.

Worth noting with the above code that if the user resaves their profile (making no changes) it will delete the user picture ... on the site I'm using it on I've got some code that checks the original user object for a picture during re-saves but I don't think it's that clean.

It's all a bit O.T.T. duplicating that much Drupal core code in a hook so probably time to whip up a patch... is it something that people need?

Sounds like what I need at least until we can pull the images from LDAP directly.

Sounds like what we need too.

Yes, I need it for my company intranet. For the moment I've created a new content type containing an image field rather than use the built-in profile image field, but that's not ideal as it means altering views from other modules so extra work for something which isn't as clean.

Yes, this is something that is needed. We would like our users to be able to upload their picture while using our Intranet version of Drupal.

Thank you
Nadim

Will work on a patch in early Feb at Drupalcon sydney. If anybody's keen to give me a hand testing or developing let me know!

I could help with testing. Thanks for working on the patch.

It seems that this bug only affects users who didn't already have a profile image before upgrading from 1.x to 2.x. I'm able to change images for users that already had them, but I'm not able to add images for users that didn't.

I've also found that ldap_user_user_update() is called twice for users without profile images, even when no changes are made to the form. If you do try to add an image, the 'picture' field seems to save correctly the first time, then immediately gets set to '0' the second time through. You can see this if you un-comment the debug code in the user_update function.

I did try running the code in #4, but as Marton pointed out it doesn't work after re-saving the page, and it doesn't fix the underlying problem.

So it's not, as I'd previously thought, related to whether or not users had an image already -- many of these accounts seem to be working fine.

I am still seeing that, in cases where I'm unable to upload an image, ldap_user_user_update() gets called twice, overriding the picture. I still can't identify a pattern for what's triggering that, but it looks like it only affects a small percentage of accounts on our site.

It looks like user_save() is being called without passing the picture file object. Instead it's only passing the file id. This patch fixes it for me.

I'm using it on my work intranet so I can test the patch Monday morning (they're not paying me enough to dial in at weekends!)

Your solution works if your user stays logged on, but once they log out and come back it will sync the ldap account and remove the image. I think I can fix the problem I will try looking into it.

Not on contract with anybody with an LDAP server and just spent way too much time trying to set up openLDAP on OS X (not fun).

If anybody has an LDAP test server I can log in to let me know and I'll sort the patch out or somebody else could work on this patch.

#4 deletes the user picture on re-sync - but there's a pretty simple solution to that.

If you are grabbing pictures from LDAP then this solution that is only just in the Beta 3 version hopefully soon dev version of 7.x-2 should fix your issue of the losing picture on login.

http://drupal.org/node/1774936

The problem is that the wrong hook is used. (Calling user_save inside a user_update hook can't be good since the hook is called from user_save...) Replacing the user_update hook with user_presave fixes this for me.

Patch against ldap 7.x-2.0-beta3 attached.

PS! This isn't related to grabbing pictures from LDAP.

This seems reasonable. Does it pass the simpletests?

OK, I just ran the simpletests for the "LDAP User" test group. There were two errors, but those were present if I used the old code too. (The failing tests were "ldap_user.ProvisionToDrupal.:provisionDrupalAccount.i=2.prov_event=1" and "ldap_user.ProvisionToDrupal.:provisionDrupalAccount.i=2.prov_event=2".)

PS! There are plenty of "Array to string conversion" notices making the test result hard to read. In addition there was two PDO exceptions. (This is with 7.x-2.0-beta3.)

Just for the record, I have been running a rather busy site with my change and provisioning works like normal and users can change/add picture.

I looked further into this. The patch works well for provisioning and synching to Drupal. But breaks provisioning and synching to ldap. The correct solution is to use hook_user_presave for the provisioning to Drupal aspect and hook_user_save for provisioning to ldap.

For those not provisioning to ldap, #21 should cause no problems. I'll work on a patch that accommodates both use cases. Thanks for following up on this hansfn.

Attached is a patch that passes all the simpletests. It does what I mentioned in #25. Can you give it a try and make sure it works.

OK, I tested your patch and it worked nicely in my case (which is provisioning to Drupal). The code seems logically correct now - thank for taking care of this.

PS! There is an empty line between the docblock and the functions ldap_user_user_update and ldap_user_user_presave. Don't know if it breaks the doc generation.

This is committed to 7.x-2.x-dev.

I show this as still unresolved in beta4. It does look like the above patch was applied to the code I'm running but user profile pictures are definitely still wiped on login/sync even if "sync on every login" is turned off. I use ldap_user in conjunction with CAS -- could the issue lie there?

It could be CAS. There is a bug in core that makes external authentication modules not interact well. Try to replicate without CAS.

A patch is needed for this.

Please test 7.x-2.x-dev with and without external authentication (CAS, LDAP, etc.)

I believe this is a duplicate of #1973352: LDAP User: User pictures disappearing on login which is related to user picture provisioning. In any event this is too broad of an issue.