Martin, Done: https://issues.apache.org/jira/browse/WICKET-6161
Thank you! 2016-05-04 23:38 GMT-07:00 Martin Grigorov <[email protected]>: > Hi Ilia, > > Please create a ticket so we don't forget it! > Thanks! > > Martin Grigorov > Wicket Training and Consulting > https://twitter.com/mtgrigorov > > On Wed, May 4, 2016 at 7:32 PM, Илья Нарыжный <[email protected]> wrote: > >> For now I just removed BookmarkableMapper. Everything works. But it seems >> to me that some cases might go wrong... And as side effect: hrefs just >> empty to pages without mounts. Probably I would expect some other behavior >> if url can't be resolved for a page. >> >> Thanks, >> >> Ilia >> On May 4, 2016 6:54 AM, "Martin Grigorov" <[email protected]> wrote: >> >> > On Wed, May 4, 2016 at 3:27 PM, Sven Meier <[email protected]> wrote: >> > >> > > Hi, >> > > >> > > well, it seems I wasn't completely out of my mind when I pushed for >> > > WICKET-5094: >> > > - I've checked 1.4 and the logic of #enforceMounts was exactly like it >> is >> > > now >> > > - the javadoc for #setEnforceMounts() matches the current behavior: >> > > >> > > "Sets whether mounts should be enforced. If true, requests for mounted >> > > targets have to done through the mounted paths. If, for instance, a >> > > bookmarkable page is mounted to a path, a request to that same page via >> > the >> > > bookmarkablePage parameter will be denied." >> > > >> > > For those trying to prevent any requests to non-mounted pages: Couldn't >> > > you just remove the BookmarkableMapper? >> > > >> > > ICompoundRequestMapper mappers = >> > getRootRequestMapperAsCompound(); >> > > mappers.forEach((mapper) -> {if (mapper instanceof >> > > BookmarkableMapper) mappers.remove(mapper); }); >> > > >> > > Personally I wouldn't mind to change/remove/rename this setting for >> > Wicket >> > > 8.x, so it is more useful. >> > > >> > >> > +1 to change the behavior to what it was after WICKET-3849 and before >> > WICKET-5094 >> > >> > >> > > >> > > Have fun >> > > Sven >> > > >> > > >> > > >> > > On 04.05.2016 08:23, Martin Grigorov wrote: >> > > >> > >> Hi, >> > >> >> > >> I also think the current behavior is not correct. See my question at >> > >> http://markmail.org/message/xmo74m3tbc5v4nwp. >> > >> I read the name of the method "enforceMounts" as "do not allow urls to >> > >> page >> > >> which are not explicitly mounted". I believe also this is the reason >> > this >> > >> method is in SecuritySettings, and not in PageSettings. >> > >> And its javadoc also says the same. That's why I've -reintroduced this >> > >> behavior with https://issues.apache.org/jira/browse/WICKET-3849. >> > >> >> > >> According to Sven the behavior in Wicket 1.4.x was different and he >> > >> changed >> > >> it with https://issues.apache.org/jira/browse/WICKET-5094. >> > >> IMO Wicket 1.4.x must had a bug but there is no one to confirm :-/ >> > >> >> > >> Martin Grigorov >> > >> Wicket Training and Consulting >> > >> https://twitter.com/mtgrigorov >> > >> >> > >> On Wed, May 4, 2016 at 7:57 AM, Илья Нарыжный <[email protected]> wrote: >> > >> >> > >> Martin, >> > >>> >> > >>> Checked this issue: >> https://issues.apache.org/jira/browse/WICKET-5094 >> > >>> Absolutely disagree with discussed behavior. It's meaningless to >> > >>> prevent accessing /wicket/bookmarkable/<CLASS> only if there is mount >> > >>> point for that page. >> > >>> Please help to find consensus. In mine case it's real security hole. >> > >>> >> > >>> Thanks, >> > >>> >> > >>> Ilia >> > >>> >> > >>> 2016-05-03 22:50 GMT-07:00 Илья Нарыжный <[email protected]>: >> > >>> >> > >>>> Martin, >> > >>>> >> > >>>> Just checked: it doesn't work as expected. It seems that this code >> > >>>> doesn't work as it was assumed: >> > >>>> >> > >>>> BookmarkableMapper.java >> > >>>> if (application.getSecuritySettings().getEnforceMounts()) >> > >>>> { >> > >>>> // we make an exception if the homepage itself was mounted, see >> > >>>> >> > >>> WICKET-1898 >> > >>> >> > >>>> if (!pageClass.equals(application.getHomePage())) >> > >>>> { >> > >>>> // WICKET-5094 only enforce mount if page is mounted >> > >>>> if (isPageMounted(pageClass, >> > >>>> application.getRootRequestMapperAsCompound())) // HERE!!! >> > >>>> { >> > >>>> return null; >> > >>>> } >> > >>>> } >> > >>>> } >> > >>>> >> > >>>> Imho condition at line marked by HERE!!! should be opposite. >> > >>>> Please check. >> > >>>> >> > >>>> In my case getSecuritySettings().setEnforceMounts(true); doesn't >> have >> > >>>> any effect. >> > >>>> >> > >>>> Thanks, >> > >>>> >> > >>>> Ilia >> > >>>> >> > >>>> 2016-05-03 10:59 GMT-07:00 Илья Нарыжный <[email protected]>: >> > >>>> >> > >>>>> Thank you Martin! I did know that there should be easier way to do >> > >>>>> that, but could not be able to find it:) >> > >>>>> >> > >>>>> Regards, >> > >>>>> >> > >>>>> Ilia >> > >>>>> >> > >>>>> 2016-05-03 0:06 GMT-07:00 Martin Grigorov <[email protected]>: >> > >>>>> >> > >>>>>> Hi, >> > >>>>>> >> > >>>>>> I always thought >> > >>>>>> that >> org.apache.wicket.settings.SecuritySettings#getEnforceMounts() >> > is >> > >>>>>> >> > >>>>> for >> > >>> >> > >>>> this. Also its javadoc seems to say that. >> > >>>>>> But there were some changes to its behavior after which I am no >> more >> > >>>>>> >> > >>>>> sure >> > >>> >> > >>>> what exactly it does :-/ >> > >>>>>> >> > >>>>>> Martin Grigorov >> > >>>>>> Wicket Training and Consulting >> > >>>>>> https://twitter.com/mtgrigorov >> > >>>>>> >> > >>>>>> On Tue, May 3, 2016 at 8:53 AM, Илья Нарыжный <[email protected]> >> > wrote: >> > >>>>>> >> > >>>>>> Yea - that's possible. Even instrumentation is possible, but >> > probably >> > >>>>>>> this problem somehow solved already in wicket. I would briefly >> > >>>>>>> summarize the problem like: >> > >>>>>>> >> > >>>>>>> Wicket allow to directly address bookmarkable pages from 3rd >> party >> > >>>>>>> libraries without good way to manage accessibility. >> > >>>>>>> Potentially it means that with having control over some 3rd >> partly >> > >>>>>>> lib >> > >>>>>>> it's possible to include "backdoor page" >> > >>>>>>> Thanks, >> > >>>>>>> >> > >>>>>>> Ilia >> > >>>>>>> >> > >>>>>>> >> > --------------------------------------------------------------------- >> > >>>>>>> To unsubscribe, e-mail: [email protected] >> > >>>>>>> For additional commands, e-mail: [email protected] >> > >>>>>>> >> > >>>>>>> >> > >>>>>>> >> > --------------------------------------------------------------------- >> > >>> To unsubscribe, e-mail: [email protected] >> > >>> For additional commands, e-mail: [email protected] >> > >>> >> > >>> >> > >>> >> > > >> > > --------------------------------------------------------------------- >> > > To unsubscribe, e-mail: [email protected] >> > > For additional commands, e-mail: [email protected] >> > > >> > > >> > >>
