fix(Session): restore session data after cookie login #1022
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| namespace OCA\User_SAML\Db; | ||
|
|
||
| use OCA\User_SAML\Model\SessionData as SessionDataModel; | ||
| use OCP\AppFramework\Db\Entity; | ||
| use OCP\DB\Types; | ||
|
|
||
| /** | ||
| * @method setTokenId(int $tokenId): void | ||
| * @method getTokenId(): int | ||
| */ | ||
| class SessionData extends Entity { | ||
| /** @var string */ | ||
| public $id; | ||
| public ?string $data = null; | ||
| protected ?int $tokenId = null; | ||
|
|
||
| public function __construct() { | ||
| $this->addType('id', Types::STRING); | ||
| // technically tokenId is BIGINT, effectively no difference in the Entity context. | ||
| // It can be set to BIGINT once dropping NC 30 support. | ||
| $this->addType('tokenId', Types::INTEGER); | ||
| $this->addType('data', Types::TEXT); | ||
| } | ||
|
|
||
| public function setId(string $id): void { | ||
| $this->id = $id; | ||
| $this->markFieldUpdated('id'); | ||
| } | ||
|
|
||
| public function setData(SessionDataModel $input): void { | ||
| $this->data = json_encode($input); | ||
| $this->markFieldUpdated('data'); | ||
| } | ||
|
|
||
| public function getData(): SessionDataModel { | ||
| $deserialized = json_decode($this->data, true); | ||
| return SessionDataModel::fromInputArray($deserialized); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| namespace OCA\User_SAML\Db; | ||
|
|
||
| use OCP\AppFramework\Db\DoesNotExistException; | ||
| use OCP\AppFramework\Db\MultipleObjectsReturnedException; | ||
| use OCP\AppFramework\Db\QBMapper; | ||
| use OCP\DB\Exception; | ||
| use OCP\IDBConnection; | ||
|
|
||
| /** @template-extends QBMapper<SessionData> */ | ||
| class SessionDataMapper extends QBMapper { | ||
| public const SESSION_DATA_TABLE_NAME = 'user_saml_session_data'; | ||
|
|
||
| public function __construct(IDBConnection $db) { | ||
| parent::__construct($db, self::SESSION_DATA_TABLE_NAME, SessionData::class); | ||
| } | ||
|
|
||
| /** | ||
| * @throws DoesNotExistException | ||
| * @throws MultipleObjectsReturnedException | ||
| * @throws Exception | ||
| */ | ||
| public function retrieve(string $sessionId): SessionData { | ||
| $qb = $this->db->getQueryBuilder(); | ||
| $qb->select('*') | ||
| ->from(self::SESSION_DATA_TABLE_NAME) | ||
| ->where($qb->expr()->eq('id', $qb->createNamedParameter($sessionId))); | ||
| return $this->findEntity($qb); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| namespace OCA\User_SAML\Jobs; | ||
|
|
||
| use OCA\User_SAML\Db\SessionDataMapper; | ||
| use OCP\AppFramework\Utility\ITimeFactory; | ||
| use OCP\BackgroundJob\IJob; | ||
| use OCP\BackgroundJob\TimedJob; | ||
| use OCP\DB\QueryBuilder\IQueryBuilder; | ||
| use OCP\IDBConnection; | ||
|
|
||
| class CleanSessionData extends TimedJob { | ||
| protected const NC_AUTH_TOKEN_TABLE = 'authtoken'; | ||
|
|
||
| public function __construct( | ||
| protected IDBConnection $dbc, | ||
| ITimeFactory $timeFactory, | ||
| ) { | ||
| parent::__construct($timeFactory); | ||
|
|
||
| $this->setInterval(86400); | ||
| $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); | ||
| $this->setAllowParallelRuns(false); | ||
| } | ||
|
|
||
| protected function run(mixed $argument): void { | ||
| $missingSessionIds = $this->findInvalidatedSessions(); | ||
| $this->deleteInvalidatedSessions($missingSessionIds); | ||
| } | ||
|
|
||
| protected function findInvalidatedSessions(): array { | ||
| $qb = $this->dbc->getQueryBuilder(); | ||
| $qb->select('s.id') | ||
| ->from(SessionDataMapper::SESSION_DATA_TABLE_NAME, 's') | ||
| ->leftJoin('s', self::NC_AUTH_TOKEN_TABLE, 'a', | ||
| $qb->expr()->eq('s.token_id', 'a.id'), | ||
| ) | ||
| ->where($qb->expr()->isNull('a.id')) | ||
| ->setMaxResults(1000); | ||
|
|
||
| $invalidatedSessionsResult = $qb->executeQuery(); | ||
| $invalidatedSessionIds = $invalidatedSessionsResult->fetchAll(\PDO::FETCH_COLUMN); | ||
| $invalidatedSessionsResult->closeCursor(); | ||
|
|
||
| return $invalidatedSessionIds; | ||
| } | ||
|
|
||
| protected function deleteInvalidatedSessions(array $invalidatedSessionIds): void { | ||
| $qb = $this->dbc->getQueryBuilder(); | ||
| $qb->delete(SessionDataMapper::SESSION_DATA_TABLE_NAME) | ||
| ->where($qb->expr()->in( | ||
| 'id', | ||
| $qb->createNamedParameter($invalidatedSessionIds, IQueryBuilder::PARAM_STR_ARRAY) | ||
| )); | ||
| $qb->executeStatement(); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the DB schema changes, this actually needs a bump to 8.0.0 🤔 However. it kills backportability to lower versions (not intended yet, but you never know). Would hence go to 7.2.0 instead although it technically is not correct. Opinions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK there is no hard requirement on needing a major bump for db changes. Even patches can have them. Admins are not fans, but it's inevitable for some changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec is unclear in its current form, but as it is an external system and it requires the changed schema, you can argue it falls into the "public api" scope. This is also largely discussed and acknowledged in semver/semver#90
If not bumping the major with a DB schema change is lived practice across Nextcloud (I am not sure, but your statement sounds like it), then it is acceptable to only bump the patch number.