3 +1, 1 +0, no -1, merged On Fri, Feb 17, 2012 at 09:42, Andreas Jonsson <[email protected]> wrote:
> +1 > > Best Regards, > > /Andreas > > 2012-02-15 11:17, Denis Gervalle skrev: > > 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 ? > > > > > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
