diff options
20 files changed, 137 insertions, 61 deletions
diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php index c777f5e5a35..844e0780ffb 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/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 539e22296f2..59b326243ee 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -115,7 +115,6 @@ class FilesPlugin extends ServerPlugin { /** * @param Tree $tree - * @param View $view * @param IConfig $config * @param IRequest $request * @param IPreview $previewManager @@ -123,14 +122,12 @@ class FilesPlugin extends ServerPlugin { * @param bool $downloadAttachment */ public function __construct(Tree $tree, - View $view, IConfig $config, IRequest $request, IPreview $previewManager, $isPublic = false, $downloadAttachment = true) { $this->tree = $tree; - $this->fileView = $view; $this->config = $config; $this->request = $request; $this->isPublic = $isPublic; diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 6d9f9b1bc8b..24c93ee571d 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -153,7 +153,6 @@ class ServerFactory { $server->addPlugin( new \OCA\DAV\Connector\Sabre\FilesPlugin( $objectTree, - $view, $this->config, $this->request, $this->previewManager, diff --git a/apps/dav/lib/HookManager.php b/apps/dav/lib/HookManager.php index 92aa4fce7fa..247d4b291af 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/lib/Server.php b/apps/dav/lib/Server.php index fca4d0ce209..1205d018241 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -171,7 +171,6 @@ class Server { $this->server->addPlugin( new FilesPlugin( $this->server->tree, - $view, \OC::$server->getConfig(), $this->request, \OC::$server->getPreviewManager(), diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index d4e9ce9dd3e..c6e833033d5 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -68,11 +68,6 @@ class FilesPluginTest extends TestCase { private $plugin; /** - * @var \OC\Files\View | \PHPUnit_Framework_MockObject_MockObject - */ - private $view; - - /** * @var \OCP\IConfig | \PHPUnit_Framework_MockObject_MockObject */ private $config; @@ -95,12 +90,7 @@ class FilesPluginTest extends TestCase { $this->tree = $this->getMockBuilder('\Sabre\DAV\Tree') ->disableOriginalConstructor() ->getMock(); - $this->view = $this->getMockBuilder('\OC\Files\View') - ->disableOriginalConstructor() - ->getMock(); - $this->config = $this->getMockBuilder('\OCP\IConfig') - ->disableOriginalConstructor() - ->getMock(); + $this->config = $this->createMock('\OCP\IConfig'); $this->config->expects($this->any())->method('getSystemValue') ->with($this->equalTo('data-fingerprint'), $this->equalTo('')) ->willReturn('my_fingerprint'); @@ -113,7 +103,6 @@ class FilesPluginTest extends TestCase { $this->plugin = new FilesPlugin( $this->tree, - $this->view, $this->config, $this->request, $this->previewManager @@ -246,7 +235,6 @@ class FilesPluginTest extends TestCase { public function testGetPublicPermissions() { $this->plugin = new FilesPlugin( $this->tree, - $this->view, $this->config, $this->getMockBuilder('\OCP\IRequest') ->disableOriginalConstructor() diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php index 2b5c9f46301..54e5283c7c1 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php @@ -380,7 +380,6 @@ class FilesReportPluginTest extends \Test\TestCase { $this->server->addPlugin( new \OCA\DAV\Connector\Sabre\FilesPlugin( $this->tree, - $this->view, $config, $this->getMockBuilder('\OCP\IRequest') ->disableOriginalConstructor() diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index f22da14a0d8..1db85b1bcaf 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 5b7d4700a5f..f980e595bf9 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 b4d16e54617..32c8d628a80 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/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 1358663ea2b..90274beba49 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -692,6 +692,7 @@ class ShareAPIController extends OCSController { if ($newPermissions !== null) { $share->setPermissions($newPermissions); + $permissions = $newPermissions; } if ($expireDate === '') { diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 890fdb6eda0..ed4aa1dba9e 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -1205,7 +1205,7 @@ class ShareAPIControllerTest extends \Test\TestCase { public function testUpdateLinkShareClear() { $ocs = $this->mockFormatShare(); - $node = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); + $node = $this->getMockBuilder(Folder::class)->getMock(); $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser) @@ -1229,6 +1229,9 @@ class ShareAPIControllerTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); + $this->shareManager->method('getSharedWith') + ->willReturn([]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42, null, '', 'false', ''); @@ -1261,6 +1264,9 @@ class ShareAPIControllerTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); + $this->shareManager->method('getSharedWith') + ->willReturn([]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42, null, 'password', 'true', '2000-01-01'); @@ -1483,6 +1489,9 @@ class ShareAPIControllerTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); + $this->shareManager->method('getSharedWith') + ->willReturn([]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42, null, null, 'true', null); @@ -1633,6 +1642,52 @@ class ShareAPIControllerTest extends \Test\TestCase { } } + public function testUpdateShareCannotIncreasePermissionsLinkShare() { + $ocs = $this->mockFormatShare(); + + $folder = $this->createMock(Folder::class); + + $share = \OC::$server->getShareManager()->newShare(); + $share + ->setId(42) + ->setSharedBy($this->currentUser) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + // note: updateShare will modify the received instance but getSharedWith will reread from the database, + // so their values will be different + $incomingShare = \OC::$server->getShareManager()->newShare(); + $incomingShare + ->setId(42) + ->setSharedBy($this->currentUser) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedWith('currentUser') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->shareManager->expects($this->any()) + ->method('getSharedWith') + ->will($this->returnValueMap([ + ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, [$incomingShare]], + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, []] + ])); + + $this->shareManager->expects($this->never())->method('updateShare'); + $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); + + try { + $ocs->updateShare(42, null, null, 'true'); + $this->fail(); + } catch (OCSNotFoundException $e) { + $this->assertEquals('Cannot increase permissions', $e->getMessage()); + } + } + public function testUpdateShareCanIncreasePermissionsIfOwner() { $ocs = $this->mockFormatShare(); diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index 07237ac7218..acfeb0e611f 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -971,3 +971,20 @@ Feature: sharing When Deleting last share Then etag of element "/" of user "user1" has changed And etag of element "/PARENT" of user "user0" has not changed + + Scenario: do not allow to increase link share permissions on reshare + Given As an "admin" + And user "admin" created a folder "/TMP" + And user "user0" exists + And creating a share with + | path | TMP | + | shareType | 0 | + | shareWith | user0 | + | permissions | 17 | + When As an "user0" + And creating a share with + | path | TMP | + | shareType | 3 | + And Updating last share with + | publicUpload | true | + Then the OCS status code should be "404" diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index 515a795d6e0..175cb4f914f 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 4c72fbc9623..81a5343fe21 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 c3fb8737420..43e09cdabcf 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 7215cbe4188..ef408aa4077 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 68787ce60f8..3cc6dc3b7ed 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/lib/private/legacy/util.php b/lib/private/legacy/util.php index 5cd92eaa415..ecc8f053704 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -98,6 +98,11 @@ class OC_Util { } // instantiate object store implementation + $name = $config['class']; + if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) { + $segments = explode('\\', $name); + OC_App::loadApp(strtolower($segments[1])); + } $config['arguments']['objectstore'] = new $config['class']($config['arguments']); // mount with plain / root object store implementation $config['class'] = '\OC\Files\ObjectStore\ObjectStoreStorage'; diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index c1fe9c382fa..42b2273e119 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); |