neos / flow-development-collection

The unified repository containing the Flow core packages, used for Flow development.
https://flow.neos.io/
MIT License
139 stars 188 forks source link

Authentication Redirect with subfolder fails #1797

Closed kaystrobach closed 5 years ago

kaystrobach commented 5 years ago

Description

instead of redirecting to /t/package/authentication/index flow 4.3.18+ redirects to `/t/t/package/authentication/index

Steps to Reproduce

  1. flow in subfolder (symlink)
  2. enable rewrite

Expected behavior

same as 4.3.17 redirect to the correct auth endpoint

Actual behavior

s.a.

Affected Versions

Flow:

albe commented 5 years ago

Probably a regression from https://github.com/neos/flow-development-collection/pull/1682/files#diff-d9281be63867d9c5a9b3d0fce5380ee5R67?

kaystrobach commented 5 years ago

Any idea how to help to fix this problem? I’m not so much into the routing component.

albe commented 5 years ago

Best thing would be to have just a simple test case that fails with 4.3.18 and is green with 4.3.17. Afterwards the change could be adjusted to cover the new "fixed" behaviour and still work as before without regression.

kaystrobach commented 5 years ago

what do you need to reproduce?

basicly i just added the .htaccess to rewrite with a subdirectory.

kaystrobach commented 5 years ago

problem is, that i need to understand that part preciscly to add more valuable information.

kaystrobach commented 5 years ago

As far as i can see the redirect is triggered here:

Packages/Framework/Neos.Flow/Classes/Security/Authentication/EntryPoint/WebRedirect.php:50

            $actionName = $this->extractRouteValue($routeValues, '@action');
            $controllerName = $this->extractRouteValue($routeValues, '@controller');
            $packageKey = $this->extractRouteValue($routeValues, '@package');
            $subPackageKey = $this->extractRouteValue($routeValues, '@subpackage');
            $uri = $this->uriBuilder->setCreateAbsoluteUri(true)->uriFor($actionName, $routeValues, $controllerName, $packageKey, $subPackageKey);

The code should always return an absolute URI, das the function setCreateAbsolute is called with the parameter true.

Out Settings for the Authentication Provider look like:

Settings.yaml

Neos:
  Flow:
    security:
      enable: true
      authorization: null
      authentication:
        providers:
          DefaultProvider:
            provider: PersistedUsernamePasswordProvider
            entryPoint: WebRedirect
            entryPointOptions:
              uri: Vendor.Package/Login/index
              routeValues:
                '@package': Vendor.Package
                '@controller': Login
                '@action': index

The uri part can be safely removed, as the first condition matches true on routeValues.

Our directory structure looks like this:

/
/testing    --> symlink to flow framework deployed with typo3.surf
/testing/index.php

.htaccess

        # Enable URL rewriting
        RewriteEngine On

        # Set flag so we know URL rewriting is available
        SetEnv FLOW_REWRITEURLS 1

        # You will have to change the path in the following option if you
        # experience problems while your installation is located in a subdirectory
        # of the website root.
        RewriteBase /testing/
 SetEnv FLOW_CONTEXT Production/Testing

        # Stop rewrite processing no matter if a package resource, robots.txt etc. exists or not
        RewriteRule ^(_Resources/Packages/|robots\.txt|favicon\.ico) - [L]

        # Stop rewrite process if the path points to a static file anyway
        RewriteCond %{REQUEST_FILENAME} -f [OR]
        RewriteCond %{REQUEST_FILENAME} -l [OR]
        RewriteCond %{REQUEST_FILENAME} -d
        RewriteRule .* - [L]

        # Perform rewriting of persistent private resources
        RewriteRule ^(_Resources/Persistent/[a-z0-9]+/(.+/)?[a-f0-9]{40})/.+(\..+) $1$3 [L]

        # Perform rewriting of persitent resource files
        RewriteRule ^(_Resources/Persistent/.{40})/.+(\..+) $1$2 [L]

        # Make sure that not existing resources don't execute TYPO3 Flow
        RewriteRule ^_Resources/.* - [L]

        # Continue only if the file/symlink/directory does not exist
        RewriteRule (.*) index.php
kaystrobach commented 5 years ago

btw. we have the identical problem in 7 project, so we pinned the framework version for now, but a fix is highly appreciated.

kaystrobach commented 5 years ago

still a problem in 4.3.21

kaystrobach commented 5 years ago

@albe do you have any idea how to solve that? maybe you can provide an entry point, so that i can dig deeper, we have that problem in some of our projects now.

albe commented 5 years ago

Looking at https://github.com/neos/flow-development-collection/compare/4.3.17...4.3.18 to find some code that changed between 4.3.17 and 4.3.18 that could be related and the only thing there is the change mentioned above. So I would first start at that location and try commenting out that section in ResolveContext. The routing is not my strongest point in Flow, but maybe @bwaidelich has a bright idea, why that change could lead to the above regression?

Edit: Well, how I read that code, the issue must be that

Therefore I'd check where the baseUri is coming from (do you have it configured in settings? otherwise it's automatically resolved from the request URI iirc) and why the resolved URI also contains that path prefix. Maybe we need to revert the fix even.

davidspiola commented 5 years ago

Hi Folks, is this issue resolved with #1682?

kaystrobach commented 5 years ago

@davidspiola was that released already, or do i need to cherry-pick that change for testing?

davidspiola commented 5 years ago

It is in the change log of 4.3.23 -> https://flowframework.readthedocs.io/en/4.3/TheDefinitiveGuide/PartV/ChangeLogs/4323.html

kaystrobach commented 5 years ago

ok, so I will deploy it to one of our staging envs now - give me a moment.

kaystrobach commented 5 years ago
Updating neos/flow (4.3.17 => 4.3.23): 

mhmm coverage takes very long this time.

kaystrobach commented 5 years ago

will tell you tomorrow. ... it's time to end work for today.

davidspiola commented 5 years ago

Ok, no stress. Good night

kaystrobach commented 5 years ago

so now i disabled the coverage, and i think i will move it to the nightly build.

will check for the current state now.

kaystrobach commented 5 years ago

deployed now (no coverage for now), but it seems to work without any problems. all the fe tests we use are green again.