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.


>
> Regards.
>
> --
> Francesco Chicchiriccò
>
> Tirasa - Open Source Excellence
> http://www.tirasa.net/
>
> Member at The Apache Software Foundation
> Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
> http://home.apache.org/~ilgrosso/
>
>


-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Reply via email to