[xwiki-devs] [Proposal] Changing the RightService API

I will make two proposals for changing the RightService API: The first does not actually change any code, but it is a preparation for future changes. I want to be sure that the methods checkAccess and hasAccessLevel aren't used interchangeably; it should be well defined when to use one or the other. The second proposal is a complete rework of the API, which might be a bit further into the future. 1. Change the semantics of hasAccessLevel slightly. Currently the method hasAccessLevel is defined as: /** * Verifies if the user identified by {@code username} has the * access level identified by {@code right} on the document with * the name {@code docname}. I would like to make a change the describing text into: /** * Decides whether the access level given by {@code right} should * be granted during rendering of the document identified by * {@code docname} by request of the user identified by {@code * username}. In other words, the rights queried for is tied to the thread executing a request on behalf of the user. In contrast, the checkAccess method queries for the configured rights of a particular user. This suggests that the underlying implementation may choose to base its decision on more things than the given user and document, and thus opens up for more possibilities to change the security policy. For instance, hasAccessLevel("programming", context.getUser(), docname, context) could be made equivalent to hasProgrammingRights(context.getWiki().getDocument(docname), context) In the current implementation, XWikiRightServiceImpl, the methods checkAccess and hasAccessLevel can be used interchangeably once the user have been authenticated. But it seems that the current usage pattern more or less matches the above change in semantics already. 2. Deprecate all methods in the current API and introduce the following new ones: boolean checkAccess(String action, EntityReference entity); boolean userHasRight(DocumentReference userIdentity, Right right, EntityReference entity); boolean hasAccessLevel(Right right); Iterable<Right> getEnabledRights(EntityType entityType); The differences are: 1. The context parameter is dropped. The right service can have an Execution instance injected instead. 2. No exceptions are thrown. Instead, on error, log and deny access. 3. Identify users by the DocumentReference of their profile page. 4. Introduction of the enum 'Right'. 5. The rights are controlled against entities identified by EntityReferences that may be of type WIKI, SPACE or DOCUMENT. 6. Add a new method, userHasRight, for querying for the specific rights of a given user on a given entity, where the entity may be wiki, space or document. This method replaces the method hasAdminRights. I guess this method will be primarily used by the user interface. 7. hasAccessLevel have the same change in semantics as mentioned above, and could thus replace the hasProgrammingRights methods. In other words, hasAccessLevel provides the answer to the question "should access for an operation that requires the specified right be granted given the current context?" Within the lifecycle of a request, first a call should be made to checkAccess to authenticate the user and to grant access to the specific action. Subsequent security checkpoinst should normally use hasAccessLevel. 8. The listAllLevels-method is moved to the Right enum class. The right service could instead provide a method that lists what rights are enabled at a particular level in the document hierarchy, i.e., the getEnabledRights method, which I guess is relevant for the rights manager. So, what do you think? Best regards, Andreas Jonsson