Bug #96813
closed
BeforeRedirectEvent is not useful at all because it accepts no arguments and the return value is ignored
100%
Description
Steps to reproduce:
- Register your event class at the
BeforeRedirectEvent - Let felogin trigger a redirect
- the event gets fired
- Problem: you have no way to do anything meaningful because the result value is not returned and no objects (e.g. the
LoginConroller) are passed as argument.
The event gets the redirect URL and the login-type passed as parameter. But you neither can return a changed redirect URL nor change the calling controller (e.g. to inject another redirect URL).
The Event should provide at least one way to actually use the result from the event.
I flagged this as a bug, not as a feature, because the feature itself (the event) is already there, but it's just dead code.
Updated by S P over 2 years ago
- Related to Bug #92068: felogin (extbase) redirect from GET/POST is not working properly added
Updated by S P over 2 years ago
Reproducing step 2 should be "Let felogin trigger a releoad redirect", of course.
Updated by S P over 2 years ago
- Related to Bug #90157: missing possibility to send return_url Parameter with 403 standard errorhandler added
Updated by S P over 2 years ago
- Related to Bug #91844: felogin Redirect to referer is not working at all with Site config 403 errorHandler added
Updated by S P over 2 years ago
- Related to Epic #96814: Totally broken felogin redirect mechanism added
Updated by S P over 2 years ago
For people with similar problems:
In the event I do this:
$myChangedRedirectUrl = $event->getRedirectUrl();
// here change $myChangedRedirectUrl
\header('Location: ' . $myChangedRedirectUrl);
throw new \TYPO3\CMS\Extbase\Mvc\Exception\StopActionException();
This is totally off-framework, but afaics the only way to achieve anything with this event. Feels totally ugly and hacky.
Updated by Oliver Hader over 2 years ago
BeforeRedirectEvent does work, I just does not match with your expectations.
- adding new setters/getters for new
modifiedRedirectUrlproperty might be fine - and also fine using
$event->getModifiedRedirectUrl() ?? $event->getRedirectUrl()when redirecting in https://github.com/TYPO3/typo3/blob/58558fee9ad4897016bd9a21a02a1c0a07595f6a/typo3/sysext/felogin/Classes/Controller/LoginController.php#L232
The original change in issue #88740 has been merged two years ago for TYPO3 v10.4.0.
Updated by S P about 2 years ago
- Subject changed from BeforeRedirectEvent is not working to BeforeRedirectEvent is not useful at all because it accepts no arguments and the return value is ignored
- Description updated (diff)
Updated by Benni Mack over 1 year ago
I'm fine changing this. Reading through the passive aggressive rants in this issue took me longer than the actual fix.
Updated by Gerrit Code Review over 1 year ago
- Status changed from New to Under Review
Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78135
Updated by Gerrit Code Review over 1 year ago
Patch set 1 for branch 11.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78059
Updated by Gerrit Code Review over 1 year ago
Patch set 2 for branch 11.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78059
Updated by Benni Mack over 1 year ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 2522a9c4d8dc9d27ff6e6c8b347a5f039548195f.
Updated by Gerrit Code Review over 1 year ago
- Status changed from Resolved to Under Review
Patch set 3 for branch 11.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78059
Updated by Gerrit Code Review over 1 year ago
Patch set 4 for branch 11.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78059
Updated by Benni Mack over 1 year ago
- Status changed from Under Review to Resolved
Applied in changeset 3e45e21862b4c69425506bb898e001ef06f021e8.