add a job to clean up expired verification tokens

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

core/Controller/LostController.php

@@ -172,7 +172,7 @@ public function resetform($token, $userId) {
 	 */
 	protected function checkPasswordResetToken(string $token, string $userId): void {
 		try {
-			$this->verificationToken->check($token, $this->userManager->get($userId), 'lostpassword');
+			$this->verificationToken->check($token, $this->userManager->get($userId), 'lostpassword', '', true);
 		} catch (InvalidTokenException $e) {
 			$error = $e->getCode() === InvalidTokenException::TOKEN_EXPIRED
 				? $this->l10n->t('Could not reset password because the token is expired')

lib/composer/composer/autoload_classmap.php

@@ -1373,6 +1373,7 @@
     'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php',
     'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php',
     'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php',
+    'OC\\Security\\VerificationToken\\CleanUpJob' => $baseDir . '/lib/private/Security/VerificationToken/CleanUpJob.php',
     'OC\\Security\\VerificationToken\\VerificationToken' => $baseDir . '/lib/private/Security/VerificationToken/VerificationToken.php',
     'OC\\Server' => $baseDir . '/lib/private/Server.php',
     'OC\\ServerContainer' => $baseDir . '/lib/private/ServerContainer.php',

lib/composer/composer/autoload_static.php

@@ -1402,6 +1402,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php',
         'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php',
         'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php',
+        'OC\\Security\\VerificationToken\\CleanUpJob' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/CleanUpJob.php',
         'OC\\Security\\VerificationToken\\VerificationToken' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/VerificationToken.php',
         'OC\\Server' => __DIR__ . '/../../..' . '/lib/private/Server.php',
         'OC\\ServerContainer' => __DIR__ . '/../../..' . '/lib/private/ServerContainer.php',

lib/private/Security/VerificationToken/CleanUpJob.php

@@ -0,0 +1,90 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de>
+ *
+ * @author Arthur Schiwon <blizzz@arthur-schiwon.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Security\VerificationToken;
+
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\IConfig;
+use OCP\ILogger;
+use OCP\IUserManager;
+use OCP\Security\VerificationToken\InvalidTokenException;
+use OCP\Security\VerificationToken\IVerificationToken;
+
+class CleanUpJob extends \OCP\BackgroundJob\Job {
+
+	/** @var int */
+	protected $runNotBefore;
+	/** @var string */
+	protected $userId;
+	/** @var string */
+	protected $subject;
+	/** @var string */
+	protected $pwdPrefix;
+	/** @var IConfig */
+	private $config;
+	/** @var IVerificationToken */
+	private $verificationToken;
+	/** @var IUserManager */
+	private $userManager;
+
+	public function __construct(ITimeFactory $time, IConfig $config, IVerificationToken $verificationToken, IUserManager $userManager) {
+		parent::__construct($time);
+		$this->config = $config;
+		$this->verificationToken = $verificationToken;
+		$this->userManager = $userManager;
+	}
+
+	public function setArgument($argument) {
+		parent::setArgument($argument);
+		$args = \json_decode($argument);
+		$this->userId = (string)$args['userId'];
+		$this->subject = (string)$args['subject'];
+		$this->pwdPrefix = (string)$args['pp'];
+		$this->runNotBefore = (int)$args['notBefore'];
+	}
+
+	protected function run($argument) {
+		try {
+			$user = $this->userManager->get($this->userId);
+			if ($user === null) {
+				return;
+			}
+			$this->verificationToken->check('irrelevant', $user, $this->subject, $this->pwdPrefix);
+		} catch (InvalidTokenException $e) {
+			if ($e->getCode() === InvalidTokenException::TOKEN_EXPIRED) {
+				// make sure to only remove expired tokens
+				$this->config->deleteUserValue($this->userId, 'core', $this->subject);
+			}
+		}
+	}
+
+	public function execute($jobList, ILogger $logger = null) {
+		if ($this->time->getTime() >= $this->runNotBefore) {
+			$jobList->remove($this, $this->argument);
+			parent::execute($jobList, $logger);
+		}
+	}
+}

lib/private/Security/VerificationToken/VerificationToken.php

@@ -27,14 +27,17 @@
 namespace OC\Security\VerificationToken;
 
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\IJobList;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\Security\ICrypto;
 use OCP\Security\ISecureRandom;
 use OCP\Security\VerificationToken\InvalidTokenException;
 use OCP\Security\VerificationToken\IVerificationToken;
+use function json_encode;
 
 class VerificationToken implements IVerificationToken {
+	protected const TOKEN_LIFETIME = 60 * 60 * 24 * 7;
 
 	/** @var IConfig */
 	private $config;
@@ -44,17 +47,21 @@ class VerificationToken implements IVerificationToken {
 	private $timeFactory;
 	/** @var ISecureRandom */
 	private $secureRandom;
+	/** @var IJobList */
+	private $jobList;
 
 	public function __construct(
 		IConfig $config,
 		ICrypto $crypto,
 		ITimeFactory $timeFactory,
-		ISecureRandom $secureRandom
+		ISecureRandom $secureRandom,
+		IJobList $jobList
 	) {
 		$this->config = $config;
 		$this->crypto = $crypto;
 		$this->timeFactory = $timeFactory;
 		$this->secureRandom = $secureRandom;
+		$this->jobList = $jobList;
 	}
 
 	/**
@@ -64,7 +71,7 @@ protected function throwInvalidTokenException(int $code): void {
 		throw new InvalidTokenException($code);
 	}
 
-	public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = ''): void {
+	public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = '', bool $expiresWithLogin = false): void {
 		if ($user === null || !$user->isEnabled()) {
 			$this->throwInvalidTokenException(InvalidTokenException::USER_UNKNOWN);
 		}
@@ -85,8 +92,8 @@ public function check(string $token, ?IUser $user, string $subject, string $pass
 			$this->throwInvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT);
 		}
 
-		if ($splitToken[0] < ($this->timeFactory->getTime() - 60 * 60 * 24 * 7) ||
-			$user->getLastLogin() > $splitToken[0]) {
+		if ($splitToken[0] < ($this->timeFactory->getTime() - self::TOKEN_LIFETIME)
+			|| ($expiresWithLogin && $user->getLastLogin() > $splitToken[0])) {
 			$this->throwInvalidTokenException(InvalidTokenException::TOKEN_EXPIRED);
 		}
 
@@ -105,6 +112,13 @@ public function create(IUser $user, string $subject, string $passwordPrefix = ''
 		$tokenValue = $this->timeFactory->getTime() .':'. $token;
 		$encryptedValue = $this->crypto->encrypt($tokenValue, $passwordPrefix . $this->config->getSystemValue('secret'));
 		$this->config->setUserValue($user->getUID(), 'core', $subject, $encryptedValue);
+		$jobArgs = json_encode([
+			'userId' => $user->getUID(),
+			'subject' => $subject,
+			'pp' => $passwordPrefix,
+			'notBefore' => $this->timeFactory->getTime() + self::TOKEN_LIFETIME * 2, // multiply to provide a grace period
+		]);
+		$this->jobList->add(CleanUpJob::class, $jobArgs);
 
 		return $token;
 	}

lib/public/Security/VerificationToken/IVerificationToken.php

@@ -46,7 +46,7 @@ interface IVerificationToken {
 	 * @throws InvalidTokenException
 	 * @since 23.0.0
 	 */
-	public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = ''): void;
+	public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = '', bool $expiresWithLogin = false): void;
 
 	/**
 	 * @since 23.0.0

tests/lib/Security/VerificationToken/VerificationTokenTest.php

@@ -28,6 +28,7 @@
 
 use OC\Security\VerificationToken\VerificationToken;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\IJobList;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\Security\ICrypto;
@@ -54,12 +55,14 @@ protected function setUp(): void {
 		$this->crypto = $this->createMock(ICrypto::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->secureRandom = $this->createMock(ISecureRandom::class);
+		$this->jobList = $this->createMock(IJobList::class);
 
 		$this->token = new VerificationToken(
 			$this->config,
 			$this->crypto,
 			$this->timeFactory,
-			$this->secureRandom
+			$this->secureRandom,
+			$this->jobList
 		);
 	}
 
@@ -177,13 +180,47 @@ public function testTokenExpired() {
 
 		$this->timeFactory->expects($this->any())
 			->method('getTime')
-			->willReturn(604801);
+			->willReturn(604800 * 3);
 
 		$this->expectException(InvalidTokenException::class);
 		$this->expectExceptionCode(InvalidTokenException::TOKEN_EXPIRED);
 		$this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar');
 	}
 
+	public function testTokenExpiredByLogin() {
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->atLeastOnce())
+			->method('isEnabled')
+			->willReturn(true);
+		$user->expects($this->atLeastOnce())
+			->method('getUID')
+			->willReturn('alice');
+		$user->expects($this->any())
+			->method('getLastLogin')
+			->willReturn(604803);
+
+		$this->config->expects($this->atLeastOnce())
+			->method('getUserValue')
+			->with('alice', 'core', 'fingerprintToken', null)
+			->willReturn('encryptedToken');
+		$this->config->expects($this->any())
+			->method('getSystemValue')
+			->with('secret')
+			->willReturn('357111317');
+
+		$this->crypto->method('decrypt')
+			->with('encryptedToken', 'foobar' . '357111317')
+			->willReturn('604800:mY70K3n');
+
+		$this->timeFactory->expects($this->any())
+			->method('getTime')
+			->willReturn(604801);
+
+		$this->expectException(InvalidTokenException::class);
+		$this->expectExceptionCode(InvalidTokenException::TOKEN_EXPIRED);
+		$this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar', true);
+	}
+
 	public function testTokenMismatch() {
 		$user = $this->createMock(IUser::class);
 		$user->expects($this->atLeastOnce())