Hi,
FYI I have now remembered why we used to have
@PreAuthorize("hasRole('" + StandardEntitlement.ANONYMOUS + "')")
on SAML2SPLogic#getMetadata: the reason was that metadata were supposed to be
get through the SAML2SPAgent, which acts as HTTP interface for all SAML 2.0
operations - this is also the reason why downloading metadata via Admin Console
used to work even with the entitlement set as above.
I wonder if we should revert such change...
Regards.
On 2017-08-14 18:13, Colm O hEigeartaigh <cohei...@apache.org> wrote:
> Excellent, thanks!
>
> On Mon, Aug 14, 2017 at 4:13 PM, Francesco Chicchiriccò <ilgro...@apache.org
> > wrote:
>
> > Hi,
> > I have committed the changes discussed below as
> >
> > https://github.com/apache/syncope/commit/e99766a441260bb47db
> > ac13efe069682dd4a442d
> > https://github.com/apache/syncope/commit/f912d90c2aa23c055cf
> > b2e143e865f02025154bd
> >
> > Regards.
> >
> > On 11/08/2017 18:02, Colm O hEigeartaigh wrote:
> >
> >> On Fri, Aug 11, 2017 at 2:59 PM, Francesco Chicchiriccò <
> >> ilgro...@apache.org> wrote:
> >>
> >> Agree. Maybe it should just be changed to
> >>>
> >>> @PreAuthorize("isAuthenticated()")
> >>>
> >> +1.
> >>
> >>
> >> b) The urlContext not validated at all. For example, you can pass through
> >>>
> >>>> something like "../../root" which is added to the metadata, e.g.
> >>>> Location="
> >>>> http://localhost:9080/syncope/../../root/assertion-consumer";.
> >>>>
> >>>> Should we implement some kind of validation rules on what is acceptable
> >>>> here?
> >>>>
> >>>> What do you have in mind here? Just forbid '../'? What could be the
> >>> issue(s) with the current implementation?
> >>>
> >>
> >> Well it makes me a bit uneasy that a user can essentially insert
> >> arbitrary
> >> URLs into a metadata document issued by Syncope. I have three suggestions:
> >>
> >> a) Restrict the acceptable values. Do we need to allow arbitrary contexts
> >> here...?
> >> b) Disallow ".." in the values
> >> c) Use Commons UrlValidator to make sure that the generated assertion
> >> consumer URL is a valid URL.
> >>
> >> Colm.
