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.

Sign up for GitHub

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

BUGFIX: Configurable uriPathSuffix in EventSourcedFrontendNodeRoutePa… #5189

Draft
wants to merge 3 commits into
base: 9.0
Choose a base branch
from
+6 −5
Draft

BUGFIX: Configurable uriPathSuffix in EventSourcedFrontendNodeRoutePa… #5189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f993ea
BUGFIX: Configurable uriPathSuffix in EventSourcedFrontendNodeRoutePa…
samsauter Jul 29, 2024
621ba52
Merge branch 'neos:9.0' into 9.0
samsauter Aug 12, 2024
684251d
BUGFIX: add `@Flow\IgnoreValidation("node")` to node's showAction().
samsauter Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Neos.Neos/Classes/Controller/Frontend/NodeController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class NodeController extends ActionController
* @Flow\SkipCsrfProtection We need to skip CSRF protection here because this action could be called
* with unsafe requests from widgets or plugins that are rendered on the node
* - For those the CSRF token is validated on the sub-request, so it is safe to be skipped here
*/
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fluke?

public function previewAction(string $node): void
{
// @todo add $renderingModeName as parameter and append it for successive links again as get parameter to node uris
Expand Down Expand Up @@ -187,9 +187,9 @@ public function previewAction(string $node): void
* @throws \Neos\Flow\Mvc\Routing\Exception\MissingActionNameException
* @throws \Neos\Flow\Session\Exception\SessionNotStartedException
* @throws \Neos\Neos\Exception
* @Flow\SkipCsrfProtection We need to skip CSRF protection here because this action could be called
* with unsafe requests from widgets or plugins that are rendered on the node
* - For those the CSRF token is validated on the sub-request, so it is safe to be skipped here
* We need to skip CSRF protection here because this action could be called with unsafe requests from widgets or plugins that are rendered on the node - For those the CSRF token is validated on the sub-request, so it is safe to be skipped here
* @Flow\SkipCsrfProtection
* @Flow\IgnoreValidation("node")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this should be needed – IIRC IngoreValidation should be only needed for object arguments!? What happens if you omit this?

Copy link
Author

@samsauter samsauter Aug 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I submit a form which redirects to a node (e.g. via Neos.Form or a Neos.Form:Builder rendered form) without this change, the request ends in a 404 and I get the error in the logs CSRF: token was empty but a valid token is required for Neos\Neos\Controller\Frontend\NodeController::showAction().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be solved by the SkipCsrfProtection, the IgnoreValidation is also necessary and gives you the same error otherwise?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly @kitsunet. Without the IgnoreValidation, I get the mentioned error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if the problem here is in any way possibly related to #4909 ? ill have to experiment with this myself

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bwaidelich and @kitsunet i guess this is related to our misconception that the NodeController is infact part of the backend authentication?
But then this part should be a 8.3 bugfix?

*/
public function showAction(string $node): void
{
Expand Down
3 changes: 2 additions & 1 deletion Neos.Neos/Classes/FrontendRouting/EventSourcedFrontendNodeRoutePartHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ public function matchWithParameters(&$requestPath, RouteParameters $parameters)
throw new ResolvedSiteNotFoundException(sprintf('No site found for siteNodeName "%s"', $siteDetectionResult->siteNodeName->value));
}

$remainingRequestPath = $this->truncateRequestPathAndReturnRemainder($requestPath, $resolvedSite->getConfiguration()->uriPathSuffix);
$uriPathSuffix = $this->options['uriPathSuffix'] ?? $resolvedSite->getConfiguration()->uriPathSuffix;
$remainingRequestPath = $this->truncateRequestPathAndReturnRemainder($requestPath, $uriPathSuffix);

$dimensionResolvingResult = $this->delegatingResolver->fromRequestToDimensionSpacePoint(
RequestToDimensionSpacePointContext::fromUriPathAndRouteParametersAndResolvedSite(
Expand Down
Loading