Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuthenticationService::getUnauthenticatedRedirectUrl( Returns Relative URLs Without Ffull Base #684

Open
ravage84 opened this issue Nov 28, 2024 · 2 comments

Comments

@ravage84
Copy link
Member

Description

Why does Authentication\AuthenticationService::getUnauthenticatedRedirectUrl() returns URLs without full base?

return $url['path'] . '?' . $url['query'] . $fragment;

Image

This makes asserting redirects because of unauthenticated access a PITA.

$this->assertRedirect('/login?redirect=' . urlencode($redirect));

Leads to the following test error because of this:

Failed asserting that 'http://localhost/coupon-portal/login?redirect=%2Fcampaigns%2Fposition' equals content in header 'Location' (/coupon-portal/login?redirect=%2Fcampaigns%2Fposition).

Because Cake\TestSuite\IntegrationTestTrait::assertRedirect() always tests against full based URLs:

https://github.com/cakephp/cakephp/blame/33626a03197758f30a8ce8c2e0a75ed8ddbce40a/src/TestSuite/IntegrationTestTrait.php#L929

Wouldn't full based URL redirects be more sensible (& secure) to begin with?

CakePHP Version

4.x & (probably) 5.x

PHP Version

irrelevant

@dereuromark
Copy link
Member

I would argue that full based URLs are usually less secure, because they could redirect you away somewhere else.
So keeping it local prevents this kind of attack from the start.
Also, there can be cookie issues across even subdomains, so keeping it local by default also prevents issues here, or forever redirects or alike on misconfig.

@ADmad
Copy link
Member

ADmad commented Nov 28, 2024

Why does Authentication\AuthenticationService::getUnauthenticatedRedirectUrl() returns URLs without full base?

Because that's what's consistently done throughout the framework unless you specifically ask for a URL with full base through a method/option.

Because Cake\TestSuite\IntegrationTestTrait::assertRedirect() always tests against full based URLs:

Then the test method should be adjusted / have a config to assert URLs without the full base.

@markstory markstory transferred this issue from cakephp/cakephp Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants