From df34571d1d888e7232487664629679130fef5e5e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 4 Jun 2018 09:51:13 +0200 Subject: [PATCH] Use constant for token version And don't set the version in the constructor. That would possible cause to many updates. Signed-off-by: Roeland Jago Douma --- .../Authentication/Token/DefaultToken.php | 4 ++-- .../Authentication/Token/DefaultTokenMapper.php | 14 +++++++------- .../Token/DefaultTokenProvider.php | 1 + .../Authentication/Token/PublicKeyToken.php | 4 ++-- .../Token/PublicKeyTokenMapper.php | 16 ++++++++-------- .../Token/PublicKeyTokenProvider.php | 3 ++- .../Token/DefaultTokenMapperTest.php | 2 ++ .../Token/DefaultTokenProviderTest.php | 1 + .../Token/PublicKeyTokenMapperTest.php | 2 ++ 9 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 29c4b5a74a..85ea0dc4cd 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -41,6 +41,8 @@ use OCP\AppFramework\Db\Entity; */ class DefaultToken extends Entity implements IToken { + const VERSION = 1; + /** @var string user UID */ protected $uid; @@ -90,8 +92,6 @@ class DefaultToken extends Entity implements IToken { $this->addType('scope', 'string'); $this->addType('expires', 'int'); $this->addType('version', 'int'); - - $this->setVersion(1); } public function getId(): int { diff --git a/lib/private/Authentication/Token/DefaultTokenMapper.php b/lib/private/Authentication/Token/DefaultTokenMapper.php index 02964e3f59..b8df00ff09 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -50,7 +50,7 @@ class DefaultTokenMapper extends QBMapper { $qb = $this->db->getQueryBuilder(); $qb->delete('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); } @@ -65,7 +65,7 @@ class DefaultTokenMapper extends QBMapper { ->where($qb->expr()->lt('last_activity', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter(IToken::TEMPORARY_TOKEN, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('remember', $qb->createNamedParameter($remember, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); } @@ -82,7 +82,7 @@ class DefaultTokenMapper extends QBMapper { $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'token', 'type', 'remember', 'last_activity', 'last_check', 'scope', 'expires', 'version') ->from('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); $data = $result->fetch(); @@ -106,7 +106,7 @@ class DefaultTokenMapper extends QBMapper { $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'token', 'type', 'remember', 'last_activity', 'last_check', 'scope', 'expires', 'version') ->from('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); $data = $result->fetch(); @@ -132,7 +132,7 @@ class DefaultTokenMapper extends QBMapper { $qb->select('id', 'uid', 'login_name', 'password', 'name', 'token', 'type', 'remember', 'last_activity', 'last_check', 'scope', 'expires', 'version') ->from('authtoken') ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))) ->setMaxResults(1000); $result = $qb->execute(); $data = $result->fetchAll(); @@ -151,7 +151,7 @@ class DefaultTokenMapper extends QBMapper { $qb->delete('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))); $qb->execute(); } @@ -164,7 +164,7 @@ class DefaultTokenMapper extends QBMapper { $qb = $this->db->getQueryBuilder(); $qb->delete('authtoken') ->where($qb->expr()->eq('name', $qb->createNamedParameter($name), IQueryBuilder::PARAM_STR)) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT))); $qb->execute(); } diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index ed3c14c174..078ab4ed8f 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -105,6 +105,7 @@ class DefaultTokenProvider implements IProvider { $dbToken->setRemember($remember); $dbToken->setLastActivity($this->time->getTime()); $dbToken->setLastCheck($this->time->getTime()); + $dbToken->setVersion(DefaultToken::VERSION); $this->mapper->insert($dbToken); diff --git a/lib/private/Authentication/Token/PublicKeyToken.php b/lib/private/Authentication/Token/PublicKeyToken.php index 18b2707577..9d01fc9ecc 100644 --- a/lib/private/Authentication/Token/PublicKeyToken.php +++ b/lib/private/Authentication/Token/PublicKeyToken.php @@ -45,6 +45,8 @@ use OCP\AppFramework\Db\Entity; */ class PublicKeyToken extends Entity implements IToken { + const VERSION = 2; + /** @var string user UID */ protected $uid; @@ -102,8 +104,6 @@ class PublicKeyToken extends Entity implements IToken { $this->addType('publicKey', 'string'); $this->addType('privateKey', 'string'); $this->addType('version', 'int'); - - $this->setVersion(2); } public function getId(): int { diff --git a/lib/private/Authentication/Token/PublicKeyTokenMapper.php b/lib/private/Authentication/Token/PublicKeyTokenMapper.php index 23982c6ba0..129b2a272b 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenMapper.php +++ b/lib/private/Authentication/Token/PublicKeyTokenMapper.php @@ -45,7 +45,7 @@ class PublicKeyTokenMapper extends QBMapper { $qb = $this->db->getQueryBuilder(); $qb->delete('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); } @@ -60,7 +60,7 @@ class PublicKeyTokenMapper extends QBMapper { ->where($qb->expr()->lt('last_activity', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter(IToken::TEMPORARY_TOKEN, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('remember', $qb->createNamedParameter($remember, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); } @@ -75,7 +75,7 @@ class PublicKeyTokenMapper extends QBMapper { $result = $qb->select('*') ->from('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); $data = $result->fetch(); @@ -97,7 +97,7 @@ class PublicKeyTokenMapper extends QBMapper { $result = $qb->select('*') ->from('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))) ->execute(); $data = $result->fetch(); @@ -123,7 +123,7 @@ class PublicKeyTokenMapper extends QBMapper { $qb->select('*') ->from('authtoken') ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))) ->setMaxResults(1000); $result = $qb->execute(); $data = $result->fetchAll(); @@ -142,7 +142,7 @@ class PublicKeyTokenMapper extends QBMapper { $qb->delete('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))); $qb->execute(); } @@ -155,7 +155,7 @@ class PublicKeyTokenMapper extends QBMapper { $qb = $this->db->getQueryBuilder(); $qb->delete('authtoken') ->where($qb->expr()->eq('name', $qb->createNamedParameter($name), IQueryBuilder::PARAM_STR)) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))); $qb->execute(); } @@ -165,7 +165,7 @@ class PublicKeyTokenMapper extends QBMapper { $qb->delete('authtoken') ->where($qb->expr()->eq('type', $qb->createNamedParameter(IToken::TEMPORARY_TOKEN))) ->andWhere($qb->expr()->neq('id', $qb->createNamedParameter($except->getId()))) - ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT))); $qb->execute(); } diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index ca7a7e37e1..b7e0d1da33 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -190,7 +190,7 @@ class PublicKeyTokenProvider implements IProvider { throw new InvalidTokenException(); } - // When changeing passwords all temp tokens are deleted + // When changing passwords all temp tokens are deleted $this->mapper->deleteTempToken($token); // Update the password for all tokens @@ -314,6 +314,7 @@ class PublicKeyTokenProvider implements IProvider { $dbToken->setRemember($remember); $dbToken->setLastActivity($this->time->getTime()); $dbToken->setLastCheck($this->time->getTime()); + $dbToken->setVersion(PublicKeyToken::VERSION); return $dbToken; } diff --git a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php index ab09c00529..bebceba62c 100644 --- a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php @@ -135,6 +135,7 @@ class DefaultTokenMapperTest extends TestCase { $token->setRemember(IToken::DO_NOT_REMEMBER); $token->setLastActivity($this->time - 60 * 60 * 24 * 3); $token->setLastCheck($this->time - 10); + $token->setVersion(DefaultToken::VERSION); $dbToken = $this->mapper->getToken($token->getToken()); @@ -164,6 +165,7 @@ class DefaultTokenMapperTest extends TestCase { $token->setRemember(IToken::DO_NOT_REMEMBER); $token->setLastActivity($this->time - 60 * 60 * 24 * 3); $token->setLastCheck($this->time - 10); + $token->setVersion(DefaultToken::VERSION); $dbToken = $this->mapper->getToken($token->getToken()); $token->setId($dbToken->getId()); // We don't know the ID diff --git a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php index 58e152457f..3fb11f410b 100644 --- a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php @@ -91,6 +91,7 @@ class DefaultTokenProviderTest extends TestCase { $toInsert->setRemember(IToken::DO_NOT_REMEMBER); $toInsert->setLastActivity($this->time); $toInsert->setLastCheck($this->time); + $toInsert->setVersion(DefaultToken::VERSION); $this->config->expects($this->any()) ->method('getSystemValue') diff --git a/tests/lib/Authentication/Token/PublicKeyTokenMapperTest.php b/tests/lib/Authentication/Token/PublicKeyTokenMapperTest.php index ce4de92e19..5a98747ab0 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenMapperTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenMapperTest.php @@ -147,6 +147,7 @@ class PublicKeyTokenMapperTest extends TestCase { $token->setLastCheck($this->time - 10); $token->setPublicKey('public key'); $token->setPrivateKey('private key'); + $token->setVersion(PublicKeyToken::VERSION); $dbToken = $this->mapper->getToken($token->getToken()); @@ -178,6 +179,7 @@ class PublicKeyTokenMapperTest extends TestCase { $token->setLastCheck($this->time - 10); $token->setPublicKey('public key'); $token->setPrivateKey('private key'); + $token->setVersion(PublicKeyToken::VERSION); $dbToken = $this->mapper->getToken($token->getToken()); $token->setId($dbToken->getId()); // We don't know the ID