Support for encrypted passwords

Marked #1633646: User import is taking unencrypted passwords only. Can we change it to md5 encrypted passwords also. as a duplicate of this issue.

The above code will only work for existing users. What about new users?

This patch worked for me. I used the Encrypted Password (Old) method to imported about 100 users from a D6 site to a D7 site with the passwords intact. @twistor These users were new to the D7 site.

I also had success with the patch in #3.

Proposal for adding automated tests for this case

I'm working on automated tests for this case. This is my proposal for developing the tests:

• Add two test methods to FeedsCSVtoUsersTest: testMD5() and testSha512();
• Add two columns to users.csv: password_md5 and password_sha512;
• Refactor FeedsCSVtoUsersTest, so less code is duplicated.

Patch

Attached is a patch with my work on the automated tests so far. In addition of the patch in #3, the patch does the following:

• Adds testMD5() method to the FeedsCSVtoUsersTest to test user import with md5 encrypted passwords;
• Adds password_md5 column to the users.csv file;
• Removes @ from pugsley@example in the last row of users.csv (see further why).

Removed @ from pugsley@example in the last row of users.csv

The last row from users.csv should represent an user with a invalid mail address, according to the test in FeedsCSVtoUsersTest. However, Drupal 7 installed on PHP 5.3 or PHP 5.2.9+ evaluates pugsley@example as a valid mail address. Since I run PHP 5.3 locally, that resulted into a broken test. Discussion about this can be found in #308138: Make valid_email_address() support IDNs. There are mixed opinions about whether or not an mail address like pugsley@example should be valid. Since it's currently considered valid, I shamelessly removed the '@' from the mail address to fix the broken test in FeedsCSVtoUsersTest.

Leaving the issue status to 'needs review', so the testbot will execute tests. Should ideally be 'needs work' because the test for sha512 crypted passwords is not yet added.

I would like a little feedback on my proposal for adding the tests for this case before I continue.

*Triggering the testbot*

In addition of the patch in #7, this patch also adds a test for D7 crypted passwords. I put back the @ for "pugsley", because now that mail address appears to be invalid again (must have been a bug in PHP I guess as meanwhile I have upgraded to PHP 5.3.18).

Thank you! I've been searching all day for a solution to this. #9 is exactly what I needed and works perfectly.

Thanks MegaChriz, you my friend are a lifesaver!

This looks pretty good. It would be nice if we could use a configurable target for this rather than add more targets. Should we be setting $count_log2 to 11? I know that's used during updates, so it kind of makes sense here. I'm not sure.

We should probably be using a db_update() rather than drupal_write_record to set the passwords.

Please don't change the email validation tests. That has been a mess. I know things might fail locally, but the testbot is the one we can all agree on. That debate is messy and out of scope for this issue.

Very nice!

I plan to work on this next Thursday. I haven't seen $count_log2 before. Can you tell me in short what it is and what it is used for?

Compared to the patch in #9, the attached patch changes the following to address the issues noted in #11:

• db_update() is used instead of drupal_write_record() to set the passwords.
• Mapping config is used instead of separate targets.
• When hashing a MD5 password using user_hash_password(), the $count_log2 parameter is not set, so "the iteration count" falls back to Drupal's default.

Switching from separate targets to mapping config does make the code more bloated in my opinion. I added five methods and three constants to FeedsUserProcessor to deal with password hashing. These are the new methods:

• passSummaryCallback
  Summary callback for pass mapper.
• passSummaryCallbackOptions
  Password encryptions labels for the summary callback.
• passFormCallback
  Settings form callback for pass mapper.
• passFormCallbackOptions
  Password encryptions labels for the form callback.
• setPassTarget
  Sets the password on the user target.

The options methods are separated from the actual callback to make them easier overridable and reusable when extending the FeedsUserProcessor class. There are two options methods because I wanted the summary for 'pass' to be short, but for the form I wanted to add a description for each option.

Maybe the whole password thing should be moved to a separate file or class to make the FeedsUserProcessor class simpler?

#13 worked great for me. perhaps that is the best solution. Though I would have thought it would be easier to add this feature to feeds_tamper. Perhaps better to have a core need like this in the core module. Thanks again!

spoke too soon... problem. When importing users with the update option I get this error,

Notice: Undefined property: stdClass::$pass in FeedsUserProcessor->entitySave() (line 85 of .../sites/all/modules/feeds/plugins/FeedsUserProcessor.inc).

The password patch #13 works with new users, but it will not update existing users. Feeds is designed to nicely handle updates based on a key and so it seems this patch should do that also.

It's easy problem.

I just changed method setPassTarget

...
$target_user->pass = $target_user->pass_crypted = $new_hash;
...
$target_user->pass = $target_user->pass_crypted = $value;
...

@MegaChriz thank you for your patch. Can somebody update his patch and add my corrections?

I am using this patch, but it didn't make it into the latest release! :-(

is #17 supposed to fix the problem of updating existing user passwords? I am not sure how that works so could @eugene.ilyin or someone else post back a complete patch?

@webservant316, unfortunately I've lost context of this problem :(

Does feeds 7.x-2.0-beta1 now handle transport of the encrypted password from source to destination?

Patch #13 plus the advice in #17 enables password processing for new and updating users. I have attached my attempt at fixing the patch. I am using feeds 7.x-2.0-beta1. I had to manually apply the patch so I am not sure if the patch will apply cleanly to 7.x-2.0-beta1.

This seems like an incredibly essential feature for feeds. Perhaps we could get some help getting this patch and feature committed?

Subscribing. I definitely think this is an important feature to have in Feeds. I know a company who was going to redo their (very large) website into Drupal but stopped because they couldn't import MD5 passwords.

Re-rolled patch against latest dev.

I manually applied this patch to feeds 7.x-2.0-beat2. It would be great to see this get committed!

+1 for getting this into a release, it would be such a big help for several major projects i've got coming up.

(i did have this patch working, but upgraded to beta2 and manually re-applied it, and now have lost the option on the Feeds module Mapping page to choose which type of encryption should apply to the password field import)

I've updated the patch for the latest dev. Most important change is that the target configuration is fixed. I've included an interdiff, but that isn't exactly a diff to the patch in #27: I first had to reroll that patch before I could make the changes that were necessary for the latest dev.

I tested the patch once manually. I tried importing users with plain passwords, md5 encrypted passwords and sha512 encrypted passwords. Login went successfully each time.

Committed #31.

most excellent, thanks!