Services User Register resource does not set the right password

Here's a patch for @scotthooker's suggestion.

Related issue: #2195489: PHP Parse error: syntax error, unexpected ':' in user_resource.inc on line 286

i dont think this patch works. $account['pass'] is actually an array so think it should be:

'pass1' => isset($account['pass']) ? $account['pass']['pass1'] : '',
'pass2' => isset($account['pass']) ? $account['pass']['pass2'] : '',

patch attached.

when i dump $account, $account['pass'] is an array so we can't get the actual value if we just use ? $account['pass']. Doing so results in a validation error for me with the following warning:

Warning: trim() expects parameter 1 to be string, array given in password_confirm_validate() (line 2881 of /var/www/dev/includes/form.inc).

In 7.x-3.7, It try to use above patch, But still not work.

$ curl -H 'Content-type: application/json' -d '{"name":"u10", "pass": "123","mail": "user@a.om"}' http://localhost/test/api/user/register

return me ok:

{"uid":"11","uri":"http://drupal/daxuebao/api/user/11"}

But I try to login:

$ curl -H 'Content-type: application/json' -d '{"username":"u10", "password": "123","mail": "user@a.om"}' http://localhost/test/api/user/login

It alert me :

["Wrong username or password."]

With the patch in comment #1, I just confirmed a user registration (with password) will work properly if you pass JSON like this:

{
"name":"bob",
"mail":"bob@hotmail.com",
"conf_mail":"bob@hotmail.com",
"pass":"secret"
}

I'd like to point out that passwords are collected on user registration, only when e-mail verification is disabled.

@yaworsk (regarding patch #2), if $account['pass'] is an array for you, then I'm pretty sure that you must be passing an array/object to it, when a string should be passed (hence the warning message in #4).

@TangMonk (#6), your registration call should work if you use patch #1.

@tyler - good point about passing the array. I've seen multiple implementations of this. I was passing
"pass":{
"pass1":"abcdef",
"pass2":"abcdef"
}
to match the actual form submission where you have to enter and confirm the password. I assume that is how this module is intended to work as the code is setting pass1 and pass2... if there is a mistake during the api call and we are just passing in the password via a string one time, that could result in problems for the end user, right?

@yaworsk, it really depends on how the Services team wants to handle this "bug". As far as I recall, the Services call always just collected one password, then used that single password to fill in the 2 password form fields, behind the scenes.

So I think passing in one password is OK, but that puts an assumption on consumers of the service, to make sure their users provide two identical passwords, before passing off the single password to services.

We'll have to wait for a Services team member to chime in on this one.

@tyler, yeah my thinking exactly. Thanks!

the patch works for me. nice work!

Patch from #2 works for me. Thanks for finding the issue guys! I was searching for problems in different places!

I don't think patch #2 will be backwards compatable.

Take a look at this commit:

http://drupalcode.org/project/services.git/commitdiff/c8e9ac50bfcce574b3...

According to the commit diff above, we used to always just pass one password string to the service resource.

Patch #2 assumes we will be passing an array/object with two strings, which will break current consumption of the resource.

I'm not on the Services team so I'm not entirely sure this is valid, or if there is a Change Record in place for this. But from my experience, the commit diff linked above did introduce this "bug" for current consumers.

Patch #1 fixes it for current consumers.

yeah, i'd defer to the services team. I'm of two minds, we could easily update the patch to check to see if the array is being passed in and if not, then just use the one value twice. However, core uses a password and a confirmation field and i'd recommend that both be actually passed into the call to mimic that core functionality. imho don't think it is a good idea to introduce a work around in services which essentially provides backward compatibility that's not actually inline with core...

We can use the versions system built into services to provide a new version with new array method, and fix the old version so it does not set a password of 1.

http://drupalcode.org/project/services.git/blob/refs/heads/7.x-3.x:/docs...

Awesome!! patch 2 worked for me.

Thank you so much peter yaworski !!

@kylebrowning, since the old and new versions of the User Register resouce proposed here both accept an array as its data, I didn't see a way to create a new version of the resource (http://cgit.drupalcode.org/services/tree/docs/services.versions.api.php?...).

I've instead modified the callback to handle a single string for legacy purposes (http://cgit.drupalcode.org/services/diff/?id=c8e9ac50bfcce574b39564a0bfe...), and to handle an array of strings for new age purposes.

Setting to RTBC, #18 fixes my use case of passing in a password string and the array handling stuff LGTM as well.

#18 fixes for me as well.

In using Recent version of services (services-7.x-3.10), there is a bug in password.

I sent this json data to druapl.

{"name":"buggi",
"mail":"buggi@gmail.com",
"pass":{ "pass1":"a111","pass2":"a111"},
"status":"1"
}

User was created but login was failed .
error message : Wrong username or password. 401

I searched codes in user_resource.inc

$pass1 = '';
$pass2 = '';
if (isset($account['pass'])) {
// For legacy usage, passwords come in as a single string. To match the
// actual form state value keys used by Drupal, we also can collect two
// passwords via an array.
if (is_array($account['pass'])) {
$pass1 = $account['pass']['pass1'];
$pass2 = $account['pass']['pass2'];
}
else {
$pass1 = $account['pass'];
$pass2 = $account['pass'];
}
}
$form_state['values']['pass'] = array(
'pass1' => $pass1,
'pass2' => $pass2
);

What problems ?

WE have a test that checks this, so I'm not sure what you're referring to.

Reopening this issue as the patch has not been committed.

This was fixed as part of the security release a few months ago