From baec42e80a74543543064f3af9946b9c4dafddeb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 17 Feb 2017 15:40:20 +0100 Subject: [PATCH 1/2] Save the scope of an auth token in the session Signed-off-by: Robin Appelman --- lib/private/Lockdown/LockdownManager.php | 41 ++++++++++++-- lib/private/Server.php | 6 +- lib/private/User/Session.php | 45 +++++++++------ tests/lib/Lockdown/LockdownManagerTest.php | 17 +++++- tests/lib/User/SessionTest.php | 65 ++++++++++++---------- 5 files changed, 118 insertions(+), 56 deletions(-) diff --git a/lib/private/Lockdown/LockdownManager.php b/lib/private/Lockdown/LockdownManager.php index 5ce52a0368..93752dc922 100644 --- a/lib/private/Lockdown/LockdownManager.php +++ b/lib/private/Lockdown/LockdownManager.php @@ -20,27 +20,60 @@ namespace OC\Lockdown; use OC\Authentication\Token\IToken; +use OCP\ISession; use OCP\Lockdown\ILockdownManager; class LockdownManager implements ILockdownManager { + /** @var ISession */ + private $sessionCallback; + private $enabled = false; /** @var array|null */ private $scope; + /** + * LockdownManager constructor. + * + * @param callable $sessionCallback we need to inject the session lazily to avoid dependency loops + */ + public function __construct(callable $sessionCallback) { + $this->sessionCallback = $sessionCallback; + } + + public function enable() { $this->enabled = true; } + /** + * @return ISession + */ + private function getSession() { + $callback = $this->sessionCallback; + return $callback(); + } + + private function getScopeAsArray() { + if (!$this->scope) { + $session = $this->getSession(); + $sessionScope = $session->get('token_scope'); + if ($sessionScope) { + $this->scope = $sessionScope; + } + } + return $this->scope; + } + public function setToken(IToken $token) { $this->scope = $token->getScopeAsArray(); + $session = $this->getSession(); + $session->set('token_scope', $this->scope); $this->enable(); } public function canAccessFilesystem() { - if (!$this->enabled) { - return true; - } - return !$this->scope || $this->scope['filesystem']; + $scope = $this->getScopeAsArray(); + return !$scope || $scope['filesystem']; } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 10f9a810de..98910b097b 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -307,7 +307,7 @@ class Server extends ServerContainer implements IServerContainer { $defaultTokenProvider = null; } - $userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $c->getConfig(), $c->getSecureRandom()); + $userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $c->getConfig(), $c->getSecureRandom(), $c->getLockdownManager()); $userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) { \OC_Hook::emit('OC_User', 'pre_createUser', array('run' => true, 'uid' => $uid, 'password' => $password)); }); @@ -930,7 +930,9 @@ class Server extends ServerContainer implements IServerContainer { }); $this->registerService('LockdownManager', function (Server $c) { - return new LockdownManager(); + return new LockdownManager(function() use ($c) { + return $c->getSession(); + }); }); $this->registerService(ICloudIdManager::class, function (Server $c) { diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 4980318b55..73a8196cec 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -51,6 +51,7 @@ use OCP\ISession; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; +use OCP\Lockdown\ILockdownManager; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Util; @@ -84,7 +85,7 @@ class Session implements IUserSession, Emitter { private $session; /** @var ITimeFactory */ - private $timeFacory; + private $timeFactory; /** @var IProvider */ private $tokenProvider; @@ -98,26 +99,33 @@ class Session implements IUserSession, Emitter { /** @var ISecureRandom */ private $random; + /** @var ILockdownManager */ + private $lockdownManager; + /** * @param IUserManager $manager * @param ISession $session - * @param ITimeFactory $timeFacory + * @param ITimeFactory $timeFactory * @param IProvider $tokenProvider * @param IConfig $config * @param ISecureRandom $random + * @param ILockdownManager $lockdownManager */ public function __construct(IUserManager $manager, ISession $session, - ITimeFactory $timeFacory, + ITimeFactory $timeFactory, $tokenProvider, IConfig $config, - ISecureRandom $random) { + ISecureRandom $random, + ILockdownManager $lockdownManager + ) { $this->manager = $manager; $this->session = $session; - $this->timeFacory = $timeFacory; + $this->timeFactory = $timeFactory; $this->tokenProvider = $tokenProvider; $this->config = $config; $this->random = $random; + $this->lockdownManager = $lockdownManager; } /** @@ -374,7 +382,7 @@ class Session implements IUserSession, Emitter { if (!is_null($request->getCookie('cookie_test'))) { return true; } - setcookie('cookie_test', 'test', $this->timeFacory->getTime() + 3600); + setcookie('cookie_test', 'test', $this->timeFactory->getTime() + 3600); return false; } @@ -464,7 +472,7 @@ class Session implements IUserSession, Emitter { ); // Set the last-password-confirm session to make the sudo mode work - $this->session->set('last-password-confirm', $this->timeFacory->getTime()); + $this->session->set('last-password-confirm', $this->timeFactory->getTime()); return true; } @@ -550,7 +558,7 @@ class Session implements IUserSession, Emitter { $this->setUser($user); $this->setLoginName($dbToken->getLoginName()); $this->setToken($dbToken->getId()); - \OC::$server->getLockdownManager()->setToken($dbToken); + $this->lockdownManager->setToken($dbToken); $this->manager->emit('\OC\User', 'postLogin', array($user, $password)); if ($this->isLoggedIn()) { @@ -626,7 +634,7 @@ class Session implements IUserSession, Emitter { // Check whether login credentials are still valid and the user was not disabled // This check is performed each 5 minutes $lastCheck = $dbToken->getLastCheck() ? : 0; - $now = $this->timeFacory->getTime(); + $now = $this->timeFactory->getTime(); if ($lastCheck > ($now - 60 * 5)) { // Checked performed recently, nothing to do now return true; @@ -747,7 +755,7 @@ class Session implements IUserSession, Emitter { // replace successfully used token with a new one $this->config->deleteUserValue($uid, 'login_token', $currentToken); $newToken = $this->random->generate(32); - $this->config->setUserValue($uid, 'login_token', $newToken, $this->timeFacory->getTime()); + $this->config->setUserValue($uid, 'login_token', $newToken, $this->timeFactory->getTime()); try { $sessionId = $this->session->getId(); @@ -766,6 +774,7 @@ class Session implements IUserSession, Emitter { $this->setUser($user); $this->setLoginName($token->getLoginName()); $this->setToken($token->getId()); + $this->lockdownManager->setToken($token); $user->updateLastLoginTimestamp(); $this->manager->emit('\OC\User', 'postRememberedLogin', [$user]); return true; @@ -776,7 +785,7 @@ class Session implements IUserSession, Emitter { */ public function createRememberMeToken(IUser $user) { $token = $this->random->generate(32); - $this->config->setUserValue($user->getUID(), 'login_token', $token, $this->timeFacory->getTime()); + $this->config->setUserValue($user->getUID(), 'login_token', $token, $this->timeFactory->getTime()); $this->setMagicInCookie($user->getUID(), $token); } @@ -814,7 +823,7 @@ class Session implements IUserSession, Emitter { $webRoot = '/'; } - $expires = $this->timeFacory->getTime() + $this->config->getSystemValue('remember_login_cookie_lifetime', 60 * 60 * 24 * 15); + $expires = $this->timeFactory->getTime() + $this->config->getSystemValue('remember_login_cookie_lifetime', 60 * 60 * 24 * 15); setcookie('nc_username', $username, $expires, $webRoot, '', $secureCookie, true); setcookie('nc_token', $token, $expires, $webRoot, '', $secureCookie, true); try { @@ -834,14 +843,14 @@ class Session implements IUserSession, Emitter { unset($_COOKIE['nc_username']); //TODO: DI unset($_COOKIE['nc_token']); unset($_COOKIE['nc_session_id']); - setcookie('nc_username', '', $this->timeFacory->getTime() - 3600, OC::$WEBROOT, '', $secureCookie, true); - setcookie('nc_token', '', $this->timeFacory->getTime() - 3600, OC::$WEBROOT, '', $secureCookie, true); - setcookie('nc_session_id', '', $this->timeFacory->getTime() - 3600, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_username', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_token', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_session_id', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, '', $secureCookie, true); // old cookies might be stored under /webroot/ instead of /webroot // and Firefox doesn't like it! - setcookie('nc_username', '', $this->timeFacory->getTime() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); - setcookie('nc_token', '', $this->timeFacory->getTime() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); - setcookie('nc_session_id', '', $this->timeFacory->getTime() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); + setcookie('nc_username', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); + setcookie('nc_token', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); + setcookie('nc_session_id', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); } /** diff --git a/tests/lib/Lockdown/LockdownManagerTest.php b/tests/lib/Lockdown/LockdownManagerTest.php index 4cbd9d71a5..1d206dbf40 100644 --- a/tests/lib/Lockdown/LockdownManagerTest.php +++ b/tests/lib/Lockdown/LockdownManagerTest.php @@ -23,18 +23,29 @@ namespace Test\Lockdown; use OC\Authentication\Token\DefaultToken; use OC\Lockdown\LockdownManager; +use OCP\ISession; use Test\TestCase; class LockdownManagerTest extends TestCase { + private $sessionCallback; + + public function setUp() { + parent::setUp(); + + $this->sessionCallback = function() { + return $this->createMock(ISession::class); + }; + } + public function testCanAccessFilesystemDisabled() { - $manager = new LockdownManager(); + $manager = new LockdownManager($this->sessionCallback); $this->assertTrue($manager->canAccessFilesystem()); } public function testCanAccessFilesystemAllowed() { $token = new DefaultToken(); $token->setScope(['filesystem' => true]); - $manager = new LockdownManager(); + $manager = new LockdownManager($this->sessionCallback); $manager->setToken($token); $this->assertTrue($manager->canAccessFilesystem()); } @@ -42,7 +53,7 @@ class LockdownManagerTest extends TestCase { public function testCanAccessFilesystemNotAllowed() { $token = new DefaultToken(); $token->setScope(['filesystem' => false]); - $manager = new LockdownManager(); + $manager = new LockdownManager($this->sessionCallback); $manager->setToken($token); $this->assertFalse($manager->canAccessFilesystem()); } diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 51560d78a6..1bcc6ce3a4 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -26,6 +26,7 @@ use OCP\IRequest; use OCP\ISession; use OCP\IUser; use OCP\IUserManager; +use OCP\Lockdown\ILockdownManager; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; @@ -50,6 +51,8 @@ class SessionTest extends \Test\TestCase { private $session; /** @var Session|\PHPUnit_Framework_MockObject_MockObject */ private $userSession; + /** @var ILockdownManager|\PHPUnit_Framework_MockObject_MockObject */ + private $lockdownManager; protected function setUp() { parent::setUp(); @@ -64,6 +67,7 @@ class SessionTest extends \Test\TestCase { $this->random = $this->createMock(ISecureRandom::class); $this->manager = $this->createMock(IUserManager::class); $this->session = $this->createMock(ISession::class); + $this->lockdownManager = $this->createMock(ILockdownManager::class); $this->userSession = $this->getMockBuilder(Session::class) ->setConstructorArgs([ $this->manager, @@ -72,6 +76,7 @@ class SessionTest extends \Test\TestCase { $this->tokenProvider, $this->config, $this->random, + $this->lockdownManager ]) ->setMethods([ 'setMagicInCookie', @@ -132,7 +137,7 @@ class SessionTest extends \Test\TestCase { ->with($expectedUser->getUID()) ->will($this->returnValue($expectedUser)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $user = $userSession->getUser(); $this->assertSame($expectedUser, $user); $this->assertSame(10000, $token->getLastCheck()); @@ -154,7 +159,7 @@ class SessionTest extends \Test\TestCase { $manager = $this->createMock(Manager::class); $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods([ 'getUser' ]) @@ -181,7 +186,7 @@ class SessionTest extends \Test\TestCase { ->method('getUID') ->will($this->returnValue('foo')); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $userSession->setUser($user); } @@ -233,7 +238,7 @@ class SessionTest extends \Test\TestCase { ->will($this->returnValue($user)); $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods([ 'prepareUserLogin' ]) @@ -280,7 +285,7 @@ class SessionTest extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue($user)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $userSession->login('foo', 'bar'); } @@ -294,7 +299,7 @@ class SessionTest extends \Test\TestCase { ->setConstructorArgs([$this->config]) ->getMock(); $backend = $this->createMock(\Test\Util\User\Dummy::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $user = $this->getMockBuilder(User::class)->setConstructorArgs(['foo', $backend])->getMock(); @@ -323,7 +328,7 @@ class SessionTest extends \Test\TestCase { public function testLoginNonExisting() { $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); $manager = $this->createMock(Manager::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $session->expects($this->never()) ->method('set'); @@ -349,7 +354,7 @@ class SessionTest extends \Test\TestCase { public function testLoginWithDifferentTokenLoginName() { $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); $manager = $this->createMock(Manager::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $username = 'user123'; $token = new \OC\Authentication\Token\DefaultToken(); $token->setLoginName($username); @@ -381,7 +386,7 @@ class SessionTest extends \Test\TestCase { /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -417,7 +422,7 @@ class SessionTest extends \Test\TestCase { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -440,7 +445,7 @@ class SessionTest extends \Test\TestCase { /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -482,7 +487,7 @@ class SessionTest extends \Test\TestCase { /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods(['login', 'isTwoFactorEnforced']) ->getMock(); @@ -529,7 +534,7 @@ class SessionTest extends \Test\TestCase { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->setMethods(['setMagicInCookie', 'setLoginName']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->getMock(); $user = $this->createMock(IUser::class); @@ -606,7 +611,7 @@ class SessionTest extends \Test\TestCase { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->setMethods(['setMagicInCookie']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->getMock(); $user = $this->createMock(IUser::class); @@ -666,7 +671,7 @@ class SessionTest extends \Test\TestCase { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->setMethods(['setMagicInCookie']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->getMock(); $user = $this->createMock(IUser::class); @@ -714,7 +719,7 @@ class SessionTest extends \Test\TestCase { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->setMethods(['setMagicInCookie']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->getMock(); $token = 'goodToken'; $oldSessionId = 'sess321'; @@ -762,7 +767,7 @@ class SessionTest extends \Test\TestCase { $session = new Memory(''); $session->set('user_id', 'foo'); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods([ 'validateSession' ]) @@ -782,7 +787,7 @@ class SessionTest extends \Test\TestCase { $manager = $this->createMock(Manager::class); $session = $this->createMock(ISession::class); $user = $this->createMock(IUser::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $random = $this->createMock(ISecureRandom::class); $config = $this->createMock(IConfig::class); @@ -823,7 +828,7 @@ class SessionTest extends \Test\TestCase { $manager = $this->createMock(Manager::class); $session = $this->createMock(ISession::class); $user = $this->createMock(IUser::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $random = $this->createMock(ISecureRandom::class); $config = $this->createMock(IConfig::class); @@ -867,7 +872,7 @@ class SessionTest extends \Test\TestCase { $session = $this->createMock(ISession::class); $token = $this->createMock(IToken::class); $user = $this->createMock(IUser::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $random = $this->createMock(ISecureRandom::class); $config = $this->createMock(IConfig::class); @@ -914,7 +919,7 @@ class SessionTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $session = $this->createMock(ISession::class); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager); $request = $this->createMock(IRequest::class); $uid = 'user123'; @@ -944,7 +949,7 @@ class SessionTest extends \Test\TestCase { $user = $this->createMock(IUser::class); $userSession = $this->getMockBuilder('\OC\User\Session') ->setMethods(['logout']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->getMock(); $request = $this->createMock(IRequest::class); @@ -973,7 +978,7 @@ class SessionTest extends \Test\TestCase { $timeFactory = $this->createMock(ITimeFactory::class); $tokenProvider = $this->createMock(IProvider::class); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods(['logout']) ->getMock(); @@ -1020,7 +1025,7 @@ class SessionTest extends \Test\TestCase { $timeFactory = $this->createMock(ITimeFactory::class); $tokenProvider = $this->createMock(IProvider::class); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random]) + ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager]) ->setMethods(['logout']) ->getMock(); @@ -1054,7 +1059,7 @@ class SessionTest extends \Test\TestCase { $session = $this->createMock(ISession::class); $timeFactory = $this->createMock(ITimeFactory::class); $tokenProvider = $this->createMock(IProvider::class); - $userSession = new \OC\User\Session($userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager); $password = '123456'; $sessionId = 'session1234'; @@ -1079,7 +1084,7 @@ class SessionTest extends \Test\TestCase { $session = $this->createMock(ISession::class); $timeFactory = $this->createMock(ITimeFactory::class); $tokenProvider = $this->createMock(IProvider::class); - $userSession = new \OC\User\Session($userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager); $session->expects($this->once()) ->method('getId') @@ -1093,7 +1098,7 @@ class SessionTest extends \Test\TestCase { $session = $this->createMock(ISession::class); $timeFactory = $this->createMock(ITimeFactory::class); $tokenProvider = $this->createMock(IProvider::class); - $userSession = new \OC\User\Session($userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random); + $userSession = new \OC\User\Session($userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager); $password = '123456'; $sessionId = 'session1234'; @@ -1133,7 +1138,7 @@ class SessionTest extends \Test\TestCase { $tokenProvider = new DefaultTokenProvider($mapper, $crypto, $this->config, $logger, $this->timeFactory); /** @var \OC\User\Session $userSession */ - $userSession = new Session($manager, $session, $this->timeFactory, $tokenProvider, $this->config, $this->random); + $userSession = new Session($manager, $session, $this->timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager); $mapper->expects($this->any()) ->method('getToken') @@ -1183,7 +1188,7 @@ class SessionTest extends \Test\TestCase { $tokenProvider = new DefaultTokenProvider($mapper, $crypto, $this->config, $logger, $this->timeFactory); /** @var \OC\User\Session $userSession */ - $userSession = new Session($manager, $session, $this->timeFactory, $tokenProvider, $this->config, $this->random); + $userSession = new Session($manager, $session, $this->timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager); $mapper->expects($this->any()) ->method('getToken') @@ -1271,6 +1276,7 @@ class SessionTest extends \Test\TestCase { $this->tokenProvider, $this->config, $this->random, + $this->lockdownManager ]) ->setMethods([ 'logClientIn', @@ -1320,6 +1326,7 @@ class SessionTest extends \Test\TestCase { $this->tokenProvider, $this->config, $this->random, + $this->lockdownManager ]) ->setMethods([ 'logClientIn', From 1d3e391ad8b9eadefab8eaa8239e6901b8466f01 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 5 Apr 2017 13:14:59 +0200 Subject: [PATCH 2/2] Add integration tests for token auth Signed-off-by: Lukas Reschke --- build/integration/features/auth.feature | 59 +++++--- build/integration/features/bootstrap/Auth.php | 135 +++++++++++++++--- 2 files changed, 158 insertions(+), 36 deletions(-) diff --git a/build/integration/features/auth.feature b/build/integration/features/auth.feature index a3af28f25c..b9f423a9e9 100644 --- a/build/integration/features/auth.feature +++ b/build/integration/features/auth.feature @@ -2,11 +2,11 @@ Feature: auth Background: Given user "user0" exists - Given a new client token is used - + Given a new restricted client token is added + Given a new unrestricted client token is added + Given the cookie jar is reset # FILES APP - Scenario: access files app anonymously When requesting "/index.php/apps/files" with "GET" Then the HTTP status code should be "401" @@ -15,12 +15,20 @@ Feature: auth When requesting "/index.php/apps/files" with "GET" using basic auth Then the HTTP status code should be "200" - Scenario: access files app with basic token auth - When requesting "/index.php/apps/files" with "GET" using basic token auth + Scenario: access files app with unrestricted basic token auth + When requesting "/index.php/apps/files" with "GET" using unrestricted basic token auth + Then the HTTP status code should be "200" + Then requesting "/remote.php/files/welcome.txt" with "GET" using browser session Then the HTTP status code should be "200" - Scenario: access files app with a client token - When requesting "/index.php/apps/files" with "GET" using a client token + Scenario: access files app with restricted basic token auth + When requesting "/index.php/apps/files" with "GET" using restricted basic token auth + Then the HTTP status code should be "200" + Then requesting "/remote.php/files/welcome.txt" with "GET" using browser session + Then the HTTP status code should be "404" + + Scenario: access files app with an unrestricted client token + When requesting "/index.php/apps/files" with "GET" using an unrestricted client token Then the HTTP status code should be "200" Scenario: access files app with browser session @@ -28,9 +36,7 @@ Feature: auth When requesting "/index.php/apps/files" with "GET" using browser session Then the HTTP status code should be "200" - # WebDAV - Scenario: using WebDAV anonymously When requesting "/remote.php/webdav" with "PROPFIND" Then the HTTP status code should be "401" @@ -39,23 +45,20 @@ Feature: auth When requesting "/remote.php/webdav" with "PROPFIND" using basic auth Then the HTTP status code should be "207" - Scenario: using WebDAV with token auth - When requesting "/remote.php/webdav" with "PROPFIND" using basic token auth + Scenario: using WebDAV with unrestricted basic token auth + When requesting "/remote.php/webdav" with "PROPFIND" using unrestricted basic token auth Then the HTTP status code should be "207" - # DAV token auth is not possible yet - #Scenario: using WebDAV with a client token - # When requesting "/remote.php/webdav" with "PROPFIND" using a client token - # Then the HTTP status code should be "207" + Scenario: using WebDAV with restricted basic token auth + When requesting "/remote.php/webdav" with "PROPFIND" using restricted basic token auth + Then the HTTP status code should be "207" Scenario: using WebDAV with browser session Given a new browser session is started When requesting "/remote.php/webdav" with "PROPFIND" using browser session Then the HTTP status code should be "207" - # OCS - Scenario: using OCS anonymously When requesting "/ocs/v1.php/apps/files_sharing/api/v1/remote_shares" with "GET" Then the OCS status code should be "997" @@ -65,11 +68,11 @@ Feature: auth Then the OCS status code should be "100" Scenario: using OCS with token auth - When requesting "/ocs/v1.php/apps/files_sharing/api/v1/remote_shares" with "GET" using basic token auth + When requesting "/ocs/v1.php/apps/files_sharing/api/v1/remote_shares" with "GET" using unrestricted basic token auth Then the OCS status code should be "100" - Scenario: using OCS with client token - When requesting "/ocs/v1.php/apps/files_sharing/api/v1/remote_shares" with "GET" using a client token + Scenario: using OCS with an unrestricted client token + When requesting "/ocs/v1.php/apps/files_sharing/api/v1/remote_shares" with "GET" using an unrestricted client token Then the OCS status code should be "100" Scenario: using OCS with browser session @@ -84,3 +87,19 @@ Feature: auth And requesting "/index.php/apps/files" with "GET" using browser session Then the HTTP status code should be "200" + # AUTH TOKENS + Scenario: Creating an auth token with regular auth token should not work + When requesting "/index.php/apps/files" with "GET" using restricted basic token auth + Then the HTTP status code should be "200" + When the CSRF token is extracted from the previous response + When a new unrestricted client token is added using restricted basic token auth + Then the HTTP status code should be "503" + + Scenario: Creating a restricted auth token with regular login should work + When a new restricted client token is added + Then the HTTP status code should be "200" + + Scenario: Creating an unrestricted auth token with regular login should work + When a new unrestricted client token is added + Then the HTTP status code should be "200" + diff --git a/build/integration/features/bootstrap/Auth.php b/build/integration/features/bootstrap/Auth.php index 46bb94a2b2..fd1b2e05a8 100644 --- a/build/integration/features/bootstrap/Auth.php +++ b/build/integration/features/bootstrap/Auth.php @@ -1,7 +1,5 @@ * @@ -21,19 +19,28 @@ * along with this program. If not, see . * */ + use GuzzleHttp\Client; use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Cookie\CookieJar; require __DIR__ . '/../../vendor/autoload.php'; trait Auth { - - private $clientToken; + /** @var string */ + private $unrestrictedClientToken; + /** @var string */ + private $restrictedClientToken; + /** @var Client */ + private $client; + /** @var string */ + private $responseXml; /** @BeforeScenario */ public function setUpScenario() { $this->client = new Client(); $this->responseXml = ''; + $this->cookieJar = new CookieJar(); } /** @@ -65,15 +72,28 @@ trait Auth { } /** - * @Given a new client token is used + * @When the CSRF token is extracted from the previous response */ - public function aNewClientTokenIsUsed() { - $this->loggingInUsingWebAs('user0'); + public function theCsrfTokenIsExtractedFromThePreviousResponse() { + $this->requestToken = substr(preg_replace('/(.*)data-requesttoken="(.*)">(.*)/sm', '\2', $this->response->getBody()->getContents()), 0, 89); + } + + /** + * @param bool $loginViaWeb + * @return object + */ + private function createClientToken($loginViaWeb = true) { + if($loginViaWeb) { + $this->loggingInUsingWebAs('user0'); + } $fullUrl = substr($this->baseUrl, 0, -5) . '/index.php/settings/personal/authtokens'; $client = new Client(); $options = [ - 'auth' => ['user0', '123456'], + 'auth' => [ + 'user0', + $loginViaWeb ? '123456' : $this->restrictedClientToken, + ], 'body' => [ 'requesttoken' => $this->requestToken, 'name' => md5(microtime()), @@ -81,34 +101,107 @@ trait Auth { 'cookies' => $this->cookieJar, ]; - $resp = $client->send($client->createRequest('POST', $fullUrl, $options)); + try { + $this->response = $client->send($client->createRequest('POST', $fullUrl, $options)); + } catch (\GuzzleHttp\Exception\ServerException $e) { + $this->response = $e->getResponse(); + } + return json_decode($this->response->getBody()->getContents()); + } - $this->clientToken = json_decode($resp->getBody()->getContents())->token; + /** + * @Given a new restricted client token is added + */ + public function aNewRestrictedClientTokenIsAdded() { + $tokenObj = $this->createClientToken(); + $newCreatedTokenId = $tokenObj->deviceToken->id; + $fullUrl = substr($this->baseUrl, 0, -5) . '/index.php/settings/personal/authtokens/' . $newCreatedTokenId; + $client = new Client(); + $options = [ + 'auth' => ['user0', '123456'], + 'headers' => [ + 'requesttoken' => $this->requestToken, + ], + 'json' => [ + 'scope' => [ + 'filesystem' => false, + ], + ], + 'cookies' => $this->cookieJar, + ]; + $this->response = $client->send($client->createRequest('PUT', $fullUrl, $options)); + $this->restrictedClientToken = $tokenObj->token; + } + + /** + * @Given a new unrestricted client token is added + */ + public function aNewUnrestrictedClientTokenIsAdded() { + $this->unrestrictedClientToken = $this->createClientToken()->token; + } + + /** + * @When a new unrestricted client token is added using restricted basic token auth + */ + public function aNewUnrestrictedClientTokenIsAddedUsingRestrictedBasicTokenAuth() { + $this->createClientToken(false); } /** * @When requesting :url with :method using basic auth + * + * @param string $url + * @param string $method */ public function requestingWithBasicAuth($url, $method) { $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:123456')); } /** - * @When requesting :url with :method using basic token auth + * @When requesting :url with :method using unrestricted basic token auth + * + * @param string $url + * @param string $method */ - public function requestingWithBasicTokenAuth($url, $method) { - $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:' . $this->clientToken)); + public function requestingWithUnrestrictedBasicTokenAuth($url, $method) { + $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:' . $this->unrestrictedClientToken), true); } /** - * @When requesting :url with :method using a client token + * @When requesting :url with :method using restricted basic token auth + * + * @param string $url + * @param string $method */ - public function requestingWithUsingAClientToken($url, $method) { - $this->sendRequest($url, $method, 'token ' . $this->clientToken); + public function requestingWithRestrictedBasicTokenAuth($url, $method) { + $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:' . $this->restrictedClientToken), true); + } + + /** + * @When requesting :url with :method using an unrestricted client token + * + * @param string $url + * @param string $method + */ + public function requestingWithUsingAnUnrestrictedClientToken($url, $method) { + $this->sendRequest($url, $method, 'token ' . $this->unrestrictedClientToken); + } + + /** + * @When requesting :url with :method using a restricted client token + * + * @param string $url + * @param string $method + */ + public function requestingWithUsingARestrictedClientToken($url, $method) { + $this->sendRequest($url, $method, 'token ' . $this->restrictedClientToken); } /** * @When requesting :url with :method using browser session + * + * @param string $url + * @param string $method */ public function requestingWithBrowserSession($url, $method) { $this->sendRequest($url, $method, null, true); @@ -116,6 +209,8 @@ trait Auth { /** * @Given a new browser session is started + * + * @param bool $remember */ public function aNewBrowserSessionIsStarted($remember = false) { $loginUrl = substr($this->baseUrl, 0, -5) . '/login'; @@ -149,6 +244,14 @@ trait Auth { $this->aNewBrowserSessionIsStarted(true); } + + /** + * @Given the cookie jar is reset + */ + public function theCookieJarIsReset() { + $this->cookieJar = new CookieJar(); + } + /** * @When the session cookie expires */