From 506ccdbd8dd7a9438fa69fe09bc9307ca5f346e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 23 Sep 2016 17:21:07 +0200 Subject: [PATCH] Introduce an event for first time login based on the last login time stamp Use firstLogin event to trigger creation of default calendar and default address book Delay login of admin user after setup so that firstLogin event can properly be processed for the admin Fixing tests ... Skeleton files are not copied over -> only 3 cache entries are remaining Use updateLastLoginTimestamp to properly setup lastLogin value for a test user --- apps/dav/lib/AppInfo/Application.php | 12 ++++++++- apps/dav/lib/HookManager.php | 7 +----- .../Sabre/RequestTest/UploadTest.php | 1 + apps/dav/tests/unit/DAV/HookManagerTest.php | 9 +++---- .../tests/Command/DeleteOrphanedFilesTest.php | 8 +++--- lib/private/Files/Node/Root.php | 1 - lib/private/Setup.php | 18 ++++++------- lib/private/User/Manager.php | 8 ------ lib/private/User/Session.php | 25 ++++++++++++++----- lib/private/User/User.php | 5 +++- tests/lib/TestCase.php | 4 +++ 11 files changed, 57 insertions(+), 41 deletions(-) diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php index c777f5e5a3..844e0780ff 100644 --- a/apps/dav/lib/AppInfo/Application.php +++ b/apps/dav/lib/AppInfo/Application.php @@ -33,6 +33,7 @@ use OCA\DAV\CardDAV\SyncService; use OCA\DAV\HookManager; use \OCP\AppFramework\App; use OCP\Contacts\IManager; +use OCP\IUser; use Symfony\Component\EventDispatcher\GenericEvent; class Application extends App { @@ -65,6 +66,16 @@ class Application extends App { $hm = $this->getContainer()->query(HookManager::class); $hm->setup(); + $dispatcher = $this->getContainer()->getServer()->getEventDispatcher(); + + // first time login event setup + $dispatcher->addListener(IUser::class . '::firstLogin', function ($event) use ($hm) { + if ($event instanceof GenericEvent) { + $hm->firstLogin($event->getSubject()); + } + }); + + // carddav/caldav sync event setup $listener = function($event) { if ($event instanceof GenericEvent) { /** @var BirthdayService $b */ @@ -77,7 +88,6 @@ class Application extends App { } }; - $dispatcher = $this->getContainer()->getServer()->getEventDispatcher(); $dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::createCard', $listener); $dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $listener); $dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::deleteCard', function($event) { diff --git a/apps/dav/lib/HookManager.php b/apps/dav/lib/HookManager.php index 92aa4fce7f..247d4b291a 100644 --- a/apps/dav/lib/HookManager.php +++ b/apps/dav/lib/HookManager.php @@ -78,10 +78,6 @@ class HookManager { 'changeUser', $this, 'changeUser'); - Util::connectHook('OC_User', - 'post_login', - $this, - 'postLogin'); } public function postCreateUser($params) { @@ -117,8 +113,7 @@ class HookManager { $this->syncService->updateUser($user); } - public function postLogin($params) { - $user = $this->userManager->get($params['uid']); + public function firstLogin(IUser $user = null) { if (!is_null($user)) { $principal = 'principals/users/' . $user->getUID(); if ($this->calDav->getCalendarsForUserCount($principal) === 0) { diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index f22da14a0d..1db85b1bca 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php @@ -88,6 +88,7 @@ class UploadTest extends RequestTest { public function testUploadOverWriteWriteLocked() { $user = $this->getUniqueID(); $view = $this->setupUser($user, 'pass'); + $this->loginAsUser($user); $view->file_put_contents('foo.txt', 'bar'); diff --git a/apps/dav/tests/unit/DAV/HookManagerTest.php b/apps/dav/tests/unit/DAV/HookManagerTest.php index 5b7d4700a5..f980e595bf 100644 --- a/apps/dav/tests/unit/DAV/HookManagerTest.php +++ b/apps/dav/tests/unit/DAV/HookManagerTest.php @@ -58,7 +58,6 @@ class HookManagerTest extends TestCase { $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); - $userManager->expects($this->once())->method('get')->willReturn($user); /** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) @@ -84,7 +83,7 @@ class HookManagerTest extends TestCase { 'contacts', ['{DAV:}displayname' => 'Contacts']); $hm = new HookManager($userManager, $syncService, $cal, $card); - $hm->postLogin(['uid' => 'newUser']); + $hm->firstLogin($user); } public function testWithExisting() { @@ -97,7 +96,6 @@ class HookManagerTest extends TestCase { $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); - $userManager->expects($this->once())->method('get')->willReturn($user); /** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) @@ -119,7 +117,7 @@ class HookManagerTest extends TestCase { $card->expects($this->never())->method('createAddressBook'); $hm = new HookManager($userManager, $syncService, $cal, $card); - $hm->postLogin(['uid' => 'newUser']); + $hm->firstLogin($user); } public function testWithBirthdayCalendar() { @@ -132,7 +130,6 @@ class HookManagerTest extends TestCase { $userManager = $this->getMockBuilder(IUserManager::class) ->disableOriginalConstructor() ->getMock(); - $userManager->expects($this->once())->method('get')->willReturn($user); /** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $syncService */ $syncService = $this->getMockBuilder(SyncService::class) @@ -158,7 +155,7 @@ class HookManagerTest extends TestCase { 'contacts', ['{DAV:}displayname' => 'Contacts']); $hm = new HookManager($userManager, $syncService, $cal, $card); - $hm->postLogin(['uid' => 'newUser']); + $hm->firstLogin($user); } public function testDeleteCalendar() { diff --git a/apps/files/tests/Command/DeleteOrphanedFilesTest.php b/apps/files/tests/Command/DeleteOrphanedFilesTest.php index b4d16e5461..32c8d628a8 100644 --- a/apps/files/tests/Command/DeleteOrphanedFilesTest.php +++ b/apps/files/tests/Command/DeleteOrphanedFilesTest.php @@ -24,8 +24,10 @@ namespace OCA\Files\Tests\Command; +use OC\Files\View; use OCA\Files\Command\DeleteOrphanedFiles; use OCP\Files\StorageNotAvailableException; +use Test\TestCase; /** * Class DeleteOrphanedFilesTest @@ -34,7 +36,7 @@ use OCP\Files\StorageNotAvailableException; * * @package OCA\Files\Tests\Command */ -class DeleteOrphanedFilesTest extends \Test\TestCase { +class DeleteOrphanedFilesTest extends TestCase { /** * @var DeleteOrphanedFiles @@ -94,7 +96,7 @@ class DeleteOrphanedFilesTest extends \Test\TestCase { $this->loginAsUser($this->user1); - $view = new \OC\Files\View('/' . $this->user1 . '/'); + $view = new View('/' . $this->user1 . '/'); $view->mkdir('files/test'); $fileInfo = $view->getFileInfo('files/test'); @@ -115,7 +117,7 @@ class DeleteOrphanedFilesTest extends \Test\TestCase { $output ->expects($this->once()) ->method('writeln') - ->with('4 orphaned file cache entries deleted'); + ->with('3 orphaned file cache entries deleted'); $this->command->execute($input, $output); diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index 515a795d6e..175cb4f914 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -378,7 +378,6 @@ class Root extends Folder implements IRootFolder { $this->newFolder('/' . $userId); } $folder = $this->newFolder('/' . $userId . '/files'); - \OC_Util::copySkeleton($userId, $folder); } $this->userFolderCache->set($userId, $folder); diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 4c72fbc962..81a5343fe2 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -363,15 +363,6 @@ class Setup { $group =\OC::$server->getGroupManager()->createGroup('admin'); $group->addUser($user); - // Create a session token for the newly created user - // The token provider requires a working db, so it's not injected on setup - /* @var $userSession User\Session */ - $userSession = \OC::$server->getUserSession(); - $defaultTokenProvider = \OC::$server->query('OC\Authentication\Token\DefaultTokenProvider'); - $userSession->setTokenProvider($defaultTokenProvider); - $userSession->login($username, $password); - $userSession->createSessionToken($request, $userSession->getUser()->getUID(), $username, $password); - //guess what this does Installer::installShippedApps(); @@ -392,6 +383,15 @@ class Setup { //and we are done $config->setSystemValue('installed', true); + + // Create a session token for the newly created user + // The token provider requires a working db, so it's not injected on setup + /* @var $userSession User\Session */ + $userSession = \OC::$server->getUserSession(); + $defaultTokenProvider = \OC::$server->query('OC\Authentication\Token\DefaultTokenProvider'); + $userSession->setTokenProvider($defaultTokenProvider); + $userSession->login($username, $password); + $userSession->createSessionToken($request, $userSession->getUser()->getUID(), $username, $password); } return $error; diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index c3fb873742..43e09cdabc 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -79,14 +79,6 @@ class Manager extends PublicEmitter implements IUserManager { /** @var \OC\User\User $user */ unset($cachedUsers[$user->getUID()]); }); - $this->listen('\OC\User', 'postLogin', function ($user) { - /** @var \OC\User\User $user */ - $user->updateLastLoginTimestamp(); - }); - $this->listen('\OC\User', 'postRememberedLogin', function ($user) { - /** @var \OC\User\User $user */ - $user->updateLastLoginTimestamp(); - }); } /** diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 7215cbe418..ef408aa407 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -51,6 +51,7 @@ use OCP\IUserSession; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Util; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class Session @@ -396,15 +397,25 @@ class Session implements IUserSession, Emitter { } } - protected function prepareUserLogin() { + protected function prepareUserLogin($firstTimeLogin) { // TODO: mock/inject/use non-static // Refresh the token \OC::$server->getCsrfTokenManager()->refreshToken(); //we need to pass the user name, which may differ from login name $user = $this->getUser()->getUID(); OC_Util::setupFS($user); - //trigger creation of user home and /files folder - \OC::$server->getUserFolder($user); + + if ($firstTimeLogin) { + // TODO: lock necessary? + //trigger creation of user home and /files folder + $userFolder = \OC::$server->getUserFolder($user); + + // copy skeleton + \OC_Util::copySkeleton($user, $userFolder); + + // trigger any other initialization + \OC::$server->getEventDispatcher()->dispatch(IUser::class . '::firstLogin', new GenericEvent($this->getUser())); + } } /** @@ -457,9 +468,10 @@ class Session implements IUserSession, Emitter { if ($user->isEnabled()) { $this->setUser($user); $this->setLoginName($uid); - $this->manager->emit('\OC\User', 'postLogin', array($user, $password)); + $firstTimeLogin = $user->updateLastLoginTimestamp(); + $this->manager->emit('\OC\User', 'postLogin', [$user, $password]); if ($this->isLoggedIn()) { - $this->prepareUserLogin(); + $this->prepareUserLogin($firstTimeLogin); return true; } else { // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory @@ -725,7 +737,8 @@ class Session implements IUserSession, Emitter { //login $this->setUser($user); - $this->manager->emit('\OC\User', 'postRememberedLogin', array($user)); + $user->updateLastLoginTimestamp(); + $this->manager->emit('\OC\User', 'postRememberedLogin', [$user]); return true; } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 68787ce60f..3cc6dc3b7e 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -180,9 +180,12 @@ class User implements IUser { * updates the timestamp of the most recent login of this user */ public function updateLastLoginTimestamp() { + $firstTimeLogin = ($this->lastLogin === 0); $this->lastLogin = time(); - \OC::$server->getConfig()->setUserValue( + $this->config->setUserValue( $this->uid, 'login', 'lastLogin', $this->lastLogin); + + return $firstTimeLogin; } /** diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index c1fe9c382f..42b2273e11 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -377,6 +377,10 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { self::logout(); \OC\Files\Filesystem::tearDown(); \OC_User::setUserId($user); + $userObject = \OC::$server->getUserManager()->get($user); + if (!is_null($userObject)) { + $userObject->updateLastLoginTimestamp(); + } \OC_Util::setupFS($user); if (\OC_User::userExists($user)) { \OC::$server->getUserFolder($user);