From eea5e1cca2d1552c35d1e3a1d07ac16351fbefd8 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 1 Mar 2024 18:37:47 +0100 Subject: 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 --- .../PasswordConfirmationMiddlewareTest.php | 60 +++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 3153d7f0b08..98847a69d2f 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -27,7 +27,9 @@ 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\ISession; use OCP\IUser; use OCP\IUserSession; @@ -48,6 +50,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { private $contoller; /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ private $timeFactory; + private IProvider|\PHPUnit\Framework\MockObject\MockObject $tokenProvider; protected function setUp(): void { $this->reflector = new ControllerMethodReflector(); @@ -56,12 +59,14 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $this->user = $this->createMock(IUser::class); $this->contoller = $this->createMock(Controller::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, ); } @@ -94,6 +99,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,6 +119,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->contoller, __FUNCTION__); @@ -117,6 +136,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $this->assertSame($exception, $thrown); } + + public function dataProvider() { return [ ['foo', 2000, 4000, true], @@ -127,4 +148,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); + } } -- cgit v1.2.3 From 0f5c8f9111c327d13f4ef3637ac11fbcc6e64ffd Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 12 Jun 2024 11:05:43 +0200 Subject: fix(Token): make new scope future compatible - "password-unconfirmable" is the effective name for 30, but a draft name was backported. Signed-off-by: Arthur Schiwon --- .../AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php | 2 +- lib/private/Template/JSConfigHelper.php | 2 +- lib/private/legacy/OC_User.php | 2 +- .../Middleware/Security/PasswordConfirmationMiddlewareTest.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index beb64d42191..4425931b8bb 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -99,7 +99,7 @@ class PasswordConfirmationMiddleware extends Middleware { return; } $scope = $token->getScopeAsArray(); - if (isset($scope['sso-based-login']) && $scope['sso-based-login'] === true) { + if (isset($scope['password-unconfirmable']) && $scope['password-unconfirmable'] === true) { // Users logging in from SSO backends cannot confirm their password by design return; } diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index d890ac785af..6942fdeebcb 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -309,6 +309,6 @@ class JSConfigHelper { return true; } $scope = $token->getScopeAsArray(); - return !isset($scope['sso-based-login']) || $scope['sso-based-login'] === false; + return !isset($scope['password-unconfirmable']) || $scope['password-unconfirmable'] === false; } } diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php index 24ffaa3b3aa..ec0b7e69c8a 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -197,7 +197,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(['password-unconfirmable' => true]); $tokenProvider->updateToken($token); } diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 98847a69d2f..102c7631003 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -170,7 +170,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $token = $this->createMock(IToken::class); $token->method('getScopeAsArray') - ->willReturn(['sso-based-login' => true]); + ->willReturn(['password-unconfirmable' => true]); $this->tokenProvider->expects($this->once()) ->method('getToken') ->with($sessionId) -- cgit v1.2.3 From 527bc5d984f2fbad6d1f5ee95085bfdf7c30634f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 13 Jun 2024 20:05:34 +0200 Subject: test(unit): adjust testSSO scenario and test class Signed-off-by: Arthur Schiwon --- .../PasswordConfirmationMiddlewareController.php | 57 ++++++++++++++++++++++ .../PasswordConfirmationMiddlewareTest.php | 15 ++++-- 2 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php (limited to 'tests') diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php new file mode 100644 index 00000000000..e06b4f2a76a --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php @@ -0,0 +1,57 @@ + + * + * @author Joas Schilling + * + * @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 . + */ + +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() { + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 102c7631003..b61732fbd4c 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -30,9 +30,11 @@ 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 { @@ -47,7 +49,7 @@ 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; @@ -57,7 +59,10 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $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); @@ -77,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__); } /** @@ -90,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__); } /** @@ -128,7 +133,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $thrown = false; try { - $this->middleware->beforeController($this->contoller, __FUNCTION__); + $this->middleware->beforeController($this->controller, __FUNCTION__); } catch (NotConfirmedException $e) { $thrown = true; } -- cgit v1.2.3