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 c777f5e5a352e..844e0780ffb92 100644 --- a/apps/dav/lib/AppInfo/Application.php +++ b/apps/dav/lib/AppInfo/Application.php @@ -33,6 +33,7 @@ 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 @@ public function registerHooks() { $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 @@ public function registerHooks() { } }; - $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 92aa4fce7fa37..247d4b291af15 100644 --- a/apps/dav/lib/HookManager.php +++ b/apps/dav/lib/HookManager.php @@ -78,10 +78,6 @@ public function setup() { 'changeUser', $this, 'changeUser'); - Util::connectHook('OC_User', - 'post_login', - $this, - 'postLogin'); } public function postCreateUser($params) { @@ -117,8 +113,7 @@ public function changeUser($params) { $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 f22da14a0d8cd..1db85b1bcaf5b 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 @@ public function testUploadOverWriteReadLocked() { 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 5b7d4700a5f87..f980e595bf917 100644 --- a/apps/dav/tests/unit/DAV/HookManagerTest.php +++ b/apps/dav/tests/unit/DAV/HookManagerTest.php @@ -58,7 +58,6 @@ public function test() { $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 @@ public function test() { '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 @@ public function testWithExisting() { $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 @@ public function testWithExisting() { $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 @@ public function testWithBirthdayCalendar() { $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 @@ public function testWithBirthdayCalendar() { '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 b4d16e546176d..32c8d628a809e 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 @@ * * @package OCA\Files\Tests\Command */ -class DeleteOrphanedFilesTest extends \Test\TestCase { +class DeleteOrphanedFilesTest extends TestCase { /** * @var DeleteOrphanedFiles @@ -94,7 +96,7 @@ public function testClearFiles() { $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 @@ public function testClearFiles() { $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 515a795d6e023..175cb4f914f03 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -378,7 +378,6 @@ public function getUserFolder($userId) { $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 4c72fbc962342..81a5343fe2103 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -363,15 +363,6 @@ public function install($options) { $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 @@ public function install($options) { //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 c3fb873742003..43e09cdabcfc3 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -79,14 +79,6 @@ public function __construct(IConfig $config) { /** @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 7215cbe418886..ef408aa40776b 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -51,6 +51,7 @@ use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Util; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class Session @@ -396,15 +397,25 @@ public function isTokenPassword($password) { } } - 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 @@ private function loginWithPassword($uid, $password) { 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 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { //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 68787ce60f875..3cc6dc3b7ed62 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -180,9 +180,12 @@ public function getLastLogin() { * 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 c1fe9c382faff..42b2273e11929 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -377,6 +377,10 @@ static protected function loginAsUser($user = '') { 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);