Vote has passed with four +1 and mostly general agreement. With Thomas, we have fix the missing features, see XWIKI-8931 and XWIKI-8933, and we are improving unit tests. So, I will switch the default RightService later today, the jira issue is XWIKI-8944 Your help will be welcome to fix any integration tests that start failing after that.
Thanks, On Sat, Mar 16, 2013 at 2:36 PM, Marius Dumitru Florea < [email protected]> wrote: > On Fri, Mar 15, 2013 at 9:44 PM, Denis Gervalle <[email protected]> wrote: > > On Fri, Mar 15, 2013 at 12:35 PM, Vincent Massol <[email protected]> > wrote: > > > >> > >> On Mar 15, 2013, at 12:14 PM, Denis Gervalle <[email protected]> wrote: > >> > >> > On Fri, Mar 15, 2013 at 12:06 PM, Jean-Vincent Drean <[email protected]> > >> wrote: > >> > > >> >> On Fri, Mar 15, 2013 at 11:25 AM, Denis Gervalle <[email protected]> > wrote: > >> >>> On Fri, Mar 15, 2013 at 10:57 AM, Vincent Massol < > [email protected]> > >> >> wrote: > >> >>> > >> >>>> > >> >>>> On Mar 15, 2013, at 10:50 AM, Denis Gervalle <[email protected]> > wrote: > >> >>>> > >> >>>>> On Fri, Mar 15, 2013 at 10:47 AM, Vincent Massol < > [email protected] > >> > > >> >>>> wrote: > >> >>>>> > >> >>>>>> > >> >>>>>> On Mar 15, 2013, at 10:33 AM, Denis Gervalle <[email protected]> > wrote: > >> >>>>>> > >> >>>>>>> On Fri, Mar 15, 2013 at 3:12 AM, Ludovic Dubost < > [email protected] > >> > > >> >>>>>> wrote: > >> >>>>>>> > >> >>>>>>>> 2013/3/14 Denis Gervalle <[email protected]> > >> >>>>>>>> > >> >>>>>>>>> On Thu, Mar 14, 2013 at 11:12 PM, Jerome Velociter < > >> >>>>>> [email protected] > >> >>>>>>>>>> wrote: > >> >>>>>>>>> > >> >>>>>>>>>> Hi Denis, > >> >>>>>>>>>> > >> >>>>>>>>>> Le 14/03/13 22:59, Denis Gervalle a écrit : > >> >>>>>>>>>> > >> >>>>>>>>>> On Thu, Mar 14, 2013 at 9:20 PM, Denis Gervalle < > [email protected]> > >> >>>>>>>> wrote: > >> >>>>>>>>>>> > >> >>>>>>>>>>> Hi devs, > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> We have a new (component based) authorization module since > a > >> >> while > >> >>>>>>>> now, > >> >>>>>>>>>>>> and I think 5.0 is the perfect time to introduce it as the > >> >> default > >> >>>>>>>>> right > >> >>>>>>>>>>>> service. First, I simply propose to change the default in > >> >>>> xwiki.cfg: > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> > >> >>>>>>>>> > >> >>>>>> > >> >> > xwiki.authentication.**rightsclass=org.xwiki.**security.authorization.** > >> >>>>>>>>>>>> internal.**XWikiCachingRightService > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> (Later, I propose that we deprecate that bridge and that we > >> >>>> create a > >> >>>>>>>>>>>> friendly (xwiki oriented) interface over the more generic > >> >>>>>>>>>>>> org.xwiki.security.**authorization.**AuthorizationManager. > But > >> >>>> leave > >> >>>>>>>>>>>> this for a > >> >>>>>>>>>>>> later proposal.) > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> So this vote is about changing the default in xwiki.cfg > before > >> >>>>>> 5.0M2. > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> pros: > >> >>>>>>>>>>>> - improved performance, since the new service is using > caching > >> >>>>>>>>>>>> techniques > >> >>>>>>>>>>>> and a single page load required lots of calls to it. > >> >>>>>>>>>>>> - ability for extension to add new rights > >> >>>>>>>>>>>> - define right declaratively > >> >>>>>>>>>>>> - separate method for checking and verifying right (throws > >> >> opposed > >> >>>>>>>> to > >> >>>>>>>>>>>> boolean return) > >> >>>>>>>>>>>> - fix some long waiting bugs like XWIKI-5174, XWIKI-6987, > as > >> >> well > >> >>>>>>>> as > >> >>>>>>>>>>>> some unstated ones > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> Also XWIKI-4550 > >> >>>>>>>>>>> > >> >>>>>>>>>>> - possibility to easily solve issues like XWIKI-4491 > >> >>>>>>>>>>>> - no more admin right per default > >> >>>>>>>>>>>> - being in good position to improve it and release > >> >> dependencies to > >> >>>>>>>>>>>> oldcore for security matters. > >> >>>>>>>>>>>> - possibility for third party to adapt the right settler to > >> >> their > >> >>>>>>>>>>>> special > >> >>>>>>>>>>>> needs (right decision is plugable) > >> >>>>>>>>>>>> - a consistant right evaluation with very few exception > that > >> >> could > >> >>>>>>>> be > >> >>>>>>>>>>>> explained and documented > >> >>>>>>>>>>>> > >> >>>>>>>>>>>> cons: > >> >>>>>>>>>>>> - no more admin right per default, but since we have DW, > the > >> >>>>>>>> initial > >> >>>>>>>>>>>> setup is no more a problem, and advanced users may use > >> >> superadmin. > >> >>>>>>>>>>>> - groups are only checked from the user wiki, not from the > >> >>>> accessed > >> >>>>>>>>>>>> entity wiki. > >> >>>>>>>>>>>> > >> >>>>>>>>>>> > >> >>>>>>>>>> This sound like a big regression. > >> >>>>>>>>>> > >> >>>>>>>>>> Can you explicit more ? Does this mean that adding a global > >> (main > >> >>>>>> wiki) > >> >>>>>>>>>> user in a local group has no effect ? > >> >>>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> You have got it right. This could be improved, and help is > >> >> welcome. > >> >>>>>> What > >> >>>>>>>>> happen is that the user groups are evaluated independently to > the > >> >>>>>>>> targeted > >> >>>>>>>>> entity, and therefore only in the user wiki. > >> >>>>>>>>> > >> >>>>>>>>> I admit this is a regression, but I have not cross lots of use > >> >> case > >> >>>>>> like > >> >>>>>>>>> those. The simple display in admin of Global user in local > Group > >> >> is > >> >>>>>> even > >> >>>>>>>>> broken (double xwiki:xwiki:...) so this does not seems to me a > >> >> common > >> >>>>>>>>> usage. > >> >>>>>>>>> You may provide access to global group in a local wiki to > achieve > >> >> the > >> >>>>>>>> same > >> >>>>>>>>> goals. > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>> This looks to be indeed a big regression. It's quite a common > use > >> >> case > >> >>>>>> to > >> >>>>>>>> have only global users and to create groups in the local wiki > that > >> >>>>>> refer to > >> >>>>>>>> local users. > >> >>>>>>>> > >> >>>>>>> ^^^^^^^^ > >> >>>>>>> I suppose you means global users here. > >> >>>>>>> > >> >>>>>>> IMHO, having user managed by a separate entity (global admin), > and > >> >>>> these > >> >>>>>>> same individual users grouped by another one (local admin) is > very > >> >>>>>> uncommon > >> >>>>>>> delegation of authority to me (but I may be wrong). On the other > >> >> hand, > >> >>>>>>> having a local admin providing access to local ressources to > global > >> >>>> group > >> >>>>>>> (and potentially some global users) makes more sense. In that > way, > >> >> the > >> >>>>>> same > >> >>>>>>> admin manage its users, and group its users, and the local admin > >> >> trust > >> >>>>>> the > >> >>>>>>> global admin to know its users. > >> >>>>>>> > >> >>>>>>> That said, I am not against any improvement on the way it > works, if > >> >> it > >> >>>>>> is a > >> >>>>>>> common use case (moreover used by workspace), we should > obviously > >> >>>> support > >> >>>>>>> it. However, I am convince that evaluating groups based on both > the > >> >>>> user > >> >>>>>>> and the targeted entity is not easily achievable and conduct to > >> very > >> >>>>>>> complex partial caching. > >> >>>>>>> > >> >>>>>>> I have currently not implemented in the security module anything > >> >> that > >> >>>>>> would > >> >>>>>>> cause all wikis to be scanned, and I would really like to avoid > >> >> that to > >> >>>>>>> happen. So, it will be difficult to avoid partial caching, but > we > >> >> need > >> >>>> to > >> >>>>>>> limit that at the higher level, the subwiki. This would allow to > >> had > >> >>>> only > >> >>>>>>> scan both the wiki of the user and the target entry to consider > our > >> >>>> cache > >> >>>>>>> valid. It means subwiki will be unable to share groups (I do not > >> >> think > >> >>>>>> this > >> >>>>>>> has ever worked), but it will keep performance on large farm. > >> >>>>>>> > >> >>>>>>> This would really need to be fixed sooner than later otherwise I > >> >> know > >> >>>>>>>> plenty of projects for which migration to 5.0 would be almost > >> >>>> impossible > >> >>>>>>>> > >> >>>>>>> > >> >>>>>>> I will need helps to achieve that for 5.0 > >> >>>>>> > >> >>>>>> ok so before changing anything we need a plan i.e. someone > >> >> volunteering > >> >>>> to > >> >>>>>> work on this, right? > >> >>>>>> > >> >>>>> > >> >>>>> Do we really need that for 5.0 ? > >> >>>>> Using the new module as a default does not means the old right > >> >> service is > >> >>>>> unavailable. Couldn't we simply define which case needs to revert > to > >> >> the > >> >>>>> old modules in the RN, and have 5.0 without this feature ? We may > >> even > >> >>>>> release XEM without it if workspace need so. > >> >>>> > >> >>>> I thought the config change you were proposing was global… I'm > lost… > >> >>>> > >> >>> > >> >>> It was, but I was not aware that workspace may need (to be > confirmed) > >> >> that > >> >>> special unsupported case. > >> >> > >> >> I don't see why you consider this use case as special, when all the > >> >> users are managed in the main wiki and you want local admins to be > >> >> able to manage groups in their wiki you need this. Or am I missing > >> >> something ? > >> >> > >> > > >> > I probably expressed myself badly. This is currently unsupported in > the > >> new > >> > module. I am just saying that even if we release a 5.0 with this > >> > regression, a simple like in xwiki.cfg will put back the old right > >> service > >> > for those who need this. First, only XEM is impacted, second only user > >> with > >> > this kind of delegation need that. So, this is not the general case > IMO. > >> > >> Sure but it's not that simple. We certainly don't want users to be in a > >> situation where their wiki doesn't work, then they spend time trying to > >> understand. After a few days they post to the list and then only change > the > >> config property. This will be bad for us. It has to work. Of course we > >> could put it in the Release Notes but it won't help much in practice. > >> > >> I'd prefer that we have someone committed to work on this before we > change > >> the default so that we're sure someone is going to work on this. > >> > >> In the meantime, maybe you could start a branch where you have it by > >> default so that we can start fixing build/tests? > >> > > > > This is really a lot of works for just a single line commit (redefining > all > > jenkins modules for the branch) > > I think everyone has been +1 on the principe to push this new component > > forward, so I see no point on doing that aside. > > > > I already made some preliminary test on my own machine, here are the > > results: > > (a few tests failed whatever the RightService used, however, here is the > > delta) > > > > UITest: 1 failure, 1 error > > SeleniumTest: 4 failures > > Rest: 2 failures > > Storage: 3 failures > > WYSIWYG: 3 failures > > > > This does not seems much to me in regards to the change. If you all > agree, > > I would like to commit on master ASAP, so all of you may help fixing > those > > tests. > > > > Thomas proposed to help me fixing and improving tests while I add the > > missing feature of global user in local group on monday (Good news, I > > already some code that seems to be working, need to optimize a bit and > > test thoroughly). > > If everyone take a minimum of time to help fixing their own tests as > > needed, I do not see why we could not be ready for 5.0M2. > > And this leave us some more time to fix potential hidden issue before RC. > > > > So, do you agree to go that way and give this long awaited improvement a > > real boost ? > > +1. I'll help fixing the failing tests. > > Thanks, > Marius > > > > > > >> > >> Thanks > >> -Vincent > >> > >> > See my reply to Ludovic for more about how we need to work on that > >> missing > >> > feature. > >> > > >> > > >> >> > >> >> JV. > >> _______________________________________________ > >> 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 > _______________________________________________ > 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

