fix PHPDoc and other minor issues

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));
 	}
 
 	/**

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']

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)) {

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;
 	}
 

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');

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);
 }

lib/private/Authentication/Token/IToken.php

@@ -42,5 +42,5 @@ interface IToken {
 	 *
 	 * @return string
 	 */
-	public function getUid();
+	public function getUID();
 }

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;

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());

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);

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;

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() {

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();
 	}

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())