diff --git a/build/integration/features/bootstrap/Auth.php b/build/integration/features/bootstrap/Auth.php index 88edcd49a5..5f36b199e0 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 977f523afd..df5ece9671 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 6f98face14..6606a3c834 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 5dd9dc5b03..25caf675a4 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 a0d07f9e2e..53ecb562a8 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 5c0b0d140a..f8a3262ca8 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 90feefb458..9b2bd18f83 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 89b8547aa5..99c123ad1a 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 fd082c837e..dbcaccaad2 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 297ebb2aaf..0351125b5d 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 e3d597fc64..60300c88b5 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 1f0fdc160e..b77bf31aa4 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 54e29a7ef5..567068ef06 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 bbbe5e0c79..c6ddeb416f 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())