From 5ae5c1fa1cf45a8c96033bba5cf1757792ad4d09 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 28 May 2026 10:45:08 -0400 Subject: [PATCH 01/11] fix(AppFramework): make basic auth parsing less brittle in PasswordConfirmationMiddleware Also make it clearer why there is a silent bypass on for various specific token retrieval scenarios. Signed-off-by: Josh --- .../PasswordConfirmationMiddleware.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 94fc329d5c71f..23b6b3517696b 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -68,7 +68,8 @@ public function beforeController(Controller $controller, string $methodName) { $sessionId = $this->session->getId(); $token = $this->tokenProvider->getToken($sessionId); } catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) { - // States we do not deal with here. + // Only valid interactive session tokens participate in password confirmation. Requests without such a + // token are left to be rejected or otherwise handled by the normal authentication/session handling. return; } @@ -80,13 +81,23 @@ public function beforeController(Controller $controller, string $methodName) { $reflectionMethod = new ReflectionMethod($controller, $methodName); if ($this->isPasswordConfirmationStrict($reflectionMethod)) { - $authHeader = $this->request->getHeader('Authorization'); - if (!str_starts_with(strtolower($authHeader), 'basic ')) { + $authHeader = strtolower($this->request->getHeader('Authorization')); + + if (!str_starts_with($authHeader, 'basic ')) { throw new NotConfirmedException('Required authorization header missing'); } - [, $password] = explode(':', base64_decode(substr($authHeader, 6)), 2); + + $decoded = base64_decode(substr($authHeader, 6), true); + + if ($decoded === false || !str_contains($decoded, ':')) { + throw new NotConfirmedException('Malformed authorization header'); + } + + [$ignoredUser, $password] = explode(':', $decoded, 2); + $loginName = $this->session->get('loginname'); $loginResult = $this->userManager->checkPassword($loginName, $password); + if ($loginResult === false) { throw new NotConfirmedException(); } From 8790170ecc048850ab3e5221c3fe49ed82bfb316 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 28 May 2026 11:56:21 -0400 Subject: [PATCH 02/11] refactor(AppFramework): PasswordConfirmationMiddleware exemptions/exclusions helpers Signed-off-by: Josh --- .../PasswordConfirmationMiddleware.php | 71 ++++++++++++++----- 1 file changed, 52 insertions(+), 19 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 23b6b3517696b..3c803d1b0522b 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -28,7 +28,17 @@ use ReflectionMethod; class PasswordConfirmationMiddleware extends Middleware { - private array $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true]; + /** + * Legacy compatibility allowlist for backends that do not participate in the + * non-strict recent-confirmation flow. New backends should prefer implementing + * IPasswordConfirmationBackend instead of being added here. + * + * @var array + */ + private array $excludedUserBackEnds = [ + 'user_saml' => true, + 'user_globalsiteselector' => true, + ]; public function __construct( private ControllerMethodReflector $reflector, @@ -52,16 +62,9 @@ public function beforeController(Controller $controller, string $methodName) { } $user = $this->userSession->getUser(); - $backendClassName = ''; - if ($user !== null) { - $backend = $user->getBackend(); - if ($backend instanceof IPasswordConfirmationBackend) { - if (!$backend->canConfirmPassword($user->getUID())) { - return; - } - } - $backendClassName = $user->getBackendClassName(); + if ($this->isBackendExemptFromPasswordConfirmation($user)) { + return; } try { @@ -73,8 +76,7 @@ public function beforeController(Controller $controller, string $methodName) { return; } - $scope = $token->getScopeAsArray(); - if (isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) && $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true) { + if ($this->isTokenExemptFromPasswordConfirmation($token)) { // Users logging in from SSO backends cannot confirm their password by design return; } @@ -103,17 +105,26 @@ public function beforeController(Controller $controller, string $methodName) { } $this->session->set('last-password-confirm', $this->timeFactory->getTime()); - } else { - $lastConfirm = (int)$this->session->get('last-password-confirm'); - // TODO: confirm excludedUserBackEnds can go away and remove it - if (!isset($this->excludedUserBackEnds[$backendClassName]) && $lastConfirm < ($this->timeFactory->getTime() - (30 * 60 + 15))) { // allow 15 seconds delay - throw new NotConfirmedException(); - } + return; + } + + $lastConfirm = (int)$this->session->get('last-password-confirm'); + $minimumRequiredConfirmTime = $this->timeFactory->getTime() - (30 * 60 + 15); // allow 15 seconds delay + + // TODO: confirm excludedUserBackEnds can go away and remove it + if ( + !$this->isLegacyBackendExcludedFromRecentConfirmation($user) + && $lastConfirm < $minimumRequiredConfirmTime + ) { + throw new NotConfirmedException(); } } private function needsPasswordConfirmation(): bool { - return $this->reflector->hasAnnotationOrAttribute('PasswordConfirmationRequired', PasswordConfirmationRequired::class); + return $this->reflector->hasAnnotationOrAttribute( + 'PasswordConfirmationRequired', + PasswordConfirmationRequired::class + ); } private function isPasswordConfirmationStrict(ReflectionMethod $reflectionMethod): bool { @@ -121,4 +132,26 @@ private function isPasswordConfirmationStrict(ReflectionMethod $reflectionMethod $attributes = $reflectionMethod->getAttributes(PasswordConfirmationRequired::class); return !empty($attributes) && ($attributes[0]->newInstance()->getStrict()); } + + private function isBackendExemptFromPasswordConfirmation(?IUser $user): bool { + if ($user === null) { + return false; + } + + $backend = $user->getBackend(); + return $backend instanceof IPasswordConfirmationBackend + && !$backend->canConfirmPassword($user->getUID()); + } + + private function isLegacyBackendExcludedFromRecentConfirmation(?IUser $user): bool { + $backendClassName = $user?->getBackendClassName() ?? ''; + return isset($this->excludedUserBackEnds[$backendClassName]); + } + + private function isTokenExemptFromPasswordConfirmation(IToken $token): bool { + $scope = $token->getScopeAsArray(); + return isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) + && $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true; + } + } From 76a1ada311553ce403d08f32d4ac3d2b04d07c6e Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 28 May 2026 12:01:57 -0400 Subject: [PATCH 03/11] refactor(AppFramework): extract p/w confirmation Authorization header parsing logic for clarity Signed-off-by: Josh --- .../PasswordConfirmationMiddleware.php | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 3c803d1b0522b..8dc5b3e2ac330 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -83,27 +83,7 @@ public function beforeController(Controller $controller, string $methodName) { $reflectionMethod = new ReflectionMethod($controller, $methodName); if ($this->isPasswordConfirmationStrict($reflectionMethod)) { - $authHeader = strtolower($this->request->getHeader('Authorization')); - - if (!str_starts_with($authHeader, 'basic ')) { - throw new NotConfirmedException('Required authorization header missing'); - } - - $decoded = base64_decode(substr($authHeader, 6), true); - - if ($decoded === false || !str_contains($decoded, ':')) { - throw new NotConfirmedException('Malformed authorization header'); - } - - [$ignoredUser, $password] = explode(':', $decoded, 2); - - $loginName = $this->session->get('loginname'); - $loginResult = $this->userManager->checkPassword($loginName, $password); - - if ($loginResult === false) { - throw new NotConfirmedException(); - } - + $this->confirmPasswordFromAuthorizationHeader(); $this->session->set('last-password-confirm', $this->timeFactory->getTime()); return; } @@ -154,4 +134,29 @@ private function isTokenExemptFromPasswordConfirmation(IToken $token): bool { && $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true; } + /** + * @throws NotConfirmedException + */ + private function confirmPasswordFromAuthorizationHeader(): void { + $authHeader = strtolower($this->request->getHeader('Authorization')); + + if (!str_starts_with($authHeader, 'basic ')) { + throw new NotConfirmedException('Required authorization header missing'); + } + + $decodedCredentials = base64_decode(substr($authHeader, 6), true); + + if ($decodedCredentials === false || !str_contains($decodedCredentials, ':')) { + throw new NotConfirmedException('Malformed authorization header'); + } + + [$ignoredUser, $password] = explode(':', $decodedCredentials, 2); + + $loginName = $this->session->get('loginname'); + $loginResult = $this->userManager->checkPassword($loginName, $password); + + if ($loginResult === false) { + throw new NotConfirmedException(); + } + } } From 9947e388b2c32112aab11cd658d52150c0cb781c Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 28 May 2026 12:05:26 -0400 Subject: [PATCH 04/11] refactor(AppFramework): replace magic numbers in PasswordConfirmationMiddleware Signed-off-by: Josh --- .../Security/PasswordConfirmationMiddleware.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 8dc5b3e2ac330..4ded207a9b537 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -28,6 +28,9 @@ use ReflectionMethod; class PasswordConfirmationMiddleware extends Middleware { + private const PASSWORD_CONFIRMATION_TIMEOUT = 30 * 60; + private const PASSWORD_CONFIRMATION_GRACE_SECONDS = 15; + /** * Legacy compatibility allowlist for backends that do not participate in the * non-strict recent-confirmation flow. New backends should prefer implementing @@ -81,15 +84,17 @@ public function beforeController(Controller $controller, string $methodName) { return; } + $now = $this->timeFactory->getTime(); $reflectionMethod = new ReflectionMethod($controller, $methodName); if ($this->isPasswordConfirmationStrict($reflectionMethod)) { $this->confirmPasswordFromAuthorizationHeader(); - $this->session->set('last-password-confirm', $this->timeFactory->getTime()); + $this->session->set('last-password-confirm', $now); return; } $lastConfirm = (int)$this->session->get('last-password-confirm'); - $minimumRequiredConfirmTime = $this->timeFactory->getTime() - (30 * 60 + 15); // allow 15 seconds delay + $minimumRequiredConfirmTime = $now + - (self::PASSWORD_CONFIRMATION_TIMEOUT + self::PASSWORD_CONFIRMATION_GRACE_SECONDS); // TODO: confirm excludedUserBackEnds can go away and remove it if ( From 9ccb3631ef0a767f07c88babaf9721cf7933f1b4 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 28 May 2026 12:24:20 -0400 Subject: [PATCH 05/11] fix(AppFramework): apply legacy PasswordConfirmation exemptions in both strict and non-strict modes Previously the legacy exemption list was only consulted in the non-strict branch. Signed-off-by: Josh --- .../PasswordConfirmationMiddleware.php | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 4ded207a9b537..ca5922a1a767e 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -20,6 +20,7 @@ use OCP\Authentication\Token\IToken; use OCP\IRequest; use OCP\ISession; +use OCP\IUser; use OCP\IUserSession; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\User\Backend\IPasswordConfirmationBackend; @@ -32,9 +33,9 @@ class PasswordConfirmationMiddleware extends Middleware { private const PASSWORD_CONFIRMATION_GRACE_SECONDS = 15; /** - * Legacy compatibility allowlist for backends that do not participate in the - * non-strict recent-confirmation flow. New backends should prefer implementing - * IPasswordConfirmationBackend instead of being added here. + * Backends that cannot participate in password confirmation are exempt from both + * strict and non-strict password confirmation checks. New backends should prefer + * implementing IPasswordConfirmationBackend instead of being added here. * * @var array */ @@ -74,13 +75,14 @@ public function beforeController(Controller $controller, string $methodName) { $sessionId = $this->session->getId(); $token = $this->tokenProvider->getToken($sessionId); } catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) { - // Only valid interactive session tokens participate in password confirmation. Requests without such a - // token are left to be rejected or otherwise handled by the normal authentication/session handling. + // Password confirmation is only enforced for requests backed by a valid interactive session token. + // Requests without such a token are left to be rejected or otherwise handled by the normal + // authentication/session middleware stack. return; } if ($this->isTokenExemptFromPasswordConfirmation($token)) { - // Users logging in from SSO backends cannot confirm their password by design + // Some session tokens are marked to skip password validation entirely. return; } @@ -96,11 +98,7 @@ public function beforeController(Controller $controller, string $methodName) { $minimumRequiredConfirmTime = $now - (self::PASSWORD_CONFIRMATION_TIMEOUT + self::PASSWORD_CONFIRMATION_GRACE_SECONDS); - // TODO: confirm excludedUserBackEnds can go away and remove it - if ( - !$this->isLegacyBackendExcludedFromRecentConfirmation($user) - && $lastConfirm < $minimumRequiredConfirmTime - ) { + if ($lastConfirm < $minimumRequiredConfirmTime) { throw new NotConfirmedException(); } } @@ -124,12 +122,20 @@ private function isBackendExemptFromPasswordConfirmation(?IUser $user): bool { } $backend = $user->getBackend(); - return $backend instanceof IPasswordConfirmationBackend - && !$backend->canConfirmPassword($user->getUID()); + + if ( + $backend instanceof IPasswordConfirmationBackend + && !$backend->canConfirmPassword($user->getUID()) + ) { + return true; + } + + return $this->isLegacyBackendExcludedFromRecentConfirmation($user); } private function isLegacyBackendExcludedFromRecentConfirmation(?IUser $user): bool { $backendClassName = $user?->getBackendClassName() ?? ''; + // TODO: confirm excludedUserBackEnds can go away and remove it return isset($this->excludedUserBackEnds[$backendClassName]); } @@ -143,9 +149,9 @@ private function isTokenExemptFromPasswordConfirmation(IToken $token): bool { * @throws NotConfirmedException */ private function confirmPasswordFromAuthorizationHeader(): void { - $authHeader = strtolower($this->request->getHeader('Authorization')); + $authHeader = $this->request->getHeader('Authorization'); - if (!str_starts_with($authHeader, 'basic ')) { + if (!str_starts_with(strtolower($authHeader), 'basic ')) { throw new NotConfirmedException('Required authorization header missing'); } From 274adbca1ddeb9d8753bd18268fd90e8d2d5cb37 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 28 May 2026 12:49:00 -0400 Subject: [PATCH 06/11] docs(AppFramework): note why auth username is ignored in PasswordConfirmationMiddleware Signed-off-by: Josh --- .../Middleware/Security/PasswordConfirmationMiddleware.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index ca5922a1a767e..5b903779b130b 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -93,7 +93,7 @@ public function beforeController(Controller $controller, string $methodName) { $this->session->set('last-password-confirm', $now); return; } - + $lastConfirm = (int)$this->session->get('last-password-confirm'); $minimumRequiredConfirmTime = $now - (self::PASSWORD_CONFIRMATION_TIMEOUT + self::PASSWORD_CONFIRMATION_GRACE_SECONDS); @@ -162,7 +162,8 @@ private function confirmPasswordFromAuthorizationHeader(): void { } [$ignoredUser, $password] = explode(':', $decodedCredentials, 2); - + // Use the session's loginname, not the one from the Authorization header, + // to prevent credential stuffing against arbitrary usernames. $loginName = $this->session->get('loginname'); $loginResult = $this->userManager->checkPassword($loginName, $password); From 739f9d9a7f7c8b888f4ac5ec0085751c9ef1fba0 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 30 May 2026 10:47:37 -0400 Subject: [PATCH 07/11] test(AppFramework): split out exempt cases - PasswordConfirmationMiddleware Signed-off-by: Josh --- .../PasswordConfirmationMiddlewareTest.php | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index c1c7e587fd256..eab8a5d3d2abc 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -93,7 +93,7 @@ public function testDifferentAnnotation(): void { $this->middleware->beforeController($this->controller, __FUNCTION__); } - #[\PHPUnit\Framework\Attributes\DataProvider('dataProvider')] + #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderNonExemptBackend')] public function testAnnotation($backend, $lastConfirm, $currentTime, $exception): void { $this->reflector->reflect($this->controller, __FUNCTION__); @@ -126,7 +126,7 @@ public function testAnnotation($backend, $lastConfirm, $currentTime, $exception) $this->assertSame($exception, $thrown); } - #[\PHPUnit\Framework\Attributes\DataProvider('dataProvider')] + #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderNonExemptBackend')] public function testAttribute($backend, $lastConfirm, $currentTime, $exception): void { $this->reflector->reflect($this->controller, __FUNCTION__); @@ -159,19 +159,41 @@ public function testAttribute($backend, $lastConfirm, $currentTime, $exception): $this->assertSame($exception, $thrown); } - - - public static function dataProvider(): array { + public static function dataProviderNonExemptBackend(): array { return [ ['foo', 2000, 4000, true], ['foo', 2000, 3000, false], - ['user_saml', 2000, 4000, false], - ['user_saml', 2000, 3000, false], ['foo', 2000, 3815, false], ['foo', 2000, 3816, true], ]; } + /** + * @dataProvider dataProviderLegacyExemptBackends + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderLegacyExemptBackends')] + public function testLegacyBackendExempt(string $backend): void { + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName') + ->willReturn($backend); + $this->userSession->method('getUser') + ->willReturn($this->user); + + // Backend is exempt — getToken() must never be called + $this->tokenProvider->expects($this->never()) + ->method('getToken'); + + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public static function dataProviderLegacyExemptBackends(): array { + return [ + ['user_saml'], + ['user_globalsiteselector'], + ]; + } + public function testSSO(): void { static $sessionId = 'mySession1d'; From 19959f2e9ed29c7bd81c472936f647c4c67dd385 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 30 May 2026 10:51:12 -0400 Subject: [PATCH 08/11] test(AppFramework): add mock controller methods for new PWC tests Signed-off-by: Josh --- ...sswordConfirmationMiddlewareController.php | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php index cd1cdaa49ca33..16900069da36d 100644 --- a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php +++ b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php @@ -32,6 +32,35 @@ public function testAnnotation() { public function testAttribute() { } + // Non-strict — for backend capability and null-user tests + #[PasswordConfirmationRequired] + public function testLegacyBackendExempt() {} + + #[PasswordConfirmationRequired] + public function testIPasswordConfirmationBackendExempt() {} + + #[PasswordConfirmationRequired] + public function testIPasswordConfirmationBackendNotExempt() {} + + #[PasswordConfirmationRequired] + public function testNullUser() {} + + // Strict — one controller method per strict-mode test + #[PasswordConfirmationRequired(strict: true)] + public function testStrictModeValidCredentials() {} + + #[PasswordConfirmationRequired(strict: true)] + public function testStrictModeMissingAuthHeader() {} + + #[PasswordConfirmationRequired(strict: true)] + public function testStrictModeMalformedBase64() {} + + #[PasswordConfirmationRequired(strict: true)] + public function testStrictModeWrongPassword() {} + + #[PasswordConfirmationRequired(strict: true)] + public function testStrictModeLegacyBackendExempt() {} + #[PasswordConfirmationRequired] public function testSSO() { } From ba23ea8a64e65ed8fc73389b66fd1ffb0cd6c500 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 30 May 2026 11:29:07 -0400 Subject: [PATCH 09/11] test(AppFramework): expand PasswordConfirmationMiddleware coverage Signed-off-by: Josh --- .../PasswordConfirmationMiddlewareTest.php | 207 +++++++++++++++++- 1 file changed, 205 insertions(+), 2 deletions(-) diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index eab8a5d3d2abc..2991104ad01a5 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -19,6 +19,7 @@ use OCP\IUser; use OCP\IUserSession; use OCP\Server; +use OCP\User\Backend\IPasswordConfirmationBackend; use Psr\Log\LoggerInterface; use Test\AppFramework\Middleware\Security\Mock\PasswordConfirmationMiddlewareController; use Test\TestCase; @@ -94,7 +95,7 @@ public function testDifferentAnnotation(): void { } #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderNonExemptBackend')] - public function testAnnotation($backend, $lastConfirm, $currentTime, $exception): void { + public function testAnnotation(string $backend, int $lastConfirm, int $currentTime, bool $exception): void $this->reflector->reflect($this->controller, __FUNCTION__); $this->user->method('getBackendClassName') @@ -127,7 +128,7 @@ public function testAnnotation($backend, $lastConfirm, $currentTime, $exception) } #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderNonExemptBackend')] - public function testAttribute($backend, $lastConfirm, $currentTime, $exception): void { + public function testAttribute(string $backend, int $lastConfirm, int $currentTime, bool $exception): void { $this->reflector->reflect($this->controller, __FUNCTION__); $this->user->method('getBackendClassName') @@ -194,6 +195,208 @@ public static function dataProviderLegacyExemptBackends(): array { ]; } + public function testIPasswordConfirmationBackendExempt(): void { + $this->reflector->reflect($this->controller, __FUNCTION__); + + $backend = $this->createMock(IPasswordConfirmationBackend::class); + $backend->method('canConfirmPassword')->with('uid')->willReturn(false); + + $this->user->method('getBackend')->willReturn($backend); + $this->user->method('getUID')->willReturn('uid'); + $this->userSession->method('getUser')->willReturn($this->user); + + // Exempt before token check + $this->tokenProvider->expects($this->never())->method('getToken'); + + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public function testIPasswordConfirmationBackendNotExempt(): void { + static $sessionId = 'mySession1d'; + $this->reflector->reflect($this->controller, __FUNCTION__); + + $backend = $this->createMock(IPasswordConfirmationBackend::class); + $backend->method('canConfirmPassword')->with('uid')->willReturn(true); + + $this->user->method('getBackend')->willReturn($backend); + $this->user->method('getUID')->willReturn('uid'); + $this->user->method('getBackendClassName')->willReturn('capable_backend'); + $this->userSession->method('getUser')->willReturn($this->user); + + $this->session->method('getId')->willReturn($sessionId); + $this->session->method('get')->with('last-password-confirm')->willReturn(2000); + $this->timeFactory->method('getTime')->willReturn(3000); // within window — no exception + + $token = $this->createMock(IToken::class); + $token->method('getScopeAsArray')->willReturn([]); + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with($sessionId) + ->willReturn($token); + + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public function testNullUser(): void { + static $sessionId = 'mySession1d'; + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->userSession->method('getUser')->willReturn(null); + + $this->session->method('getId')->willReturn($sessionId); + $this->session->method('get')->with('last-password-confirm')->willReturn(2000); + $this->timeFactory->method('getTime')->willReturn(3000); // within window — no exception + + $token = $this->createMock(IToken::class); + $token->method('getScopeAsArray')->willReturn([]); + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with($sessionId) + ->willReturn($token); + + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public function testStrictModeValidCredentials(): void { + static $sessionId = 'mySession1d'; + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName')->willReturn('foo'); + $this->userSession->method('getUser')->willReturn($this->user); + $this->session->method('getId')->willReturn($sessionId); + $this->timeFactory->method('getTime')->willReturn(9999); + + $token = $this->createMock(IToken::class); + $token->method('getScopeAsArray')->willReturn([]); + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with($sessionId) + ->willReturn($token); + + $this->request->method('getHeader') + ->with('Authorization') + ->willReturn('Basic ' . base64_encode('user:correctpassword')); + + $this->session->method('get') + ->with('loginname') + ->willReturn('user'); + + $loginUser = $this->createMock(IUser::class); + $this->userManager->expects($this->once()) + ->method('checkPassword') + ->with('user', 'correctpassword') + ->willReturn($loginUser); + + // Timestamp must be written after successful strict confirmation + $this->session->expects($this->once()) + ->method('set') + ->with('last-password-confirm', 9999); + + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public function testStrictModeMissingAuthHeader(): void { + static $sessionId = 'mySession1d'; + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName')->willReturn('foo'); + $this->userSession->method('getUser')->willReturn($this->user); + $this->session->method('getId')->willReturn($sessionId); + $this->timeFactory->method('getTime')->willReturn(9999); + + $token = $this->createMock(IToken::class); + $token->method('getScopeAsArray')->willReturn([]); + $this->tokenProvider->method('getToken')->willReturn($token); + + $this->request->method('getHeader') + ->with('Authorization') + ->willReturn(''); // no header + + $this->session->expects($this->never())->method('set'); + $this->userManager->expects($this->never())->method('checkPassword'); + $this->expectException(NotConfirmedException::class); + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + /** + * @dataProvider dataProviderMalformedAuthHeaders + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderMalformedAuthHeaders')] + public function testStrictModeMalformedBase64(string $headerValue): void { + static $sessionId = 'mySession1d'; + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName')->willReturn('foo'); + $this->userSession->method('getUser')->willReturn($this->user); + $this->session->method('getId')->willReturn($sessionId); + $this->timeFactory->method('getTime')->willReturn(9999); + + $token = $this->createMock(IToken::class); + $token->method('getScopeAsArray')->willReturn([]); + $this->tokenProvider->method('getToken')->willReturn($token); + + $this->request->method('getHeader') + ->with('Authorization') + ->willReturn($headerValue); + + $this->session->expects($this->never())->method('set'); + $this->userManager->expects($this->never())->method('checkPassword'); + $this->expectException(NotConfirmedException::class); + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public static function dataProviderMalformedAuthHeaders(): array { + return [ + 'invalid base64' => ['Basic !!!notbase64!!!'], + 'no colon in decoded' => ['Basic ' . base64_encode('nodivider')], + ]; + } + + public function testStrictModeWrongPassword(): void { + static $sessionId = 'mySession1d'; + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName')->willReturn('foo'); + $this->userSession->method('getUser')->willReturn($this->user); + $this->session->method('getId')->willReturn($sessionId); + $this->timeFactory->method('getTime')->willReturn(9999); + + $token = $this->createMock(IToken::class); + $token->method('getScopeAsArray')->willReturn([]); + $this->tokenProvider->method('getToken')->willReturn($token); + + $this->request->method('getHeader') + ->with('Authorization') + ->willReturn('Basic ' . base64_encode('user:wrongpassword')); + + $this->session->method('get') + ->with('loginname') + ->willReturn('user'); + + $this->userManager->method('checkPassword') + ->with('user', 'wrongpassword') + ->willReturn(false); + + $this->session->expects($this->never())->method('set'); + + $this->expectException(NotConfirmedException::class); + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + + public function testStrictModeLegacyBackendExempt(): void { + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName')->willReturn('user_saml'); + $this->userSession->method('getUser')->willReturn($this->user); + + // Must exit before reaching the auth header or token checks + $this->tokenProvider->expects($this->never())->method('getToken'); + $this->request->expects($this->never())->method('getHeader'); + $this->userManager->expects($this->never())->method('checkPassword'); + + $this->middleware->beforeController($this->controller, __FUNCTION__); + } + public function testSSO(): void { static $sessionId = 'mySession1d'; From 14ee86f8b1337c6fb6a821234f4b74422ca71283 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 30 May 2026 11:32:14 -0400 Subject: [PATCH 10/11] chore(PasswordConfirmationMiddlewareTest): typo fixup Signed-off-by: Josh --- .../Middleware/Security/PasswordConfirmationMiddlewareTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 2991104ad01a5..1dae157a8d85a 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -95,7 +95,7 @@ public function testDifferentAnnotation(): void { } #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderNonExemptBackend')] - public function testAnnotation(string $backend, int $lastConfirm, int $currentTime, bool $exception): void + public function testAnnotation(string $backend, int $lastConfirm, int $currentTime, bool $exception): void { $this->reflector->reflect($this->controller, __FUNCTION__); $this->user->method('getBackendClassName') From 7b4d6c9bdf5d8c5a8842bd59cb54bf1721713048 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 30 May 2026 12:00:03 -0400 Subject: [PATCH 11/11] refactor(Template): align JSConfigHelper PasswordConfirmation logic w/ Middleware Signed-off-by: Josh --- lib/private/Template/JSConfigHelper.php | 55 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index 4907274f7304e..1d1f09e6b852a 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -40,8 +40,17 @@ class JSConfigHelper { - /** @var array user back-ends excluded from password verification */ - private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true]; + /** + * Backends that cannot participate in password confirmation are exempt from both + * strict and non-strict password confirmation checks. New backends should prefer + * implementing IPasswordConfirmationBackend instead of being added here. + * + * @var array + */ + private array $excludedUserBackEnds = [ + 'user_saml' => true, + 'user_globalsiteselector' => true, + ]; public function __construct( protected ServerVersion $serverVersion, @@ -63,18 +72,15 @@ public function __construct( } public function getConfig(): string { - $userBackendAllowsPasswordConfirmation = true; + $userCanUsePasswordConfirmation = false; + $canUserValidatePassword = $this->canUserValidatePassword(); + if ($this->currentUser !== null) { $uid = $this->currentUser->getUID(); - $backend = $this->currentUser->getBackend(); - if ($backend instanceof IPasswordConfirmationBackend) { - $userBackendAllowsPasswordConfirmation = $backend->canConfirmPassword($uid) && $this->canUserValidatePassword(); - } elseif (isset($this->excludedUserBackEnds[$this->currentUser->getBackendClassName()])) { - $userBackendAllowsPasswordConfirmation = false; - } else { - $userBackendAllowsPasswordConfirmation = $this->canUserValidatePassword(); - } + $userCanUsePasswordConfirmation = + $this->canBackendConfirmPassword($this->currentUser) + && $canUserValidatePassword; } else { $uid = null; } @@ -126,7 +132,7 @@ public function getConfig(): string { } if ($this->currentUser instanceof IUser) { - if ($this->canUserValidatePassword()) { + if ($canUserValidatePassword) { $lastConfirmTimestamp = $this->session->get('last-password-confirm'); if (!is_int($lastConfirmTimestamp)) { $lastConfirmTimestamp = 0; @@ -174,7 +180,7 @@ public function getConfig(): string { $array = [ '_oc_debug' => $this->config->getSystemValue('debug', false) ? 'true' : 'false', '_oc_isadmin' => $uid !== null && $this->groupManager->isAdmin($uid) ? 'true' : 'false', - 'backendAllowsPasswordConfirmation' => $userBackendAllowsPasswordConfirmation ? 'true' : 'false', + 'backendAllowsPasswordConfirmation' => $userCanUsePasswordConfirmation ? 'true' : 'false', 'oc_dataURL' => is_string($dataLocation) ? '"' . $dataLocation . '"' : 'false', '_oc_webroot' => '"' . \OC::$WEBROOT . '"', '_oc_appswebroots' => str_replace('\\/', '/', json_encode($apps_paths)), // Ugly unescape slashes waiting for better solution @@ -304,10 +310,31 @@ protected function canUserValidatePassword(): bool { try { $token = $this->tokenProvider->getToken($this->session->getId()); } catch (ExpiredTokenException|WipeTokenException|InvalidTokenException|SessionNotAvailableException) { - // actually we do not know, so we fall back to this statement + // If token lookup fails, fall back to allowing password validation in the UI. return true; } $scope = $token->getScopeAsArray(); return !isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) || $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === false; } + + private function canBackendConfirmPassword(?IUser $user): bool { + if ($user === null) { + return false; + } + + $backend = $user->getBackend(); + if ( + $backend instanceof IPasswordConfirmationBackend + && !$backend->canConfirmPassword($user->getUID()) + ) { + return false; + } + + return !$this->isLegacyBackendExcludedFromPasswordConfirmation($user); + } + + private function isLegacyBackendExcludedFromPasswordConfirmation(?IUser $user): bool { + $backendClassName = $user?->getBackendClassName() ?? ''; + return isset($this->excludedUserBackEnds[$backendClassName]); + } }