From f0f8bdd495ff958ce536e577e42586090b6bcd8f Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 2 May 2016 19:58:19 +0200 Subject: [PATCH] PHPDoc and other minor fixes --- core/Controller/TokenController.php | 6 ++-- .../Authentication/Token/DefaultToken.php | 14 ++++++++ .../Token/DefaultTokenProvider.php | 18 ++++++---- lib/private/Server.php | 6 ++-- lib/private/User/Session.php | 35 +++++++++++-------- tests/lib/user/session.php | 31 +++++++++------- 6 files changed, 74 insertions(+), 36 deletions(-) diff --git a/core/Controller/TokenController.php b/core/Controller/TokenController.php index db48ea27b9..6f98face14 100644 --- a/core/Controller/TokenController.php +++ b/core/Controller/TokenController.php @@ -65,15 +65,17 @@ class TokenController extends Controller { * * @param string $user * @param string $password + * @param string $name the name of the client + * @return Response */ public function generateToken($user, $password, $name = 'unknown client') { if (is_null($user) || is_null($password)) { - $response = new Response([]); + $response = new Response(); $response->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); return $response; } if ($this->userManager->checkPassword($user, $password) === false) { - $response = new Response([]); + $response = new Response(); $response->setStatus(Http::STATUS_UNAUTHORIZED); return $response; } diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 70562502b7..5dd9dc5b03 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -24,6 +24,20 @@ namespace OC\Authentication\Token; use OCP\AppFramework\Db\Entity; +/** + * @method void setId(int $id) + * @method void setUid(string $uid); + * @method void setPassword(string $password) + * @method string getPassword() + * @method void setName(string $name) + * @method string getName() + * @method void setToken(string $token) + * @method string getToken() + * @method void setType(string $type) + * @method int getType() + * @method void setLastActivity(int $lastActivity) + * @method int getLastActivity() + */ class DefaultToken extends Entity implements IToken { /** diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index 97567e53cd..a0d07f9e2e 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -24,6 +24,7 @@ namespace OC\Authentication\Token; use OC\Authentication\Exceptions\InvalidTokenException; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; use OCP\ILogger; use OCP\Security\ICrypto; @@ -42,17 +43,21 @@ class DefaultTokenProvider implements IProvider { /** @var ILogger $logger */ private $logger; + /** @var ITimeFactory $time */ + private $time; + /** * @param DefaultTokenMapper $mapper * @param ICrypto $crypto * @param IConfig $config * @param ILogger $logger */ - public function __construct(DefaultTokenMapper $mapper, ICrypto $crypto, IConfig $config, ILogger $logger) { + public function __construct(DefaultTokenMapper $mapper, ICrypto $crypto, IConfig $config, ILogger $logger, ITimeFactory $time) { $this->mapper = $mapper; $this->crypto = $crypto; $this->config = $config; $this->logger = $logger; + $this->time = $time; } /** @@ -61,7 +66,7 @@ class DefaultTokenProvider implements IProvider { * @param string $token * @param string $uid * @param string $password - * @apram int $type token type + * @param int $type token type * @return DefaultToken */ public function generateToken($token, $uid, $password, $name, $type = IToken::TEMPORARY_TOKEN) { @@ -71,7 +76,7 @@ class DefaultTokenProvider implements IProvider { $dbToken->setName($name); $dbToken->setToken($this->hashToken($token)); $dbToken->setType($type); - $dbToken->setLastActivity(time()); + $dbToken->setLastActivity($this->time->getTime()); $this->mapper->insert($dbToken); @@ -88,7 +93,7 @@ class DefaultTokenProvider implements IProvider { throw new InvalidTokenException(); } /** @var DefaultToken $token */ - $token->setLastActivity(time()); + $token->setLastActivity($this->time->getTime()); $this->mapper->update($token); } @@ -126,7 +131,7 @@ class DefaultTokenProvider implements IProvider { * Invalidate (delete) old session tokens */ public function invalidateOldTokens() { - $olderThan = time() - (int) $this->config->getSystemValue('session_lifetime', 60 * 60 * 24); + $olderThan = $this->time->getTime() - (int) $this->config->getSystemValue('session_lifetime', 60 * 60 * 24); $this->logger->info('Invalidating tokens older than ' . date('c', $olderThan)); $this->mapper->invalidateOld($olderThan); } @@ -153,7 +158,8 @@ class DefaultTokenProvider implements IProvider { * @return string */ private function hashToken($token) { - return hash('sha512', $token); + $secret = $this->config->getSystemValue('secret'); + return hash('sha512', $token . $secret); } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index b17c1a384d..cbab1f09eb 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -218,11 +218,13 @@ class Server extends ServerContainer implements IServerContainer { $crypto = $c->getCrypto(); $config = $c->getConfig(); $logger = $c->getLogger(); - return new \OC\Authentication\Token\DefaultTokenProvider($mapper, $crypto, $config, $logger); + $timeFactory = new TimeFactory(); + return new \OC\Authentication\Token\DefaultTokenProvider($mapper, $crypto, $config, $logger, $timeFactory); }); $this->registerService('UserSession', function (Server $c) { $manager = $c->getUserManager(); $session = new \OC\Session\Memory(''); + $timeFactory = new TimeFactory(); // Token providers might require a working database. This code // might however be called when ownCloud is not yet setup. if (\OC::$server->getSystemConfig()->getValue('installed', false)) { @@ -235,7 +237,7 @@ class Server extends ServerContainer implements IServerContainer { $tokenProviders = []; } - $userSession = new \OC\User\Session($manager, $session, $defaultTokenProvider, $tokenProviders); + $userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $tokenProviders); $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 72fb8bc41d..0f191d52d4 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -40,7 +40,9 @@ use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Hooks\Emitter; use OC_User; +use OC_Util; use OCA\DAV\Connector\Sabre\Auth; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; use OCP\ISession; use OCP\IUser; @@ -78,6 +80,11 @@ class Session implements IUserSession, Emitter { private $session; /* + * @var ITimeFactory + */ + private $timeFacory; + + /** * @var DefaultTokenProvider */ private $tokenProvider; @@ -96,9 +103,11 @@ class Session implements IUserSession, Emitter { * @param ISession $session * @param IProvider[] $tokenProviders */ - public function __construct(IUserManager $manager, ISession $session, $tokenProvider, array $tokenProviders = []) { + public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider, + array $tokenProviders = []) { $this->manager = $manager; $this->session = $session; + $this->timeFacory = $timeFacory; $this->tokenProvider = $tokenProvider; $this->tokenProviders = $tokenProviders; } @@ -199,8 +208,7 @@ class Session implements IUserSession, Emitter { } protected function validateSession(IUser $user) { - // TODO: use ISession::getId(), https://github.com/owncloud/core/pull/24229 - $sessionId = session_id(); + $sessionId = $this->session->getId(); try { $token = $this->tokenProvider->getToken($sessionId); } catch (InvalidTokenException $ex) { @@ -212,14 +220,15 @@ class Session implements IUserSession, Emitter { // Check whether login credentials are still valid // This check is performed each 5 minutes $lastCheck = $this->session->get('last_login_check') ? : 0; - if ($lastCheck < (time() - 60 * 5)) { + $now = $this->timeFacory->getTime(); + if ($lastCheck < ($now - 60 * 5)) { $pwd = $this->tokenProvider->getPassword($token, $sessionId); if ($this->manager->checkPassword($user->getUID(), $pwd) === false) { // Password has changed -> log user out $this->logout(); return false; } - $this->session->set('last_login_check', time()); + $this->session->set('last_login_check', $now); } // Session is valid, so the token can be refreshed @@ -323,7 +332,7 @@ class Session implements IUserSession, Emitter { \OC::$server->getCsrfTokenManager()->refreshToken(); //we need to pass the user name, which may differ from login name $user = $this->getUser()->getUID(); - \OC_Util::setupFS($user); + OC_Util::setupFS($user); //trigger creation of user home and /files folder \OC::$server->getUserFolder($user); } @@ -383,10 +392,9 @@ class Session implements IUserSession, Emitter { return false; } $name = isset($request->server['HTTP_USER_AGENT']) ? $request->server['HTTP_USER_AGENT'] : 'unknown browser'; - // TODO: use ISession::getId(), https://github.com/owncloud/core/pull/24229 $loggedIn = $this->login($uid, $password); if ($loggedIn) { - $sessionId = session_id(); + $sessionId = $this->session->getId(); $this->tokenProvider->generateToken($sessionId, $uid, $password, $name); } return $loggedIn; @@ -423,9 +431,10 @@ class Session implements IUserSession, Emitter { private function updateToken(IProvider $provider, IToken $token) { // To save unnecessary DB queries, this is only done once a minute $lastTokenUpdate = $this->session->get('last_token_update') ? : 0; - if ($lastTokenUpdate < (time() - 60)) { + $now = $this->timeFacory->getTime(); + if ($lastTokenUpdate < ($now - 60)) { $provider->updateToken($token); - $this->session->set('last_token_update', time()); + $this->session->set('last_token_update', $now); } } @@ -438,8 +447,7 @@ class Session implements IUserSession, Emitter { $authHeader = $request->getHeader('Authorization'); if (strpos($authHeader, 'token ') === false) { // No auth header, let's try session id - // TODO: use ISession::getId(), https://github.com/owncloud/core/pull/24229 - $sessionId = session_id(); + $sessionId = $this->session->getId(); return $this->validateToken($request, $sessionId); } else { $token = substr($authHeader, 6); @@ -488,8 +496,7 @@ class Session implements IUserSession, Emitter { $this->manager->emit('\OC\User', 'logout'); $user = $this->getUser(); if (!is_null($user)) { - // TODO: use ISession::getId(), https://github.com/owncloud/core/pull/24229 - $this->tokenProvider->invalidateToken(session_id()); + $this->tokenProvider->invalidateToken($this->session->getId()); } $this->setUser(null); $this->setLoginName(null); diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php index ab252357d8..bbbe5e0c79 100644 --- a/tests/lib/user/session.php +++ b/tests/lib/user/session.php @@ -18,12 +18,19 @@ use OC\User\User; */ class Session extends \Test\TestCase { + /** @var \OCP\AppFramework\Utility\ITimeFactory */ + private $timeFactory; + /** @var \OC\Authentication\Token\DefaultTokenProvider */ protected $defaultProvider; protected function setUp() { parent::setUp(); + $this->timeFactory = $this->getMock('\OCP\AppFramework\Utility\ITimeFactory'); + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->will($this->returnValue(10000)); $this->defaultProvider = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider') ->disableOriginalConstructor() ->getMock(); @@ -59,7 +66,7 @@ class Session extends \Test\TestCase { ->will($this->returnValue(true)); $session->expects($this->at(2)) ->method('set') - ->with('last_login_check', $this->equalTo(time(), 10)); + ->with('last_login_check', 10000); $session->expects($this->at(3)) ->method('get') @@ -77,7 +84,7 @@ class Session extends \Test\TestCase { ->with($expectedUser->getUID()) ->will($this->returnValue($expectedUser)); - $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $user = $userSession->getUser(); $this->assertSame($expectedUser, $user); } @@ -100,7 +107,7 @@ class Session extends \Test\TestCase { ->getMock(); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->defaultProvider, [$this->defaultProvider]]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]]) ->setMethods([ 'getUser' ]) @@ -127,7 +134,7 @@ class Session extends \Test\TestCase { ->method('getUID') ->will($this->returnValue('foo')); - $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $userSession->setUser($user); } @@ -179,7 +186,7 @@ class Session extends \Test\TestCase { ->will($this->returnValue($user)); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->defaultProvider, [$this->defaultProvider]]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]]) ->setMethods([ 'prepareUserLogin' ]) @@ -223,7 +230,7 @@ class Session extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue($user)); - $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $userSession->login('foo', 'bar'); } @@ -259,7 +266,7 @@ class Session extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $userSession->login('foo', 'bar'); } @@ -279,7 +286,7 @@ class Session extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $userSession->login('foo', 'bar'); } @@ -334,7 +341,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->defaultProvider, [$this->defaultProvider])); + array($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider])); $granted = $userSession->loginWithCookie('foo', $token); @@ -379,7 +386,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->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $granted = $userSession->loginWithCookie('foo', 'badToken'); $this->assertSame($granted, false); @@ -422,7 +429,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->defaultProvider, [$this->defaultProvider]); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]); $granted = $userSession->loginWithCookie('foo', $token); $this->assertSame($granted, false); @@ -447,7 +454,7 @@ class Session extends \Test\TestCase { $session = new Memory(''); $session->set('user_id', 'foo'); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->defaultProvider, [$this->defaultProvider]]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, [$this->defaultProvider]]) ->setMethods([ 'validateSession' ])