Skip to content

Commit 0e2e0fb

Browse files
committed
fix(Session): restore session data after cookie login
when a browser is closed and the session lost, a new session might be started with the help of the remember me cookie. For we keep some data in the session, like the IdP identifier, and some SAML data needed for SLO, those were lost in this process. This made especially logout behaviour not acting as expected. Now we keep the required data also in the database and can restore it on a cookie login event. Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
1 parent 3448746 commit 0e2e0fb

File tree

12 files changed

+374
-28
lines changed

12 files changed

+374
-28
lines changed

appinfo/info.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ The following providers are supported and tested at the moment:
2020
* Any other provider that authenticates using the environment variable
2121
2222
While theoretically any other authentication provider implementing either one of those standards is compatible, we like to note that they are not part of any internal test matrix.]]></description>
23-
<version>7.1.1</version>
23+
<version>7.1.2-beta.1</version>
2424
<licence>agpl</licence>
2525
<author>Lukas Reschke</author>
2626
<namespace>User_SAML</namespace>

lib/AppInfo/Application.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@
1616
use OCA\User_SAML\DavPlugin;
1717
use OCA\User_SAML\GroupBackend;
1818
use OCA\User_SAML\GroupManager;
19+
use OCA\User_SAML\Listener\CookieLoginEventListener;
1920
use OCA\User_SAML\Listener\LoadAdditionalScriptsListener;
2021
use OCA\User_SAML\Listener\SabrePluginEventListener;
2122
use OCA\User_SAML\Middleware\OnlyLoggedInMiddleware;
2223
use OCA\User_SAML\SAMLSettings;
24+
use OCA\User_SAML\Service\SessionService;
2325
use OCA\User_SAML\UserBackend;
2426
use OCP\AppFramework\App;
2527
use OCP\AppFramework\Bootstrap\IBootContext;
@@ -38,6 +40,8 @@
3840
use OCP\IUserSession;
3941
use OCP\L10N\IFactory;
4042
use OCP\Server;
43+
use OCP\User\Events\BeforeUserLoggedInWithCookieEvent;
44+
use OCP\User\Events\UserLoggedInWithCookieEvent;
4145
use Psr\Container\ContainerInterface;
4246
use Psr\Log\LoggerInterface;
4347
use Throwable;
@@ -53,11 +57,14 @@ public function register(IRegistrationContext $context): void {
5357
$context->registerMiddleware(OnlyLoggedInMiddleware::class);
5458
$context->registerEventListener(BeforeTemplateRenderedEvent::class, LoadAdditionalScriptsListener::class);
5559
$context->registerEventListener(SabrePluginAddEvent::class, SabrePluginEventListener::class);
60+
$context->registerEventListener(BeforeUserLoggedInWithCookieEvent::class, CookieLoginEventListener::class);
61+
$context->registerEventListener(UserLoggedInWithCookieEvent::class, CookieLoginEventListener::class);
5662
$context->registerService(DavPlugin::class, fn (ContainerInterface $c) => new DavPlugin(
5763
$c->get(ISession::class),
5864
$c->get(IConfig::class),
5965
$_SERVER,
60-
$c->get(SAMLSettings::class)
66+
$c->get(SAMLSettings::class),
67+
$c->get(SessionService::class),
6168
));
6269
}
6370

