search

OpenConext / Stepup-saml-bundle

A PHP Symfony bundle that adds SAML capabilities to your application using simplesamlphp/saml2
Apache License 2.0
14 stars 24 forks source link

Why does HostedEntities::generateUrl reset the request context? #42

Closed ddeboer closed 8 years ago

ddeboer commented 8 years ago

At https://github.com/SURFnet/Stepup-saml-bundle/blob/develop/src/Entity/HostedEntities.php#L141. This prevents me from setting a custom host on the request context.

Introduced by https://github.com/SURFnet/Stepup-saml-bundle/commit/f9852abf656b76e5cf520b3e7c4d3c255fc38ea5.

DRvanR commented 8 years ago

Hello,

this is done so that when you generate the EntityId for an Hosted Entity when in a subrequest (e.g. as result of forwarding the request) the correct routing context (the one from the masterrequest) is used so that the domain name of the host handling the request is included in the full URL (rather than localhost).

What issue requires that you actually override the hostname (thus pretending to be a different host than the one the request went to)? Perhaps the same could be achieved in a different manner :smile:

ddeboer commented 8 years ago

Symfony allows setting values such as host on RequestContext. HostedEntities currently overrides any RequestContext values set by the user.

My use case: I need to offer a metadata endpoint to our identity provider even for our local development environments, that cannot be reached from outside. The way I solved this, is by setting our dev hostname in the RequestContext and then offer that on our publicly available endpoint (that I called /metadata-dev).

DRvanR commented 8 years ago

HostedEntities currently overrides any RequestContext values set by the user.

That is correct, so that it always generates the correct URL including the HostName of the current host.

With regards to your use case, I can see the problem. Personally I've never had the problem as I've always used a local IdP for developement purposes (a quick SimpleSamlPHP install usually does the trick). Perhaps that is a solution? Not sure if it helps though :wink: Alternatively, you could just comment out the offending lines, generate the metadata, save the xml in an accessible location and then change the metadata URL in the metadata XML so that it points to that xml file. Also not quite sure if this would actually help you...

With respect to the library, I do not really see how this could be solved in such a manner that all usecases are covered (with respect to subrequest and overridable RoutingContext values). I'm reluctant to allow overriding the host for the hosted entity as there is no good way to detect something like that (at least not that I can see). How would the library decide to use the RoutingContext if it does not know whether or not the hostname has been altered? This potentially means that the application cannot be trusted as to stating which host it actually is, which would be nasty with regards to being a SAML Entity...

@rjkip any thoughts?

As an aside: the documentation you are referring to is for a console application and seems indeed to be intended for outside a web context (see the note in the section here). Doing this is in a web context could mean that the routes you generate are incorrect.

DRvanR commented 8 years ago

Hi @ddeboer, is this still an issue? Been thinking about it, but couldn't really find a way to change this in a reliable and safe way, without resorting to provide an optional override of the hostname, which is a bit counterintuitive...

DRvanR commented 8 years ago

Hello @ddeboer, I'm going to close this issue due to inactivity.

Should this still be an issue, feel free to open a new issue. Thanks!