Merge pull request #45812 from nextcloud/backport/43942/stable26

[stable26] fix(Session): avoid password confirmation on SSO

core/Controller/OCJSController.php

@@ -28,6 +28,7 @@
 namespace OC\Core\Controller;
 
 use bantu\IniGetWrapper\IniGetWrapper;
+use OC\Authentication\Token\IProvider;
 use OC\CapabilitiesManager;
 use OC\Template\JSConfigHelper;
 use OCP\App\IAppManager;
@@ -59,7 +60,9 @@ class OCJSController extends Controller {
 								IniGetWrapper $iniWrapper,
 								IURLGenerator $urlGenerator,
 								CapabilitiesManager $capabilitiesManager,
-								IInitialStateService $initialStateService) {
+								IInitialStateService $initialStateService,
+								IProvider $tokenProvider,
+	) {
 		parent::__construct($appName, $request);
 
 		$this->helper = new JSConfigHelper(
@@ -73,7 +76,8 @@ class OCJSController extends Controller {
 			$iniWrapper,
 			$urlGenerator,
 			$capabilitiesManager,
-			$initialStateService
+			$initialStateService,
+			$tokenProvider
 		);
 	}
 

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -275,7 +275,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 					$c->get(IControllerMethodReflector::class),
 					$c->get(ISession::class),
 					$c->get(IUserSession::class),
-					$c->get(ITimeFactory::class)
+					$c->get(ITimeFactory::class),
+					$c->get(\OC\Authentication\Token\IProvider::class),
 				)
 			);
 			$dispatcher->registerMiddleware(

lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php

@@ -25,11 +25,16 @@ namespace OC\AppFramework\Middleware\Security;
 
 use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Authentication\Exceptions\ExpiredTokenException;
+use OC\Authentication\Exceptions\InvalidTokenException;
+use OC\Authentication\Exceptions\WipeTokenException;
+use OC\Authentication\Token\IProvider;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\ISession;
 use OCP\IUserSession;
+use OCP\Session\Exceptions\SessionNotAvailableException;
 use OCP\User\Backend\IPasswordConfirmationBackend;
 
 class PasswordConfirmationMiddleware extends Middleware {
@@ -43,6 +48,7 @@ class PasswordConfirmationMiddleware extends Middleware {
 	private $timeFactory;
 	/** @var array */
 	private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
+	private IProvider $tokenProvider;
 
 	/**
 	 * PasswordConfirmationMiddleware constructor.
@@ -53,13 +59,16 @@ class PasswordConfirmationMiddleware extends Middleware {
 	 * @param ITimeFactory $timeFactory
 	 */
 	public function __construct(ControllerMethodReflector $reflector,
-								ISession $session,
-								IUserSession $userSession,
-								ITimeFactory $timeFactory) {
+		ISession $session,
+		IUserSession $userSession,
+		ITimeFactory $timeFactory,
+		IProvider $tokenProvider,
+	) {
 		$this->reflector = $reflector;
 		$this->session = $session;
 		$this->userSession = $userSession;
 		$this->timeFactory = $timeFactory;
+		$this->tokenProvider = $tokenProvider;
 	}
 
 	/**
@@ -82,8 +91,21 @@ class PasswordConfirmationMiddleware extends Middleware {
 				$backendClassName = $user->getBackendClassName();
 			}
 
+			try {
+				$sessionId = $this->session->getId();
+				$token = $this->tokenProvider->getToken($sessionId);
+			} catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) {
+				// States we do not deal with here.
+				return;
+			}
+			$scope = $token->getScopeAsArray();
+			if (isset($scope['password-unconfirmable']) && $scope['password-unconfirmable'] === true) {
+				// Users logging in from SSO backends cannot confirm their password by design
+				return;
+			}
+
 			$lastConfirm = (int) $this->session->get('last-password-confirm');
-			// we can't check the password against a SAML backend, so skip password confirmation in this case
+			// 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();
 			}

lib/private/Authentication/Token/PublicKeyTokenProvider.php

@@ -242,6 +242,7 @@ class PublicKeyTokenProvider implements IProvider {
 				IToken::TEMPORARY_TOKEN,
 				$token->getRemember()
 			);
+			$newToken->setScope($token->getScopeAsArray());
 
 			$this->mapper->delete($token);
 

lib/private/Template/JSConfigHelper.php

@@ -34,6 +34,10 @@ declare(strict_types=1);
 namespace OC\Template;
 
 use bantu\IniGetWrapper\IniGetWrapper;
+use OC\Authentication\Exceptions\ExpiredTokenException;
+use OC\Authentication\Exceptions\InvalidTokenException;
+use OC\Authentication\Exceptions\WipeTokenException;
+use OC\Authentication\Token\IProvider;
 use OC\CapabilitiesManager;
 use OC\Share\Share;
 use OCP\App\AppPathNotFoundException;
@@ -49,47 +53,28 @@ use OCP\ISession;
 use OCP\IURLGenerator;
 use OCP\ILogger;
 use OCP\IUser;
+use OCP\Session\Exceptions\SessionNotAvailableException;
 use OCP\User\Backend\IPasswordConfirmationBackend;
 use OCP\Util;
 
 class JSConfigHelper {
-	protected IL10N $l;
-	protected Defaults $defaults;
-	protected IAppManager $appManager;
-	protected ISession $session;
-	protected ?IUser $currentUser;
-	protected IConfig $config;
-	protected IGroupManager $groupManager;
-	protected IniGetWrapper $iniWrapper;
-	protected IURLGenerator $urlGenerator;
-	protected CapabilitiesManager $capabilitiesManager;
-	protected IInitialStateService $initialStateService;
-
 	/** @var array user back-ends excluded from password verification */
 	private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
 
-	public function __construct(IL10N $l,
-								Defaults $defaults,
-								IAppManager $appManager,
-								ISession $session,
-								?IUser $currentUser,
-								IConfig $config,
-								IGroupManager $groupManager,
-								IniGetWrapper $iniWrapper,
-								IURLGenerator $urlGenerator,
-								CapabilitiesManager $capabilitiesManager,
-								IInitialStateService $initialStateService) {
-		$this->l = $l;
-		$this->defaults = $defaults;
-		$this->appManager = $appManager;
-		$this->session = $session;
-		$this->currentUser = $currentUser;
-		$this->config = $config;
-		$this->groupManager = $groupManager;
-		$this->iniWrapper = $iniWrapper;
-		$this->urlGenerator = $urlGenerator;
-		$this->capabilitiesManager = $capabilitiesManager;
-		$this->initialStateService = $initialStateService;
+	public function __construct(
+		protected IL10N                $l,
+		protected Defaults             $defaults,
+		protected IAppManager          $appManager,
+		protected ISession             $session,
+		protected ?IUser               $currentUser,
+		protected IConfig              $config,
+		protected IGroupManager        $groupManager,
+		protected IniGetWrapper        $iniWrapper,
+		protected IURLGenerator        $urlGenerator,
+		protected CapabilitiesManager  $capabilitiesManager,
+		protected IInitialStateService $initialStateService,
+		protected IProvider            $tokenProvider,
+	) {
 	}
 
 	public function getConfig(): string {
@@ -155,9 +140,13 @@ class JSConfigHelper {
 		}
 
 		if ($this->currentUser instanceof IUser) {
-			$lastConfirmTimestamp = $this->session->get('last-password-confirm');
-			if (!is_int($lastConfirmTimestamp)) {
-				$lastConfirmTimestamp = 0;
+			if ($this->canUserValidatePassword()) {
+				$lastConfirmTimestamp = $this->session->get('last-password-confirm');
+				if (!is_int($lastConfirmTimestamp)) {
+					$lastConfirmTimestamp = 0;
+				}
+			} else {
+				$lastConfirmTimestamp = PHP_INT_MAX;
 			}
 		} else {
 			$lastConfirmTimestamp = 0;
@@ -310,4 +299,15 @@ class JSConfigHelper {
 
 		return $result;
 	}
+
+	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
+			return true;
+		}
+		$scope = $token->getScopeAsArray();
+		return !isset($scope['password-unconfirmable']) || $scope['password-unconfirmable'] === false;
+	}
 }

lib/private/TemplateLayout.php

@@ -43,6 +43,7 @@
 namespace OC;
 
 use bantu\IniGetWrapper\IniGetWrapper;
+use OC\Authentication\Token\IProvider;
 use OC\Search\SearchQuery;
 use OC\Template\CSSResourceLocator;
 use OC\Template\JSConfigHelper;
@@ -235,7 +236,8 @@ class TemplateLayout extends \OC_Template {
 				\OC::$server->get(IniGetWrapper::class),
 				\OC::$server->getURLGenerator(),
 				\OC::$server->getCapabilitiesManager(),
-				\OC::$server->query(IInitialStateService::class)
+				\OCP\Server::get(IInitialStateService::class),
+				\OCP\Server::get(IProvider::class),
 			);
 			$config = $jsConfigHelper->getConfig();
 			if (\OC::$server->getContentSecurityPolicyNonceManager()->browserSupportsCspV3()) {

lib/private/legacy/OC_User.php

@@ -35,7 +35,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>
  *
  */
-
+use OC\Authentication\Token\IProvider;
 use OC\User\LoginException;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\ILogger;
@@ -193,6 +193,14 @@ class OC_User {
 
 				$userSession->createSessionToken($request, $uid, $uid, $password);
 				$userSession->createRememberMeToken($userSession->getUser());
+
+				if (empty($password)) {
+					$tokenProvider = \OC::$server->get(IProvider::class);
+					$token = $tokenProvider->getToken($userSession->getSession()->getId());
+					$token->setScope(['password-unconfirmable' => true]);
+					$tokenProvider->updateToken($token);
+				}
+
 				// setup the filesystem
 				OC_Util::setupFS($uid);
 				// first call the post_login hooks, the login-process needs to be

tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php

@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace Test\AppFramework\Middleware\Security\Mock;
+
+use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
+
+class PasswordConfirmationMiddlewareController extends \OCP\AppFramework\Controller {
+	public function testNoAnnotationNorAttribute() {
+	}
+
+	/**
+	 * @TestAnnotation
+	 */
+	public function testDifferentAnnotation() {
+	}
+
+	/**
+	 * @PasswordConfirmationRequired
+	 */
+	public function testAnnotation() {
+	}
+
+	/**
+	 * @PasswordConfirmationRequired
+	 */
+	public function testAttribute() {
+	}
+
+	/**
+	 * @PasswordConfirmationRequired
+	 */
+	public function testSSO() {
+	}
+}

tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php

@@ -27,10 +27,14 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
 use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\AppFramework\Controller;
+use OC\Authentication\Token\IProvider;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OC\Authentication\Token\IToken;
+use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUser;
 use OCP\IUserSession;
+use Test\AppFramework\Middleware\Security\Mock\PasswordConfirmationMiddlewareController;
 use Test\TestCase;
 
 class PasswordConfirmationMiddlewareTest extends TestCase {
@@ -45,23 +49,29 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 	/** @var PasswordConfirmationMiddleware */
 	private $middleware;
 	/** @var Controller */
-	private $contoller;
+	private $controller;
 	/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
 	private $timeFactory;
+	private IProvider|\PHPUnit\Framework\MockObject\MockObject $tokenProvider;
 
 	protected function setUp(): void {
 		$this->reflector = new ControllerMethodReflector();
 		$this->session = $this->createMock(ISession::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->user = $this->createMock(IUser::class);
-		$this->contoller = $this->createMock(Controller::class);
+		$this->controller = new PasswordConfirmationMiddlewareController(
+			'test',
+			$this->createMock(IRequest::class)
+		);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->tokenProvider = $this->createMock(IProvider::class);
 
 		$this->middleware = new PasswordConfirmationMiddleware(
 			$this->reflector,
 			$this->session,
 			$this->userSession,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->tokenProvider,
 		);
 	}
 
@@ -72,7 +82,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 		$this->userSession->expects($this->never())
 			->method($this->anything());
 
-		$this->middleware->beforeController($this->contoller, __FUNCTION__);
+		$this->middleware->beforeController($this->controller, __FUNCTION__);
 	}
 
 	/**
@@ -85,7 +95,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 		$this->userSession->expects($this->never())
 			->method($this->anything());
 
-		$this->middleware->beforeController($this->contoller, __FUNCTION__);
+		$this->middleware->beforeController($this->controller, __FUNCTION__);
 	}
 
 	/**
@@ -94,6 +104,13 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 	 */
 	public function testAnnotation($backend, $lastConfirm, $currentTime, $exception) {
 		$this->reflector->reflect(__CLASS__, __FUNCTION__);
+		$token = $this->createMock(IToken::class);
+		$token->method('getScopeAsArray')
+			->willReturn([]);
+		$this->tokenProvider->expects($this->once())
+			->method('getToken')
+			->willReturn($token);
+
 
 		$this->user->method('getBackendClassName')
 			->willReturn($backend);
@@ -107,9 +124,16 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 		$this->timeFactory->method('getTime')
 			->willReturn($currentTime);
 
+		$token = $this->createMock(IToken::class);
+		$token->method('getScopeAsArray')
+			->willReturn([]);
+		$this->tokenProvider->expects($this->once())
+			->method('getToken')
+			->willReturn($token);
+
 		$thrown = false;
 		try {
-			$this->middleware->beforeController($this->contoller, __FUNCTION__);
+			$this->middleware->beforeController($this->controller, __FUNCTION__);
 		} catch (NotConfirmedException $e) {
 			$thrown = true;
 		}
@@ -117,6 +141,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 		$this->assertSame($exception, $thrown);
 	}
 
+
+
 	public function dataProvider() {
 		return [
 			['foo', 2000, 4000, true],
@@ -127,4 +153,41 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 			['foo', 2000, 3816, true],
 		];
 	}
+
+	public function testSSO() {
+		static $sessionId = 'mySession1d';
+
+		$this->reflector->reflect($this->controller, __FUNCTION__);
+
+		$this->user->method('getBackendClassName')
+			->willReturn('fictional_backend');
+		$this->userSession->method('getUser')
+			->willReturn($this->user);
+
+		$this->session->method('get')
+			->with('last-password-confirm')
+			->willReturn(0);
+		$this->session->method('getId')
+			->willReturn($sessionId);
+
+		$this->timeFactory->method('getTime')
+			->willReturn(9876);
+
+		$token = $this->createMock(IToken::class);
+		$token->method('getScopeAsArray')
+			->willReturn(['password-unconfirmable' => true]);
+		$this->tokenProvider->expects($this->once())
+			->method('getToken')
+			->with($sessionId)
+			->willReturn($token);
+
+		$thrown = false;
+		try {
+			$this->middleware->beforeController($this->controller, __FUNCTION__);
+		} catch (NotConfirmedException) {
+			$thrown = true;
+		}
+
+		$this->assertSame(false, $thrown);
+	}
 }