Redmine

IMHO, it is better to remove '|| (User.current == @user)' from the patch. Non-admin users can know whether they are admin or not by accessing /users.(xml|json).

Index: app/views/users/show.api.rsb
===================================================================
--- app/views/users/show.api.rsb    (revision 17471)
+++ app/views/users/show.api.rsb    (working copy)
@@ -1,6 +1,7 @@
 api.user do
   api.id         @user.id
   api.login      @user.login if User.current.admin? || (User.current == @user)
+  api.admin      @user.admin? if User.current.admin? || (User.current == @user)
   api.firstname  @user.firstname
   api.lastname   @user.lastname
   api.mail       @user.mail if User.current.admin? || !@user.pref.hide_mail

When a non-admin user gets their information via XML API, the response contains an '<admin>' element and they can know that they are non-admin user by seeing the value. But when a non-admin user accesses JSON API, it is impossible to make sure that whether they are admin or not because the response does not have 'admin' key, as you already mentioned. The behavior and specification will be different between XML and JSON. I think it is confusing.

Go MAEDA wrote:

IMHO, it is better to remove '|| (User.current @user)' from the patch. Non-admin users can know whether they are admin or not by accessing /users.(xml|json).

Unfortunately, only admin users can access /users.(xml|json). The endpoint is not accessible to "normal" users at all. Even in the web UI, there is currently no possibility for a non-admin user to find which users have admin permissions. Thus, I think there is a general need for any kind of check to restrict this information to only authorized persons.

For the API, I think the most common use-case for this added flag is for an API client to find whether its given credentials grant them admin permissions. If we remove the check for || (User.current @user), this check will be sort-of possible still, since admin users will still return true in /users/current.(json|xml) here while non-admin users will not contain the field at all (both in JSON and XML). Deoending on the client, this might make it a bit awkward to distinguish between a true value and a missing tag, but could probably be made to work.

If we retain the check, for XML, we will have a true or false value in the output for the current user, making it more consistent here. It will still not include the field for other users.

JSON will still only include the field if the value is true (which I think is a bug in the API builder which should be fixed separately). However, I think JSON clients are usually better equipped to handle "fluid" responses than XML clients.

Thus, in the end, I still think that it might a bit more consistent to include the field for the current user (esp. if or when we get to fix this API builder issue), handling the field the same way as the other filtered user fields. But I don't want to fight over this. If you think it should be removed to only show the field to admin users in any case, I can live with that too.

In both cases, you the following rule holds:

• If the admin field is present and contains a true value, the user is an admin
• If you are querying users/current.(xml|json) and the admin field is missing or false, the current user is not an admin
• If you are querying a different user and the field is not included, you don't know anything.