Extend the User service with a user.load service

Forgot the attachment, sorry.

An example JSON string for the attributes could be:
{ "Name": "admin", "status": true }

Thank you for pointing that out. I was worried about the information coming in from outside. However, I see that _db_query_callback seems to handle all that. The only other thing that springs to mind is that a JSON supplied value would arrive as an object, which the foreach takes care of. Though I have warbled on about that elsewhere.

All in all, it could be reduced to:

function _user_service_load_user($attrs) {
  if (is_object($attrs) {
    $attrs = (array)$attrs
  }
  return user_load($attrs);
}

Badly formed requests will cause SQL errors (but they probably still could even with all that checking code) which will be logged - no bad thing. But it won't stop the request and the external caller will simply get a 404 not found error - not quite right, but good enough.

I agree that it would be better if the user.get service accepted either, but I don't know if that can be done for XML-RPC. It would need an either/or specification for the parameters to avoid breaking current client side code. Something like user.get(int (optional), struct (optional)) perhaps.

Merging into user_get would be the best solution, but changing the external API is a dangerous game, there's probably a lot of code out there using Services.

I'm in favour of reducing it to a call to user_load once the input data has been forced to an array.
This could be:

function user_service_load($attrs) {
  $account = user_load((array)$attrs);
  ...
}

and

function user_service_load_access($attrs) {
  global $user;
  $account = user_load((array)$attrs);
  ...
}

At which point, further code reductions can be made, simply by passing the original calls to xxx_get along to xxx_load.
For example:

function user_service_get_access($uid) {
  return user_service_load_access(array("uid" => $uid)); 
}

and

function user_service_get($uid) {
  return user_service_load(array("uid" => $uid));
}

HTH

John

@greg.harvey I think that the patch should take into account the various front-ends, as you yourself have remarked with respect to XMLRPC.

FYI Apart from this issue, I am also attempting to get a small group of patches applied to the users service:
http://drupal.org/node/806856 the error code goes AWOL when using the XMLRPC front end
http://drupal.org/node/807638 allow user.services to transparently apply additional protection against specific user modification/deletion - using the User Protect module (a necessary patch to that module has already been committed)
http://drupal.org/node/679494 a plea to change the argument to user.save back to <struct> (as it was) instead of <array> (as it is now)
You may not be interested in all of these requests, obviously.

Personally my opinion is that it is best not to change the existing published user.service API at all - it's an external API, and there will be all sorts of applications which use it. The last thing I want is to offer a patch which could potentially break these applications.

We seem to agree that while user.get(uid) is useful within the Drupal environment, it has less sense to an outside application. Better to use the username or email. I also agree with #10, though if things have been this way for so long, I don't see it as "critical".

Which is why I came to the conclusion of adding a new method - user.load - which would connect directly to the Drupal function user_load. Since this is essentially a superset of the user.get method, I don't think this would actually be such a huge increase in code (see #5).

I'll gladly join forces with you in getting this patch approved, though things seem to have gone very quiet recently. Of course this may be due to other reasons on the part of the maintainers, we all have lives to lead...

Greg - I'd left #5 as an idea, because I was expecting some feedback before writing (yet another) patch. Look forward to seeing/reviewing your patch.

I'm doing (more or less) the same thing - an external application (not mine, and I really don't want to know about it ;-) wants to add/modify/delete users and their roles. Be wary of the fact that, much as within Drupal Web UI itself, uid = 1 can be deleted via Services. Uid = 0 can also be deleted via Services, though it's a little more difficult to do via the Drupal Web UI. The first will cause problems for administrators, the second will probably break Drupal. This is why I am also pushing for a transparent User Protect integration.

Sorry Greg, but doesn't work for me...

Operations carried out:

1. Disabled user service
2. Applied patch
3. Enabled user service
4. Went to admin/build/services/browse/user.get set 51 for the uid - perfect
5. Went to admin/build/services/browse/user.load set { "uid": 51 } as the JSON input for 'array' - got the error "The parameters you supplied did not match any user in the system."

One minor point. I've battled the last two weeks trying to clear up the difference between "array" and "struct" in services (particularly user.save) - now you've called the parameter for user.load "array", of type "struct"! Can we call it "attrs" or something?

A quick dsm($uid); shows that the function user_service_load_access($uid) doesn't receive a number (how could it?), but the attributes. These need to be passed to user_load to get the actual uid.

Here then the patched patch... (I'm not trying to be facetious, I appreciate your help, and I'd really like to see this get through)

Sorry if I'm being a pain - and yes, I've spent some time on this too. The service should run irrespective of the server.