lib/Controller/SAMLController.php

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OCA\User_SAML\Exceptions\UserFilterViolationException;
1818
use OCA\User_SAML\Helper\TXmlHelper;
1919
use OCA\User_SAML\SAMLSettings;
20+
use OCA\User_SAML\Service\SessionService;
2021
use OCA\User_SAML\UserBackend;
2122
use OCA\User_SAML\UserData;
2223
use OCA\User_SAML\UserResolver;
@@ -57,6 +58,7 @@ public function __construct(
5758
private UserData $userData,
5859
private ICrypto $crypto,
5960
private ITrustedDomainHelper $trustedDomainHelper,
61+
private SessionService $sessionService,
6062
) {
6163
parent::__construct($appName, $request);
6264
}
@@ -232,7 +234,7 @@ public function login(int $idp = 1): Http\RedirectResponse|Http\TemplateResponse
232234
if (empty($ssoUrl)) {
233235
$ssoUrl = $this->urlGenerator->getAbsoluteURL('/');
234236
}
235-
$this->session->set('user_saml.samlUserData', $_SERVER);
237+
$this->sessionService->prepareEnvironmentBasedSession($_SERVER);
236238
try {
237239
$this->userData->setAttributes($this->session->get('user_saml.samlUserData'));
238240
$this->autoprovisionIfPossible();
@@ -335,8 +337,8 @@ public function assertionConsumerService(): Http\RedirectResponse {
335337
$AuthNRequestID = $data['AuthNRequestID'];
336338
$idp = $data['Idp'];
337339
// need to keep the IdP config ID during session lifetime (SAMLSettings::getPrefix)
338-
$this->session->set('user_saml.Idp', $idp);
339-
if (is_null($AuthNRequestID) || $AuthNRequestID === '' || is_null($idp)) {
340+
$this->sessionService->storeIdentityProviderInSession($idp);
341+
if (is_null($AuthNRequestID) || $AuthNRequestID === '') {
340342
$this->logger->debug('Invalid auth payload', ['app' => 'user_saml']);
341343
return new Http\RedirectResponse($this->urlGenerator->getAbsoluteURL('/'));
342344
}
@@ -383,14 +385,8 @@ public function assertionConsumerService(): Http\RedirectResponse {
383385
return $response;
384386
}
385387

386-
$this->session->set('user_saml.samlUserData', $auth->getAttributes());
387-
$this->session->set('user_saml.samlNameId', $auth->getNameId());
388-
$this->session->set('user_saml.samlNameIdFormat', $auth->getNameIdFormat());
389-
$this->session->set('user_saml.samlNameIdNameQualifier', $auth->getNameIdNameQualifier());
390-
$this->session->set('user_saml.samlNameIdSPNameQualifier', $auth->getNameIdSPNameQualifier());
391-
$this->session->set('user_saml.samlSessionIndex', $auth->getSessionIndex());
392-
$this->session->set('user_saml.samlSessionExpiration', $auth->getSessionExpiration());
393-
$this->logger->debug('Session values set', ['app' => 'user_saml']);
388+
$this->sessionService->prepareSession($auth);
389+
394390
try {
395391
$user = $this->userResolver->findExistingUser($this->userBackend->getCurrentUserId());
396392
$firstLogin = $user->updateLastLoginTimestamp();
@@ -510,14 +506,14 @@ public function singleLogoutService(): Http\RedirectResponse {
510506
*/
511507
private function tryProcessSLOResponse(?int $idp): array {
512508
$idps = ($idp !== null) ? [$idp] : array_keys($this->samlSettings->getListOfIdps());
513-
foreach ($idps as $idp) {
509+
foreach ($idps as $identityProviderId) {
514510
try {
515-
$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
511+
$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($identityProviderId));
516512
// validator (called with processSLO()) needs an XML entity loader
517513
$targetUrl = $this->callWithXmlEntityLoader(fn (): string => $auth->processSLO(
518514
true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
519515
null,
520-
$this->samlSettings->usesSloWebServerDecode($idp),
516+
$this->samlSettings->usesSloWebServerDecode($identityProviderId),
521517
null,
522518
true
523519
));

lib/DavPlugin.php

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,40 +8,41 @@
88
namespace OCA\User_SAML;
99

1010
use OCA\DAV\Connector\Sabre\Auth;
11-
use OCP\IConfig;
11+
use OCA\User_SAML\Service\SessionService;
12+
use OCP\AppFramework\Services\IAppConfig;
1213
use OCP\ISession;
1314
use Sabre\DAV\Server;
1415
use Sabre\DAV\ServerPlugin;
1516
use Sabre\HTTP\RequestInterface;
1617
use Sabre\HTTP\ResponseInterface;
1718

1819
class DavPlugin extends ServerPlugin {
19-
/** @var Server */
20-
private $server;
20+
/** @noinspection PhpPropertyOnlyWrittenInspection */
21+
private Server $server;
2122

2223
public function __construct(
2324
private readonly ISession $session,
24-
private readonly IConfig $config,
25-
private array $auth,
25+
private readonly IAppConfig $config,
26+
private readonly array $auth,
2627
private readonly SAMLSettings $samlSettings,
28+
private readonly SessionService $sessionService,
2729
) {
2830
}
2931

30-
public function initialize(Server $server) {
31-
// before auth
32+
public function initialize(Server $server): void {
3233
$server->on('beforeMethod:*', $this->beforeMethod(...), 9);
3334
$this->server = $server;
3435
}
3536

36-
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
37+
public function beforeMethod(RequestInterface $request, ResponseInterface $response): void {
3738
if (
38-
$this->config->getAppValue('user_saml', 'type') === 'environment-variable'
39+
$this->config->getAppValueString('type', 'unset') === 'environment-variable'
3940
&& !$this->session->exists('user_saml.samlUserData')
4041
) {
4142
$uidMapping = $this->samlSettings->get(1)['general-uid_mapping'];
4243
if (isset($this->auth[$uidMapping])) {
4344
$this->session->set(Auth::DAV_AUTHENTICATED, $this->auth[$uidMapping]);
44-
$this->session->set('user_saml.samlUserData', $this->auth);
45+
$this->sessionService->prepareEnvironmentBasedSession($this->auth);
4546
}
4647
}
4748
}

lib/Db/SessionData.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
namespace OCA\User_SAML\Db;
9+
10+
use OCA\User_SAML\Model\SessionData as SessionDataModel;
11+
use OCP\AppFramework\Db\Entity;
12+
use OCP\DB\Types;
13+
14+
class SessionData extends Entity {
15+
/** @var string */
16+
public $id;
17+
public ?string $data = null;
18+
19+
public function __construct() {
20+
$this->addType('id', Types::STRING);
21+
$this->addType('data', Types::TEXT);
22+
}
23+
24+
public function setId(string $id): void {
25+
$this->id = $id;
26+
$this->markFieldUpdated('id');
27+
}
28+
29+
public function setData(SessionDataModel $input): void {
30+
$this->data = json_encode($input);
31+
$this->markFieldUpdated('data');
32+
}
33+
34+
public function getData(): SessionDataModel {
35+
$deserialized = json_decode($this->data, true);
36+
return SessionDataModel::fromInputArray($deserialized);
37+
}
38+
}

lib/Db/SessionDataMapper.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
namespace OCA\User_SAML\Db;
9+
10+
use OCP\AppFramework\Db\DoesNotExistException;
11+
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
12+
use OCP\AppFramework\Db\QBMapper;
13+
use OCP\DB\Exception;
14+
use OCP\IDBConnection;
15+
16+
/** @template-extends QBMapper<SessionData> */
17+
class SessionDataMapper extends QBMapper {
18+
public const SESSION_DATA_TABLE_NAME = 'user_saml_session_data';
19+
20+
public function __construct(IDBConnection $db) {
21+
parent::__construct($db, self::SESSION_DATA_TABLE_NAME, SessionData::class);
22+
}
23+
24+
/**
25+
* @throws DoesNotExistException
26+
* @throws MultipleObjectsReturnedException
27+
* @throws Exception
28+
*/
29+
public function retrieve(string $sessionId): SessionData {
30+
$qb = $this->db->getQueryBuilder();
31+
$qb->select('*')
32+
->from(self::SESSION_DATA_TABLE_NAME)
33+
->where($qb->expr()->eq('id', $qb->createNamedParameter($sessionId)));
34+
return $this->findEntity($qb);
35+
}
36+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
namespace OCA\User_SAML\Listener;
9+
10+
use OCA\User_SAML\Service\SessionService;
11+
use OCP\EventDispatcher\Event;
12+
use OCP\EventDispatcher\IEventListener;
13+
use OCP\User\Events\BeforeUserLoggedInWithCookieEvent;
14+
use OCP\User\Events\UserLoggedInWithCookieEvent;
15+
16+
/** @template-implements IEventListener<BeforeUserLoggedInWithCookieEvent|UserLoggedInWithCookieEvent|Event> */
17+
class CookieLoginEventListener implements IEventListener {
18+
protected ?string $oldSessionId = null;
19+
20+
public function __construct(
21+
protected readonly SessionService $sessionService,
22+
) {
23+
}
24+
25+
public function handle(Event $event): void {
26+
if ($event instanceof BeforeUserLoggedInWithCookieEvent) {
27+
$this->prepareRestoreOfSession();
28+
return;
29+
}
30+
31+
if ($event instanceof UserLoggedInWithCookieEvent) {
32+
$this->restoreSession();
33+
return;
34+
}
35+
}
36+
37+
protected function prepareRestoreOfSession(): void {
38+
if (isset($_COOKIE['nc_session_id'])) {
39+
$this->oldSessionId = $_COOKIE['nc_session_id'];
40+
}
41+
}
42+
43+
protected function restoreSession(): void {
44+
if ($this->oldSessionId !== null) {
45+
$this->sessionService->restoreSession($this->oldSessionId);
46+
}
47+
}
48+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\User_SAML\Migration;
11+
12+
use Closure;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\DB\Types;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\SimpleMigrationStep;
17+
use Override;
18+
19+
class Version7001Date20251215192613 extends SimpleMigrationStep {
20+
public const SESSION_DATA_TABLE_NAME = 'user_saml_session_data';
21+
22+
#[Override]
23+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
24+
/** @var ISchemaWrapper $schema */
25+
$schema = $schemaClosure();
26+
27+
if ($schema->hasTable(self::SESSION_DATA_TABLE_NAME)) {
28+
return null;
29+
}
30+
31+
$table = $schema->createTable(self::SESSION_DATA_TABLE_NAME);
32+
$table->addColumn('id', Types::STRING, [
33+
'notnull' => true,
34+
'length' => 200,
35+
]);
36+
$table->addColumn('data', Types::TEXT, [
37+
'notnull' => true,
38+
]);
39+
$table->setPrimaryKey(['id']);
40+
41+
return $schema;
42+
}
43+
44+
}

0 commit comments

Comments
 (0)