From baaaec77abdc1f974abdaab62963cbcee5d3f1c3 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 1/6] Introduce an event for first time login based on the last login time stamp --- lib/private/Files/Node/Root.php | 1 - lib/private/User/Manager.php | 8 -------- lib/private/User/Session.php | 21 +++++++++++++++++---- lib/private/User/User.php | 5 ++++- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index ed225ae11f66..2415588a753e 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -349,7 +349,6 @@ public function getUserFolder($userId) { $folder = $folder->get($dir); } catch (NotFoundException $e) { $folder = $folder->newFolder($dir); - \OC_Util::copySkeleton($userId, $folder); } return $folder; diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 42213074fd8b..04cafa81e6bc 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -78,14 +78,6 @@ public function __construct(IConfig $config = null) { /** @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 9bdb9a4659c9..45edb447e356 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -49,6 +49,7 @@ use OCP\IUserSession; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Util; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class Session @@ -371,15 +372,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())); + } } /** @@ -431,9 +442,10 @@ private function loginWithPassword($uid, $password) { if ($user->isEnabled()) { $this->setUser($user); $this->setLoginName($uid); + $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 @@ -688,6 +700,7 @@ public function loginWithCookie($uid, $currentToken) { //login $this->setUser($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 023378edc1f0..0350fba61397 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; } /** From 6c8d3ce4cb07c871a54a10cb97a55873be6bd12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 23 Sep 2016 17:22:09 +0200 Subject: [PATCH 2/6] Use firstLogin event to trigger creation of default calendar and default address book --- apps/dav/lib/AppInfo/Application.php | 12 +++++++++++- apps/dav/lib/HookManager.php | 7 +------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php index b1181564cfef..452f7740653b 100644 --- a/apps/dav/lib/AppInfo/Application.php +++ b/apps/dav/lib/AppInfo/Application.php @@ -29,6 +29,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 { @@ -58,6 +59,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 */ @@ -70,7 +81,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 79c3a212dea3..d8e828c03ea7 100644 --- a/apps/dav/lib/HookManager.php +++ b/apps/dav/lib/HookManager.php @@ -84,10 +84,6 @@ public function setup() { 'changeUser', $this, 'changeUser'); - Util::connectHook('OC_User', - 'post_login', - $this, - 'postLogin'); } public function postCreateUser($params) { @@ -123,8 +119,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(); $calendars = $this->calDav->getCalendarsForUser($principal); From 8b064e1f1114d87992c3818f33367690088dbefd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 23 Sep 2016 17:23:55 +0200 Subject: [PATCH 3/6] Delay login of admin user after setup so that firstLogin event can properly be processed for the admin --- lib/private/Setup.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/private/Setup.php b/lib/private/Setup.php index ea9006818d91..b505c527c512 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -366,15 +366,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(); @@ -395,6 +386,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; From 12b00a2346dfd3505d238a0ea0bdd92d31123f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 23 Sep 2016 23:45:08 +0200 Subject: [PATCH 4/6] Fixing tests ... --- apps/dav/tests/unit/DAV/HookManagerTest.php | 9 +++------ tests/lib/TestCase.php | 1 + 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/dav/tests/unit/DAV/HookManagerTest.php b/apps/dav/tests/unit/DAV/HookManagerTest.php index 4a0e65e4cbce..d79da2f45d51 100644 --- a/apps/dav/tests/unit/DAV/HookManagerTest.php +++ b/apps/dav/tests/unit/DAV/HookManagerTest.php @@ -60,7 +60,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) @@ -90,7 +89,7 @@ public function test() { 'contacts', ['{DAV:}displayname' => $this->l10n->t('Contacts')]); $hm = new HookManager($userManager, $syncService, $cal, $card, $this->l10n); - $hm->postLogin(['uid' => 'newUser']); + $hm->firstLogin($user); } public function testWithExisting() { @@ -103,7 +102,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) @@ -129,7 +127,7 @@ public function testWithExisting() { $card->expects($this->never())->method('createAddressBook'); $hm = new HookManager($userManager, $syncService, $cal, $card, $this->l10n); - $hm->postLogin(['uid' => 'newUser']); + $hm->firstLogin($user); } public function testWithBirthdayCalendar() { @@ -142,7 +140,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) @@ -174,7 +171,7 @@ public function testWithBirthdayCalendar() { 'contacts', ['{DAV:}displayname' => $this->l10n->t('Contacts')]); $hm = new HookManager($userManager, $syncService, $cal, $card, $this->l10n); - $hm->postLogin(['uid' => 'newUser']); + $hm->firstLogin($user); } public function testDeleteCalendar() { diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 575dab0c248c..bf63d8c4d6af 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -332,6 +332,7 @@ static protected function tearDownAfterClassCleanStrayLocks() { static protected function loginAsUser($user = '') { self::logout(); \OC\Files\Filesystem::tearDown(); + \OC::$server->getConfig()->setUserValue($user, 'login', 'lastLogin', time()); \OC_User::setUserId($user); \OC_Util::setupFS($user); if (\OC_User::userExists($user)) { From 7652e667094e5f444fbb1f814bd5180bb812eb9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 4 Oct 2016 13:47:30 +0200 Subject: [PATCH 5/6] Skeleton files are not copied over -> only 3 cache entries are remaining --- apps/files/tests/Command/DeleteOrphanedFilesTest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/files/tests/Command/DeleteOrphanedFilesTest.php b/apps/files/tests/Command/DeleteOrphanedFilesTest.php index 65f0f5ab8db5..93a43e012c2d 100644 --- a/apps/files/tests/Command/DeleteOrphanedFilesTest.php +++ b/apps/files/tests/Command/DeleteOrphanedFilesTest.php @@ -23,8 +23,10 @@ namespace OCA\Files\Tests\Command; +use OC\Files\View; use OCA\Files\Command\DeleteOrphanedFiles; use OCP\Files\StorageNotAvailableException; +use Test\TestCase; /** * Class DeleteOrphanedFilesTest @@ -33,7 +35,7 @@ * * @package OCA\Files\Tests\Command */ -class DeleteOrphanedFilesTest extends \Test\TestCase { +class DeleteOrphanedFilesTest extends TestCase { /** * @var DeleteOrphanedFiles @@ -93,7 +95,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'); @@ -114,7 +116,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); From a68d63132738ae343160c8298d0dc3bf0dc05569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 4 Oct 2016 14:37:11 +0200 Subject: [PATCH 6/6] Use updateLastLoginTimestamp to properly setup lastLogin value for a test user --- .../tests/unit/Connector/Sabre/RequestTest/UploadTest.php | 1 + tests/lib/TestCase.php | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index bb27b6fa18a1..b5fdef74feee 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php @@ -86,6 +86,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/tests/lib/TestCase.php b/tests/lib/TestCase.php index bf63d8c4d6af..a0798b37131b 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -332,8 +332,11 @@ static protected function tearDownAfterClassCleanStrayLocks() { static protected function loginAsUser($user = '') { self::logout(); \OC\Files\Filesystem::tearDown(); - \OC::$server->getConfig()->setUserValue($user, 'login', 'lastLogin', time()); \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);