Merge pull request #1022 from nextcloud/fix/noid/restore-session-state-after-cookie-login

fix(Session): restore session data after cookie login

Author: blizzz

appinfo/info.xml

@@ -3,7 +3,7 @@
  - SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  - SPDX-License-Identifier: AGPL-3.0-or-later
 -->
-<info>
+<info xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd">
 	<id>user_saml</id>
 	<name>SSO &amp; SAML authentication</name>
 	<summary>Authenticate using single sign-on</summary>
@@ -20,7 +20,7 @@ The following providers are supported and tested at the moment:
 	* Any other provider that authenticates using the environment variable
 
 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>
-	<version>7.1.1</version>
+	<version>7.1.2-beta.1</version>
 	<licence>agpl</licence>
 	<author>Lukas Reschke</author>
 	<namespace>User_SAML</namespace>
@@ -39,6 +39,9 @@ While theoretically any other authentication provider implementing either one of
 	<dependencies>
 		<nextcloud min-version="30" max-version="33" />
 	</dependencies>
+	<background-jobs>
+		<job>OCA\User_SAML\Jobs\CleanSessionData</job>
+	</background-jobs>
 	<repair-steps>
 		<post-migration>
 			<step>OCA\User_SAML\Migration\RememberLocalGroupsForPotentialMigrations</step>

composer.json

@@ -23,11 +23,11 @@
 		"post-update-cmd": [
 			"[ $COMPOSER_DEV_MODE -eq 0 ] || composer bin all update --ansi"
 		],
-		"cs:fix": "php-cs-fixer fix",
-		"cs:check": "php-cs-fixer fix --dry-run --diff",
-		"psalm": "psalm",
-		"psalm:fix": "psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
-		"psalm:update-baseline": "psalm --threads=1 --update-baseline",
+		"cs:fix": "@php php-cs-fixer fix",
+		"cs:check": "@php php-cs-fixer fix --dry-run --diff",
+		"psalm": "@php psalm",
+		"psalm:fix": "@php psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
+		"psalm:update-baseline": "@php psalm --threads=1 --update-baseline",
 		"lint": "find . -name \\*.php -not -path '*/vendor/*' -print0 | xargs -0 -n1 php -l",
 		"rector:check": "rector --dry-run",
 		"rector:fix": "rector",

lib/AppInfo/Application.php

@@ -16,10 +16,13 @@
 use OCA\User_SAML\DavPlugin;
 use OCA\User_SAML\GroupBackend;
 use OCA\User_SAML\GroupManager;
+use OCA\User_SAML\Listener\CookieLoginEventListener;
 use OCA\User_SAML\Listener\LoadAdditionalScriptsListener;
+use OCA\User_SAML\Listener\LoginEventListener;
 use OCA\User_SAML\Listener\SabrePluginEventListener;
 use OCA\User_SAML\Middleware\OnlyLoggedInMiddleware;
 use OCA\User_SAML\SAMLSettings;
+use OCA\User_SAML\Service\SessionService;
 use OCA\User_SAML\UserBackend;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
@@ -38,6 +41,9 @@
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
 use OCP\Server;
+use OCP\User\Events\BeforeUserLoggedInWithCookieEvent;
+use OCP\User\Events\UserLoggedInEvent;
+use OCP\User\Events\UserLoggedInWithCookieEvent;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 use Throwable;
@@ -53,11 +59,15 @@ public function register(IRegistrationContext $context): void {
 		$context->registerMiddleware(OnlyLoggedInMiddleware::class);
 		$context->registerEventListener(BeforeTemplateRenderedEvent::class, LoadAdditionalScriptsListener::class);
 		$context->registerEventListener(SabrePluginAddEvent::class, SabrePluginEventListener::class);
+		$context->registerEventListener(BeforeUserLoggedInWithCookieEvent::class, CookieLoginEventListener::class);
+		$context->registerEventListener(UserLoggedInWithCookieEvent::class, CookieLoginEventListener::class);
+		$context->registerEventListener(UserLoggedInEvent::class, LoginEventListener::class);
 		$context->registerService(DavPlugin::class, fn (ContainerInterface $c) => new DavPlugin(
 			$c->get(ISession::class),
 			$c->get(IConfig::class),
 			$_SERVER,
-			$c->get(SAMLSettings::class)
+			$c->get(SAMLSettings::class),
+			$c->get(SessionService::class),
 		));
 	}
 

lib/Controller/SAMLController.php

@@ -17,6 +17,7 @@
 use OCA\User_SAML\Exceptions\UserFilterViolationException;
 use OCA\User_SAML\Helper\TXmlHelper;
 use OCA\User_SAML\SAMLSettings;
+use OCA\User_SAML\Service\SessionService;
 use OCA\User_SAML\UserBackend;
 use OCA\User_SAML\UserData;
 use OCA\User_SAML\UserResolver;
