From 98b465a8b9c6900f12ca2efa5d51036b6ccc4b8b Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 17 May 2016 17:20:54 +0200 Subject: [PATCH] a single token provider suffices --- .../Authentication/Token/DefaultToken.php | 9 +++ .../Token/DefaultTokenProvider.php | 16 +++-- .../Authentication/Token/IProvider.php | 37 +++++++++++ lib/private/Authentication/Token/IToken.php | 10 ++- lib/private/Server.php | 6 +- lib/private/User/Session.php | 65 +++++++------------ tests/lib/user/session.php | 22 +++---- 7 files changed, 96 insertions(+), 69 deletions(-) diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 25caf675a4..08451a4615 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -77,5 +77,14 @@ class DefaultToken extends Entity implements IToken { public function getUID() { return $this->uid; } + + /** + * Get the (encrypted) login password + * + * @return string + */ + public function getPassword() { + return parent::getPassword(); + } } diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index deca5b409e..a335b79e33 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -103,25 +103,27 @@ class DefaultTokenProvider implements IProvider { } /** - * @param string $token + * Get a token by token id + * + * @param string $tokenId * @throws InvalidTokenException * @return DefaultToken */ - public function getToken($token) { + public function getToken($tokenId) { try { - return $this->mapper->getToken($this->hashToken($token)); + return $this->mapper->getToken($this->hashToken($tokenId)); } catch (DoesNotExistException $ex) { throw new InvalidTokenException(); } } /** - * @param DefaultToken $savedToken - * @param string $token session token + * @param IToken $savedToken + * @param string $tokenId session token * @return string */ - public function getPassword(DefaultToken $savedToken, $token) { - return $this->decryptPassword($savedToken->getPassword(), $token); + public function getPassword(IToken $savedToken, $tokenId) { + return $this->decryptPassword($savedToken->getPassword(), $tokenId); } /** diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php index f8a3262ca8..1fd3a70fbb 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -26,6 +26,27 @@ use OC\Authentication\Exceptions\InvalidTokenException; interface IProvider { + /** + * Create and persist a new token + * + * @param string $token + * @param string $uid + * @param string $password + * @param string $name + * @param int $type token type + * @return DefaultToken + */ + public function generateToken($token, $uid, $password, $name, $type = IToken::TEMPORARY_TOKEN); + + /** + * Get a token by token id + * + * @param string $tokenId + * @throws InvalidTokenException + * @return IToken + */ + public function getToken($tokenId) ; + /** * @param string $token * @throws InvalidTokenException @@ -33,10 +54,26 @@ interface IProvider { */ public function validateToken($token); + /** + * Invalidate (delete) the given session token + * + * @param string $token + */ + public function invalidateToken($token); + /** * Update token activity timestamp * * @param IToken $token */ public function updateToken(IToken $token); + + /** + * Get the (unencrypted) password of the given token + * + * @param IToken $token + * @param string $tokenId + * @return string + */ + public function getPassword(IToken $token, $tokenId); } diff --git a/lib/private/Authentication/Token/IToken.php b/lib/private/Authentication/Token/IToken.php index 9b2bd18f83..2a01ea75ea 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -22,9 +22,6 @@ namespace OC\Authentication\Token; -/** - * @since 9.1.0 - */ interface IToken { const TEMPORARY_TOKEN = 0; @@ -43,4 +40,11 @@ interface IToken { * @return string */ public function getUID(); + + /** + * Get the (encrypted) login password + * + * @return string + */ + public function getPassword(); } diff --git a/lib/private/Server.php b/lib/private/Server.php index 8ece9addd3..a4294ee2c8 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -231,15 +231,11 @@ class Server extends ServerContainer implements IServerContainer { // might however be called when ownCloud is not yet setup. if (\OC::$server->getSystemConfig()->getValue('installed', false)) { $defaultTokenProvider = $c->query('OC\Authentication\Token\DefaultTokenProvider'); - $tokenProviders = [ - $defaultTokenProvider, - ]; } else { $defaultTokenProvider = null; - $tokenProviders = []; } - $userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $tokenProviders); + $userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider); $userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) { \OC_Hook::emit('OC_User', 'pre_createUser', array('run' => true, 'uid' => $uid, 'password' => $password)); }); diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index c9f42d7e41..3f074fa8ad 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -35,7 +35,6 @@ namespace OC\User; use OC; use OC\Authentication\Exceptions\InvalidTokenException; -use OC\Authentication\Token\DefaultTokenProvider; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Hooks\Emitter; @@ -69,35 +68,20 @@ use OCP\Session\Exceptions\SessionNotAvailableException; * @package OC\User */ class Session implements IUserSession, Emitter { - /* - * @var Manager $manager - */ - + + /** @var Manager $manager */ private $manager; - /* - * @var ISession $session - */ + /** @var ISession $session */ private $session; - /* - * @var ITimeFactory - */ + /** @var ITimeFactory */ private $timeFacory; - /** - * @var DefaultTokenProvider - */ + /** @var IProvider */ private $tokenProvider; - /** - * @var IProvider[] - */ - private $tokenProviders; - - /** - * @var User $activeUser - */ + /** @var User $activeUser */ protected $activeUser; /** @@ -105,20 +89,18 @@ class Session implements IUserSession, Emitter { * @param ISession $session * @param ITimeFactory $timeFacory * @param IProvider $tokenProvider - * @param IProvider[] $tokenProviders */ - public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider, array $tokenProviders = []) { + public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider) { $this->manager = $manager; $this->session = $session; $this->timeFacory = $timeFacory; $this->tokenProvider = $tokenProvider; - $this->tokenProviders = $tokenProviders; } /** - * @param DefaultTokenProvider $provider + * @param IProvider $provider */ - public function setTokenProvider(DefaultTokenProvider $provider) { + public function setTokenProvider(IProvider $provider) { $this->tokenProvider = $provider; } @@ -246,7 +228,7 @@ class Session implements IUserSession, Emitter { } // Session is valid, so the token can be refreshed - $this->updateToken($this->tokenProvider, $token); + $this->updateToken($token); } /** @@ -418,34 +400,31 @@ class Session implements IUserSession, Emitter { * @return boolean */ private function validateToken($token) { - foreach ($this->tokenProviders as $provider) { - try { - $token = $provider->validateToken($token); - if (!is_null($token)) { - $result = $this->loginWithToken($token->getUID()); - if ($result) { - // Login success - $this->updateToken($provider, $token); - return true; - } + try { + $token = $this->tokenProvider->validateToken($token); + if (!is_null($token)) { + $result = $this->loginWithToken($token->getUID()); + if ($result) { + // Login success + $this->updateToken($token); + return true; } - } catch (InvalidTokenException $ex) { - } + } catch (InvalidTokenException $ex) { + } return false; } /** - * @param IProvider $provider * @param IToken $token */ - private function updateToken(IProvider $provider, IToken $token) { + private function updateToken(IToken $token) { // To save unnecessary DB queries, this is only done once a minute $lastTokenUpdate = $this->session->get('last_token_update') ? : 0; $now = $this->timeFacory->getTime(); if ($lastTokenUpdate < ($now - 60)) { - $provider->updateToken($token); + $this->tokenProvider->updateToken($token); $this->session->set('last_token_update', $now); } } diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php index c6ddeb416f..710d5ae20b 100644 --- a/tests/lib/user/session.php +++ b/tests/lib/user/session.php @@ -88,7 +88,7 @@ class Session extends \Test\TestCase { ->with($expectedUser->getUID()) ->will($this->returnValue($expectedUser)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $user = $userSession->getUser(); $this->assertSame($expectedUser, $user); } @@ -111,7 +111,7 @@ class Session extends \Test\TestCase { ->getMock(); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider]) ->setMethods([ 'getUser' ]) @@ -138,7 +138,7 @@ class Session extends \Test\TestCase { ->method('getUID') ->will($this->returnValue('foo')); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $userSession->setUser($user); } @@ -190,7 +190,7 @@ class Session extends \Test\TestCase { ->will($this->returnValue($user)); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider]) ->setMethods([ 'prepareUserLogin' ]) @@ -237,7 +237,7 @@ class Session extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue($user)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $userSession->login('foo', 'bar'); } @@ -273,7 +273,7 @@ class Session extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $userSession->login('foo', 'bar'); } @@ -293,7 +293,7 @@ class Session extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $userSession->login('foo', 'bar'); } @@ -348,7 +348,7 @@ class Session extends \Test\TestCase { //override, otherwise tests will fail because of setcookie() array('setMagicInCookie'), //there are passed as parameters to the constructor - array($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider])); + array($manager, $session, $this->timeFactory, $this->defaultProvider)); $granted = $userSession->loginWithCookie('foo', $token); @@ -393,7 +393,7 @@ class Session extends \Test\TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $granted = $userSession->loginWithCookie('foo', 'badToken'); $this->assertSame($granted, false); @@ -436,7 +436,7 @@ class Session extends \Test\TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider); $granted = $userSession->loginWithCookie('foo', $token); $this->assertSame($granted, false); @@ -461,7 +461,7 @@ class Session extends \Test\TestCase { $session = new Memory(''); $session->set('user_id', 'foo'); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider]) ->setMethods([ 'validateSession' ])