aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArthur Schiwon <blizzz@arthur-schiwon.de>2024-03-01 18:37:47 +0100
committerAndy Scherzinger <info@andy-scherzinger.de>2024-06-11 20:19:18 +0200
commitf0494ec17a97f83d0eb23b7a69f73e0afdf3c019 (patch)
treece49db3df9a13aa304220e1e657a5a3778259032
parentf211e9a767f64f3e86d4b3dcb11230cb9cc9cbfc (diff)
downloadnextcloud-server-f0494ec17a97f83d0eb23b7a69f73e0afdf3c019.tar.gz
nextcloud-server-f0494ec17a97f83d0eb23b7a69f73e0afdf3c019.zip
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>
-rw-r--r--core/Controller/OCJSController.php5
-rw-r--r--lib/private/AppFramework/DependencyInjection/DIContainer.php3
-rw-r--r--lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php26
-rw-r--r--lib/private/Authentication/Token/PublicKeyTokenProvider.php1
-rw-r--r--lib/private/Template/JSConfigHelper.php73
-rw-r--r--lib/private/TemplateLayout.php4
-rw-r--r--lib/private/legacy/OC_User.php9
-rw-r--r--tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php4
-rw-r--r--tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php60
9 files changed, 143 insertions, 42 deletions
diff --git a/core/Controller/OCJSController.php b/core/Controller/OCJSController.php
index dbb203e827f..4dc3a0a4b9c 100644
--- 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;
@@ -65,6 +66,7 @@ class OCJSController extends Controller {
IURLGenerator $urlGenerator,
CapabilitiesManager $capabilitiesManager,
IInitialStateService $initialStateService,
+ IProvider $tokenProvider,
) {
parent::__construct($appName, $request);
@@ -79,7 +81,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 a69fc24c978..f920ff4f207 100644
--- 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 351f47ea924..27328e17b03 100644
--- 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 3a15ba006d4..8a6b0b6fed7 100644
--- 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 88d09355bde..48bf84d7306 100644
--- 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,48 +53,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 {
@@ -156,9 +142,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;
@@ -313,4 +303,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 966e8bf9478..d17189a3bd0 100644
--- 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->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 3fe3f173b99..1d6085f72ee 100644
--- a/lib/private/legacy/OC_User.php
+++ b/lib/private/legacy/OC_User.php
@@ -36,6 +36,7 @@
*
*/
+use OC\Authentication\Token\IProvider;
use OC\User\LoginException;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IGroupManager;
@@ -197,6 +198,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 5b83575f711..941906d8bb6 100644
--- 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 3752259c61b..ed51837acbf 100644
--- 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);
+ }
}