| # |
Jun 21st 2021, 15:47 |
 kevin.pfeifer |
i see, thx :) |
| # |
Jun 21st 2021, 15:47 |
ndm |
Not with regards to preventing absolute URLs from slipping in, no, `getLoginRedirect()` will prevent that just fine, it just needs to be actually used :) |
| # |
Jun 21st 2021, 15:44 |
kevin.pfeifer |
so this isn’t an issue with the authentication plugin? I don’t have a “pure” authentication app ready to test it |
| # |
Jun 21st 2021, 15:40 |
ndm |
You may want to open an issue with cakedc/users so that they can possibly fix it. |
| # |
Jun 21st 2021, 15:35 |
kevin.pfeifer |
but isn't that exactly what is being discussed here? https://github.com/cakephp/authentication/issues/450 |
| # |
Jun 21st 2021, 15:32 |
kevin.pfeifer |
I can confirm the external redirect from the `redirect_to=<url_encoded-URL>` param when using cakedc/users which is based on the authentication plugin |
| # |
Jun 21st 2021, 14:36 |
a.vanniel |
we jumped on the Authentication middleware with version 1.x ... I guess we forgot to change this part for 2.x ... I feel a bit stupid now. |
| # |
Jun 21st 2021, 14:31 |
ndm |
`getLoginRedirect()` was introduced with some early 1.x version I think, so at some point the docs were different. |
| # |
Jun 21st 2021, 14:25 |
a.vanniel |
we actually used that page to build what we have now... has this been changed? or did we just mess things up at some point? |
| # |
Jun 21st 2021, 14:25 |
ndm |
https://book.cakephp.org/authentication/2/en/index.html#building-a-login-action |
| # |
Jun 21st 2021, 14:23 |
a.vanniel |
where do I find docs to do this properly ... or in other words, where do I find docs to read how this should be fixed? |
| # |
Jun 21st 2021, 14:21 |
a.vanniel |
```$redirect = $this->request->getQuery('redirect', [ 'controller' => 'Pages', 'action' => 'display', 'home' ]);``` I think this must be old and has not been updated at some point... |
| # |
Jun 21st 2021, 14:21 |
a.vanniel |
o dear... I just noticed something... |
| # |
Jun 21st 2021, 14:20 |
ndm |
What does your redirect code look like? |
| # |
Jun 21st 2021, 14:20 |
a.vanniel |
(running Authentication 2.6.1 by the way) |
| # |
Jun 21st 2021, 14:19 |
a.vanniel |
might we have done something wrong on our end to allow redirect to go to an external page after loggin in? I just tried it again... came to https://www.our-site.com/users/login?redirect=%2F and I changed the redirect to https://www.some-evil-site.com/ and got redirected to the external site after logging in... |
| # |
Jun 21st 2021, 14:17 |
a.vanniel |
odd... |
| # |
Jun 21st 2021, 14:02 |
ndm |
Yes, it really should. https://github.com/cakephp/authentication/blob/2.6.1/tests/TestCase/AuthenticationServiceTest.php#L751-L755 |
| # |
Jun 21st 2021, 13:59 |
slackebot1 |
illegal urls. |
| # |
Jun 21st 2021, 13:59 |
a.vanniel |
@ndm really? because I manually changed redirect for the Authentication middleware plugin in the login page and after logging in, i was redirected to the google search engine landing page... Also, I am looking to do this in a place somewhere other then login ... it was just an example ... not meant to say there is a bug or security hole in there... I just was trying to figure out if and how that middleware handles external |
| # |
Jun 21st 2021, 13:56 |
ndm |
Sorry, ment to tag @a.vanniel |
| # |
Jun 21st 2021, 13:55 |
ndm |
@alexdd55976 Use the authentication component, `getLoginRedirect()` will only return relative URLs, if someone crams in an absolute URL (internal or external), you'd receive `null`. |
| # |
Jun 21st 2021, 13:07 |
a.vanniel |
for now I did what you suggested regarding the host by the way... checking if the host in the redirect url (full) matches the current host... if not, it most likely isn't internal |
| # |
Jun 21st 2021, 13:06 |
a.vanniel |
It would be great to have an isInternal method inside Router... I don't think many people actually need this though.. I will open an issue on github. see where it goes. good luck on your 2nd shot... I need to wait until 12th of juli for the 2nd one. |
| # |
Jun 21st 2021, 13:01 |
kevin.pfeifer |
I would suggest you create an issue in the cakephp/cakephp repo. I dont know it/cant check right since im on my way to get my 2. vacine shot :) Or someone else can answer that ^^ |
| # |
Jun 21st 2021, 12:22 |
a.vanniel |
What I don't really get, but there might some logic to this, is why the Google website counts as an existing route....? |
| # |
Jun 21st 2021, 12:17 |
a.vanniel |
would normalize be able to help me out here? |
| # |
Jun 21st 2021, 12:16 |
slackebot1 |
would like to prevent manipulation of the redirect param so that someone is not redirected anywhere externally after payment is done and after a user is coming back... the url is stored in the database for several reasons, one of which is because the payment provider is a pain in the behind about return urls. |
| # |
Jun 21st 2021, 12:16 |
a.vanniel |
it's not that I would want this to be added to the middleware per se... it was more of a thing that I am using similar logic (redirect param) and after doing the thing, redirect the user to the redirect param and when I looked at the Authentication middleware I did not see any security in place to prevent someone/something from redirecting someone to something externally after a login... I am doing this after a payment and I |
| # |
Jun 21st 2021, 12:11 |
kevin.pfeifer |
we could make that a configurable option |
| # |
Jun 21st 2021, 12:11 |
kevin.pfeifer |
then that would have to be added to this function i guess https://github.com/cakephp/authentication/blob/de989c759937406f514a5a31313b36578005b07c/src/AuthenticationService.php#L360 |
| # |
Jun 21st 2021, 12:09 |
a.vanniel |
maybe I am just making things more complex than need be but it feels like a securiy hole in the location I have this ?redirect= param... |
| # |
Jun 21st 2021, 12:08 |
a.vanniel |
but that is when a user is about to or just has been redirected, right? |
| # |
Jun 21st 2021, 12:07 |
kevin.pfeifer |
you could use https://github.com/cakephp/cakephp/blob/master/src/Routing/Router.php#L225 and catch the MissingRouteException |
| # |
Jun 21st 2021, 12:07 |
a.vanniel |
alright... thing is that with the Authentication middleware, relative redirects are used, to make sure the param does not get too large I guess... |
| # |
Jun 21st 2021, 12:06 |
a.vanniel |
I thought I'd take a look at Router::routeExists() but that actually returns true for external URLs. ... |
| # |
Jun 21st 2021, 12:06 |
kevin.pfeifer |
to be fair, all the redirect param logic I know always uses absolute urls |
| # |
Jun 21st 2021, 12:04 |
kevin.pfeifer |
well if you force your redirect param to be absoulte all the time then you can check that pretty easily with a parse_url |
| # |
Jun 21st 2021, 11:59 |
a.vanniel |
I though I would check how the Authentication middleware handles this... but when you open a login screen, change the ?redirect= to something else and log in, you are then redirected to the external page I entered in ?redirect=... I had expected this would really force redirect to be local/matching route but it doesn't |
| # |
Jun 21st 2021, 11:57 |
a.vanniel |
the latter might not work when a relative path unless the Router first qualifies the use given url ofcourse |
| # |
Jun 21st 2021, 11:57 |
slackebot1 |
matches a route" and the "check for host" method... |