Re: [xwiki-devs] [Proposal] Add api to allow scripts choose with the permission of their author.

On Mon, Mar 22, 2010 at 18:57, Caleb James DeLisle &lt;calebdelisle(a)lavabit.com Well, I am not sure this is the proper way to do so. Switching user will in fact also change the author that is doing the save, and therefore loose any information about the real author of the change, something the current API currently prevent. Wouldn't it be better to just do something similar to saveWithProgrammingRight(), by simply checking edit right against the content author, and proceed to the normal saveDocument() ? For this one, checking against the content author, that the document is viewable could be enough as well, avoiding a useless context switch, no ? With the above remarks, I see no real inconvenience to have these two functions added in the old API, since it fit well with the saveWithPR one. It could reduce the need of PR which is always welcome. I would also like to add a configuration parameter to xwiki.cfg (these I do not agree that the author could be safer. Both have issues. Do you really need this ? Denis > WDYT? > > > Caleb > > > > Caleb James DeLisle wrote: > > > > Denis Gervalle wrote: > >> On Wed, Mar 17, 2010 at 10:29, Caleb James DeLisle < > calebdelisle(a)lavabit.com > >>> >>> After realizing that it would be a huge pain to prevent the author > being > >>> set to the > >>> viewer on document save I would like to propose this: > >>> > >>> api.Document.saveAsContentAuthor() > >>> > >> api.XWiki.getDocumentAsContentAuthor(String Space, String Name) > >> > >>> The names are relatively self explanatory, They will act like the > normal > >>> load and save > >>> methods but checking rights on the document's content author instead of > the > >>> viewer. > >>> > >> Sorry, but I really need some more explanation, which document ? the > content > >> author of which document ? > > > > It would be the document in the context which (we hope) is the document > containing > > the code being executed. This is the same way that hasProgrammingRights > works. > > > > The security could be improved by throwing an error if > context.get("isInRenderingEngine") > > does not return Boolean.TRUE. > > > > Caleb > > > >> > >>> If these prove safe and useful then we could look into adding rename > and > >>> delete later. > >>> > >> Cannot say if it is useful or safe, it depends on the interpretation I > made > >> of your proposal. For sure, it could became very unsafe depending on the > >> implementation! > >> > >> Denis > >> > >>> WDYT? > >>> > >>> Caleb > >>> > >>> > >>> Caleb James DeLisle wrote: > >>>> Vincent Massol wrote: > >>>>> On Mar 16, 2010, at 12:52 PM, Caleb James DeLisle wrote: > >>>>> > >>>>>> Vincent Massol wrote: > >>>>>>> On Mar 16, 2010, at 11:08 AM, Caleb James DeLisle wrote: > >>>>>>> > >>>>>>>> Denis Gervalle wrote: > >>>>>>>>> On Tue, Mar 16, 2010 at 03:46, Caleb James DeLisle < > >>> calebdelisle(a)lavabit.com > >>>>>>>>>> >>>>>>>>>> Right now a script which is saved with programming access is > >>> allowed to do > >>>>>>>>>> things which > >>>>>>>>>> the viewer (who executes it) is not allowed to do. I would like > to > >>> extend > >>>>>>>>>> this to scripts which do not > >>>>>>>>>> have programming access. > >>>>>>>>>> > >>>>>>>>>> Use case example: > >>>>>>>>>> A page where the viewer can submit an email address to be put on > a > >>> mailing > >>>>>>>>>> list. The page needs to be able to > >>>>>>>>>> save the email address to a list but the viewer should not be > able > >>> to > >>>>>>>>>> directly edit (or view) the list. > >>>>>>>>>> > >>>>>>>>>> I would like to add to xwiki.api.Context: > >>>>>>>>>> void setScriptUser(String) > >>>>>>>>>> String getScriptUser() > >>>>>>>>>> (These names are the best I could think of, suggestions would be > >>> great) > >>>>>>>>>> the input can be either "viewer", "author" or "default" and it > >>> stores this > >>>>>>>>>> in the XWikiContext > >>>>>>>>>> If the value is set to author, hasAccessLevel will return an > access > >>> check > >>>>>>>>>> on the author, not the viewer. > >>>>>>>>>> > >>>>>>>>>> When includeForm is called, or the include macro is invoked, the > >>> only > >>>>>>>>>> explicit change is that changes made by > >>>>>>>>>> the called document will be reset when the include ends. > >>>>>>>>>> If scrips are set to run with author's permissions: > >>>>>>>>>> If DocumentA includes documentB, documentB executes with > >>> documentB's > >>>>>>>>>> author's permissions. > >>>>>>>>>> If DocumentA includes _in context_ documentB, documentB executes > >>> with > >>>>>>>>>> documentA's author's permissions. > >>>>>>>>>> When include of documentB ends, scriptUser is set back to the > value > >>> which > >>>>>>>>>> documentA left it at. > >>>>>>>>>> > >>>>>>>>>> I think this will help us achieve the goal of no documents with > >>> programming > >>>>>>>>>> rights in the XE xar file. > >>>>>>>>>> > >>>>>>>>>> WDYT? > >>>>>>>>>> > >>>>>>>>>> Caleb > >>>>>>>>>> > >>>>>>>>>> _______________________________________________ > >>>>>>>>>> devs mailing list > >>>>>>>>>> devs(a)xwiki.org > >>>>>>>>>> http://lists.xwiki.org/mailman/listinfo/devs > >>>>>>>>>> > >>>>>>>>> First of all, I am personally against adding anything to the > >>> RightService in > >>>>>>>>> its current state. This is already spaghetti code which already > lead > >>> to some > >>>>>>>>> security risks, and trying to improve that partly could be an > >>> impossible > >>>>>>>>> challenge. Moreover, this code is absolutely not optimized, you > just > >>> have to > >>>>>>>>> put it under debug logging to understand what I mean. > >>>>>>>> +1 I read it and I know exactly what you mean, I *think* a rights > >>> check could be done > >>>>>>>> in one query. > >>>>>>>> > >>>>>>>> I was only talking about modifying api.Context and api.Api I have > no > >>> desire to touch > >>>>>>>> user.impl.xwiki.ANYTHING! > >>>>>>>> > >>>>>>>>> Regarding the way you expect the inclusion of documents to > function, > >>> I am > >>>>>>>>> not convince that this is the right choice. The behavior of > >>> inclusion has > >>>>>>>>> been broken since Syntax 2.0 has been introduced, and I have > always > >>> feel > >>>>>>>>> this to be a serious regression. My principal use case is a > document > >>> (that > >>>>>>>>> could have been created by a user) editable by a user, and that > >>> store > >>>>>>>>> objects in itself, and use an inclusion in context for its > content. > >>> In this > >>>>>>>>> case, the user may save the document, which should therefore be > >>> without any > >>>>>>>>> special rights, but the code may need to have higher privileges, > and > >>> still > >>>>>>>>> require to be included in context for access to the including > >>> documents. > >>>>>>>>> This is the typical Sheet case shown in many sample of XWiki > >>> Programming, > >>>>>>>>> but with programming right requirement. I have used that many > times > >>> before > >>>>>>>>> it became broken, and this design is still missing to me. > >>>>>>>> You bring up an important point which I think is worthy of it's > own > >>> email. I have thought > >>>>>>>> it would be nice to allow PR code to extend the api or to add > >>> velocity macros > >>>>>>>> which will run with programming right. > >>>>>>>> > >>>>>>>>> I am convince that the rights given to a block of code should be > >>> tightly > >>>>>>>>> linked to the content author > >>>>>>>> I hereby ammend my proposal to include contentAuthor as a > possibility > >>> for the set method, I had > >>>>>>>> forgotten about it. > >>>>>>>> > >>>>>>>>> of its containing document, and absolutely not > >>>>>>>>> based on any author in the context in which it is executed. > >>>>>>>> Do you mean the viewer? I believe code should never be executed in > >>> context of it's viewer > >>>>>>>> because code may set a trap which saves itself when the viewer has > >>> greater privileges than the > >>>>>>>> code. I propose the set method because I am -1 to changing default > >>> behavior. > >>>>>>>>> This has been > >>>>>>>>> broken in Syntax 2.0, and this currently leads to elevation of > >>> privilege > >>>>>>>>> through inclusions. For the same reason, I do not follow Alex on > the > >>> idea > >>>>>>>>> that the right should be set in place of a user. This would lead > to > >>> many > >>>>>>>>> problem in regards of who and how this could be set, stored and > >>> inherited. > >>>>>>>>> I completely agree that the programming right is currently not > >>> perfect, but > >>>>>>>>> in the same time, I do not see the real value you expect to > provide. > >>> There > >>>>>>>>> is already a similar mechanism in place, using the content > author, > >>> opposed > >>>>>>>>> to the last author of a page. > >>>>>>>> I want to extend that same mechanism to authors who do not have > >>> programming rights, > >>>>>>>> PR is dangerous and should be used as little as possible and if we > >>> need it in the default > >>>>>>>> xar that is an API flaw IMO. > >>>>>>>> > >>>>>>>>> This is what make commenting page possible > >>>>>>>>> without breaking its programming behavior. Extending this to > manage > >>> other > >>>>>>>>> objects in my use case above, is probably a good track to follow. > >>> But again, > >>>>>>>>> this should be part of a global reengineering of the > RightService. > >>>>>>>>> > >>>>>>>>> If you want to rethink the RightService starting from scratch, I > >>> will surely > >>>>>>>>> participate ! I have already some ideas, but not been able to put > >>> them > >>>>>>>>> properly in a document. Time is always missing, but I hope this > to > >>> change... > >>>>>>>> I absolutely do want to rethink the RightService, however I think > it > >>> is held up by the fact > >>>>>>>> that components have no access to the HttpServletRequest. > >>>>>>> Just answering this last point. This is not quite true. Components > do > >>> have access to the Container component which represents the > environment. > >>>>>> Getting the HttpServletRequest requires an if instanceof cast hack. > >>> Case in point Request, Response and Session are not used anywhere in > the > >>> core. > >>>>>>> However, I don't think it's good that the authentication service > >>> relies on it. What do you need exactly from the request? You should > expose > >>> that need in the Execution Context. > >>>>>> For simple form/cookie authentication the cookies, request > parameters, > >>> and maybe the user agent and ip address for a "nothing up my sleeve" > hash > >>> should suffice. > >>>>>> For foaf+ssl, I suspect request headers will be needed and some way > to > >>> prove that it is in fact https. > >>>>>> I'm not sure what openId will need. > >>>>>> If we are to continue using org.securityfilter we must pass it a > full > >>> httpServletRequest. > >>>>>>> For example imagine that you need some unique id representing the > >>> session. If so, then this could be stored in the EC directly or through > a > >>> more coarsed-grained object. > >>>>>>> This would allow the authentication service to work in different > >>> environments: servlet, portlet, deamon threads, etc. > >>>>>> In the case of daemon threads we can either make them craft a dummy > >>> HttpServletRequest or we make them conform to a standard which we > invent (x, > >>> y and z must be in the ExecutionContext). > >>>>>> I don't like seeing component implementations become dependent on > their > >>> container but it bothers me to see new code go into the old core > because the > >>> necessary resources are hidden and exposing them is politically too > >>> difficult. > >>>>>>> The other option (maybe even better) is to authenticate the user at > >>> the level of the environment, i.e. in Container implementations. And > the > >>> output of the environment level would be to have a User object put in > the > >>> Execution Context. I feel this second option is even better than the > first > >>> since it works without introducing an additional variable in the EC and > >>> after all the authentication is related to the environment to some > degree. > >>>>>> If I understand you right this would and preclude advanced > >>> authentication such as foaf+ssl or openid which is not supported by the > >>> environment. > >>>>>> I also seem to remember hearing that tomcat was mangling session ids > >>> (users becoming logged in as someone else.) > >>>>>> I propose that we make available the actual request, response and > >>> session using generics instead of the casting trick then we make it a > best > >>> practice to avoid using them unless necessary. > >>>>> Adding new methods to the XWiki session, request and response is ok > and > >>> is easy to do. The problem is that once you get past the environment > ring, > >>> no component should access the environment anymore as otherwise it > makes > >>> these components work only in a specific environment. > >>>>> Here's what I mean: > >>>>> http://tinycoke.com/_6rr5a0xjRU7Xg/architecture-layers.png > >>>> I agree with the line of thinking, the question is how to express that > in > >>> the code. > >>>> I imagine the xwiki-authentication directory containing: > >>>> xwiki-authentication-api > >>>> and > >>>> xwiki-authentication-servlet-openid > >>>> and so on. > >>>> > >>>> Another thought is it would be helpful to provide utility interfaces > >>> which extend both portlet and servlet request, response and session. > >>>> This would allow for component implementations which can support both. > >>>> > >>>> How about that? > >>>> > >>>> Caleb > >>>> > >>>>> wdyt? > >>>>> > >>>>> Thanks > >>>>> -Vincent > >>>>> > >>>>> _______________________________________________ > >>>>> devs mailing list > >>>>> devs(a)xwiki.org > >>>>> http://lists.xwiki.org/mailman/listinfo/devs > >>>>> > >>>> _______________________________________________ > >>>> devs mailing list > >>>> devs(a)xwiki.org > >>>> http://lists.xwiki.org/mailman/listinfo/devs > >>>> > >>> _______________________________________________ > >>> devs mailing list > >>> devs(a)xwiki.org > >>> http://lists.xwiki.org/mailman/listinfo/devs > >>> > >> > >> > > > > _______________________________________________ > > devs mailing list > > devs(a)xwiki.org > > http://lists.xwiki.org/mailman/listinfo/devs > > > > _______________________________________________ > devs mailing list > devs(a)xwiki.org > http://lists.xwiki.org/mailman/listinfo/devs > -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO