fix(Session): avoid password confirmation on SSO

SSO backends like SAML and OIDC tried a trick to suppress password
confirmations as they are not possible by design. At least for SAML it was
not reliable when existing user backends where used as user repositories.

Now we are setting a special scope with the token, and also make sure that
the scope is taken over when tokens are regenerated.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

diff --git a/core/Controller/OCJSController.php b/core/Controller/OCJSController.php
index e909343912574afd5df06dd08024c3e5751e8f0e..d20665ccfeab7082a7651c62b17570b0eff3fff3 100644 (file)
--- a/core/Controller/OCJSController.php
+++ b/core/Controller/OCJSController.php
@@ -29,6 +29,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;
@@ -64,6 +65,7 @@ class OCJSController extends Controller {
                IURLGenerator $urlGenerator,
                CapabilitiesManager $capabilitiesManager,
                IInitialStateService $initialStateService,
+               IProvider $tokenProvider,
        ) {
                parent::__construct($appName, $request);
 
@@ -78,7 +80,8 @@ class OCJSController extends Controller {
                        $iniWrapper,
                        $urlGenerator,
                        $capabilitiesManager,
-                       $initialStateService
+                       $initialStateService,
+                       $tokenProvider
                );
        }
 

diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
index a0951e75523230d9dc388da5f86bfe4b60929e06..bbbbca4e00fca6bc2f2dd9d3a77fdccd1e85af7c 100644 (file)
--- a/lib/private/AppFramework/DependencyInjection/DIContainer.php
+++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -276,7 +276,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(

diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php
index 351f47ea924590d0e4873e7b556fda305c6b92ff..27328e17b03c74065ec8545e07af23a41f8d1c07 100644 (file)
--- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php
@@ -25,12 +25,17 @@ namespace OC\AppFramework\Middleware\Security;
 
 use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Authentication\Token\IProvider;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Authentication\Exceptions\ExpiredTokenException;
+use OCP\Authentication\Exceptions\InvalidTokenException;
+use OCP\Authentication\Exceptions\WipeTokenException;
 use OCP\ISession;
 use OCP\IUserSession;
+use OCP\Session\Exceptions\SessionNotAvailableException;
 use OCP\User\Backend\IPasswordConfirmationBackend;
 use ReflectionMethod;
 
@@ -45,6 +50,7 @@ class PasswordConfirmationMiddleware extends Middleware {
        private $timeFactory;
        /** @var array */
        private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
+       private IProvider $tokenProvider;
 
        /**
         * PasswordConfirmationMiddleware constructor.
@@ -57,11 +63,14 @@ class PasswordConfirmationMiddleware extends Middleware {
        public function __construct(ControllerMethodReflector $reflector,
                ISession $session,
                IUserSession $userSession,
-               ITimeFactory $timeFactory) {
+               ITimeFactory $timeFactory,
+               IProvider $tokenProvider,
+       ) {
                $this->reflector = $reflector;
                $this->session = $session;
                $this->userSession = $userSession;
                $this->timeFactory = $timeFactory;
+               $this->tokenProvider = $tokenProvider;
        }
 
        /**
@@ -86,8 +95,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['sso-based-login']) && $scope['sso-based-login'] === 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();
                        }

diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php
index 3a15ba006d461e3c5ed8f12224ccf4663443e75a..8a6b0b6fed7350abc4b45d1bb4682576f931b5ba 100644 (file)
--- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php
+++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php
@@ -265,6 +265,7 @@ class PublicKeyTokenProvider implements IProvider {
                                OCPIToken::TEMPORARY_TOKEN,
                                $token->getRemember()
                        );
+                       $newToken->setScope($token->getScopeAsArray());
                        $this->cacheToken($newToken);
 
                        $this->cacheInvalidHash($token->getToken());

diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php
index 8cba93f1f4e693d121fc73f5c6b2739fe02b953b..cca3d64654445ed0407ac59b298281f18924fadd 100644 (file)
--- a/lib/private/Template/JSConfigHelper.php
+++ b/lib/private/Template/JSConfigHelper.php
@@ -34,10 +34,14 @@ declare(strict_types=1);
 namespace OC\Template;
 
 use bantu\IniGetWrapper\IniGetWrapper;
+use OC\Authentication\Token\IProvider;
 use OC\CapabilitiesManager;
 use OC\Share\Share;
 use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
+use OCP\Authentication\Exceptions\ExpiredTokenException;
+use OCP\Authentication\Exceptions\InvalidTokenException;
+use OCP\Authentication\Exceptions\WipeTokenException;
 use OCP\Constants;
 use OCP\Defaults;
 use OCP\Files\FileInfo;
@@ -49,47 +53,29 @@ use OCP\ILogger;
 use OCP\ISession;
 use OCP\IURLGenerator;
 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 +141,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;
@@ -311,4 +301,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['sso-based-login']) || $scope['sso-based-login'] === false;
+       }
 }

diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php
index 96d0ae3e5174de86d9e6cb8c7980b2d6136b1c05..7835e974b859818cb14ada2f438d374b17008f2e 100644 (file)
--- a/lib/private/TemplateLayout.php
+++ b/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;
@@ -259,7 +260,8 @@ class TemplateLayout extends \OC_Template {
                                \OC::$server->get(IniGetWrapper::class),
                                \OC::$server->getURLGenerator(),
                                \OC::$server->getCapabilitiesManager(),
-                               \OCP\Server::get(IInitialStateService::class)
+                               \OCP\Server::get(IInitialStateService::class),
+                               \OCP\Server::get(IProvider::class),
                        );
                        $config = $jsConfigHelper->getConfig();
                        if (\OC::$server->getContentSecurityPolicyNonceManager()->browserSupportsCspV3()) {

diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php
index dc172ba4144f90a26ab7c4a8e0b83211ab8a10b6..7cf0b3487a92e45195f1c4747c23247004e0b887 100644 (file)
--- a/lib/private/legacy/OC_User.php
+++ b/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\IGroupManager;
@@ -196,6 +196,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(['sso-based-login' => true]);
+                                       $tokenProvider->updateToken($token);
+                               }
+
                                // setup the filesystem
                                OC_Util::setupFS($uid);
                                // first call the post_login hooks, the login-process needs to be

diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php
index 5b83575f7117c37c6fa08393c903770a22a71480..941906d8bb654ed5c705fdc4b5310f6185b0f322 100644 (file)
--- a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php
+++ b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php
@@ -46,4 +46,8 @@ class PasswordConfirmationMiddlewareController extends \OCP\AppFramework\Control
        #[PasswordConfirmationRequired]
        public function testAttribute() {
        }
+
+       #[PasswordConfirmationRequired]
+       public function testSSO() {
+       }
 }

diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php
index 3752259c61baa2eeaec0f03535c4d524c8984dae..ed51837acbfc22984e1c5836c108ed4a85e3780f 100644 (file)
--- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php
@@ -26,7 +26,9 @@ namespace Test\AppFramework\Middleware\Security;
 use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
 use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Authentication\Token\IProvider;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Authentication\Token\IToken;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUser;
@@ -49,6 +51,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
        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();
@@ -56,6 +59,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
                $this->userSession = $this->createMock(IUserSession::class);
                $this->user = $this->createMock(IUser::class);
                $this->timeFactory = $this->createMock(ITimeFactory::class);
+               $this->tokenProvider = $this->createMock(IProvider::class);
                $this->controller = new PasswordConfirmationMiddlewareController(
                        'test',
                        $this->createMock(IRequest::class)
@@ -65,7 +69,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
                        $this->reflector,
                        $this->session,
                        $this->userSession,
-                       $this->timeFactory
+                       $this->timeFactory,
+                       $this->tokenProvider,
                );
        }
 
@@ -107,6 +112,13 @@ 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->controller, __FUNCTION__);
@@ -135,6 +147,13 @@ 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->controller, __FUNCTION__);
@@ -145,6 +164,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
                $this->assertSame($exception, $thrown);
        }
 
+
+
        public function dataProvider() {
                return [
                        ['foo', 2000, 4000, true],
@@ -155,4 +176,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(['sso-based-login' => 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);
+       }
 }