2012-03-28 19:08, Denis Gervalle skrev: > On Wed, Mar 28, 2012 at 11:24, Vincent Massol <[email protected]> wrote: > >> Hi Denis, >> >> On Mar 28, 2012, at 11:04 AM, Denis Gervalle wrote: >> >> [snip] >> >>>> Denis, do you plan to refactor some existing code to use this new API? >> IMO >>>> this would be the best next step to prove that it works. >>> >>> >>> It works, I use it everyday ! Their minor differences only impact really >>> special usage. >> >> We all know the "it works on my machine" syndrome but that doesn't prove >> anything… :) >> >> As I said in another mail we really need a very code unit test coverage. >> >>>> Fo you have any idea about what code could be migrated? >>>> >>> >>> IMO, we may start using it for new components, or components that we are >>> refactoring, since it will allow being independent of the old core for >>> these component. This module may works for a while in parallel with the >>> existing service. >> >> I was asking for an example of what we could start with. Do you have any >> idea? >> >>> By merging it, and providing it as an option, it will allows more user to >>> try it. There is a bridge between the old service and this new one that >> you >>> may simply activate in xwiki.cfg. This new service improve performance, >>> allow adding more rights on the fly and allow customizing the way it >> works, >>> so it is an answer to those wanting these improvements. >>> >>> >>>> We need to be careful not to keep this new security module not used for >>>> too long as otherwise it could "rot". >>> >>> >>> It will not stay unused on my side, so be sure I will stay close to it, >>> since I have written it because I need those improvements. I really >>> encourage anyone to try it ! >>> >>> >>>> So IMO we need to start using it ASAP and then have a plan for switching >>>> to it. >>>> >>> >>> IMO, the switch will be welcome when we have discussed the small >>> differences it has, and fix the few we do not tolerate. In particular, >> the >>> startup of an empty wiki require login has superadmin with this new >> module. >>> I know this require more documentation to be written, I am really aware >> of >>> that, this just require time to be done... hopes Andreas and others could >>> helps as well. >> >> For me you are now responsible for this part (even though technically >> we're all responsible of course since we use no code ownership ;) - but for >> example Thomas and I know more the Rendering and are fixing stuff there, >> while others have more knowledge in other parts). > > > Have I said the contrary ? All I said is that I will do my best supporting > it, but this is really a large part, and some help is also welcome. I have > taken the time to have it at the current state in the hope we will be able > to move out of the crappy service we use currently more that 20 times per > requests. It is now mature enough to start using it, since we do, and... >
I will also support this module, of course, and help out fixing issues with it, and add more unit tests. I feel much more motivated to work on it when it is on track to replace the old implementation. Best Regards, /Andreas >> I understand you wanted this in the platform because you're using it >> already. > > > ...no, I am not so concerned about having it in platform for using it. I > now trust my module more than the existing implementation. I would like to > have it in platform to encourage others to look at it, use it, adopt it and > contribute to it, since I am really confident it is really better than the > current implementation. > > >> I just want to be sure we're going to maintain it since I can see it'll >> require a lot more work before being to full switch to this implementation. >> > > IMO, there is even more work to do to use the current right service, since > its own test cover only 52% of the right service itself and the > documentation is really sparse and almost unwritable ! (BTW, these tests > cannot be used on our bridge since these use implementation method that are > not published in the interface.) > > >> >> Thanks >> -Vincent >> >>>> Thanks >>>> -Vincent >>>> >>>> On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote: >>>> >>>>> 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 >> > > > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
