Use rest demo client ( https://github.com/moodlehq/sample-ws-clients/tree/master/PHP-REST ), call get_users_by_id() with an admin user token. Check that the return user (not the same user) has a email address.

Description

This function should return the email of the user on some conditions, namely:

1) if the invoking user was a site admin, or
2) if the invoking user could check hidden user details (moodle/user:viewhiddendetails , although I'm not 100% certain on this one)

Neither of the conditions are met, because the underlying function to get user details, namely user_get_user_details() only returns email if those conditions are met:

1) if the capability 'moodle/course:useremail' is true, which will never be upon this webservice call
2) if the user has the email visible to everyone
3) or if the invoking user is in the same course of the requested user and the requested user is allowing email sharing only for members of his courses

So , the underlying function does not check if the calling user is admin, which I think it's wrong, because the admin should have access to the e-mail of all users, and also the users that have the capability moodle/user:viewhiddendetails should as well, but on this one I am not sure.

I was developing the implementation for get_users() when I stumbled upon this bug. This might be critical for some integrations I think (at least on mine is), because normally an email is an important key.

Edit: I forgot to mention that there is only one condition on which the e-mail is returned: when the invoking user is the same as the requested user.

The function user_get_user_details is used at 3 functions, on enrol/externallib.php and user/externallib.php, so these also suffer from the same bug.
My fix allows the admin user to have access to the email in any situation, which seems to me the correct behaviour.

The remaining behaviour can be added at the externallib.php functions. For example, I think that the users with the capability moodle/user:viewhiddendetails or moodle/user:update should be able to consult another user's email, but I can be wrong on this one of course. If so, these fields can be added to the answer, like it is being done on user/externallib.php, lines 395- 403 aprox.

Fábio Souto
added a comment - 08/Feb/12 7:08 PM Here goes a fix for this one
https://github.com/fabiomsouto/moodle/compare/MOODLE_22_STABLE...MDL-31520-MOODLE_22_STABLE
The function user_get_user_details is used at 3 functions, on enrol/externallib.php and user/externallib.php, so these also suffer from the same bug.
My fix allows the admin user to have access to the email in any situation, which seems to me the correct behaviour.
The remaining behaviour can be added at the externallib.php functions. For example, I think that the users with the capability moodle/user:viewhiddendetails or moodle/user:update should be able to consult another user's email, but I can be wrong on this one of course. If so, these fields can be added to the answer, like it is being done on user/externallib.php, lines 395- 403 aprox.
Cheers
Fabio

Jérôme Mouneyrac
added a comment - 01/May/12 10:49 AM Peer reviewed. It seems a good fix to me. There are some cases where no capability are checked (when $course is empty) so I think it is necessary to add a check if the user is admin.
Cheery-pick into 2.1 and master too.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Eloy Lafuente (stronk7)
added a comment - 12/May/12 1:35 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Eloy Lafuente (stronk7)
added a comment - 19/May/12 7:01 PM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Dan Poltawski
added a comment - 01/Jun/12 5:31 PM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao

Thanks Fabio, this has been integrated now (sorry about the delay).
During integration I did make one change worth noting. I moved the is admin check to the front of that queue of or's as an optimisation (bool check is quicker than some of the other checks there).

Sam Hemelryk
added a comment - 06/Jun/12 6:39 AM Thanks Fabio, this has been integrated now (sorry about the delay).
During integration I did make one change worth noting. I moved the is admin check to the front of that queue of or's as an optimisation (bool check is quicker than some of the other checks there).
Cheers
Sam

This works fine when I use Moodle's built in test client at /admin/webservice/testclient.php. I eventually got this working with the PHP REST client from github. Passing.

The testing instructions with this could have been more thorough. There are a bunch of steps that are required but which are not mentioned. In particular the process to create a user token and the need to enable the REST protocol.

Andrew Davis
added a comment - 06/Jun/12 5:40 PM This works fine when I use Moodle's built in test client at /admin/webservice/testclient.php. I eventually got this working with the PHP REST client from github. Passing.
The testing instructions with this could have been more thorough. There are a bunch of steps that are required but which are not mentioned. In particular the process to create a user token and the need to enable the REST protocol.

Eloy Lafuente (stronk7)
added a comment - 12/Jun/12 9:46 AM We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.
Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.
Many thanks for your collaboration!
Ciao

Rajesh Taneja
added a comment - 15/Jun/12 4:28 PM Sorry guys,
It seems on 21 it was wrongly merged.
+if ($isadmin
+ of $currentuser
It should be or and not of
Can you please rectify this or should this be handled by new issue ?