We use this on an intranet where all URL's except a few are redirected to the login page. Some of these are typical bots that look for security issues.
One that we've been hit with recently is a request like this:
https://ourdomain.tld/:88/favicon.ico
This redirects to:
/saml/login?destination=/:88/favicon.ico
Which logs an exception like this:
InvalidArgumentException encountered during initiating SAML login: The internal path component ':88/favicon.ico' is external. You are not allowed to specify an external URL together with internal:/
And redirects away, but becauase there's a destination query string, it goes straight back to that page, which redirects again back to saml/login, resulting in a redirect loop and lots of warnings in our logs.
Not sure. Ignore invalid destinations? Could also make sure to remove the destination when redirecting, would still come back then in our case, but without the destination.
Comments
Comment #2
roderik(1AM typo in commit message. Commit is here.)
.
Thanks for the report. Yes, this is a bug... which I never consciously registered.
There's supposedly-always an exception handler active, which takes care of returning a (redirect) response to an error page. But the 'destination' parameter should always be removed in that case. In other words: the 'destination' wasn't removed early enough.
This makes DOMAIN/saml/login?destination=/:88/favicon.ico at DOMAIN/:88/favicon.ico, instead of the error page... which is a bug.
Bug fixed by removing the 'destination' properly/earlier. "The error page" by default is the homepage (with the standard message saying there was an error which has been logged). If that isn't good for your specific case, you can set an "Error redirect URL" in config.
(I briefly considered (additionally) checking/removing invalid 'destination' values, but that seems to be the
RequestSanitizer::checkDestination()'s job, so I won't duplicate that.)Comment #3
roderik