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 e06803d0bfc..52dd6644d2e 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -29,10 +29,8 @@ use OCP\AppFramework\Db\Entity; * @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 @@ class DefaultToken extends Entity implements IToken { 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 36a8b1d5464..13407a688d3 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -266,6 +266,28 @@ class DefaultTokenProvider implements IProvider { $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 e1cc8182ff0..707645a09e9 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -138,4 +138,14 @@ interface IProvider { * @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 a24d31e2ed2..0e32e3adfd6 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -94,4 +94,18 @@ interface IToken extends JsonSerializable { * @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 08c74961c0d..e2643d315ce 100644 --- a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php @@ -418,4 +418,46 @@ class DefaultTokenProviderTest extends TestCase { $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 00000000000..62198d0bb37 --- /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 c6f6d2322bb..dc02dfb114f 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 e9de3538b77..8d7a4a7b57d 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 9988e2118f4..ba65c44f79d 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 00000000000..8abf01bae09 --- /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 52dd6644d2e..f41d0b8b7c4 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 41d1b9f203d..70a450602da 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 13407a688d3..4e87424e55c 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 707645a09e9..8b812a9533c 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 0e32e3adfd6..6586a5b2fd7 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 e2643d315ce..ccf654bcdfd 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 5c1b3eea2f2..60a4d48b650 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 ccddc9a8f71..689cb0f4377 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 97c8098caa3..c933a1bf21e 100644 --- a/apps/oauth2/composer/composer/autoload_classmap.php +++ b/apps/oauth2/composer/composer/autoload_classmap.php @@ -15,5 +15,6 @@ return array( '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 bc196bad81a..2dab44c2e6e 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 b97d85ae3e6..b7de44f11f8 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -65,9 +65,11 @@ class OauthApiController extends Controller { * @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 00000000000..54add100fa7 --- /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 9237b4b1b3c..8e6d6d55e2d 100644 --- a/apps/oauth2/lib/Controller/LoginRedirectorController.php +++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php @@ -61,11 +61,20 @@ class LoginRedirectorController extends Controller { * * @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 b7de44f11f8..9d0b8830533 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -23,7 +23,10 @@ namespace OCA\OAuth2\Controller; 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 @@ use OCP\Security\ISecureRandom; 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 @@ class OauthApiController extends Controller { 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 @@ class OauthApiController extends Controller { * @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 9d0b8830533..4d368801cca 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 @@ class OauthApiController extends Controller { } $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 4d368801cca..8c96a3feee1 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -90,6 +90,7 @@ class OauthApiController extends Controller { */ 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 @@ class OauthApiController extends Controller { ], 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 @@ class OauthApiController extends Controller { $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 @@ class OauthApiController extends Controller { ], 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 @@ class OauthApiController extends Controller { $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 0e7fbf892b6..3bd396a0b97 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -291,7 +291,7 @@ class ClientFlowLoginController extends Controller { ); 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 b33d3379be4..584e3ebed54 100644 --- a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php +++ b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php @@ -86,6 +86,24 @@ class LoginRedirectorControllerTest extends TestCase { ->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 c90e2bf711f..790dba0a598 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 @@ class OauthApiControllerTest extends TestCase { $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') - ->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); + $accessToken->setClientId(42); - $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->accessTokenMapper->method('getByCode') + ->with('validrefresh') + ->willReturn($accessToken); + + $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')); + } }