Add admin flag to users API
|Assignee:||Go MAEDA||% Done:|
Currently, it is not possible to distinguish admin users from "normal" users via the API. This patch adds the
admin flag to the users API. The flag is added to the index action (visible only to admins) and on the show action to admins only.
Note that due to a peculiarity of the API builder, the field is only included in JSON responses if the value is
true. For XML, it is included with
false values respectively. With the JSON API, it is thus not possible to distinguish a list of non-admin users from a list of users without the permission to see the admin status.
#2 Updated by Go MAEDA over 1 year ago
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
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.
#3 Updated by Holger Just over 1 year ago
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
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
/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
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
adminfield is present and contains a
truevalue, the user is an admin
- If you are querying
adminfield 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.
#6 Updated by Marius BALTEANU over 1 year ago
- Status changed from Reopened to Closed
- Assignee changed from Holger Just to Go MAEDA