From 340939e688fab5c52061bc9e358587fbd8ec9fc8 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 1 Mar 2024 18:37:47 +0100 Subject: [PATCH 1/2] 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 --- core/Controller/OCJSController.php | 5 +- .../DependencyInjection/DIContainer.php | 3 +- .../PasswordConfirmationMiddleware.php | 26 ++++++- .../Token/PublicKeyTokenProvider.php | 1 + lib/private/Template/JSConfigHelper.php | 73 ++++++++++--------- lib/private/TemplateLayout.php | 4 +- lib/private/legacy/OC_User.php | 9 +++ ...sswordConfirmationMiddlewareController.php | 4 + .../PasswordConfirmationMiddlewareTest.php | 60 ++++++++++++++- 9 files changed, 143 insertions(+), 42 deletions(-) diff --git a/core/Controller/OCJSController.php b/core/Controller/OCJSController.php index 8eed7a871c6..11a6e5827d8 100644 --- a/core/Controller/OCJSController.php +++ b/core/Controller/OCJSController.php @@ -6,6 +6,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; @@ -42,6 +43,7 @@ class OCJSController extends Controller { IURLGenerator $urlGenerator, CapabilitiesManager $capabilitiesManager, IInitialStateService $initialStateService, + IProvider $tokenProvider, ) { parent::__construct($appName, $request); @@ -56,7 +58,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 9e3119d262a..4add17396b0 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -249,7 +249,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 b95a69dc3e2..9fa4aedd401 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -7,12 +7,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; @@ -27,6 +32,7 @@ class PasswordConfirmationMiddleware extends Middleware { private $timeFactory; /** @var array */ private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true]; + private IProvider $tokenProvider; /** * PasswordConfirmationMiddleware constructor. @@ -39,11 +45,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; } /** @@ -68,8 +77,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 5363b82df69..767ece1e551 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -243,6 +243,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 2f2ff9d64e3..5c38ae4cc72 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -8,10 +8,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; @@ -23,48 +27,30 @@ use OCP\ILogger; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUser; +use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Share\IManager as IShareManager; 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 { @@ -130,9 +116,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; @@ -287,4 +277,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 0ff56ece635..e04bc0330c0 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -8,6 +8,7 @@ namespace OC; use bantu\IniGetWrapper\IniGetWrapper; +use OC\Authentication\Token\IProvider; use OC\Search\SearchQuery; use OC\Template\CSSResourceLocator; use OC\Template\JSConfigHelper; @@ -224,7 +225,8 @@ class TemplateLayout extends \OC_Template { \OC::$server->get(IniGetWrapper::class), \OC::$server->getURLGenerator(), \OC::$server->get(CapabilitiesManager::class), - \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 e7708525c76..66d28771ae1 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ +use OC\Authentication\Token\IProvider; use OC\User\LoginException; use OCP\EventDispatcher\IEventDispatcher; use OCP\IGroupManager; @@ -166,6 +167,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 82d6cf8f251..02159661ff6 100644 --- a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php +++ b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php @@ -30,4 +30,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 3613ca624a3..f1030c449e2 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -9,7 +9,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; @@ -32,6 +34,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(); @@ -39,6 +42,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) @@ -48,7 +52,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $this->reflector, $this->session, $this->userSession, - $this->timeFactory + $this->timeFactory, + $this->tokenProvider, ); } @@ -90,6 +95,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__); @@ -118,6 +130,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__); @@ -128,6 +147,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $this->assertSame($exception, $thrown); } + + public function dataProvider() { return [ ['foo', 2000, 4000, true], @@ -138,4 +159,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); + } } From f6d6efef3a26fc5524988cdfba780dce035cd61b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 15 Mar 2024 12:51:31 +0100 Subject: [PATCH 2/2] refactor(Token): introduce scope constants Signed-off-by: Arthur Schiwon --- .../lib/Controller/AuthSettingsController.php | 4 ++-- .../Controller/AuthSettingsControllerTest.php | 20 +++++++++---------- .../Personal/Security/AuthtokensTest.php | 5 +++-- .../PasswordConfirmationMiddleware.php | 3 ++- .../Authentication/Token/PublicKeyToken.php | 3 ++- lib/private/Lockdown/LockdownManager.php | 4 ++-- lib/private/Template/JSConfigHelper.php | 3 ++- lib/private/legacy/OC_User.php | 3 ++- lib/public/Authentication/Token/IToken.php | 9 +++++++++ .../PasswordConfirmationMiddlewareTest.php | 2 +- .../Token/PublicKeyTokenTest.php | 5 +++-- tests/lib/Lockdown/Filesystem/NoFSTest.php | 5 +++-- tests/lib/Lockdown/LockdownManagerTest.php | 5 +++-- 13 files changed, 44 insertions(+), 27 deletions(-) diff --git a/apps/settings/lib/Controller/AuthSettingsController.php b/apps/settings/lib/Controller/AuthSettingsController.php index 8a01c7c2468..8f8ceb7d9ca 100644 --- a/apps/settings/lib/Controller/AuthSettingsController.php +++ b/apps/settings/lib/Controller/AuthSettingsController.php @@ -241,8 +241,8 @@ class AuthSettingsController extends Controller { $currentName = $token->getName(); if ($scope !== $token->getScopeAsArray()) { - $token->setScope(['filesystem' => $scope['filesystem']]); - $this->publishActivity($scope['filesystem'] ? Provider::APP_TOKEN_FILESYSTEM_GRANTED : Provider::APP_TOKEN_FILESYSTEM_REVOKED, $token->getId(), ['name' => $currentName]); + $token->setScope([IToken::SCOPE_FILESYSTEM => $scope[IToken::SCOPE_FILESYSTEM]]); + $this->publishActivity($scope[IToken::SCOPE_FILESYSTEM] ? Provider::APP_TOKEN_FILESYSTEM_GRANTED : Provider::APP_TOKEN_FILESYSTEM_REVOKED, $token->getId(), ['name' => $currentName]); } if (mb_strlen($name) > 128) { diff --git a/apps/settings/tests/Controller/AuthSettingsControllerTest.php b/apps/settings/tests/Controller/AuthSettingsControllerTest.php index b744b942e09..747515898ea 100644 --- a/apps/settings/tests/Controller/AuthSettingsControllerTest.php +++ b/apps/settings/tests/Controller/AuthSettingsControllerTest.php @@ -267,7 +267,7 @@ class AuthSettingsControllerTest extends TestCase { $token->expects($this->once()) ->method('getScopeAsArray') - ->willReturn(['filesystem' => true]); + ->willReturn([IToken::SCOPE_FILESYSTEM => true]); $token->expects($this->once()) ->method('setName') @@ -277,7 +277,7 @@ class AuthSettingsControllerTest extends TestCase { ->method('updateToken') ->with($this->equalTo($token)); - $this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true], $newName)); + $this->assertSame([], $this->controller->update($tokenId, [IToken::SCOPE_FILESYSTEM => true], $newName)); } public function dataUpdateFilesystemScope(): array { @@ -310,17 +310,17 @@ class AuthSettingsControllerTest extends TestCase { $token->expects($this->once()) ->method('getScopeAsArray') - ->willReturn(['filesystem' => $filesystem]); + ->willReturn([IToken::SCOPE_FILESYSTEM => $filesystem]); $token->expects($this->once()) ->method('setScope') - ->with($this->equalTo(['filesystem' => $newFilesystem])); + ->with($this->equalTo([IToken::SCOPE_FILESYSTEM => $newFilesystem])); $this->tokenProvider->expects($this->once()) ->method('updateToken') ->with($this->equalTo($token)); - $this->assertSame([], $this->controller->update($tokenId, ['filesystem' => $newFilesystem], 'App password')); + $this->assertSame([], $this->controller->update($tokenId, [IToken::SCOPE_FILESYSTEM => $newFilesystem], 'App password')); } public function testUpdateNoChange(): void { @@ -339,7 +339,7 @@ class AuthSettingsControllerTest extends TestCase { $token->expects($this->once()) ->method('getScopeAsArray') - ->willReturn(['filesystem' => true]); + ->willReturn([IToken::SCOPE_FILESYSTEM => true]); $token->expects($this->never()) ->method('setName'); @@ -351,7 +351,7 @@ class AuthSettingsControllerTest extends TestCase { ->method('updateToken') ->with($this->equalTo($token)); - $this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true], 'App password')); + $this->assertSame([], $this->controller->update($tokenId, [IToken::SCOPE_FILESYSTEM => true], 'App password')); } public function testUpdateExpired() { @@ -371,7 +371,7 @@ class AuthSettingsControllerTest extends TestCase { ->method('updateToken') ->with($this->equalTo($token)); - $this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true], 'App password')); + $this->assertSame([], $this->controller->update($tokenId, [IToken::SCOPE_FILESYSTEM => true], 'App password')); } public function testUpdateTokenWrongUser() { @@ -389,7 +389,7 @@ class AuthSettingsControllerTest extends TestCase { $this->tokenProvider->expects($this->never()) ->method('updateToken'); - $response = $this->controller->update($tokenId, ['filesystem' => true], 'App password'); + $response = $this->controller->update($tokenId, [IToken::SCOPE_FILESYSTEM => true], 'App password'); $this->assertSame([], $response->getData()); $this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus()); } @@ -403,7 +403,7 @@ class AuthSettingsControllerTest extends TestCase { $this->tokenProvider->expects($this->never()) ->method('updateToken'); - $response = $this->controller->update(42, ['filesystem' => true], 'App password'); + $response = $this->controller->update(42, [IToken::SCOPE_FILESYSTEM => true], 'App password'); $this->assertSame([], $response->getData()); $this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus()); } diff --git a/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php b/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php index 5ccec936555..13d720c201e 100644 --- a/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php +++ b/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php @@ -30,6 +30,7 @@ use OC\Authentication\Token\PublicKeyToken; use OCA\Settings\Settings\Personal\Security\Authtokens; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; +use OCP\Authentication\Token\IToken; use OCP\ISession; use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; @@ -108,7 +109,7 @@ class AuthtokensTest extends TestCase { 'type' => 0, 'canDelete' => false, 'current' => true, - 'scope' => ['filesystem' => true], + 'scope' => [IToken::SCOPE_FILESYSTEM => true], 'canRename' => false, ], [ @@ -117,7 +118,7 @@ class AuthtokensTest extends TestCase { 'lastActivity' => 0, 'type' => 0, 'canDelete' => true, - 'scope' => ['filesystem' => true], + 'scope' => [IToken::SCOPE_FILESYSTEM => true], 'canRename' => true, ], ] diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 9fa4aedd401..5ff9d7386da 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -15,6 +15,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Exceptions\WipeTokenException; +use OCP\Authentication\Token\IToken; use OCP\ISession; use OCP\IUserSession; use OCP\Session\Exceptions\SessionNotAvailableException; @@ -85,7 +86,7 @@ class PasswordConfirmationMiddleware extends Middleware { return; } $scope = $token->getScopeAsArray(); - if (isset($scope['sso-based-login']) && $scope['sso-based-login'] === true) { + 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 return; } diff --git a/lib/private/Authentication/Token/PublicKeyToken.php b/lib/private/Authentication/Token/PublicKeyToken.php index 0b7a2589f3e..961b7191d84 100644 --- a/lib/private/Authentication/Token/PublicKeyToken.php +++ b/lib/private/Authentication/Token/PublicKeyToken.php @@ -9,6 +9,7 @@ declare(strict_types=1); namespace OC\Authentication\Token; use OCP\AppFramework\Db\Entity; +use OCP\Authentication\Token\IToken; /** * @method void setId(int $id) @@ -162,7 +163,7 @@ class PublicKeyToken extends Entity implements INamedToken, IWipeableToken { $scope = json_decode($this->getScope(), true); if (!$scope) { return [ - 'filesystem' => true + IToken::SCOPE_FILESYSTEM => true ]; } return $scope; diff --git a/lib/private/Lockdown/LockdownManager.php b/lib/private/Lockdown/LockdownManager.php index 779b1ea2650..3b45709d5c9 100644 --- a/lib/private/Lockdown/LockdownManager.php +++ b/lib/private/Lockdown/LockdownManager.php @@ -5,7 +5,7 @@ */ namespace OC\Lockdown; -use OC\Authentication\Token\IToken; +use OCP\Authentication\Token\IToken; use OCP\ISession; use OCP\Lockdown\ILockdownManager; @@ -60,6 +60,6 @@ class LockdownManager implements ILockdownManager { public function canAccessFilesystem() { $scope = $this->getScopeAsArray(); - return !$scope || $scope['filesystem']; + return !$scope || $scope[IToken::SCOPE_FILESYSTEM]; } } diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index 5c38ae4cc72..a41e99ae8c4 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -16,6 +16,7 @@ use OCP\App\IAppManager; use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Exceptions\WipeTokenException; +use OCP\Authentication\Token\IToken; use OCP\Constants; use OCP\Defaults; use OCP\Files\FileInfo; @@ -286,6 +287,6 @@ class JSConfigHelper { return true; } $scope = $token->getScopeAsArray(); - return !isset($scope['sso-based-login']) || $scope['sso-based-login'] === false; + return !isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) || $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === false; } } diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php index 66d28771ae1..f9f751f7b14 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -7,6 +7,7 @@ */ use OC\Authentication\Token\IProvider; use OC\User\LoginException; +use OCP\Authentication\Token\IToken; use OCP\EventDispatcher\IEventDispatcher; use OCP\IGroupManager; use OCP\ISession; @@ -171,7 +172,7 @@ class OC_User { if (empty($password)) { $tokenProvider = \OC::$server->get(IProvider::class); $token = $tokenProvider->getToken($userSession->getSession()->getId()); - $token->setScope(['sso-based-login' => true]); + $token->setScope([IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true]); $tokenProvider->updateToken($token); } diff --git a/lib/public/Authentication/Token/IToken.php b/lib/public/Authentication/Token/IToken.php index 4f232b83d4e..8c047280924 100644 --- a/lib/public/Authentication/Token/IToken.php +++ b/lib/public/Authentication/Token/IToken.php @@ -34,6 +34,15 @@ interface IToken extends JsonSerializable { */ public const REMEMBER = 1; + /** + * @since 30.0.0 + */ + public const SCOPE_FILESYSTEM = 'filesystem'; + /** + * @since 30.0.0 + */ + public const SCOPE_SKIP_PASSWORD_VALIDATION = 'password-unconfirmable'; + /** * Get the token ID * @since 28.0.0 diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index f1030c449e2..beee7151264 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -181,7 +181,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $token = $this->createMock(IToken::class); $token->method('getScopeAsArray') - ->willReturn(['sso-based-login' => true]); + ->willReturn([IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true]); $this->tokenProvider->expects($this->once()) ->method('getToken') ->with($sessionId) diff --git a/tests/lib/Authentication/Token/PublicKeyTokenTest.php b/tests/lib/Authentication/Token/PublicKeyTokenTest.php index acbddebea35..cc8890002e9 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenTest.php @@ -9,11 +9,12 @@ declare(strict_types=1); namespace Test\Authentication\Token; use OC\Authentication\Token\PublicKeyToken; +use OCP\Authentication\Token\IToken; use Test\TestCase; class PublicKeyTokenTest extends TestCase { public function testSetScopeAsArray() { - $scope = ['filesystem' => false]; + $scope = [IToken::SCOPE_FILESYSTEM => false]; $token = new PublicKeyToken(); $token->setScope($scope); $this->assertEquals(json_encode($scope), $token->getScope()); @@ -21,7 +22,7 @@ class PublicKeyTokenTest extends TestCase { } public function testDefaultScope() { - $scope = ['filesystem' => true]; + $scope = [IToken::SCOPE_FILESYSTEM => true]; $token = new PublicKeyToken(); $this->assertEquals($scope, $token->getScopeAsArray()); } diff --git a/tests/lib/Lockdown/Filesystem/NoFSTest.php b/tests/lib/Lockdown/Filesystem/NoFSTest.php index 08429228647..7a636fbaaaa 100644 --- a/tests/lib/Lockdown/Filesystem/NoFSTest.php +++ b/tests/lib/Lockdown/Filesystem/NoFSTest.php @@ -9,6 +9,7 @@ namespace Test\Lockdown\Filesystem; use OC\Authentication\Token\PublicKeyToken; use OC\Files\Filesystem; use OC\Lockdown\Filesystem\NullStorage; +use OCP\Authentication\Token\IToken; use Test\Traits\UserTrait; /** @@ -20,7 +21,7 @@ class NoFSTest extends \Test\TestCase { protected function tearDown(): void { $token = new PublicKeyToken(); $token->setScope([ - 'filesystem' => true + IToken::SCOPE_FILESYSTEM => true ]); \OC::$server->get('LockdownManager')->setToken($token); parent::tearDown(); @@ -30,7 +31,7 @@ class NoFSTest extends \Test\TestCase { parent::setUp(); $token = new PublicKeyToken(); $token->setScope([ - 'filesystem' => false + IToken::SCOPE_FILESYSTEM => false ]); \OC::$server->get('LockdownManager')->setToken($token); diff --git a/tests/lib/Lockdown/LockdownManagerTest.php b/tests/lib/Lockdown/LockdownManagerTest.php index 5ff5a84e800..bb71a6e63de 100644 --- a/tests/lib/Lockdown/LockdownManagerTest.php +++ b/tests/lib/Lockdown/LockdownManagerTest.php @@ -8,6 +8,7 @@ namespace Test\Lockdown; use OC\Authentication\Token\PublicKeyToken; use OC\Lockdown\LockdownManager; +use OCP\Authentication\Token\IToken; use OCP\ISession; use Test\TestCase; @@ -29,7 +30,7 @@ class LockdownManagerTest extends TestCase { public function testCanAccessFilesystemAllowed() { $token = new PublicKeyToken(); - $token->setScope(['filesystem' => true]); + $token->setScope([IToken::SCOPE_FILESYSTEM => true]); $manager = new LockdownManager($this->sessionCallback); $manager->setToken($token); $this->assertTrue($manager->canAccessFilesystem()); @@ -37,7 +38,7 @@ class LockdownManagerTest extends TestCase { public function testCanAccessFilesystemNotAllowed() { $token = new PublicKeyToken(); - $token->setScope(['filesystem' => false]); + $token->setScope([IToken::SCOPE_FILESYSTEM => false]); $manager = new LockdownManager($this->sessionCallback); $manager->setToken($token); $this->assertFalse($manager->canAccessFilesystem());