Maybe we could revert but also explicitly allow the admin role?
Colm.
On Wed, Aug 16, 2017 at 8:26 AM, Francesco Chicchiriccò <ilgro...@apache.org
> wrote:
> 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.
>
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
