From 0cdef7530be4919919b8f50a6b15c2a5d6fa849d Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 4 May 2026 10:54:56 +0200 Subject: [PATCH] fix: Prevent error page to display arbitrary error messages Signed-off-by: Carl Schwan --- lib/AppInfo/Application.php | 7 +++++-- lib/Controller/SAMLController.php | 11 +++++++---- tests/unit/Controller/SAMLControllerTest.php | 3 ++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 167700f31..398a96b9f 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -10,6 +10,7 @@ namespace OCA\User_SAML\AppInfo; use OC\Security\CSRF\CsrfTokenManager; +use OC\User\DisabledUserException; use OC\User\LoginException; use OC_User; use OCA\DAV\Events\SabrePluginAddEvent; @@ -128,12 +129,14 @@ public function boot(IBootContext $context): void { if ($request->getPathInfo() === '/apps/user_saml/saml/error') { return; } + /** @psalm-suppress UndefinedClass */ $targetUrl = $urlGenerator->linkToRouteAbsolute( 'user_saml.SAML.genericError', [ - 'message' => $e->getMessage() + 'reason' => $e instanceof DisabledUserException ? 'userDisabled' : 'authFailed', ] ); + $logger->error('Login failure', ['exception' => $e]); header('Location: ' . $targetUrl); exit(); } @@ -151,7 +154,7 @@ public function boot(IBootContext $context): void { $targetUrl = $urlGenerator->linkToRouteAbsolute( 'user_saml.SAML.genericError', [ - 'message' => $l10n->t('This user account is disabled, please contact your administrator.') + 'reason' => 'userDisabled', ] ); header('Location: ' . $targetUrl); diff --git a/lib/Controller/SAMLController.php b/lib/Controller/SAMLController.php index dd48473e7..36f46c40b 100644 --- a/lib/Controller/SAMLController.php +++ b/lib/Controller/SAMLController.php @@ -550,10 +550,13 @@ public function notPermitted(): Http\TemplateResponse { * @NoCSRFRequired * @OnlyUnauthenticatedUsers */ - public function genericError(string $message): Http\TemplateResponse { - if (empty($message)) { - $message = $this->l->t('Unknown error, please check the log file for more details.'); - } + public function genericError(string $reason): Http\TemplateResponse { + $allowedMessages = [ + 'userDisabled' => $this->l->t('This user account is disabled, please contact your administrator.'), + 'authFailed' => $this->l->t('Authentication failed.'), + ]; + + $message = $allowedMessages[$reason] ?? $this->l->t('Unknown error, please check the log file for more details.'); return new Http\TemplateResponse($this->appName, 'error', ['message' => $message], 'guest'); } diff --git a/tests/unit/Controller/SAMLControllerTest.php b/tests/unit/Controller/SAMLControllerTest.php index bcba9df70..fb8e67a17 100644 --- a/tests/unit/Controller/SAMLControllerTest.php +++ b/tests/unit/Controller/SAMLControllerTest.php @@ -344,7 +344,8 @@ public function testGenericError($messageSend, $messageExpected) { public function dataTestGenericError() { return [ ['messageSend' => '', 'messageExpected' => 'Unknown error, please check the log file for more details.'], - ['messageSend' => 'test message', 'messageExpected' => 'test message'], + ['messageSend' => 'userDisabled', 'messageExpected' => 'This user account is disabled, please contact your administrator.'], + ['messageSend' => 'authFailed', 'messageExpected' => 'Authentication failed.'], ]; }