From 46aafe4f11e971e442baa3283906878ef5dae856 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 12:39:00 +0200 Subject: [PATCH] Certain tokens can expire However due to the nature of what we store in the token (encrypted passwords etc). We can't just delete the tokens because that would make the oauth refresh useless. Signed-off-by: Roeland Jago Douma --- .../Version13000Date20180516101403.php | 56 ++++++++++++++ lib/composer/composer/ClassLoader.php | 15 ++-- lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 14 +++- .../Exceptions/ExpiredTokenException.php | 40 ++++++++++ .../Authentication/Token/DefaultToken.php | 16 ++++ .../Token/DefaultTokenMapper.php | 6 +- .../Token/DefaultTokenProvider.php | 19 ++++- .../Authentication/Token/IProvider.php | 2 + lib/private/Authentication/Token/IToken.php | 7 ++ .../Token/DefaultTokenProviderTest.php | 75 +++++++++++++++++++ version.php | 2 +- 12 files changed, 238 insertions(+), 16 deletions(-) create mode 100644 core/Migrations/Version13000Date20180516101403.php create mode 100644 lib/private/Authentication/Exceptions/ExpiredTokenException.php diff --git a/core/Migrations/Version13000Date20180516101403.php b/core/Migrations/Version13000Date20180516101403.php new file mode 100644 index 0000000000..62198d0bb3 --- /dev/null +++ b/core/Migrations/Version13000Date20180516101403.php @@ -0,0 +1,56 @@ + + * + * @author Roeland Jago Douma + * + * @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 . + * + */ + +namespace OC\Core\Migrations; + +use OCP\DB\ISchemaWrapper; +use OCP\Migration\SimpleMigrationStep; +use OCP\Migration\IOutput; + +class Version13000Date20180516101403 extends SimpleMigrationStep { + + /** + * @param IOutput $output + * @param \Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, \Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('authtoken'); + + if (!$table->hasColumn('expires')) { + $table->addColumn('expires', 'integer', [ + 'notnull' => false, + 'length' => 4, + 'default' => null, + 'unsigned' => true, + ]); + + return $schema; + } + return null; + } +} diff --git a/lib/composer/composer/ClassLoader.php b/lib/composer/composer/ClassLoader.php index c6f6d2322b..dc02dfb114 100644 --- a/lib/composer/composer/ClassLoader.php +++ b/lib/composer/composer/ClassLoader.php @@ -43,8 +43,7 @@ namespace Composer\Autoload; class ClassLoader { // PSR-4 - private $firstCharsPsr4 = array(); - private $prefixLengthsPsr4 = array(); // For BC with legacy static maps + private $prefixLengthsPsr4 = array(); private $prefixDirsPsr4 = array(); private $fallbackDirsPsr4 = array(); @@ -171,10 +170,11 @@ class ClassLoader } } elseif (!isset($this->prefixDirsPsr4[$prefix])) { // Register directories for a new namespace. - if ('\\' !== substr($prefix, -1)) { + $length = strlen($prefix); + if ('\\' !== $prefix[$length - 1]) { throw new \InvalidArgumentException("A non-empty PSR-4 prefix must end with a namespace separator."); } - $this->firstCharsPsr4[$prefix[0]] = true; + $this->prefixLengthsPsr4[$prefix[0]][$prefix] = $length; $this->prefixDirsPsr4[$prefix] = (array) $paths; } elseif ($prepend) { // Prepend directories for an already registered namespace. @@ -221,10 +221,11 @@ class ClassLoader if (!$prefix) { $this->fallbackDirsPsr4 = (array) $paths; } else { - if ('\\' !== substr($prefix, -1)) { + $length = strlen($prefix); + if ('\\' !== $prefix[$length - 1]) { throw new \InvalidArgumentException("A non-empty PSR-4 prefix must end with a namespace separator."); } - $this->firstCharsPsr4[$prefix[0]] = true; + $this->prefixLengthsPsr4[$prefix[0]][$prefix] = $length; $this->prefixDirsPsr4[$prefix] = (array) $paths; } } @@ -372,7 +373,7 @@ class ClassLoader $logicalPathPsr4 = strtr($class, '\\', DIRECTORY_SEPARATOR) . $ext; $first = $class[0]; - if (isset($this->firstCharsPsr4[$first]) || isset($this->prefixLengthsPsr4[$first])) { + if (isset($this->prefixLengthsPsr4[$first])) { $subPath = $class; while (false !== $lastPos = strrpos($subPath, '\\')) { $subPath = substr($subPath, 0, $lastPos); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index e9de3538b7..8d7a4a7b57 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -394,6 +394,7 @@ return array( 'OC\\Authentication\\Token\\DefaultTokenCleanupJob' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenCleanupJob.php', 'OC\\Authentication\\Token\\DefaultTokenMapper' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenMapper.php', 'OC\\Authentication\\Token\\DefaultTokenProvider' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenProvider.php', + 'OC\\Authentication\\Token\\ExpiredTokenException' => $baseDir . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php', 'OC\\Authentication\\Token\\IProvider' => $baseDir . '/lib/private/Authentication/Token/IProvider.php', 'OC\\Authentication\\Token\\IToken' => $baseDir . '/lib/private/Authentication/Token/IToken.php', 'OC\\Authentication\\TwoFactorAuth\\Manager' => $baseDir . '/lib/private/Authentication/TwoFactorAuth/Manager.php', @@ -537,6 +538,7 @@ return array( 'OC\\Core\\Migrations\\Version13000Date20170814074715' => $baseDir . '/core/Migrations/Version13000Date20170814074715.php', 'OC\\Core\\Migrations\\Version13000Date20170919121250' => $baseDir . '/core/Migrations/Version13000Date20170919121250.php', 'OC\\Core\\Migrations\\Version13000Date20170926101637' => $baseDir . '/core/Migrations/Version13000Date20170926101637.php', + 'OC\\Core\\Migrations\\Version13000Date20180516101403' => $baseDir . '/core/Migrations/Version13000Date20180516101403.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', 'OC\\DB\\AdapterMySQL' => $baseDir . '/lib/private/DB/AdapterMySQL.php', 'OC\\DB\\AdapterOCI8' => $baseDir . '/lib/private/DB/AdapterOCI8.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 9988e2118f..ba65c44f79 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -6,8 +6,14 @@ namespace Composer\Autoload; class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c { - public static $firstCharsPsr4 = array ( - 'O' => true, + public static $prefixLengthsPsr4 = array ( + 'O' => + array ( + 'OC\\Settings\\' => 12, + 'OC\\Core\\' => 8, + 'OC\\' => 3, + 'OCP\\' => 4, + ), ); public static $prefixDirsPsr4 = array ( @@ -418,6 +424,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Authentication\\Token\\DefaultTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenCleanupJob.php', 'OC\\Authentication\\Token\\DefaultTokenMapper' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenMapper.php', 'OC\\Authentication\\Token\\DefaultTokenProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenProvider.php', + 'OC\\Authentication\\Token\\ExpiredTokenException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php', 'OC\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IProvider.php', 'OC\\Authentication\\Token\\IToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IToken.php', 'OC\\Authentication\\TwoFactorAuth\\Manager' => __DIR__ . '/../../..' . '/lib/private/Authentication/TwoFactorAuth/Manager.php', @@ -561,6 +568,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version13000Date20170814074715' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170814074715.php', 'OC\\Core\\Migrations\\Version13000Date20170919121250' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170919121250.php', 'OC\\Core\\Migrations\\Version13000Date20170926101637' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170926101637.php', + 'OC\\Core\\Migrations\\Version13000Date20180516101403' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20180516101403.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', 'OC\\DB\\AdapterMySQL' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterMySQL.php', 'OC\\DB\\AdapterOCI8' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterOCI8.php', @@ -986,7 +994,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c public static function getInitializer(ClassLoader $loader) { return \Closure::bind(function () use ($loader) { - $loader->firstCharsPsr4 = ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c::$firstCharsPsr4; + $loader->prefixLengthsPsr4 = ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c::$prefixLengthsPsr4; $loader->prefixDirsPsr4 = ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c::$prefixDirsPsr4; $loader->classMap = ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c::$classMap; diff --git a/lib/private/Authentication/Exceptions/ExpiredTokenException.php b/lib/private/Authentication/Exceptions/ExpiredTokenException.php new file mode 100644 index 0000000000..8abf01bae0 --- /dev/null +++ b/lib/private/Authentication/Exceptions/ExpiredTokenException.php @@ -0,0 +1,40 @@ + + * + * @author Roeland Jago Douma + * + * @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 . + * + */ +namespace OC\Authentication\Token; + +use OC\Authentication\Exceptions\InvalidTokenException; + +class ExpiredTokenException extends InvalidTokenException { + /** @var IToken */ + private $token; + + public function __construct(IToken $token) { + parent::__construct(); + + $this->token = $token; + } + + public function getToken() { + return $this->token; + } +} diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 52dd6644d2..f41d0b8b7c 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -91,10 +91,15 @@ class DefaultToken extends Entity implements IToken { */ protected $scope; + /** @var int */ + protected $expires; + public function __construct() { $this->addType('type', 'int'); $this->addType('lastActivity', 'int'); $this->addType('lastCheck', 'int'); + $this->addType('scope', 'string'); + $this->addType('expires', 'int'); } public function getId() { @@ -180,4 +185,15 @@ class DefaultToken extends Entity implements IToken { public function setPassword($password = null) { parent::setPassword($password); } + + public function setExpires($expires) { + parent::setExpires($expires); + } + + /** + * @return int|null + */ + public function getExpires() { + return parent::getExpires(); + } } diff --git a/lib/private/Authentication/Token/DefaultTokenMapper.php b/lib/private/Authentication/Token/DefaultTokenMapper.php index 41d1b9f203..70a450602d 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -78,7 +78,7 @@ class DefaultTokenMapper extends Mapper { public function getToken($token) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope') + $result = $qb->select('*') ->from('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))) ->execute(); @@ -101,7 +101,7 @@ class DefaultTokenMapper extends Mapper { public function getTokenById($id) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check', 'scope') + $result = $qb->select('*') ->from('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->execute(); @@ -126,7 +126,7 @@ class DefaultTokenMapper extends Mapper { public function getTokenByUser(IUser $user) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope') + $qb->select('*') ->from('authtoken') ->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))) ->setMaxResults(1000); diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index 13407a688d..4e87424e55 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -155,13 +155,20 @@ class DefaultTokenProvider implements IProvider { * @param string $tokenId * @throws InvalidTokenException * @return DefaultToken + * @throws ExpiredTokenException */ public function getToken($tokenId) { try { - return $this->mapper->getToken($this->hashToken($tokenId)); + $token = $this->mapper->getToken($this->hashToken($tokenId)); } catch (DoesNotExistException $ex) { throw new InvalidTokenException(); } + + if ($token->getExpires() !== null && $token->getExpires() < $this->time->getTime()) { + throw new ExpiredTokenException($token); + } + + return $token; } /** @@ -170,13 +177,21 @@ class DefaultTokenProvider implements IProvider { * @param string $tokenId * @throws InvalidTokenException * @return DefaultToken + * @throws ExpiredTokenException + * @return IToken */ public function getTokenById($tokenId) { try { - return $this->mapper->getTokenById($tokenId); + $token = $this->mapper->getTokenById($tokenId); } catch (DoesNotExistException $ex) { throw new InvalidTokenException(); } + + if ($token->getExpires() !== null && $token->getExpires() < $this->time->getTime()) { + throw new ExpiredTokenException($token); + } + + return $token; } /** diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php index 707645a09e..8b812a9533 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -51,6 +51,7 @@ interface IProvider { * * @param string $tokenId * @throws InvalidTokenException + * @throws ExpiredTokenException * @return IToken */ public function getToken($tokenId); @@ -61,6 +62,7 @@ interface IProvider { * @param string $tokenId * @throws InvalidTokenException * @return DefaultToken + * @throws ExpiredTokenException */ public function getTokenById($tokenId); diff --git a/lib/private/Authentication/Token/IToken.php b/lib/private/Authentication/Token/IToken.php index 0e32e3adfd..6586a5b2fd 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -108,4 +108,11 @@ interface IToken extends JsonSerializable { * @param string $password */ public function setPassword($password); + + /** + * Set the expiration time of the token + * + * @param int|null $expires + */ + public function setExpires($expires); } diff --git a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php index e2643d315c..ccf654bcdf 100644 --- a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php @@ -25,6 +25,7 @@ namespace Test\Authentication\Token; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\DefaultToken; use OC\Authentication\Token\DefaultTokenProvider; +use OC\Authentication\Token\ExpiredTokenException; use OC\Authentication\Token\IToken; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Mapper; @@ -397,6 +398,63 @@ class DefaultTokenProviderTest extends TestCase { $this->tokenProvider->renewSessionToken('oldId', 'newId'); } + public function testGetToken() { + $token = new DefaultToken(); + + $this->config->method('getSystemValue') + ->with('secret') + ->willReturn('mysecret'); + + $this->mapper->method('getToken') + ->with( + $this->callback(function ($token) { + return hash('sha512', 'unhashedTokenmysecret') === $token; + }) + )->willReturn($token); + + $this->assertSame($token, $this->tokenProvider->getToken('unhashedToken')); + } + + public function testGetInvalidToken() { + $this->expectException(InvalidTokenException::class); + + $this->config->method('getSystemValue') + ->with('secret') + ->willReturn('mysecret'); + + $this->mapper->method('getToken') + ->with( + $this->callback(function ($token) { + return hash('sha512', 'unhashedTokenmysecret') === $token; + }) + )->willThrowException(new InvalidTokenException()); + + $this->tokenProvider->getToken('unhashedToken'); + } + + public function testGetExpiredToken() { + $token = new DefaultToken(); + $token->setExpires(42); + + $this->config->method('getSystemValue') + ->with('secret') + ->willReturn('mysecret'); + + $this->mapper->method('getToken') + ->with( + $this->callback(function ($token) { + return hash('sha512', 'unhashedTokenmysecret') === $token; + }) + )->willReturn($token); + + try { + $this->tokenProvider->getToken('unhashedToken'); + } catch (ExpiredTokenException $e) { + $this->assertSame($token, $e->getToken()); + } + + } + public function testGetTokenById() { $token = $this->createMock(DefaultToken::class); @@ -419,6 +477,23 @@ class DefaultTokenProviderTest extends TestCase { $this->tokenProvider->getTokenById(42); } + public function testGetExpiredTokenById() { + $token = new DefaultToken(); + $token->setExpires(42); + + $this->mapper->expects($this->once()) + ->method('getTokenById') + ->with($this->equalTo(42)) + ->willReturn($token); + + try { + $this->tokenProvider->getTokenById(42); + $this->fail(); + } catch (ExpiredTokenException $e) { + $this->assertSame($token, $e->getToken()); + } + } + public function testRotate() { $token = new DefaultToken(); $token->setPassword('oldencryptedpassword'); diff --git a/version.php b/version.php index 5c1b3eea2f..60a4d48b65 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = array(13, 0, 2, 1); +$OC_Version = array(13, 0, 2, 2); // The human readable string $OC_VersionString = '13.0.2';