As part of the implementation for Add a /users REST API endpoint, I would like to propose that we add a method hasAccess to UserManager, to allow checking what rights users have on other user resources, generically (without the need to resolve the target user resource beforehand).
The signature would be:
/**
* Checks if the user identified by {@code user} has the access identified by {@code right} on the other user
* identified by {@code target}.
*
* @param right the right to check
* @param user the user to check the right for
* @param target the target user on which to check the right
* @return {@code true} if the user has the specified right on the target user, {@code false} otherwise
*/
boolean hasAccess(Right right, UserReference user, UserReference target);
I don’t understand what you mean by to allow checking what rights users have on other user resources or if the user identified by {@code user} has the access identified by {@code right} on the other user identified by {@code target}. Could you elaborate?
At first sight, it feels weird for a user api to have a hasAccess API. That feels better targeted for an Authorization API.
AuthorizationManager targets are very entity oriented, so it feels better to have it in UserManager for now (and then the document implementation of UserManager is of course relying on AuthorizationManager). It’s not so different from UserManager#exist conceptually.
Thx but I still don’t understand what this is about
None of to allow checking what rights users have on other user resources or if the user identified by {@code user} has the access identified by {@code right} on the other user identified by {@code target} means anything to me (and they don’t mean the same thing so that’s weird too).
Maybe an example would help understand the need because a user having a right on another user doesn’t mean anything to me. Only thing that comes to mind is impersonation but I doubt that it’s the need.
Right, this might need some rephrasing work.
It’s about a user having specific rights on another user resource.
So, for a document store it would be a document and the rights can be mapped 1:1 with AuthorizationManager’s. If we had an external LDAP store, this would allow to check if a given user can access a specific record in read/write/delete in the same way.
So basically, if I get it right, this is about verifying if a given user can access another user’s data.
And there’s no notion of it being more fine-grained than that (ie we could imagine that a user could access a user’s first name and last name but not its phone number)?
If I still understand, it seems the idea is to generify the idea of being able to put rights on user profile page in XWiki, and to have a generic api so that it would work for any future user store.
Now a big problem is that in XWiki we can put permissions on a user page so that all members of a group can see it for ex. Since you’re passing UserReference and since we don’t have a new API for groups, we don’t know if that’ll work in the future for groups (it would work only if a Group Reference extends a UserReference, and that’s doubtful. We quickly discussed it (well, I proposed it and nobody replied) in the past (see User and Group APIs), and one idea was to have Users and Groups being Actors (i.e. have an ActorReference when you mean a reference to a Group or a User). Nothing was agreed though.
Answering to myself: Regarding the location, AuthorizationManager wouldn’t work since right now that API is only for Entities (documents, spaces, wikis, etc), and a UserReference is not an Entity in XWiki (maybe it should have been but it’s too late). So indeed, it makes sense to have that API inside the User module. So it could go in UserManager or in some new UserAuthorizationManager component.
So to conclude the biggest problem for me is the group use case.
What you are suggesting seems out of scope for this proposal to me.
If we end up having proper groups definition, I could expect a GroupManager component to provide this method for groups, and we could also have a generic ActorManager to dispatch the calls to the right manager. But in the end, it should not change the fact that the semantics for hasAccess(Right right, UserReference user, UserReference target) already need to include group membership in the computation (like an implementation for document would, by relying on AuthorizationManager).
Yes, it’s probably too complicated to introduce this now. We’ve discussed in the past about the idea of providing rights on xproperties but we haven’t concluded. I guess that if we go this way in the future, we would still be able to add a new signature like:
`List|Map<String, Boolean> hasAccess(Right right, UserReference user, UserReference target, List userProperties);``
(where userProperties would list all properties for which we want to check access for)
And the signature you propose could be kept as meaning: “does the user have access to all properties of the specified user?”.
