From 46bdf6ea2b1e10c2f4d2fae214ecc81b188fa981 Mon Sep 17 00:00:00 2001
From: Christoph Wurst <christoph@owncloud.com>
Date: Fri, 6 May 2016 16:31:40 +0200
Subject: [PATCH] fix PHPDoc and other minor issues
---
build/integration/features/bootstrap/Auth.php | 4 +-
core/Controller/LoginController.php | 4 +-
core/Controller/TokenController.php | 3 +-
.../Authentication/Token/DefaultToken.php | 2 +-
.../Token/DefaultTokenProvider.php | 11 +++--
.../Authentication/Token/IProvider.php | 2 +-
lib/private/Authentication/Token/IToken.php | 2 +-
lib/private/Files/Filesystem.php | 2 +-
lib/private/Updater.php | 1 +
lib/private/User/Session.php | 45 +++++++++----------
lib/private/legacy/api.php | 2 +-
.../token/defaulttokenmappertest.php | 4 +-
.../token/defaulttokenprovidertest.php | 36 +++++++--------
tests/lib/user/session.php | 27 ++++++-----
14 files changed, 77 insertions(+), 68 deletions(-)
diff --git a/build/integration/features/bootstrap/Auth.php b/build/integration/features/bootstrap/Auth.php
index 88edcd49a5b..5f36b199e06 100644
--- a/build/integration/features/bootstrap/Auth.php
+++ b/build/integration/features/bootstrap/Auth.php
@@ -61,14 +61,14 @@ trait Auth {
* @When requesting :url with :method using basic auth
*/
public function requestingWithBasicAuth($url, $method) {
- $this->sendRequest($url, $method, 'basic ' . base64_encode('user:user'));
+ $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:123456'));
}
/**
* @When requesting :url with :method using basic token auth
*/
public function requestingWithBasicTokenAuth($url, $method) {
- $this->sendRequest($url, $method, 'basic ' . base64_encode('user:' . $this->clientToken));
+ $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:' . $this->clientToken));
}
/**
diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php
index 977f523afda..df5ece9671b 100644
--- a/core/Controller/LoginController.php
+++ b/core/Controller/LoginController.php
@@ -98,7 +98,7 @@ class LoginController extends Controller {
* @param string $redirect_url
* @param string $remember_login
*
- * @return TemplateResponse
+ * @return TemplateResponse|RedirectResponse
*/
public function showLoginForm($user, $redirect_url, $remember_login) {
if ($this->userSession->isLoggedIn()) {
@@ -106,7 +106,6 @@ class LoginController extends Controller {
}
$parameters = array();
- $id = $this->session->getId();
$loginMessages = $this->session->get('loginMessages');
$errors = [];
$messages = [];
@@ -177,7 +176,6 @@ class LoginController extends Controller {
}
}
if (!$loginResult) {
- $id = $this->session->getId();
$this->session->set('loginMessages', [
[],
['invalidpassword']
diff --git a/core/Controller/TokenController.php b/core/Controller/TokenController.php
index 6f98face144..6606a3c8345 100644
--- a/core/Controller/TokenController.php
+++ b/core/Controller/TokenController.php
@@ -27,6 +27,7 @@ use OC\Authentication\Token\DefaultTokenProvider;
use OC\Authentication\Token\IToken;
use OC\User\Manager;
use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\Response;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
@@ -66,7 +67,7 @@ class TokenController extends Controller {
* @param string $user
* @param string $password
* @param string $name the name of the client
- * @return Response
+ * @return JSONResponse
*/
public function generateToken($user, $password, $name = 'unknown client') {
if (is_null($user) || is_null($password)) {
diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php
index 5dd9dc5b039..25caf675a43 100644
--- a/lib/private/Authentication/Token/DefaultToken.php
+++ b/lib/private/Authentication/Token/DefaultToken.php
@@ -74,7 +74,7 @@ class DefaultToken extends Entity implements IToken {
return $this->id;
}
- public function getUid() {
+ public function getUID() {
return $this->uid;
}
diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php
index a0d07f9e2e2..53ecb562a8d 100644
--- a/lib/private/Authentication/Token/DefaultTokenProvider.php
+++ b/lib/private/Authentication/Token/DefaultTokenProvider.php
@@ -51,6 +51,7 @@ class DefaultTokenProvider implements IProvider {
* @param ICrypto $crypto
* @param IConfig $config
* @param ILogger $logger
+ * @param ITimeFactory $time
*/
public function __construct(DefaultTokenMapper $mapper, ICrypto $crypto, IConfig $config, ILogger $logger, ITimeFactory $time) {
$this->mapper = $mapper;
@@ -66,6 +67,7 @@ class DefaultTokenProvider implements IProvider {
* @param string $token
* @param string $uid
* @param string $password
+ * @param string $name
* @param int $type token type
* @return DefaultToken
*/
@@ -86,7 +88,8 @@ class DefaultTokenProvider implements IProvider {
/**
* Update token activity timestamp
*
- * @param DefaultToken $token
+ * @throws InvalidTokenException
+ * @param IToken $token
*/
public function updateToken(IToken $token) {
if (!($token instanceof DefaultToken)) {
@@ -101,6 +104,7 @@ class DefaultTokenProvider implements IProvider {
/**
* @param string $token
* @throws InvalidTokenException
+ * @return DefaultToken
*/
public function getToken($token) {
try {
@@ -113,6 +117,7 @@ class DefaultTokenProvider implements IProvider {
/**
* @param DefaultToken $savedToken
* @param string $token session token
+ * @return string
*/
public function getPassword(DefaultToken $savedToken, $token) {
return $this->decryptPassword($savedToken->getPassword(), $token);
@@ -139,13 +144,13 @@ class DefaultTokenProvider implements IProvider {
/**
* @param string $token
* @throws InvalidTokenException
- * @return IToken user UID
+ * @return DefaultToken user UID
*/
public function validateToken($token) {
$this->logger->debug('validating default token <' . $token . '>');
try {
$dbToken = $this->mapper->getToken($this->hashToken($token));
- $this->logger->debug('valid token for ' . $dbToken->getUid());
+ $this->logger->debug('valid token for ' . $dbToken->getUID());
return $dbToken;
} catch (DoesNotExistException $ex) {
$this->logger->warning('invalid token');
diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php
index 5c0b0d140ae..f8a3262ca8b 100644
--- a/lib/private/Authentication/Token/IProvider.php
+++ b/lib/private/Authentication/Token/IProvider.php
@@ -36,7 +36,7 @@ interface IProvider {
/**
* Update token activity timestamp
*
- * @param DefaultToken $token
+ * @param IToken $token
*/
public function updateToken(IToken $token);
}
diff --git a/lib/private/Authentication/Token/IToken.php b/lib/private/Authentication/Token/IToken.php
index 90feefb4589..9b2bd18f83b 100644
--- a/lib/private/Authentication/Token/IToken.php
+++ b/lib/private/Authentication/Token/IToken.php
@@ -42,5 +42,5 @@ interface IToken {
*
* @return string
*/
- public function getUid();
+ public function getUID();
}
diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php
index 89b8547aa52..99c123ad1a1 100644
--- a/lib/private/Files/Filesystem.php
+++ b/lib/private/Files/Filesystem.php
@@ -404,7 +404,7 @@ class Filesystem {
if (is_null($userObject)) {
\OCP\Util::writeLog('files', ' Backends provided no user object for ' . $user, \OCP\Util::ERROR);
- throw new \OC\User\NoUserException('Backend provided no user object for ' . $user);
+ throw new \OC\User\NoUserException('Backends provided no user object for ' . $user);
}
self::$usersSetup[$user] = true;
diff --git a/lib/private/Updater.php b/lib/private/Updater.php
index fd082c837e0..dbcaccaad26 100644
--- a/lib/private/Updater.php
+++ b/lib/private/Updater.php
@@ -216,6 +216,7 @@ class Updater extends BasicEmitter {
try {
Setup::updateHtaccess();
Setup::protectDataDirectory();
+ // TODO: replace with the new repair step mechanism https://github.com/owncloud/core/pull/24378
Setup::installBackgroundJobs();
} catch (\Exception $e) {
throw new \Exception($e->getMessage());
diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index 297ebb2aaf0..0351125b5d9 100644
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -97,11 +97,14 @@ class Session implements IUserSession, Emitter {
/**
* @var User $activeUser
+ */
protected $activeUser;
/**
* @param IUserManager $manager
* @param ISession $session
+ * @param ITimeFactory $timeFacory
+ * @param IProvider $tokenProvider
* @param IProvider[] $tokenProviders
*/
public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider,
@@ -219,7 +222,7 @@ class Session implements IUserSession, Emitter {
} catch (InvalidTokenException $ex) {
// Session was invalidated
$this->logout();
- return false;
+ return;
}
// Check whether login credentials are still valid
@@ -231,15 +234,13 @@ class Session implements IUserSession, Emitter {
if ($this->manager->checkPassword($user->getUID(), $pwd) === false) {
// Password has changed -> log user out
$this->logout();
- return false;
+ return;
}
$this->session->set('last_login_check', $now);
}
// Session is valid, so the token can be refreshed
$this->updateToken($this->tokenProvider, $token);
-
- return true;
}
/**
@@ -301,9 +302,7 @@ class Session implements IUserSession, Emitter {
$this->manager->emit('\OC\User', 'preLogin', array($uid, $password));
$user = $this->manager->checkPassword($uid, $password);
if ($user === false) {
- // Password auth failed, maybe it's a token
- $request = \OC::$server->getRequest();
- if ($this->validateToken($request, $password)) {
+ if ($this->validateToken($password)) {
$user = $this->getUser();
}
}
@@ -349,9 +348,8 @@ class Session implements IUserSession, Emitter {
* @return boolean if the login was successful
*/
public function tryBasicAuthLogin(IRequest $request) {
- // TODO: use $request->server instead of super globals
- if (!empty($_SERVER['PHP_AUTH_USER']) && !empty($_SERVER['PHP_AUTH_PW'])) {
- $result = $this->login($_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']);
+ if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) {
+ $result = $this->login($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW']);
if ($result === true) {
/**
* Add DAV authenticated. This should in an ideal world not be
@@ -363,14 +361,14 @@ class Session implements IUserSession, Emitter {
$this->session->set(
Auth::DAV_AUTHENTICATED, $this->getUser()->getUID()
);
+ return true;
}
- return $result;
}
return false;
}
private function loginWithToken($uid) {
- //$this->manager->emit('\OC\User', 'preTokenLogin', array($uid));
+ // TODO: $this->manager->emit('\OC\User', 'preTokenLogin', array($uid));
$user = $this->manager->get($uid);
if (is_null($user)) {
// user does not exist
@@ -379,7 +377,7 @@ class Session implements IUserSession, Emitter {
//login
$this->setUser($user);
- //$this->manager->emit('\OC\User', 'postTokenLogin', array($user));
+ // TODO: $this->manager->emit('\OC\User', 'postTokenLogin', array($user));
return true;
}
@@ -410,16 +408,15 @@ class Session implements IUserSession, Emitter {
}
/**
- * @param IRequest $request
* @param string $token
* @return boolean
*/
- private function validateToken(IRequest $request, $token) {
+ private function validateToken($token) {
foreach ($this->tokenProviders as $provider) {
try {
$token = $provider->validateToken($token);
if (!is_null($token)) {
- $result = $this->loginWithToken($token->getUid());
+ $result = $this->loginWithToken($token->getUID());
if ($result) {
// Login success
$this->updateToken($provider, $token);
@@ -458,13 +455,13 @@ class Session implements IUserSession, Emitter {
// No auth header, let's try session id
try {
$sessionId = $this->session->getId();
- return $this->validateToken($request, $sessionId);
+ return $this->validateToken($sessionId);
} catch (SessionNotAvailableException $ex) {
return false;
}
} else {
$token = substr($authHeader, 6);
- return $this->validateToken($request, $token);
+ return $this->validateToken($token);
}
}
@@ -530,9 +527,9 @@ class Session implements IUserSession, Emitter {
public function setMagicInCookie($username, $token) {
$secureCookie = OC::$server->getRequest()->getServerProtocol() === 'https';
$expires = time() + OC::$server->getConfig()->getSystemValue('remember_login_cookie_lifetime', 60 * 60 * 24 * 15);
- setcookie("oc_username", $username, $expires, OC::$WEBROOT, '', $secureCookie, true);
- setcookie("oc_token", $token, $expires, OC::$WEBROOT, '', $secureCookie, true);
- setcookie("oc_remember_login", "1", $expires, OC::$WEBROOT, '', $secureCookie, true);
+ setcookie('oc_username', $username, $expires, OC::$WEBROOT, '', $secureCookie, true);
+ setcookie('oc_token', $token, $expires, OC::$WEBROOT, '', $secureCookie, true);
+ setcookie('oc_remember_login', '1', $expires, OC::$WEBROOT, '', $secureCookie, true);
}
/**
@@ -542,9 +539,9 @@ class Session implements IUserSession, Emitter {
//TODO: DI for cookies and IRequest
$secureCookie = OC::$server->getRequest()->getServerProtocol() === 'https';
- unset($_COOKIE["oc_username"]); //TODO: DI
- unset($_COOKIE["oc_token"]);
- unset($_COOKIE["oc_remember_login"]);
+ unset($_COOKIE['oc_username']); //TODO: DI
+ unset($_COOKIE['oc_token']);
+ unset($_COOKIE['oc_remember_login']);
setcookie('oc_username', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true);
setcookie('oc_token', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true);
setcookie('oc_remember_login', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true);
diff --git a/lib/private/legacy/api.php b/lib/private/legacy/api.php
index e3d597fc64e..60300c88b57 100644
--- a/lib/private/legacy/api.php
+++ b/lib/private/legacy/api.php
@@ -358,7 +358,7 @@ class OC_API {
try {
$loginSuccess = $userSession->tryTokenLogin($request);
if (!$loginSuccess) {
- $loginSuccess = $userSession->tryBasicAuthLogin();
+ $loginSuccess = $userSession->tryBasicAuthLogin($request);
}
} catch (\OC\User\LoginException $e) {
return false;
diff --git a/tests/lib/authentication/token/defaulttokenmappertest.php b/tests/lib/authentication/token/defaulttokenmappertest.php
index 1f0fdc160e9..b77bf31aa48 100644
--- a/tests/lib/authentication/token/defaulttokenmappertest.php
+++ b/tests/lib/authentication/token/defaulttokenmappertest.php
@@ -79,12 +79,12 @@ class DefaultTokenMapperTest extends TestCase {
private function getNumberOfTokens() {
$qb = $this->dbConnection->getQueryBuilder();
- $result = $qb->select($qb->createFunction('COUNT(*)'))
+ $result = $qb->select($qb->createFunction('count(*) as `count`'))
->from('authtoken')
->execute()
->fetch();
print_r($result);
- return (int) $result['COUNT(*)'];
+ return (int) $result['count'];
}
public function testInvalidate() {
diff --git a/tests/lib/authentication/token/defaulttokenprovidertest.php b/tests/lib/authentication/token/defaulttokenprovidertest.php
index 54e29a7ef52..567068ef06a 100644
--- a/tests/lib/authentication/token/defaulttokenprovidertest.php
+++ b/tests/lib/authentication/token/defaulttokenprovidertest.php
@@ -36,18 +36,25 @@ class DefaultTokenProviderTest extends TestCase {
private $crypto;
private $config;
private $logger;
+ private $timeFactory;
+ private $time;
protected function setUp() {
parent::setUp();
- $this->mapper = $this->getMock('\OC\Authentication\Token\DefaultTokenMapper')
+ $this->mapper = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenMapper')
->disableOriginalConstructor()
->getMock();
$this->crypto = $this->getMock('\OCP\Security\ICrypto');
$this->config = $this->getMock('\OCP\IConfig');
$this->logger = $this->getMock('\OCP\ILogger');
+ $this->timeFactory = $this->getMock('\OCP\AppFramework\Utility\ITimeFactory');
+ $this->time = 1313131;
+ $this->timeFactory->expects($this->any())
+ ->method('getTime')
+ ->will($this->returnValue($this->time));
- $this->tokenProvider = new DefaultTokenProvider($this->mapper, $this->crypto, $this->config, $this->logger);
+ $this->tokenProvider = new DefaultTokenProvider($this->mapper, $this->crypto, $this->config, $this->logger, $this->timeFactory);
}
public function testGenerateToken() {
@@ -61,11 +68,11 @@ class DefaultTokenProviderTest extends TestCase {
$toInsert->setUid($uid);
$toInsert->setPassword('encryptedpassword');
$toInsert->setName($name);
- $toInsert->setToken(hash('sha512', $token));
+ $toInsert->setToken(hash('sha512', $token . '1f4h9s'));
$toInsert->setType($type);
- $toInsert->setLastActivity(time());
+ $toInsert->setLastActivity($this->time);
- $this->config->expects($this->once())
+ $this->config->expects($this->any())
->method('getSystemValue')
->with('secret')
->will($this->returnValue('1f4h9s'));
@@ -83,27 +90,20 @@ class DefaultTokenProviderTest extends TestCase {
}
public function testUpdateToken() {
- $tk = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider')
- ->disableOriginalConstructor()
- ->getMock();
- $tk->expects($this->once())
- ->method('setLastActivity')
- ->with(time());
+ $tk = new DefaultToken();
$this->mapper->expects($this->once())
->method('update')
->with($tk);
$this->tokenProvider->updateToken($tk);
+
+ $this->assertEquals($this->time, $tk->getLastActivity());
}
public function testGetPassword() {
$token = 'token1234';
- $tk = $this->getMockBuilder('\OC\Authentication\Token\DefaultToken')
- ->disableOriginalConstructor()
- ->getMock();
- $tk->expects($this->once())
- ->method('getPassword')
- ->will($this->returnValue('someencryptedvalue'));
+ $tk = new DefaultToken();
+ $tk->setPassword('someencryptedvalue');
$this->config->expects($this->once())
->method('getSystemValue')
->with('secret')
@@ -134,7 +134,7 @@ class DefaultTokenProviderTest extends TestCase {
->will($this->returnValue(150));
$this->mapper->expects($this->once())
->method('invalidateOld')
- ->with(time() - 150);
+ ->with($this->time - 150);
$this->tokenProvider->invalidateOldTokens();
}
diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php
index bbbe5e0c796..c6ddeb416fb 100644
--- a/tests/lib/user/session.php
+++ b/tests/lib/user/session.php
@@ -45,39 +45,43 @@ class Session extends \Test\TestCase {
->method('get')
->with('user_id')
->will($this->returnValue($expectedUser->getUID()));
+ $sessionId = 'abcdef12345';
$manager = $this->getMockBuilder('\OC\User\Manager')
->disableOriginalConstructor()
->getMock();
+ $session->expects($this->once())
+ ->method('getId')
+ ->will($this->returnValue($sessionId));
$this->defaultProvider->expects($this->once())
->method('getToken')
->will($this->returnValue($token));
- // TODO: check passed session id once it's mockable
- $session->expects($this->at(1))
- ->method('last_login_check')
+ $session->expects($this->at(2))
+ ->method('get')
+ ->with('last_login_check')
->will($this->returnValue(null)); // No check has been run yet
$this->defaultProvider->expects($this->once())
->method('getPassword')
- // TODO: check passed UID and session id once it's mockable
+ ->with($token, $sessionId)
->will($this->returnValue('password123'));
$manager->expects($this->once())
->method('checkPassword')
->with($expectedUser->getUID(), 'password123')
->will($this->returnValue(true));
- $session->expects($this->at(2))
+ $session->expects($this->at(3))
->method('set')
->with('last_login_check', 10000);
- $session->expects($this->at(3))
+ $session->expects($this->at(4))
->method('get')
->with('last_token_update')
->will($this->returnValue(null)); // No check run so far
$this->defaultProvider->expects($this->once())
->method('updateToken')
->with($token);
- $session->expects($this->at(4))
+ $session->expects($this->at(5))
->method('set')
- ->with('last_token_update', $this->equalTo(time(), 10));
+ ->with('last_token_update', $this->equalTo(10000));
$manager->expects($this->any())
->method('get')
@@ -171,7 +175,7 @@ class Session extends \Test\TestCase {
$backend = $this->getMock('\Test\Util\User\Dummy');
$user = $this->getMock('\OC\User\User', array(), array('foo', $backend));
- $user->expects($this->once())
+ $user->expects($this->any())
->method('isEnabled')
->will($this->returnValue(true));
$user->expects($this->any())
@@ -197,6 +201,9 @@ class Session extends \Test\TestCase {
$this->assertEquals($user, $userSession->getUser());
}
+ /**
+ * @expectedException \OC\User\LoginException
+ */
public function testLoginValidPasswordDisabled() {
$session = $this->getMock('\OC\Session\Memory', array(), array(''));
$session->expects($this->never())
@@ -219,7 +226,7 @@ class Session extends \Test\TestCase {
$backend = $this->getMock('\Test\Util\User\Dummy');
$user = $this->getMock('\OC\User\User', array(), array('foo', $backend));
- $user->expects($this->once())
+ $user->expects($this->any())
->method('isEnabled')
->will($this->returnValue(false));
$user->expects($this->never())
--
GitLab