AbstractLoginFormAuthenticator::supports() always returns false if application lives under a directory #44893

weaverryan · 2022-01-02T18:24:57Z

Discussed in #44329

^{Originally posted by php4fan November 28, 2021}

Symfony version(s) affected

5.4.0

Description

Say you have an application deployed to https://somedomain.com/my/path, as opposed to just https://somedomain.com/.
I think you usually call that an application that "lives under a subdirectory" (or directory, or folder).

I expect EVERYTHING in the framework to work out of the box with ZERO configuration in that scenario. Generally, Symfony does know how to handle that.

Out-of-the-box login authentication however seems to be one of those things that do not work as expected in the under-a-directory scenario.

I think that's due to the AbstractLoginFormAuthenticator::supports() method whose current implementation is:

    return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getPathInfo();

In this scenario, $this->getLoginUrl($request) returns /my/path/login, while $request->getPathInfo() returns just /login when the url being requested is /my/path/info.

How to reproduce

Deploy a Symfony application under a directory.
Make a LoginFormAuthenticator with make:auth
Try to log in.

Possible Solution

No response

Additional Context

No response

The text was updated successfully, but these errors were encountered:

weaverryan · 2022-01-02T18:27:14Z

This was originally #44318. I've re-reopened it because, to me, it does indeed look like an issue. It's fairly simple: if you deploy your site to a sub-directory (e.g. https://example.com/cool-app), then the AbstractLoginFormAuthenticator will always return false:

return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getPathInfo();

In this scenario, $this->getLoginUrl($request) returns /my/path/login, while
$request->getPathInfo() returns just /login when the url being requested is /my/path/info.

Let me know if I'm mistaken about this being an issue :).

Crell · 2022-01-12T20:21:38Z

A closely related issue: The same incorrect behavior happens if the site is not using clean URLs, vis, example.com/index.php/login.

The root problem seems to be the use of UrlGenerator to make the path in getLoginUrl() (the default generated version). The generate() method, by design, creates a link to the specified route as a browser would need it. However, that's not the value you want when comparing against an internal path. You instead want just the path equivalent of it.

There doesn't see to be any obvious way to get that, other than simply hard coding the login path, rather than login route name. But that is the fix that would be needed.

a-r-m-i-n · 2022-04-05T15:53:15Z

I can confirm this issue. When Symfony is running in a sub-directory (in my case an apache alias) the supports() always returns false, because /login does not match with /sub/login.

As workaround, I've overwritten the supports method like this:

    public function supports(Request $request): bool
    {
        return parent::supports($request) || ($request->isMethod('POST') && $this->getLoginUrl($request) === '/sub' . $request->getPathInfo());
    }

That works.

weaverryan · 2022-04-10T23:41:39Z

It would need some testing, but the solution might be a more generic version of what @a-r-m-i-n suggests:

public function supports(Request $request): bool
    {
    return parent::supports($request) || ($request->isMethod('POST') && $this->getLoginUrl($request) === $request->getBaseUrl() . $request->getPathInfo());
}

Basically, leverage $request->getBaseUrl() to "re-add" any subdirectory or index.php prefix before doing the comparison.

sgehrig · 2022-08-17T15:30:23Z

@weaverryan Is there anything new on this issue? If not, I'd try to prepare a PR. We just stumbled upon the same issue and it cost us quite some time until we realised that this one line causes issues.

wouterj · 2022-08-18T09:12:16Z

@sgehrig it would be great if you can submit a PR to 5.4 if you've verified that the code works :)
(and maybe somehow write a test, but I think that will be a tricky one)

… rewriting or from a sub folder

sgehrig · 2022-08-18T11:25:38Z

@wouterj Done. See #47317.

… url rewriting or from a sub folder (sgehrig) This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Security] Fix login url matching when app is not run with url rewriting or from a sub folder | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44893 | License | MIT | Doc PR | not required Uses the fix suggested by `@weaverryan` in #44893 (comment). I also added three tests for scenarios which I could replicate from running a simple app on a real webserver (Apache and Nginx). This, however, might not be sufficient because there could be other combinations of server variables like `DOCUMENT_ROOT`, `PHP_SELF`, `SCRIPT_FILENAME`, `SCRIPT_NAME` and possibly others depending on the server configuration and setup. As long as `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` and `\Symfony\Component\HttpFoundation\Request::getPathInfo()` work correctly, I assume that the fix will also be correct in all those constellations. The fix is based on the assumptions that: - `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` always returns an empty string when the application is run from root without the front controller script in the URL (using URL rewriting for example) - `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` always returns the path from the server root to the application base path (possibly including the front controller script) - `\Symfony\Component\HttpFoundation\Request::getPathInfo()` always returns just the *routed* part of the request Please advise if you'd need some more tests. Commits ------- ff340e2 [Security] Fix login url matching when app is not run with url rewriting or from a sub folder

