Hi,
I have committed the changes discussed below as
https://github.com/apache/syncope/commit/e99766a441260bb47dbac13efe069682dd4a442d
https://github.com/apache/syncope/commit/f912d90c2aa23c055cfb2e143e865f02025154bd
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.
--
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/
