xperseguers / t3ext-oidc

TYPO3 Extension oidc. This extension uses OpenID Connect to authenticate users.
https://extensions.typo3.org/extension/oidc
GNU General Public License v2.0
9 stars 30 forks source link

[FEATURE] Try a second pageArguments fetch if page was not found (404) #128

Closed denislorch closed 5 months ago

denislorch commented 10 months ago

If we open a page that does not exist, the typoscript fetch fail and throw the error "Failed to initialize TSFE". With this PR, we fetch it a second time without path and query arguments (=> root page).

liayn commented 6 months ago

Can you please elaborate a bit more, how the bug is triggered, which is fixed with this code change? (maybe open an issue for this)

denislorch commented 5 months ago

In our project we implemented an auto login for a specific site tree (intranet) with an user authenticator middleware, which triggers the OAuthService redirect, similar to the Causal\Oidc\Controller\LoginController::performRedirectToLogin() action. The given URL might be anything, typed in from an user.

If the login is triggered by a non existing page in this site tree, eg. https//intranet/existing-site/0815 the sso login works, but the callback processing Causal\Oidc\Service\AuthenticationService::getLocalTSFE() does not, because the site 0815 does not exist. In this case, we try a second fetch without any query and path parameters https://intranet/ to resolve those TSFE settings.

With this change, i solve this login lookup edge case and show in the end the expected 404 page.

liayn commented 5 months ago

Okay, thanks for explaining this.

But why not directly trigger the page error handler instead?

denislorch commented 5 months ago

Yes, trigger the error handler early would work, but in my opinion, leads in a worse UX for the user. My approach was to map the users login without an error, before running in the regular error handling. On further page actions he's already logged in.

xperseguers commented 5 months ago

That sounds like something that could be implemented in a custom 404 handler if you need so, purpose of this extension is to authenticate a legit request.

denislorch commented 5 months ago

Thanks for your advice, with this in mind we can close this PR and avoid mixing this things.