1. Yes it does work fine with XMLRPC. I checked too. I won't bore you with all the XML - just the request, which I also checked:

<?xml version="1.0"?>
<methodCall>
<methodName>user.load</methodName>
<params>
  <param>
    <value><string>6018d475e8ddf59814ad89d0c3f8d6e0</string></value>
  </param>
  <param>
    <value><struct>
      <member><name>uid</name><value><int>51</int></value></member>
    </struct></value>
  </param>
</params>
</methodCall>

Yes, uid 51 does exist (user.get worked just fine in my previous post). Yes, XMLRPC gives me back the user <struct> correctly. Same values as user.get(51).

2. Well, yes I see your point. Of course it *is* an array in PHP, called $array, just a <struct> in XMLRPC.

Maybe there *is* an issue with the JSON server, or Web UI. I won't argue that one. Let me just say that it causes the error message because you're getting an object (via the JSON parser), not an array. That's why I simply cast to an array - just as node.save does (services/services/node_service.inc node_service_save($edit) and node_service_save_access($node)).

Ok, let's keep the Drupalers happy, if we're mapping a drupal function, we map the parameter names too - however confusing that may be to a non drupaler/XMLRPCer.

With regard to $uid on access, I simply put a watchdog in there:
watchdog('xmlrpc', print_r($uid, TRUE));
just before global $user; Try it, $uid is an array: Array ( [uid] => 51 ) . In your (first) case it would probably be Array ( [name] => 'newname' )
It works because in the test, while the first test fails; $user->uid is not equal to $uid, the second test passes because $user->uid is still not equal to $uid, *and* user_access('get any user data') is true. So as long as the user logged in ($user) has "get any user data" permission (a very high probability, given the service), it will always return TRUE, not FALSE.

I rest my case, m'lud. New patch with 'array' name attached.

Yes it works for JSON - at least via the Web UI. The workaround is to cast to an array, as per node.save. However user.save is broken in that respect (IMHO), but that's another battle.
Give it a try XMLRPC and JSON (web UI) It works for me, but it's nice to know if it works for others too!

That it gets in soon is another matter ;-)

Good. A healthy discussion is always better than pistols at dawn (I'm a poor shot). Bon chance!

As always, it's a bit more complicated than that. If you use API keys (which I'm inferring you do) then you're quite likely to be an anonymous user - see http://drupal.org/node/807638#comment-3013154. I don't use that technique, preferring that the XMLRPC client logs in as a specific user, with obviously a specific role and permissions. (Call me paranoid if you like ;-)

So I think this is the reason for the double test. Though I admit that reading the last paragraph back, I can't see how an anon user can do anything with the user services, given the test. But (my) life is too short for such details...

I'm not prepared to touch that code per se, but the mentioned thread is mine (http://drupal.org/node/807638) where I am trying to intregrate User Protect in a fairly transparent way (using menu_get_item) so that the XMLRPC client can't delete users that I'd prefer to keep, for sentimental reasons, such as uid = 0, and uid = 1, for example.

Again, after a couple of abortive patches, I stopped with a question, or idea if you prefer, but again, it's still waiting a reply...

The point is that I'm coming to the conclusion that I have a different perspective of 'protection' to the maintainers - or the original module developers. This may not be very fair, because I haven't looked at the development code, and hayrocker does mention that "things are changing down the road".

While it is perfectly reasonable that there are no sanity checks within the drupal core functions, that is, if you're creating a module or theme, it's your responsibility to "do the right thing"TM. But if you make these functions available to the outside world, via XMLRPC for example, then some sanity checking is necessary - otherwise a zealous (to use just one adjective) client can easily "do the wrong thing"TM - delete uid = 0, uid = 1, and so on. True, the argument goes that you can do that via the Web UI too, well at least uid = 1, but does that make it right? OK, you say, "there's a module for that", so I'll add User Protect, for example, but I have to hack services because it's not using the 'classic' access functions that I can 'override' via hook_menu_alter().

As another example the XMLRPC parser doesn't take into account the declared parameters and their types. You can easily send a <struct> to user.get and that works just fine (just tried it ;-) - because it doesn't check the type, the access method returns TRUE for the wrong reasons explained above, and you get the user back. Very flexible I admit, but by design? Hmm. Ah, it won't work in the Web UI because that *does* check the types, so there's no JSON filter...

I think I'm ranting, but it's mostly frustration. This is a good module, wildly useful, I just don't want to be clobbered by a malicious competitor who is writing the client side code - hope he doesn't read these threads...

John