From 0885bd4ee543d8d9e4f3e561b44fcc90376dd45a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 15 May 2018 21:10:43 +0200 Subject: [PATCH 1/9] 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 127430ea6c..024a6097fa 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -28,10 +28,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() @@ -173,4 +171,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 f099aca9d9..4f435e80e7 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -261,6 +261,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 9f280263d7..5951cabad7 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -135,4 +135,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 49745b266c..16d97e6556 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -93,4 +93,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 96fdbaa176..eb69005655 100644 --- a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php @@ -417,4 +417,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 f2a31151576d00a6452bdfa8646adf6ba892b526 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 12:39:00 +0200 Subject: [PATCH 2/9] 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 --- db_structure.xml | 9 +++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../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 +- 11 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 lib/private/Authentication/Exceptions/ExpiredTokenException.php diff --git a/db_structure.xml b/db_structure.xml index 583e9bb852..72d7ebc647 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1181,6 +1181,15 @@ false + + expires + integer + + false + true + 4 + + authtoken_token_index true diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 93e792a7f4..39e96f3341 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -363,6 +363,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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 88bb9c65ff..1242f67deb 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -393,6 +393,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', diff --git a/lib/private/Authentication/Exceptions/ExpiredTokenException.php b/lib/private/Authentication/Exceptions/ExpiredTokenException.php new file mode 100644 index 0000000000..8abf01bae0 --- /dev/null +++ b/lib/private/Authentication/Exceptions/ExpiredTokenException.php @@ -0,0 +1,40 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OC\Authentication\Token; + +use OC\Authentication\Exceptions\InvalidTokenException; + +class ExpiredTokenException extends InvalidTokenException { + /** @var IToken */ + private $token; + + public function __construct(IToken $token) { + parent::__construct(); + + $this->token = $token; + } + + public function getToken() { + return $this->token; + } +} diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 024a6097fa..583423166d 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -90,10 +90,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() { @@ -179,4 +184,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 00e367801e..763fd118f4 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -72,7 +72,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(); @@ -96,7 +96,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(); @@ -121,7 +121,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 4f435e80e7..eee4db20af 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -150,13 +150,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; } /** @@ -165,13 +172,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 5951cabad7..22a0994755 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -48,6 +48,7 @@ interface IProvider { * * @param string $tokenId * @throws InvalidTokenException + * @throws ExpiredTokenException * @return IToken */ public function getToken($tokenId); @@ -58,6 +59,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 16d97e6556..0abdf643c1 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -107,4 +107,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 eb69005655..31b2d8a620 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; @@ -396,6 +397,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); @@ -418,6 +476,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 e59ffed5f2..ed1abb314c 100644 --- a/version.php +++ b/version.php @@ -26,7 +26,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(12, 0, 7, 1); +$OC_Version = array(12, 0, 7, 2); // The human readable string $OC_VersionString = '12.0.7'; From 1525cc286c8f13acf6ae956c4f43a680f08aab15 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 10:35:18 +0200 Subject: [PATCH 3/9] Set OAuth token expiration Signed-off-by: Roeland Jago Douma --- apps/oauth2/appinfo/info.xml | 8 +- .../lib/Controller/OauthApiController.php | 4 +- .../lib/Migration/SetTokenExpiration.php | 77 +++++++++++++++++++ 3 files changed, 87 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 9df251d8f1..6b00713f54 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -6,7 +6,7 @@ agpl Lukas Reschke OAuth2 - 1.0.5 + 1.0.6 @@ -16,6 +16,12 @@ + + + OCA\OAuth2\Migration\SetTokenExpiration + + + OCA\OAuth2\Settings\Admin diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index b97d85ae3e..b7de44f11f 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 0000000000..54add100fa --- /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 aa78c30666de8ec1b5471022fd6d428f788aad29 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 11:24:48 +0200 Subject: [PATCH 4/9] 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 9237b4b1b3..8e6d6d55e2 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 c2a21ea464951cba23ec9bf4f7a81cedae4948ed Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 11:50:37 +0200 Subject: [PATCH 5/9] 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 b7de44f11f..9d0b883053 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 335f4efa830dc060f997a23a6220e9803300f19e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 15:09:03 +0200 Subject: [PATCH 6/9] 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 9d0b883053..4d368801cc 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 44735dedd13f924c82f2d329554dd4eca530fe38 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 15:09:35 +0200 Subject: [PATCH 7/9] 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 4d368801cc..8c96a3feee 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 742bc972a3..2584d252e1 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -286,7 +286,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 c2f09e43959f31b3c9548c0bac5dcf68652b83b6 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 May 2018 20:40:46 +0200 Subject: [PATCH 8/9] 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 b33d3379be..584e3ebed5 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 c90e2bf711..790dba0a59 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')); + } } From 3e57666093f9a45ee2c13e045f39cc30ae01166d Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 23 May 2018 17:01:54 +0200 Subject: [PATCH 9/9] Properly set expires to NULL when creating a token Signed-off-by: Roeland Jago Douma --- lib/private/Authentication/Token/DefaultToken.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 583423166d..4869661a8f 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -99,6 +99,9 @@ class DefaultToken extends Entity implements IToken { $this->addType('lastCheck', 'int'); $this->addType('scope', 'string'); $this->addType('expires', 'int'); + + $this->setExpires(null); + $this->markFieldUpdated('expires'); } public function getId() {