On 16/08/2017 13:06, Colm O hEigeartaigh wrote:
Maybe we could revert but also explicitly allow the admin role?

Why? The idea is that metadata should be downloaded via the URL managed by the agent (e.g. /syncope-console/saml2sp/metadata), not via REST.

Regards.

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.

--
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/

Reply via email to