fix(password-confirmation): exempt unsupported backends from strict and non-strict confirmation checks

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;
@@ -28,7 +29,20 @@
 use ReflectionMethod;
 
 class PasswordConfirmationMiddleware extends Middleware {
-	private array $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
+	private const PASSWORD_CONFIRMATION_TIMEOUT = 30 * 60;
+	private const PASSWORD_CONFIRMATION_GRACE_SECONDS = 15;
+
+	/**
+	 * 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<string, true>
+	 */
+	private array $excludedUserBackEnds = [
+		'user_saml' => true,
+		'user_globalsiteselector' => true,
+	];
 
 	public function __construct(
 		private ControllerMethodReflector $reflector,
@@ -52,62 +66,109 @@ 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 {
 			$sessionId = $this->session->getId();
 			$token = $this->tokenProvider->getToken($sessionId);
 		} catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) {
-			// States we do not deal with here.
+			// 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;
 		}
 
-		$scope = $token->getScopeAsArray();
-		if (isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) && $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true) {
-			// Users logging in from SSO backends cannot confirm their password by design
+		if ($this->isTokenExemptFromPasswordConfirmation($token)) {
+			// Some session tokens are marked to skip password validation entirely.
 			return;
 		}
 
+		$now = $this->timeFactory->getTime();
 		$reflectionMethod = new ReflectionMethod($controller, $methodName);
 		if ($this->isPasswordConfirmationStrict($reflectionMethod)) {
-			$authHeader = $this->request->getHeader('Authorization');
-			if (!str_starts_with(strtolower($authHeader), 'basic ')) {
-				throw new NotConfirmedException('Required authorization header missing');
-			}
-			[, $password] = explode(':', base64_decode(substr($authHeader, 6)), 2);
-			$loginName = $this->session->get('loginname');
-			$loginResult = $this->userManager->checkPassword($loginName, $password);
-			if ($loginResult === false) {
-				throw new NotConfirmedException();
-			}
-
-			$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();
-			}
+			$this->confirmPasswordFromAuthorizationHeader();
+			$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);
+
+		if ($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 {
 		/** @var ReflectionAttribute<PasswordConfirmationRequired>[] $attributes */
 		$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();
+
+		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]);
+	}
+
+	private function isTokenExemptFromPasswordConfirmation(IToken $token): bool {
+		$scope = $token->getScopeAsArray();
+		return isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION])
+			&& $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true;
+	}
+
+	/**
+	 * @throws NotConfirmedException
+	 */
+	private function confirmPasswordFromAuthorizationHeader(): void {
+		$authHeader = $this->request->getHeader('Authorization');
+
+		if (!str_starts_with(strtolower($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);
+		// 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);
+
+		if ($loginResult === false) {
+			throw new NotConfirmedException();
+		}
+	}
 }

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<string, true>
+	 */
+	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]);
+	}
 }

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() {
 	}