… url rewriting or from a sub folder (sgehrig) This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Security] Fix login url matching when app is not run with url rewriting or from a sub folder | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44893 | License | MIT | Doc PR | not required Uses the fix suggested by `@weaverryan` in symfony/symfony#44893 (comment). I also added three tests for scenarios which I could replicate from running a simple app on a real webserver (Apache and Nginx). This, however, might not be sufficient because there could be other combinations of server variables like `DOCUMENT_ROOT`, `PHP_SELF`, `SCRIPT_FILENAME`, `SCRIPT_NAME` and possibly others depending on the server configuration and setup. As long as `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` and `\Symfony\Component\HttpFoundation\Request::getPathInfo()` work correctly, I assume that the fix will also be correct in all those constellations. The fix is based on the assumptions that: - `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` always returns an empty string when the application is run from root without the front controller script in the URL (using URL rewriting for example) - `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` always returns the path from the server root to the application base path (possibly including the front controller script) - `\Symfony\Component\HttpFoundation\Request::getPathInfo()` always returns just the *routed* part of the request Please advise if you'd need some more tests. Commits ------- ff340e2128 [Security] Fix login url matching when app is not run with url rewriting or from a sub folder

derrabus added Bug Security labels Jan 2, 2022

carsonbot added the Status: Needs Review label Jan 2, 2022

sgehrig mentioned this issue Aug 18, 2022

Problem with AbstractLoginFormAuthenticator::supports #42542

Closed

sgehrig added a commit to sgehrig/symfony that referenced this issue Aug 18, 2022

[symfony#44893] fixes login url matching when app is not run with url…

e6e71c2

… rewriting or from a sub folder

sgehrig mentioned this issue Aug 18, 2022

[Security] Fix login url matching when app is not run with url rewriting or from a sub folder #47317

Merged

sgehrig added a commit to sgehrig/symfony that referenced this issue Aug 18, 2022

[symfony#44893] fixes complaint about void in tests (fabbot.io)

808030d

sgehrig added a commit to sgehrig/symfony that referenced this issue Aug 18, 2022

[symfony#44893] fixes CS as per fabbot.io

1ffa730

sgehrig added a commit to sgehrig/symfony that referenced this issue Aug 18, 2022

[symfony#44893] adds some negative tests

847b190

wouterj mentioned this issue Sep 20, 2022

Authenticator does not support the request. when using reverse proxy path #47633

Closed

fabpot closed this as completed Sep 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AbstractLoginFormAuthenticator::supports() always returns false if application lives under a directory #44893

AbstractLoginFormAuthenticator::supports() always returns false if application lives under a directory #44893

weaverryan commented Jan 2, 2022

Symfony version(s) affected

Description

How to reproduce

Possible Solution

Additional Context

weaverryan commented Jan 2, 2022

Crell commented Jan 12, 2022

a-r-m-i-n commented Apr 5, 2022

weaverryan commented Apr 10, 2022

sgehrig commented Aug 17, 2022

wouterj commented Aug 18, 2022 •

edited

sgehrig commented Aug 18, 2022

AbstractLoginFormAuthenticator::supports() always returns false if application lives under a directory #44893

AbstractLoginFormAuthenticator::supports() always returns false if application lives under a directory #44893

Comments

weaverryan commented Jan 2, 2022

Discussed in #44329

Symfony version(s) affected

Description

How to reproduce

Possible Solution

Additional Context

weaverryan commented Jan 2, 2022

Crell commented Jan 12, 2022

a-r-m-i-n commented Apr 5, 2022

weaverryan commented Apr 10, 2022

sgehrig commented Aug 17, 2022

wouterj commented Aug 18, 2022 • edited

sgehrig commented Aug 18, 2022

wouterj commented Aug 18, 2022 •

edited