Skip to content

Commit eec5c87

Browse files
blizzzbackportbot[bot]
authored andcommitted
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]> [skip ci]
1 parent 415be1f commit eec5c87

File tree

9 files changed

+139
-41
lines changed

9 files changed

+139
-41
lines changed

core/Controller/OCJSController.php

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

3131
use bantu\IniGetWrapper\IniGetWrapper;
32+
use OC\Authentication\Token\IProvider;
3233
use OC\CapabilitiesManager;
3334
use OC\Template\JSConfigHelper;
3435
use OCP\App\IAppManager;
@@ -64,6 +65,7 @@ public function __construct(
6465
IURLGenerator $urlGenerator,
6566
CapabilitiesManager $capabilitiesManager,
6667
IInitialStateService $initialStateService,
68+
IProvider $tokenProvider,
6769
) {
6870
parent::__construct($appName, $request);
6971

@@ -78,7 +80,8 @@ public function __construct(
7880
$iniWrapper,
7981
$urlGenerator,
8082
$capabilitiesManager,
81-
$initialStateService
83+
$initialStateService,
84+
$tokenProvider
8285
);
8386
}
8487

lib/private/AppFramework/DependencyInjection/DIContainer.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ public function __construct(string $appName, array $urlParams = [], ServerContai
276276
$c->get(IControllerMethodReflector::class),
277277
$c->get(ISession::class),
278278
$c->get(IUserSession::class),
279-
$c->get(ITimeFactory::class)
279+
$c->get(ITimeFactory::class),
280+
$c->get(\OC\Authentication\Token\IProvider::class),
280281
)
281282
);
282283
$dispatcher->registerMiddleware(

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

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

2626
use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
2727
use OC\AppFramework\Utility\ControllerMethodReflector;
28+
use OC\Authentication\Token\IProvider;
2829
use OCP\AppFramework\Controller;
2930
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
3031
use OCP\AppFramework\Middleware;
3132
use OCP\AppFramework\Utility\ITimeFactory;
33+
use OCP\Authentication\Exceptions\ExpiredTokenException;
34+
use OCP\Authentication\Exceptions\InvalidTokenException;
35+
use OCP\Authentication\Exceptions\WipeTokenException;
3236
use OCP\ISession;
3337
use OCP\IUserSession;
38+
use OCP\Session\Exceptions\SessionNotAvailableException;
3439
use OCP\User\Backend\IPasswordConfirmationBackend;
3540
use ReflectionMethod;
3641

@@ -45,6 +50,7 @@ class PasswordConfirmationMiddleware extends Middleware {
4550
private $timeFactory;
4651
/** @var array */
4752
private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
53+
private IProvider $tokenProvider;
4854

4955
/**
5056
* PasswordConfirmationMiddleware constructor.
@@ -57,11 +63,14 @@ class PasswordConfirmationMiddleware extends Middleware {
5763
public function __construct(ControllerMethodReflector $reflector,
5864
ISession $session,
5965
IUserSession $userSession,
60-
ITimeFactory $timeFactory) {
66+
ITimeFactory $timeFactory,
67+
IProvider $tokenProvider,
68+
) {
6169
$this->reflector = $reflector;
6270
$this->session = $session;
6371
$this->userSession = $userSession;
6472
$this->timeFactory = $timeFactory;
73+
$this->tokenProvider = $tokenProvider;
6574
}
6675

6776
/**
@@ -86,8 +95,21 @@ public function beforeController($controller, $methodName) {
8695
$backendClassName = $user->getBackendClassName();
8796
}
8897

98+
try {
99+
$sessionId = $this->session->getId();
100+
$token = $this->tokenProvider->getToken($sessionId);
101+
} catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) {
102+
// States we do not deal with here.
103+
return;
104+
}
105+
$scope = $token->getScopeAsArray();
106+
if (isset($scope['sso-based-login']) && $scope['sso-based-login'] === true) {
107+
// Users logging in from SSO backends cannot confirm their password by design
108+
return;
109+
}
110+
89111
$lastConfirm = (int) $this->session->get('last-password-confirm');
90-
// we can't check the password against a SAML backend, so skip password confirmation in this case
112+
// TODO: confirm excludedUserBackEnds can go away and remove it
91113
if (!isset($this->excludedUserBackEnds[$backendClassName]) && $lastConfirm < ($this->timeFactory->getTime() - (30 * 60 + 15))) { // allow 15 seconds delay
92114
throw new NotConfirmedException();
93115
}

lib/private/Authentication/Token/PublicKeyTokenProvider.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): OCPI
265265
OCPIToken::TEMPORARY_TOKEN,
266266
$token->getRemember()
267267
);
268+
$newToken->setScope($token->getScopeAsArray());
268269
$this->cacheToken($newToken);
269270

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

lib/private/Template/JSConfigHelper.php

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,14 @@
3434
namespace OC\Template;
3535

3636
use bantu\IniGetWrapper\IniGetWrapper;
37+
use OC\Authentication\Token\IProvider;
3738
use OC\CapabilitiesManager;
3839
use OC\Share\Share;
3940
use OCP\App\AppPathNotFoundException;
4041
use OCP\App\IAppManager;
42+
use OCP\Authentication\Exceptions\ExpiredTokenException;
43+
use OCP\Authentication\Exceptions\InvalidTokenException;
44+
use OCP\Authentication\Exceptions\WipeTokenException;
4145
use OCP\Constants;
4246
use OCP\Defaults;
4347
use OCP\Files\FileInfo;
@@ -53,43 +57,24 @@
5357
use OCP\Util;
5458

5559
class JSConfigHelper {
56-
protected IL10N $l;
57-
protected Defaults $defaults;
58-
protected IAppManager $appManager;
59-
protected ISession $session;
60-
protected ?IUser $currentUser;
61-
protected IConfig $config;
62-
protected IGroupManager $groupManager;
63-
protected IniGetWrapper $iniWrapper;
64-
protected IURLGenerator $urlGenerator;
65-
protected CapabilitiesManager $capabilitiesManager;
66-
protected IInitialStateService $initialStateService;
6760

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

71-
public function __construct(IL10N $l,
72-
Defaults $defaults,
73-
IAppManager $appManager,
74-
ISession $session,
75-
?IUser $currentUser,
76-
IConfig $config,
77-
IGroupManager $groupManager,
78-
IniGetWrapper $iniWrapper,
79-
IURLGenerator $urlGenerator,
80-
CapabilitiesManager $capabilitiesManager,
81-
IInitialStateService $initialStateService) {
82-
$this->l = $l;
83-
$this->defaults = $defaults;
84-
$this->appManager = $appManager;
85-
$this->session = $session;
86-
$this->currentUser = $currentUser;
87-
$this->config = $config;
88-
$this->groupManager = $groupManager;
89-
$this->iniWrapper = $iniWrapper;
90-
$this->urlGenerator = $urlGenerator;
91-
$this->capabilitiesManager = $capabilitiesManager;
92-
$this->initialStateService = $initialStateService;
64+
public function __construct(
65+
protected IL10N $l,
66+
protected Defaults $defaults,
67+
protected IAppManager $appManager,
68+
protected ISession $session,
69+
protected ?IUser $currentUser,
70+
protected IConfig $config,
71+
protected IGroupManager $groupManager,
72+
protected IniGetWrapper $iniWrapper,
73+
protected IURLGenerator $urlGenerator,
74+
protected CapabilitiesManager $capabilitiesManager,
75+
protected IInitialStateService $initialStateService,
76+
protected IProvider $tokenProvider,
77+
) {
9378
}
9479

9580
public function getConfig(): string {
@@ -155,9 +140,13 @@ public function getConfig(): string {
155140
}
156141

157142
if ($this->currentUser instanceof IUser) {
158-
$lastConfirmTimestamp = $this->session->get('last-password-confirm');
159-
if (!is_int($lastConfirmTimestamp)) {
160-
$lastConfirmTimestamp = 0;
143+
if ($this->canUserValidatePassword()) {
144+
$lastConfirmTimestamp = $this->session->get('last-password-confirm');
145+
if (!is_int($lastConfirmTimestamp)) {
146+
$lastConfirmTimestamp = 0;
147+
}
148+
} else {
149+
$lastConfirmTimestamp = PHP_INT_MAX;
161150
}
162151
} else {
163152
$lastConfirmTimestamp = 0;
@@ -311,4 +300,15 @@ public function getConfig(): string {
311300

312301
return $result;
313302
}
303+
304+
protected function canUserValidatePassword(): bool {
305+
try {
306+
$token = $this->tokenProvider->getToken($this->session->getId());
307+
} catch (ExpiredTokenException|WipeTokenException|InvalidTokenException|SessionNotAvailableException) {
308+
// actually we do not know, so we fall back to this statement
309+
return true;
310+
}
311+
$scope = $token->getScopeAsArray();
312+
return !isset($scope['sso-based-login']) || $scope['sso-based-login'] === false;
313+
}
314314
}

lib/private/TemplateLayout.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
namespace OC;
4444

4545
use bantu\IniGetWrapper\IniGetWrapper;
46+
use OC\Authentication\Token\IProvider;
4647
use OC\Search\SearchQuery;
4748
use OC\Template\CSSResourceLocator;
4849
use OC\Template\JSConfigHelper;

lib/private/legacy/OC_User.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,14 @@ public static function loginWithApache(\OCP\Authentication\IApacheBackend $backe
196196

197197
$userSession->createSessionToken($request, $uid, $uid, $password);
198198
$userSession->createRememberMeToken($userSession->getUser());
199+
200+
if (empty($password)) {
201+
$tokenProvider = \OC::$server->get(IProvider::class);
202+
$token = $tokenProvider->getToken($userSession->getSession()->getId());
203+
$token->setScope(['sso-based-login' => true]);
204+
$tokenProvider->updateToken($token);
205+
}
206+
199207
// setup the filesystem
200208
OC_Util::setupFS($uid);
201209
// 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
@@ -46,4 +46,8 @@ public function testAnnotation() {
4646
#[PasswordConfirmationRequired]
4747
public function testAttribute() {
4848
}
49+
50+
#[PasswordConfirmationRequired]
51+
public function testSSO() {
52+
}
4953
}

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

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
2727
use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware;
2828
use OC\AppFramework\Utility\ControllerMethodReflector;
29+
use OC\Authentication\Token\IProvider;
2930
use OCP\AppFramework\Utility\ITimeFactory;
31+
use OCP\Authentication\Token\IToken;
3032
use OCP\IRequest;
3133
use OCP\ISession;
3234
use OCP\IUser;
@@ -49,13 +51,15 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
4951
private $controller;
5052
/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
5153
private $timeFactory;
54+
private IProvider|\PHPUnit\Framework\MockObject\MockObject $tokenProvider;
5255

5356
protected function setUp(): void {
5457
$this->reflector = new ControllerMethodReflector();
5558
$this->session = $this->createMock(ISession::class);
5659
$this->userSession = $this->createMock(IUserSession::class);
5760
$this->user = $this->createMock(IUser::class);
5861
$this->timeFactory = $this->createMock(ITimeFactory::class);
62+
$this->tokenProvider = $this->createMock(IProvider::class);
5963
$this->controller = new PasswordConfirmationMiddlewareController(
6064
'test',
6165
$this->createMock(IRequest::class)
@@ -65,7 +69,8 @@ protected function setUp(): void {
6569
$this->reflector,
6670
$this->session,
6771
$this->userSession,
68-
$this->timeFactory
72+
$this->timeFactory,
73+
$this->tokenProvider,
6974
);
7075
}
7176

@@ -107,6 +112,13 @@ public function testAnnotation($backend, $lastConfirm, $currentTime, $exception)
107112
$this->timeFactory->method('getTime')
108113
->willReturn($currentTime);
109114

115+
$token = $this->createMock(IToken::class);
116+
$token->method('getScopeAsArray')
117+
->willReturn([]);
118+
$this->tokenProvider->expects($this->once())
119+
->method('getToken')
120+
->willReturn($token);
121+
110122
$thrown = false;
111123
try {
112124
$this->middleware->beforeController($this->controller, __FUNCTION__);
@@ -135,6 +147,13 @@ public function testAttribute($backend, $lastConfirm, $currentTime, $exception)
135147
$this->timeFactory->method('getTime')
136148
->willReturn($currentTime);
137149

150+
$token = $this->createMock(IToken::class);
151+
$token->method('getScopeAsArray')
152+
->willReturn([]);
153+
$this->tokenProvider->expects($this->once())
154+
->method('getToken')
155+
->willReturn($token);
156+
138157
$thrown = false;
139158
try {
140159
$this->middleware->beforeController($this->controller, __FUNCTION__);
@@ -145,6 +164,8 @@ public function testAttribute($backend, $lastConfirm, $currentTime, $exception)
145164
$this->assertSame($exception, $thrown);
146165
}
147166

167+
168+
148169
public function dataProvider() {
149170
return [
150171
['foo', 2000, 4000, true],
@@ -155,4 +176,41 @@ public function dataProvider() {
155176
['foo', 2000, 3816, true],
156177
];
157178
}
179+
180+
public function testSSO() {
181+
static $sessionId = 'mySession1d';
182+
183+
$this->reflector->reflect($this->controller, __FUNCTION__);
184+
185+
$this->user->method('getBackendClassName')
186+
->willReturn('fictional_backend');
187+
$this->userSession->method('getUser')
188+
->willReturn($this->user);
189+
190+
$this->session->method('get')
191+
->with('last-password-confirm')
192+
->willReturn(0);
193+
$this->session->method('getId')
194+
->willReturn($sessionId);
195+
196+
$this->timeFactory->method('getTime')
197+
->willReturn(9876);
198+
199+
$token = $this->createMock(IToken::class);
200+
$token->method('getScopeAsArray')
201+
->willReturn(['sso-based-login' => true]);
202+
$this->tokenProvider->expects($this->once())
203+
->method('getToken')
204+
->with($sessionId)
205+
->willReturn($token);
206+
207+
$thrown = false;
208+
try {
209+
$this->middleware->beforeController($this->controller, __FUNCTION__);
210+
} catch (NotConfirmedException) {
211+
$thrown = true;
212+
}
213+
214+
$this->assertSame(false, $thrown);
215+
}
158216
}

0 commit comments

Comments
 (0)