@@ -57,6 +58,7 @@ public function __construct(
 		private UserData $userData,
 		private ICrypto $crypto,
 		private ITrustedDomainHelper $trustedDomainHelper,
+		private SessionService $sessionService,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -232,7 +234,7 @@ public function login(int $idp = 1): Http\RedirectResponse|Http\TemplateResponse
 				if (empty($ssoUrl)) {
 					$ssoUrl = $this->urlGenerator->getAbsoluteURL('/');
 				}
-				$this->session->set('user_saml.samlUserData', $_SERVER);
+				$this->sessionService->prepareEnvironmentBasedSession($_SERVER);
 				try {
 					$this->userData->setAttributes($this->session->get('user_saml.samlUserData'));
 					$this->autoprovisionIfPossible();
@@ -335,8 +337,8 @@ public function assertionConsumerService(): Http\RedirectResponse {
 		$AuthNRequestID = $data['AuthNRequestID'];
 		$idp = $data['Idp'];
 		// need to keep the IdP config ID during session lifetime (SAMLSettings::getPrefix)
-		$this->session->set('user_saml.Idp', $idp);
-		if (is_null($AuthNRequestID) || $AuthNRequestID === '' || is_null($idp)) {
+		$this->sessionService->storeIdentityProviderInSession($idp);
+		if (is_null($AuthNRequestID) || $AuthNRequestID === '') {
 			$this->logger->debug('Invalid auth payload', ['app' => 'user_saml']);
 			return new Http\RedirectResponse($this->urlGenerator->getAbsoluteURL('/'));
 		}
@@ -383,14 +385,8 @@ public function assertionConsumerService(): Http\RedirectResponse {
 			return $response;
 		}
 
-		$this->session->set('user_saml.samlUserData', $auth->getAttributes());
-		$this->session->set('user_saml.samlNameId', $auth->getNameId());
-		$this->session->set('user_saml.samlNameIdFormat', $auth->getNameIdFormat());
-		$this->session->set('user_saml.samlNameIdNameQualifier', $auth->getNameIdNameQualifier());
-		$this->session->set('user_saml.samlNameIdSPNameQualifier', $auth->getNameIdSPNameQualifier());
-		$this->session->set('user_saml.samlSessionIndex', $auth->getSessionIndex());
-		$this->session->set('user_saml.samlSessionExpiration', $auth->getSessionExpiration());
-		$this->logger->debug('Session values set', ['app' => 'user_saml']);
+		$this->sessionService->storeAuthDataInSession($auth);
+
 		try {
 			$user = $this->userResolver->findExistingUser($this->userBackend->getCurrentUserId());
 			$firstLogin = $user->updateLastLoginTimestamp();
@@ -510,14 +506,14 @@ public function singleLogoutService(): Http\RedirectResponse {
 	 */
 	private function tryProcessSLOResponse(?int $idp): array {
 		$idps = ($idp !== null) ? [$idp] : array_keys($this->samlSettings->getListOfIdps());
-		foreach ($idps as $idp) {
+		foreach ($idps as $identityProviderId) {
 			try {
-				$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
+				$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($identityProviderId));
 				// validator (called with processSLO()) needs an XML entity loader
 				$targetUrl = $this->callWithXmlEntityLoader(fn (): string => $auth->processSLO(
 					true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
 					null,
-					$this->samlSettings->usesSloWebServerDecode($idp),
+					$this->samlSettings->usesSloWebServerDecode($identityProviderId),
 					null,
 					true
 				));

lib/DavPlugin.php

@@ -8,40 +8,41 @@
 namespace OCA\User_SAML;
 
 use OCA\DAV\Connector\Sabre\Auth;
-use OCP\IConfig;
+use OCA\User_SAML\Service\SessionService;
+use OCP\AppFramework\Services\IAppConfig;
 use OCP\ISession;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
 
 class DavPlugin extends ServerPlugin {
-	/** @var Server */
-	private $server;
+	/** @noinspection PhpPropertyOnlyWrittenInspection */
+	private Server $server;
 
 	public function __construct(
 		private readonly ISession $session,
-		private readonly IConfig $config,
-		private array $auth,
+		private readonly IAppConfig $config,
+		private readonly array $auth,
 		private readonly SAMLSettings $samlSettings,
+		private readonly SessionService $sessionService,
 	) {
 	}
 
-	public function initialize(Server $server) {
-		// before auth
+	public function initialize(Server $server): void {
 		$server->on('beforeMethod:*', $this->beforeMethod(...), 9);
 		$this->server = $server;
 	}
 
-	public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
+	public function beforeMethod(RequestInterface $request, ResponseInterface $response): void {
 		if (
-			$this->config->getAppValue('user_saml', 'type') === 'environment-variable'
+			$this->config->getAppValueString('type', 'unset') === 'environment-variable'
 			&& !$this->session->exists('user_saml.samlUserData')
 		) {
 			$uidMapping = $this->samlSettings->get(1)['general-uid_mapping'];
 			if (isset($this->auth[$uidMapping])) {
 				$this->session->set(Auth::DAV_AUTHENTICATED, $this->auth[$uidMapping]);
-				$this->session->set('user_saml.samlUserData', $this->auth);
+				$this->sessionService->prepareEnvironmentBasedSession($this->auth);
 			}
 		}
 	}

lib/Db/SessionData.php

@@ -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);
+	}
+}

lib/Db/SessionDataMapper.php

@@ -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);
+	}
+}

lib/Jobs/CleanSessionData.php

@@ -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();
+	}
+}