Skip to content

Commit 340939e

Browse files
committed
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 <[email protected]>
1 parent 31b0a44 commit 340939e

File tree

9 files changed

+143
-42
lines changed

9 files changed

+143
-42
lines changed

core/Controller/OCJSController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace OC\Core\Controller;
77

88
use bantu\IniGetWrapper\IniGetWrapper;
9+
use OC\Authentication\Token\IProvider;
910
use OC\CapabilitiesManager;
1011
use OC\Template\JSConfigHelper;
1112
use OCP\App\IAppManager;
@@ -42,6 +43,7 @@ public function __construct(
4243
IURLGenerator $urlGenerator,
4344
CapabilitiesManager $capabilitiesManager,
4445
IInitialStateService $initialStateService,
46+
IProvider $tokenProvider,
4547
) {
4648
parent::__construct($appName, $request);
4749

@@ -56,7 +58,8 @@ public function __construct(
5658
$iniWrapper,
5759
$urlGenerator,
5860
$capabilitiesManager,
59-
$initialStateService
61+
$initialStateService,
62+
$tokenProvider
6063
);
6164
}
6265

lib/private/AppFramework/DependencyInjection/DIContainer.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
249249
$c->get(IControllerMethodReflector::class),
250250
$c->get(ISession::class),
251251
$c->get(IUserSession::class),
252-
$c->get(ITimeFactory::class)
252+
$c->get(ITimeFactory::class),
253+
$c->get(\OC\Authentication\Token\IProvider::class),
253254
)
254255
);
255256
$dispatcher->registerMiddleware(

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,17 @@
77

88
use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
99
use OC\AppFramework\Utility\ControllerMethodReflector;
10+
use OC\Authentication\Token\IProvider;
1011
use OCP\AppFramework\Controller;
1112
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
1213
use OCP\AppFramework\Middleware;
1314
use OCP\AppFramework\Utility\ITimeFactory;
15+
use OCP\Authentication\Exceptions\ExpiredTokenException;
16+
use OCP\Authentication\Exceptions\InvalidTokenException;
17+
use OCP\Authentication\Exceptions\WipeTokenException;
1418
use OCP\ISession;
1519
use OCP\IUserSession;
20+
use OCP\Session\Exceptions\SessionNotAvailableException;
1621
use OCP\User\Backend\IPasswordConfirmationBackend;
1722
use ReflectionMethod;
1823

@@ -27,6 +32,7 @@ class PasswordConfirmationMiddleware extends Middleware {
2732
private $timeFactory;
2833
/** @var array */
2934
private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
35+
private IProvider $tokenProvider;
3036

3137
/**
3238
* PasswordConfirmationMiddleware constructor.
@@ -39,11 +45,14 @@ class PasswordConfirmationMiddleware extends Middleware {
3945
public function __construct(ControllerMethodReflector $reflector,
4046
ISession $session,
4147
IUserSession $userSession,
42-
ITimeFactory $timeFactory) {
48+
ITimeFactory $timeFactory,
49+
IProvider $tokenProvider,
50+
) {
4351
$this->reflector = $reflector;
4452
$this->session = $session;
4553
$this->userSession = $userSession;
4654
$this->timeFactory = $timeFactory;
55+
$this->tokenProvider = $tokenProvider;
4756
}
4857

4958
/**
@@ -68,8 +77,21 @@ public function beforeController($controller, $methodName) {
6877
$backendClassName = $user->getBackendClassName();
6978
}
7079

80+
try {
81+
$sessionId = $this->session->getId();
82+
$token = $this->tokenProvider->getToken($sessionId);
83+
} catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) {
84+
// States we do not deal with here.
85+
return;
86+
}
87+
$scope = $token->getScopeAsArray();
88+
if (isset($scope['sso-based-login']) && $scope['sso-based-login'] === true) {
89+
// Users logging in from SSO backends cannot confirm their password by design
90+
return;
91+
}
92+
7193
$lastConfirm = (int) $this->session->get('last-password-confirm');
72-
// we can't check the password against a SAML backend, so skip password confirmation in this case
94+
// TODO: confirm excludedUserBackEnds can go away and remove it
7395
if (!isset($this->excludedUserBackEnds[$backendClassName]) && $lastConfirm < ($this->timeFactory->getTime() - (30 * 60 + 15))) { // allow 15 seconds delay
7496
throw new NotConfirmedException();
7597
}

lib/private/Authentication/Token/PublicKeyTokenProvider.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): OCPI
243243
OCPIToken::TEMPORARY_TOKEN,
244244
$token->getRemember()
245245
);
246+
$newToken->setScope($token->getScopeAsArray());
246247
$this->cacheToken($newToken);
247248

248249
$this->cacheInvalidHash($token->getToken());

lib/private/Template/JSConfigHelper.php

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88
namespace OC\Template;
99

1010
use bantu\IniGetWrapper\IniGetWrapper;
11+
use OC\Authentication\Token\IProvider;
1112
use OC\CapabilitiesManager;
1213
use OC\Share\Share;
1314
use OCP\App\AppPathNotFoundException;
1415
use OCP\App\IAppManager;
16+
use OCP\Authentication\Exceptions\ExpiredTokenException;
17+
use OCP\Authentication\Exceptions\InvalidTokenException;
18+
use OCP\Authentication\Exceptions\WipeTokenException;
1519
use OCP\Constants;
1620
use OCP\Defaults;
1721
use OCP\Files\FileInfo;
@@ -23,48 +27,30 @@
2327
use OCP\ISession;
2428
use OCP\IURLGenerator;
2529
use OCP\IUser;
30+
use OCP\Session\Exceptions\SessionNotAvailableException;
2631
use OCP\Share\IManager as IShareManager;
2732
use OCP\User\Backend\IPasswordConfirmationBackend;
2833
use OCP\Util;
2934

3035
class JSConfigHelper {
31-
protected IL10N $l;
32-
protected Defaults $defaults;
33-
protected IAppManager $appManager;
34-
protected ISession $session;
35-
protected ?IUser $currentUser;
36-
protected IConfig $config;
37-
protected IGroupManager $groupManager;
38-
protected IniGetWrapper $iniWrapper;
39-
protected IURLGenerator $urlGenerator;
40-
protected CapabilitiesManager $capabilitiesManager;
41-
protected IInitialStateService $initialStateService;
4236

4337
/** @var array user back-ends excluded from password verification */
4438
private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
4539

46-
public function __construct(IL10N $l,
47-
Defaults $defaults,
48-
IAppManager $appManager,
49-
ISession $session,
50-
?IUser $currentUser,
51-
IConfig $config,
52-
IGroupManager $groupManager,
53-
IniGetWrapper $iniWrapper,
54-
IURLGenerator $urlGenerator,
55-
CapabilitiesManager $capabilitiesManager,
56-
IInitialStateService $initialStateService) {
57-
$this->l = $l;
58-
$this->defaults = $defaults;
59-
$this->appManager = $appManager;
60-
$this->session = $session;
61-
$this->currentUser = $currentUser;
62-
$this->config = $config;
63-
$this->groupManager = $groupManager;
64-
$this->iniWrapper = $iniWrapper;
65-
$this->urlGenerator = $urlGenerator;
66-
$this->capabilitiesManager = $capabilitiesManager;
67-
$this->initialStateService = $initialStateService;
40+
public function __construct(
41+
protected IL10N $l,
42+
protected Defaults $defaults,
43+
protected IAppManager $appManager,
44+
protected ISession $session,
45+
protected ?IUser $currentUser,
46+
protected IConfig $config,
47+
protected IGroupManager $groupManager,
48+
protected IniGetWrapper $iniWrapper,
49+
protected IURLGenerator $urlGenerator,
50+
protected CapabilitiesManager $capabilitiesManager,
51+
protected IInitialStateService $initialStateService,
52+
protected IProvider $tokenProvider,
53+
) {
6854
}
6955

7056
public function getConfig(): string {
@@ -130,9 +116,13 @@ public function getConfig(): string {
130116
}
131117

132118
if ($this->currentUser instanceof IUser) {
133-
$lastConfirmTimestamp = $this->session->get('last-password-confirm');
134-
if (!is_int($lastConfirmTimestamp)) {
135-
$lastConfirmTimestamp = 0;
119+
if ($this->canUserValidatePassword()) {
120+
$lastConfirmTimestamp = $this->session->get('last-password-confirm');
121+
if (!is_int($lastConfirmTimestamp)) {
122+
$lastConfirmTimestamp = 0;
123+
}
124+
} else {
125+
$lastConfirmTimestamp = PHP_INT_MAX;
136126
}
137127
} else {
138128
$lastConfirmTimestamp = 0;
@@ -287,4 +277,15 @@ public function getConfig(): string {
287277

288278
return $result;
289279
}
280+
281+
protected function canUserValidatePassword(): bool {
282+
try {
283+
$token = $this->tokenProvider->getToken($this->session->getId());
284+
} catch (ExpiredTokenException|WipeTokenException|InvalidTokenException|SessionNotAvailableException) {
285+
// actually we do not know, so we fall back to this statement
286+
return true;
287+
}
288+
$scope = $token->getScopeAsArray();
289+
return !isset($scope['sso-based-login']) || $scope['sso-based-login'] === false;
290+
}
290291
}

lib/private/TemplateLayout.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OC;
99

1010
use bantu\IniGetWrapper\IniGetWrapper;
11+
use OC\Authentication\Token\IProvider;
1112
use OC\Search\SearchQuery;
1213
use OC\Template\CSSResourceLocator;
1314
use OC\Template\JSConfigHelper;
@@ -224,7 +225,8 @@ public function __construct($renderAs, $appId = '') {
224225
\OC::$server->get(IniGetWrapper::class),
225226
\OC::$server->getURLGenerator(),
226227
\OC::$server->get(CapabilitiesManager::class),
227-
\OCP\Server::get(IInitialStateService::class)
228+
\OCP\Server::get(IInitialStateService::class),
229+
\OCP\Server::get(IProvider::class),
228230
);
229231
$config = $jsConfigHelper->getConfig();
230232
if (\OC::$server->getContentSecurityPolicyNonceManager()->browserSupportsCspV3()) {

lib/private/legacy/OC_User.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
66
* SPDX-License-Identifier: AGPL-3.0-only
77
*/
8+
use OC\Authentication\Token\IProvider;
89
use OC\User\LoginException;
910
use OCP\EventDispatcher\IEventDispatcher;
1011
use OCP\IGroupManager;
@@ -166,6 +167,14 @@ public static function loginWithApache(\OCP\Authentication\IApacheBackend $backe
166167

167168
$userSession->createSessionToken($request, $uid, $uid, $password);
168169
$userSession->createRememberMeToken($userSession->getUser());
170+
171+
if (empty($password)) {
172+
$tokenProvider = \OC::$server->get(IProvider::class);
173+
$token = $tokenProvider->getToken($userSession->getSession()->getId());
174+
$token->setScope(['sso-based-login' => true]);
175+
$tokenProvider->updateToken($token);
176+
}
177+
169178
// setup the filesystem
170179
OC_Util::setupFS($uid);
171180
// first call the post_login hooks, the login-process needs to be

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,8 @@ public function testAnnotation() {
3030
#[PasswordConfirmationRequired]
3131
public function testAttribute() {
3232
}
33+
34+
#[PasswordConfirmationRequired]
35+
public function testSSO() {
36+
}
3337
}

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

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
1010
use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware;
1111
use OC\AppFramework\Utility\ControllerMethodReflector;
12+
use OC\Authentication\Token\IProvider;
1213
use OCP\AppFramework\Utility\ITimeFactory;
14+
use OCP\Authentication\Token\IToken;
1315
use OCP\IRequest;
1416
use OCP\ISession;
1517
use OCP\IUser;
@@ -32,13 +34,15 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
3234
private $controller;
3335
/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
3436
private $timeFactory;
37+
private IProvider|\PHPUnit\Framework\MockObject\MockObject $tokenProvider;
3538

3639
protected function setUp(): void {
3740
$this->reflector = new ControllerMethodReflector();
3841
$this->session = $this->createMock(ISession::class);
3942
$this->userSession = $this->createMock(IUserSession::class);
4043
$this->user = $this->createMock(IUser::class);
4144
$this->timeFactory = $this->createMock(ITimeFactory::class);
45+
$this->tokenProvider = $this->createMock(IProvider::class);
4246
$this->controller = new PasswordConfirmationMiddlewareController(
4347
'test',
4448
$this->createMock(IRequest::class)
@@ -48,7 +52,8 @@ protected function setUp(): void {
4852
$this->reflector,
4953
$this->session,
5054
$this->userSession,
51-
$this->timeFactory
55+
$this->timeFactory,
56+
$this->tokenProvider,
5257
);
5358
}
5459

@@ -90,6 +95,13 @@ public function testAnnotation($backend, $lastConfirm, $currentTime, $exception)
9095
$this->timeFactory->method('getTime')
9196
->willReturn($currentTime);
9297

98+
$token = $this->createMock(IToken::class);
99+
$token->method('getScopeAsArray')
100+
->willReturn([]);
101+
$this->tokenProvider->expects($this->once())
102+
->method('getToken')
103+
->willReturn($token);
104+
93105
$thrown = false;
94106
try {
95107
$this->middleware->beforeController($this->controller, __FUNCTION__);
@@ -118,6 +130,13 @@ public function testAttribute($backend, $lastConfirm, $currentTime, $exception)
118130
$this->timeFactory->method('getTime')
119131
->willReturn($currentTime);
120132

133+
$token = $this->createMock(IToken::class);
134+
$token->method('getScopeAsArray')
135+
->willReturn([]);
136+
$this->tokenProvider->expects($this->once())
137+
->method('getToken')
138+
->willReturn($token);
139+
121140
$thrown = false;
122141
try {
123142
$this->middleware->beforeController($this->controller, __FUNCTION__);
@@ -128,6 +147,8 @@ public function testAttribute($backend, $lastConfirm, $currentTime, $exception)
128147
$this->assertSame($exception, $thrown);
129148
}
130149

150+
151+
131152
public function dataProvider() {
132153
return [
133154
['foo', 2000, 4000, true],
@@ -138,4 +159,41 @@ public function dataProvider() {
138159
['foo', 2000, 3816, true],
139160
];
140161
}
162+
163+
public function testSSO() {
164+
static $sessionId = 'mySession1d';
165+
166+
$this->reflector->reflect($this->controller, __FUNCTION__);
167+
168+
$this->user->method('getBackendClassName')
169+
->willReturn('fictional_backend');
170+
$this->userSession->method('getUser')
171+
->willReturn($this->user);
172+
173+
$this->session->method('get')
174+
->with('last-password-confirm')
175+
->willReturn(0);
176+
$this->session->method('getId')
177+
->willReturn($sessionId);
178+
179+
$this->timeFactory->method('getTime')
180+
->willReturn(9876);
181+
182+
$token = $this->createMock(IToken::class);
183+
$token->method('getScopeAsArray')
184+
->willReturn(['sso-based-login' => true]);
185+
$this->tokenProvider->expects($this->once())
186+
->method('getToken')
187+
->with($sessionId)
188+
->willReturn($token);
189+
190+
$thrown = false;
191+
try {
192+
$this->middleware->beforeController($this->controller, __FUNCTION__);
193+
} catch (NotConfirmedException) {
194+
$thrown = true;
195+
}
196+
197+
$this->assertSame(false, $thrown);
198+
}
141199
}

0 commit comments

Comments
 (0)