From 8fcb7d4334526a30164db1d0c45b89f9f38614bd Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 15 May 2018 21:10:43 +0200 Subject: [PATCH 1/8] Allow the rotation of tokens This for example will allow rotating the apptoken for oauth Signed-off-by: Roeland Jago Douma --- .../Authentication/Token/DefaultToken.php | 10 ++++- .../Token/DefaultTokenProvider.php | 22 ++++++++++ .../Authentication/Token/IProvider.php | 10 +++++ lib/private/Authentication/Token/IToken.php | 14 +++++++ .../Token/DefaultTokenProviderTest.php | 42 +++++++++++++++++++ 5 files changed, 96 insertions(+), 2 deletions(-) diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index e06803d0bfcf4..52dd6644d2e7f 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -29,10 +29,8 @@ * @method void setId(int $id) * @method void setUid(string $uid); * @method void setLoginName(string $loginName) - * @method void setPassword(string $password) * @method void setName(string $name) * @method string getName() - * @method void setToken(string $token) * @method string getToken() * @method void setType(string $type) * @method int getType() @@ -174,4 +172,12 @@ public function setScope($scope) { parent::setScope((string)$scope); } } + + public function setToken($token) { + parent::setToken($token); + } + + public function setPassword($password = null) { + parent::setPassword($password); + } } diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index 36a8b1d5464fa..13407a688d30b 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -266,6 +266,28 @@ public function invalidateOldTokens() { $this->mapper->invalidateOld($rememberThreshold, IToken::REMEMBER); } + /** + * Rotate the token. Usefull for for example oauth tokens + * + * @param IToken $token + * @param string $oldTokenId + * @param string $newTokenId + * @return IToken + */ + public function rotate(IToken $token, $oldTokenId, $newTokenId) { + try { + $password = $this->getPassword($token, $oldTokenId); + $token->setPassword($this->encryptPassword($password, $newTokenId)); + } catch (PasswordlessTokenException $e) { + + } + + $token->setToken($this->hashToken($newTokenId)); + $this->updateToken($token); + + return $token; + } + /** * @param string $token * @return string diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php index e1cc8182ff095..707645a09e94b 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -138,4 +138,14 @@ public function getPassword(IToken $token, $tokenId); * @throws InvalidTokenException */ public function setPassword(IToken $token, $tokenId, $password); + + /** + * Rotate the token. Usefull for for example oauth tokens + * + * @param IToken $token + * @param string $oldTokenId + * @param string $newTokenId + * @return IToken + */ + public function rotate(IToken $token, $oldTokenId, $newTokenId); } diff --git a/lib/private/Authentication/Token/IToken.php b/lib/private/Authentication/Token/IToken.php index a24d31e2ed297..0e32e3adfd621 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -94,4 +94,18 @@ public function getScopeAsArray(); * @param array $scope */ public function setScope($scope); + + /** + * Set the token + * + * @param string $token + */ + public function setToken($token); + + /** + * Set the password + * + * @param string $password + */ + public function setPassword($password); } diff --git a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php index 08c74961c0d34..e2643d315ceb4 100644 --- a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php @@ -418,4 +418,46 @@ public function testGetInvalidTokenById() { $this->tokenProvider->getTokenById(42); } + + public function testRotate() { + $token = new DefaultToken(); + $token->setPassword('oldencryptedpassword'); + + $this->config->method('getSystemValue') + ->with('secret') + ->willReturn('mysecret'); + + $this->crypto->method('decrypt') + ->with('oldencryptedpassword', 'oldtokenmysecret') + ->willReturn('mypassword'); + $this->crypto->method('encrypt') + ->with('mypassword', 'newtokenmysecret') + ->willReturn('newencryptedpassword'); + + $this->mapper->expects($this->once()) + ->method('update') + ->with($this->callback(function (DefaultToken $token) { + return $token->getPassword() === 'newencryptedpassword' && + $token->getToken() === hash('sha512', 'newtokenmysecret'); + })); + + $this->tokenProvider->rotate($token, 'oldtoken', 'newtoken'); + } + + public function testRotateNoPassword() { + $token = new DefaultToken(); + + $this->config->method('getSystemValue') + ->with('secret') + ->willReturn('mysecret'); + + $this->mapper->expects($this->once()) + ->method('update') + ->with($this->callback(function (DefaultToken $token) { + return $token->getPassword() === null && + $token->getToken() === hash('sha512', 'newtokenmysecret'); + })); + + $this->tokenProvider->rotate($token, 'oldtoken', 'newtoken'); + } } From 46aafe4f11e971e442baa3283906878ef5dae856 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 12:39:00 +0200 Subject: [PATCH 2/8] 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 0000000000000..62198d0bb3788 --- /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 c6f6d2322bb36..dc02dfb114fb6 100644 --- a/lib/composer/composer/ClassLoader.php +++ b/lib/composer/composer/ClassLoader.php @@ -43,8 +43,7 @@ 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 @@ public function addPsr4($prefix, $paths, $prepend = false) } } 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 @@ public function setPsr4($prefix, $paths) 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 @@ private function findFileWithExtension($class, $ext) $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 e9de3538b7763..8d7a4a7b57dad 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -394,6 +394,7 @@ '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 @@ '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 9988e2118f4a4..ba65c44f79db8 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -6,8 +6,14 @@ 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 0000000000000..8abf01bae0989 --- /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 52dd6644d2e7f..f41d0b8b7c4b1 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 @@ public function setToken($token) { 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 41d1b9f203db9..70a450602daf8 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -78,7 +78,7 @@ public function invalidateOld($olderThan, $remember = IToken::DO_NOT_REMEMBER) { 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 @@ public function getToken($token) { 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 @@ public function getTokenById($id) { 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 13407a688d30b..4e87424e55cb4 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -155,13 +155,20 @@ public function getTokenByUser(IUser $user) { * @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 @@ public function getToken($tokenId) { * @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 707645a09e94b..8b812a9533c24 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -51,6 +51,7 @@ public function generateToken($token, $uid, $loginName, $password, $name, $type * * @param string $tokenId * @throws InvalidTokenException + * @throws ExpiredTokenException * @return IToken */ public function getToken($tokenId); @@ -61,6 +62,7 @@ public function getToken($tokenId); * @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 0e32e3adfd621..6586a5b2fd753 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -108,4 +108,11 @@ public function setToken($token); * @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 e2643d315ceb4..ccf654bcdfd4c 100644 --- a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php @@ -25,6 +25,7 @@ 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 @@ public function testRenewSessionTokenWithPassword() { $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 @@ public function testGetInvalidTokenById() { $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 5c1b3eea2f2bd..60a4d48b65041 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'; From 000cf1951c9e5a7090b16df7613139c3b8313e1e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 10:35:18 +0200 Subject: [PATCH 3/8] Set OAuth token expiration Signed-off-by: Roeland Jago Douma --- apps/oauth2/appinfo/info.xml | 8 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Controller/OauthApiController.php | 4 +- .../lib/Migration/SetTokenExpiration.php | 77 +++++++++++++++++++ 5 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 apps/oauth2/lib/Migration/SetTokenExpiration.php diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index ccddc9a8f7159..689cb0f437743 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -6,7 +6,7 @@ agpl Lukas Reschke OAuth2 - 1.1.0 + 1.1.1 @@ -15,6 +15,12 @@ + + + OCA\OAuth2\Migration\SetTokenExpiration + + + OCA\OAuth2\Settings\Admin diff --git a/apps/oauth2/composer/composer/autoload_classmap.php b/apps/oauth2/composer/composer/autoload_classmap.php index 97c8098caa3db..c933a1bf21e3b 100644 --- a/apps/oauth2/composer/composer/autoload_classmap.php +++ b/apps/oauth2/composer/composer/autoload_classmap.php @@ -15,5 +15,6 @@ 'OCA\\OAuth2\\Db\\ClientMapper' => $baseDir . '/../lib/Db/ClientMapper.php', 'OCA\\OAuth2\\Exceptions\\AccessTokenNotFoundException' => $baseDir . '/../lib/Exceptions/AccessTokenNotFoundException.php', 'OCA\\OAuth2\\Exceptions\\ClientNotFoundException' => $baseDir . '/../lib/Exceptions/ClientNotFoundException.php', + 'OCA\\OAuth2\\Migration\\SetTokenExpiration' => $baseDir . '/../lib/Migration/SetTokenExpiration.php', 'OCA\\OAuth2\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php', ); diff --git a/apps/oauth2/composer/composer/autoload_static.php b/apps/oauth2/composer/composer/autoload_static.php index bc196bad81aa9..2dab44c2e6e8e 100644 --- a/apps/oauth2/composer/composer/autoload_static.php +++ b/apps/oauth2/composer/composer/autoload_static.php @@ -27,6 +27,7 @@ class ComposerStaticInitOAuth2 'OCA\\OAuth2\\Db\\ClientMapper' => __DIR__ . '/..' . '/../lib/Db/ClientMapper.php', 'OCA\\OAuth2\\Exceptions\\AccessTokenNotFoundException' => __DIR__ . '/..' . '/../lib/Exceptions/AccessTokenNotFoundException.php', 'OCA\\OAuth2\\Exceptions\\ClientNotFoundException' => __DIR__ . '/..' . '/../lib/Exceptions/ClientNotFoundException.php', + 'OCA\\OAuth2\\Migration\\SetTokenExpiration' => __DIR__ . '/..' . '/../lib/Migration/SetTokenExpiration.php', 'OCA\\OAuth2\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php', ); diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index b97d85ae3e67c..b7de44f11f851 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -65,9 +65,11 @@ public function __construct($appName, * @NoCSRFRequired * * @param string $code + * @param string $client_id + * @param string $client_secret * @return JSONResponse */ - public function getToken($code) { + public function getToken($code, $client_id, $client_secret) { $accessToken = $this->accessTokenMapper->getByCode($code); $decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code); $newCode = $this->secureRandom->generate(128); diff --git a/apps/oauth2/lib/Migration/SetTokenExpiration.php b/apps/oauth2/lib/Migration/SetTokenExpiration.php new file mode 100644 index 0000000000000..54add100fa770 --- /dev/null +++ b/apps/oauth2/lib/Migration/SetTokenExpiration.php @@ -0,0 +1,77 @@ + + * + * @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 OCA\OAuth2\Migration; + +use OC\Authentication\Exceptions\InvalidTokenException; +use OC\Authentication\Token\IProvider as TokenProvider; +use OCA\OAuth2\Db\AccessToken; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class SetTokenExpiration implements IRepairStep { + + /** @var IDBConnection */ + private $connection; + + /** @var ITimeFactory */ + private $time; + + /** @var TokenProvider */ + private $tokenProvider; + + public function __construct(IDBConnection $connection, + ITimeFactory $timeFactory, + TokenProvider $tokenProvider) { + $this->connection = $connection; + $this->time = $timeFactory; + $this->tokenProvider = $tokenProvider; + } + + public function getName() { + return 'Update OAuth token expiration times'; + } + + public function run(IOutput $output) { + $qb = $this->connection->getQueryBuilder(); + $qb->select('*') + ->from('oauth2_access_tokens'); + + $cursor = $qb->execute(); + + while($row = $cursor->fetch()) { + $token = AccessToken::fromRow($row); + try { + $appToken = $this->tokenProvider->getTokenById($token->getTokenId()); + $appToken->setExpires($this->time->getTime() + 3600); + $this->tokenProvider->updateToken($appToken); + } catch (InvalidTokenException $e) { + //Skip this token + } + } + $cursor->closeCursor(); + } + +} From a04ea70fcaedc602fa3e8aeb77dadab5506f1786 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 11:24:48 +0200 Subject: [PATCH 4/8] Fail if the response type is not properly set Signed-off-by: Roeland Jago Douma --- .../lib/Controller/LoginRedirectorController.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/oauth2/lib/Controller/LoginRedirectorController.php b/apps/oauth2/lib/Controller/LoginRedirectorController.php index 9237b4b1b3c16..8e6d6d55e2dc3 100644 --- a/apps/oauth2/lib/Controller/LoginRedirectorController.php +++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php @@ -61,11 +61,20 @@ public function __construct($appName, * * @param string $client_id * @param string $state + * @param string $response_type * @return RedirectResponse */ public function authorize($client_id, - $state) { + $state, + $response_type) { $client = $this->clientMapper->getByIdentifier($client_id); + + if ($response_type !== 'code') { + //Fail + $url = $client->getRedirectUri() . '?error=unsupported_response_type&state=' . $state; + return new RedirectResponse($url); + } + $this->session->set('oauth.state', $state); $targetUrl = $this->urlGenerator->linkToRouteAbsolute( From 30750e4f9239b9e255e02c13d7497d7ff2cc8b86 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 11:50:37 +0200 Subject: [PATCH 5/8] Authenticate the clients on requesting a token Signed-off-by: Roeland Jago Douma --- .../lib/Controller/OauthApiController.php | 47 ++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index b7de44f11f851..9d0b883053301 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -23,7 +23,10 @@ use OC\Authentication\Token\DefaultTokenMapper; use OCA\OAuth2\Db\AccessTokenMapper; +use OCA\OAuth2\Db\ClientMapper; +use OCA\OAuth2\Exceptions\AccessTokenNotFoundException; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; use OCP\Security\ICrypto; @@ -32,6 +35,8 @@ class OauthApiController extends Controller { /** @var AccessTokenMapper */ private $accessTokenMapper; + /** @var ClientMapper */ + private $clientMapper; /** @var ICrypto */ private $crypto; /** @var DefaultTokenMapper */ @@ -44,6 +49,7 @@ class OauthApiController extends Controller { * @param IRequest $request * @param ICrypto $crypto * @param AccessTokenMapper $accessTokenMapper + * @param ClientMapper $clientMapper * @param DefaultTokenMapper $defaultTokenMapper * @param ISecureRandom $secureRandom */ @@ -51,11 +57,13 @@ public function __construct($appName, IRequest $request, ICrypto $crypto, AccessTokenMapper $accessTokenMapper, + ClientMapper $clientMapper, DefaultTokenMapper $defaultTokenMapper, ISecureRandom $secureRandom) { parent::__construct($appName, $request); $this->crypto = $crypto; $this->accessTokenMapper = $accessTokenMapper; + $this->clientMapper = $clientMapper; $this->defaultTokenMapper = $defaultTokenMapper; $this->secureRandom = $secureRandom; } @@ -64,13 +72,48 @@ public function __construct($appName, * @PublicPage * @NoCSRFRequired * + * @param string $grant_type * @param string $code + * @param string $refresh_token * @param string $client_id * @param string $client_secret * @return JSONResponse */ - public function getToken($code, $client_id, $client_secret) { - $accessToken = $this->accessTokenMapper->getByCode($code); + public function getToken($grant_type, $code, $refresh_token, $client_id, $client_secret) { + + if ($grant_type !== 'authorization_code' && $grant_type !== 'refresh_token') { + return new JSONResponse([ + 'error' => 'invalid_grant', + ], Http::STATUS_BAD_REQUEST); + } + + // We handle the initial and refresh tokens the same way + if ($grant_type === 'refresh_token' ) { + $code = $refresh_token; + } + + try { + $accessToken = $this->accessTokenMapper->getByCode($code); + } catch (AccessTokenNotFoundException $e) { + return new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + } + + try { + $client = $this->clientMapper->getByUid($accessToken->getClientId()); + } catch (ClientNotFoundException $e) { + return new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + } + + if ($client->getClientIdentifier() !== $client_id || $client->getSecret() !== $client_secret) { + return new JSONResponse([ + 'error' => 'invalid_client', + ], Http::STATUS_BAD_REQUEST); + } + $decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code); $newCode = $this->secureRandom->generate(128); $accessToken->setHashedCode(hash('sha512', $newCode)); From d03265fb62484536d00b90974f27b0e6282c2e6a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 15:09:03 +0200 Subject: [PATCH 6/8] Rotate token On a refresh token request: * rorate * reset expire Signed-off-by: Roeland Jago Douma --- .../lib/Controller/OauthApiController.php | 54 +++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 9d0b883053301..4d368801cca13 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -21,13 +21,17 @@ namespace OCA\OAuth2\Controller; -use OC\Authentication\Token\DefaultTokenMapper; +use OC\Authentication\Exceptions\InvalidTokenException; +use OC\Authentication\Token\ExpiredTokenException; +use OC\Authentication\Token\IProvider as TokenProvider; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\ClientMapper; use OCA\OAuth2\Exceptions\AccessTokenNotFoundException; +use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; @@ -39,10 +43,12 @@ class OauthApiController extends Controller { private $clientMapper; /** @var ICrypto */ private $crypto; - /** @var DefaultTokenMapper */ - private $defaultTokenMapper; + /** @var TokenProvider */ + private $tokenProvider; /** @var ISecureRandom */ private $secureRandom; + /** @var ITimeFactory */ + private $time; /** * @param string $appName @@ -50,22 +56,25 @@ class OauthApiController extends Controller { * @param ICrypto $crypto * @param AccessTokenMapper $accessTokenMapper * @param ClientMapper $clientMapper - * @param DefaultTokenMapper $defaultTokenMapper + * @param TokenProvider $tokenProvider * @param ISecureRandom $secureRandom + * @param ITimeFactory $time */ public function __construct($appName, IRequest $request, ICrypto $crypto, AccessTokenMapper $accessTokenMapper, ClientMapper $clientMapper, - DefaultTokenMapper $defaultTokenMapper, - ISecureRandom $secureRandom) { + TokenProvider $tokenProvider, + ISecureRandom $secureRandom, + ITimeFactory $time) { parent::__construct($appName, $request); $this->crypto = $crypto; $this->accessTokenMapper = $accessTokenMapper; $this->clientMapper = $clientMapper; - $this->defaultTokenMapper = $defaultTokenMapper; + $this->tokenProvider = $tokenProvider; $this->secureRandom = $secureRandom; + $this->time = $time; } /** @@ -115,18 +124,41 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client } $decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code); - $newCode = $this->secureRandom->generate(128); + + try { + $appToken = $this->tokenProvider->getTokenById($accessToken->getTokenId()); + } catch (ExpiredTokenException $e) { + $appToken = $e->getToken(); + } catch (InvalidTokenException $e) { + //We can't do anything... + $this->accessTokenMapper->delete($accessToken); + return new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + } + + $newToken = $this->secureRandom->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); + + $appToken = $this->tokenProvider->rotate( + $appToken, + $decryptedToken, + $newToken + ); + $appToken->setExpires($this->time->getTime() + 3600); + $this->tokenProvider->updateToken($appToken); + + $newCode = $this->secureRandom->generate(128, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); $accessToken->setHashedCode(hash('sha512', $newCode)); - $accessToken->setEncryptedToken($this->crypto->encrypt($decryptedToken, $newCode)); + $accessToken->setEncryptedToken($this->crypto->encrypt($newToken, $newCode)); $this->accessTokenMapper->update($accessToken); return new JSONResponse( [ - 'access_token' => $decryptedToken, + 'access_token' => $newToken, 'token_type' => 'Bearer', 'expires_in' => 3600, 'refresh_token' => $newCode, - 'user_id' => $this->defaultTokenMapper->getTokenById($accessToken->getTokenId())->getUID(), + 'user_id' => $appToken->getUID(), ] ); } From 73f8373151be49eb654ecc421ccb949e80e2f19a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 15:09:35 +0200 Subject: [PATCH 7/8] Don't use special chars to avoid confusion Signed-off-by: Roeland Jago Douma --- apps/oauth2/lib/Controller/OauthApiController.php | 7 +++++++ core/Controller/ClientFlowLoginController.php | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 4d368801cca13..8c96a3feee159 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -90,6 +90,7 @@ public function __construct($appName, */ public function getToken($grant_type, $code, $refresh_token, $client_id, $client_secret) { + // We only handle two types if ($grant_type !== 'authorization_code' && $grant_type !== 'refresh_token') { return new JSONResponse([ 'error' => 'invalid_grant', @@ -117,6 +118,7 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client ], Http::STATUS_BAD_REQUEST); } + // The client id and secret must match. Else we don't provide an access token! if ($client->getClientIdentifier() !== $client_id || $client->getSecret() !== $client_secret) { return new JSONResponse([ 'error' => 'invalid_client', @@ -125,6 +127,7 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client $decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code); + // Obtain the appToken assoicated try { $appToken = $this->tokenProvider->getTokenById($accessToken->getTokenId()); } catch (ExpiredTokenException $e) { @@ -137,6 +140,7 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client ], Http::STATUS_BAD_REQUEST); } + // Rotate the apptoken (so the old one becomes invalid basically) $newToken = $this->secureRandom->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); $appToken = $this->tokenProvider->rotate( @@ -144,9 +148,12 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client $decryptedToken, $newToken ); + + // Expiration is in 1 hour again $appToken->setExpires($this->time->getTime() + 3600); $this->tokenProvider->updateToken($appToken); + // Generate a new refresh token and encrypt the new apptoken in the DB $newCode = $this->secureRandom->generate(128, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); $accessToken->setHashedCode(hash('sha512', $newCode)); $accessToken->setEncryptedToken($this->crypto->encrypt($newToken, $newCode)); diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 0e7fbf892b60b..3bd396a0b9728 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -291,7 +291,7 @@ public function generateAppPassword($stateToken, ); if($client) { - $code = $this->random->generate(128); + $code = $this->random->generate(128, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); $accessToken = new AccessToken(); $accessToken->setClientId($client->getId()); $accessToken->setEncryptedToken($this->crypto->encrypt($token, $code)); From 3c002706a4d1e264518b1017f3a8d32576c9e9f8 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 20:40:46 +0200 Subject: [PATCH 8/8] Add tests Signed-off-by: Roeland Jago Douma --- .../LoginRedirectorControllerTest.php | 20 +- .../Controller/OauthApiControllerTest.php | 344 ++++++++++++++++-- 2 files changed, 324 insertions(+), 40 deletions(-) diff --git a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php index b33d3379be432..584e3ebed54b4 100644 --- a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php +++ b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php @@ -86,6 +86,24 @@ public function testAuthorize() { ->willReturn('https://example.com/?clientIdentifier=foo'); $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo'); - $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState')); + $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code')); + } + + public function testAuthorizeWrongResponseType() { + $client = new Client(); + $client->setClientIdentifier('MyClientIdentifier'); + $client->setRedirectUri('http://foo.bar'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientId') + ->willReturn($client); + $this->session + ->expects($this->never()) + ->method('set'); + + + $expected = new RedirectResponse('http://foo.bar?error=unsupported_response_type&state=MyState'); + $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'wrongcode')); } } diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index c90e2bf711f26..790dba0a598d0 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -21,12 +21,22 @@ namespace OCA\OAuth2\Tests\Controller; +use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\DefaultToken; use OC\Authentication\Token\DefaultTokenMapper; +use OC\Authentication\Token\ExpiredTokenException; +use OC\Authentication\Token\IProvider as TokenProvider; +use OC\Authentication\Token\IToken; use OCA\OAuth2\Controller\OauthApiController; use OCA\OAuth2\Db\AccessToken; use OCA\OAuth2\Db\AccessTokenMapper; +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; +use OCA\OAuth2\Exceptions\AccessTokenNotFoundException; +use OCA\OAuth2\Exceptions\ClientNotFoundException; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; @@ -39,10 +49,14 @@ class OauthApiControllerTest extends TestCase { private $crypto; /** @var AccessTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ private $accessTokenMapper; - /** @var DefaultTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ - private $defaultTokenMapper; + /** @var ClientMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $clientMapper; + /** @var TokenProvider|\PHPUnit_Framework_MockObject_MockObject */ + private $tokenProvider; /** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ private $secureRandom; + /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */ + private $time; /** @var OauthApiController */ private $oauthApiController; @@ -52,55 +66,307 @@ public function setUp() { $this->request = $this->createMock(IRequest::class); $this->crypto = $this->createMock(ICrypto::class); $this->accessTokenMapper = $this->createMock(AccessTokenMapper::class); - $this->defaultTokenMapper = $this->createMock(DefaultTokenMapper::class); + $this->clientMapper = $this->createMock(ClientMapper::class); + $this->tokenProvider = $this->createMock(TokenProvider::class); $this->secureRandom = $this->createMock(ISecureRandom::class); + $this->time = $this->createMock(ITimeFactory::class); $this->oauthApiController = new OauthApiController( 'oauth2', $this->request, $this->crypto, $this->accessTokenMapper, - $this->defaultTokenMapper, - $this->secureRandom + $this->clientMapper, + $this->tokenProvider, + $this->secureRandom, + $this->time ); } - public function testGetToken() { + public function testGetTokenInvalidGrantType() { + $expected = new JSONResponse([ + 'error' => 'invalid_grant', + ], Http::STATUS_BAD_REQUEST); + + $this->assertEquals($expected, $this->oauthApiController->getToken('foo', null, null, null, null)); + } + + public function testGetTokenInvalidCode() { + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + + $this->accessTokenMapper->method('getByCode') + ->with('invalidcode') + ->willThrowException(new AccessTokenNotFoundException()); + + $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'invalidcode', null, null, null)); + } + + public function testGetTokenInvalidRefreshToken() { + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + + $this->accessTokenMapper->method('getByCode') + ->with('invalidrefresh') + ->willThrowException(new AccessTokenNotFoundException()); + + $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'invalidrefresh', null, null)); + } + + public function testGetTokenClientDoesNotExist() { + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + $accessToken = new AccessToken(); - $accessToken->setEncryptedToken('MyEncryptedToken'); - $accessToken->setTokenId(123); - $this->accessTokenMapper - ->expects($this->once()) - ->method('getByCode') + $accessToken->setClientId(42); + + $this->accessTokenMapper->method('getByCode') + ->with('validrefresh') ->willReturn($accessToken); - $this->crypto - ->expects($this->once()) - ->method('decrypt') - ->with('MyEncryptedToken', 'MySecretCode') - ->willReturn('MyDecryptedToken'); - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with(128) - ->willReturn('NewToken'); - $token = new DefaultToken(); - $token->setUid('JohnDoe'); - $this->defaultTokenMapper - ->expects($this->once()) - ->method('getTokenById') - ->with(123) - ->willReturn($token); - - $expected = new JSONResponse( - [ - 'access_token' => 'MyDecryptedToken', - 'token_type' => 'Bearer', - 'expires_in' => 3600, - 'refresh_token' => 'NewToken', - 'user_id' => 'JohnDoe', - ] - ); - $this->assertEquals($expected, $this->oauthApiController->getToken('MySecretCode')); + + $this->clientMapper->method('getByUid') + ->with(42) + ->willThrowException(new ClientNotFoundException()); + + $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', null, null)); + } + + public function invalidClientProvider() { + return [ + ['invalidClientId', 'invalidClientSecret'], + ['clientId', 'invalidClientSecret'], + ['invalidClientId', 'clientSecret'], + ]; + } + + /** + * @dataProvider invalidClientProvider + * + * @param string $clientId + * @param string $clientSecret + */ + public function testGetTokenInvalidClient($clientId, $clientSecret) { + $expected = new JSONResponse([ + 'error' => 'invalid_client', + ], Http::STATUS_BAD_REQUEST); + + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + + $this->accessTokenMapper->method('getByCode') + ->with('validrefresh') + ->willReturn($accessToken); + + $client = new Client(); + $client->setClientIdentifier('clientId'); + $client->setSecret('clientSecret'); + $this->clientMapper->method('getByUid') + ->with(42) + ->willReturn($client); + + $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', $clientId, $clientSecret)); + } + + public function testGetTokenInvalidAppToken() { + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + $accessToken->setTokenId(1337); + $accessToken->setEncryptedToken('encryptedToken'); + + $this->accessTokenMapper->method('getByCode') + ->with('validrefresh') + ->willReturn($accessToken); + + $client = new Client(); + $client->setClientIdentifier('clientId'); + $client->setSecret('clientSecret'); + $this->clientMapper->method('getByUid') + ->with(42) + ->willReturn($client); + + $this->crypto->method('decrypt') + ->with( + 'encryptedToken', + 'validrefresh' + )->willReturn('decryptedToken'); + + $this->tokenProvider->method('getTokenById') + ->with(1337) + ->willThrowException(new InvalidTokenException()); + + $this->accessTokenMapper->expects($this->once()) + ->method('delete') + ->with($accessToken); + + $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); } + public function testGetTokenValidAppToken() { + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + $accessToken->setTokenId(1337); + $accessToken->setEncryptedToken('encryptedToken'); + + $this->accessTokenMapper->method('getByCode') + ->with('validrefresh') + ->willReturn($accessToken); + + $client = new Client(); + $client->setClientIdentifier('clientId'); + $client->setSecret('clientSecret'); + $this->clientMapper->method('getByUid') + ->with(42) + ->willReturn($client); + + $this->crypto->method('decrypt') + ->with( + 'encryptedToken', + 'validrefresh' + )->willReturn('decryptedToken'); + + $appToken = new DefaultToken(); + $appToken->setUid('userId'); + $this->tokenProvider->method('getTokenById') + ->with(1337) + ->willThrowException(new ExpiredTokenException($appToken)); + + $this->accessTokenMapper->expects($this->never()) + ->method('delete') + ->with($accessToken); + + $this->secureRandom->method('generate') + ->will($this->returnCallback(function ($len) { + return 'random'.$len; + })); + + $this->tokenProvider->expects($this->once()) + ->method('rotate') + ->with( + $appToken, + 'decryptedToken', + 'random72' + )->willReturn($appToken); + + $this->time->method('getTime') + ->willReturn(1000); + + $this->tokenProvider->expects($this->once()) + ->method('updateToken') + ->with( + $this->callback(function (DefaultToken $token) { + return $token->getExpires() === 4600; + }) + ); + + $this->crypto->method('encrypt') + ->with('random72', 'random128') + ->willReturn('newEncryptedToken'); + + $this->accessTokenMapper->expects($this->once()) + ->method('update') + ->with( + $this->callback(function (AccessToken $token) { + return $token->getHashedCode() === hash('sha512', 'random128') && + $token->getEncryptedToken() === 'newEncryptedToken'; + }) + ); + + $expected = new JSONResponse([ + 'access_token' => 'random72', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'refresh_token' => 'random128', + 'user_id' => 'userId', + ]); + + $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); + } + + public function testGetTokenExpiredAppToken() { + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + $accessToken->setTokenId(1337); + $accessToken->setEncryptedToken('encryptedToken'); + + $this->accessTokenMapper->method('getByCode') + ->with('validrefresh') + ->willReturn($accessToken); + + $client = new Client(); + $client->setClientIdentifier('clientId'); + $client->setSecret('clientSecret'); + $this->clientMapper->method('getByUid') + ->with(42) + ->willReturn($client); + + $this->crypto->method('decrypt') + ->with( + 'encryptedToken', + 'validrefresh' + )->willReturn('decryptedToken'); + + $appToken = new DefaultToken(); + $appToken->setUid('userId'); + $this->tokenProvider->method('getTokenById') + ->with(1337) + ->willReturn($appToken); + + $this->accessTokenMapper->expects($this->never()) + ->method('delete') + ->with($accessToken); + + $this->secureRandom->method('generate') + ->will($this->returnCallback(function ($len) { + return 'random'.$len; + })); + + $this->tokenProvider->expects($this->once()) + ->method('rotate') + ->with( + $appToken, + 'decryptedToken', + 'random72' + )->willReturn($appToken); + + $this->time->method('getTime') + ->willReturn(1000); + + $this->tokenProvider->expects($this->once()) + ->method('updateToken') + ->with( + $this->callback(function (DefaultToken $token) { + return $token->getExpires() === 4600; + }) + ); + + $this->crypto->method('encrypt') + ->with('random72', 'random128') + ->willReturn('newEncryptedToken'); + + $this->accessTokenMapper->expects($this->once()) + ->method('update') + ->with( + $this->callback(function (AccessToken $token) { + return $token->getHashedCode() === hash('sha512', 'random128') && + $token->getEncryptedToken() === 'newEncryptedToken'; + }) + ); + + $expected = new JSONResponse([ + 'access_token' => 'random72', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'refresh_token' => 'random128', + 'user_id' => 'userId', + ]); + + $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); + } }