Re: [xwiki-devs] [VOTE] Merge experimental xwiki-platform-security on master

Hi all, As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done: 1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO. 2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined. 3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode. 4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code. 5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first. 6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right. The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed. Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation. Here is my +1 for the merge on 4.x, WDYT ?