From 56bdb4337836ae5dc680935e0ec1c9dd39cf5c01 Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Thu, 11 Apr 2019 16:21:21 +0200 Subject: [PATCH 01/28] Set version to 10.2.0 RC1 --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index 35d810c0bdab..c763517a5448 100644 --- a/version.php +++ b/version.php @@ -25,10 +25,10 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [10, 2, 0, 0]; +$OC_Version = [10, 2, 0, 1]; // The human readable string -$OC_VersionString = '10.2.0 prealpha'; +$OC_VersionString = '10.2.0 RC1'; $OC_VersionCanBeUpgradedFrom = [[8, 2, 11],[9, 0, 9],[9, 1]]; From c8a593302c765d1de81b8f1f00ca420185dd43c8 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 16 Apr 2019 22:40:24 +0200 Subject: [PATCH 02/28] Fix missing progress bar in files drop view The uploader is expecting the progress bar element to be in the DOM when created. This fix makes sure to append the PublicUploadView's element to the DOM before rendering it. --- apps/files_sharing/js/PublicUploadView.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/js/PublicUploadView.js b/apps/files_sharing/js/PublicUploadView.js index 23fe0f8274e7..6f5e8f550df6 100644 --- a/apps/files_sharing/js/PublicUploadView.js +++ b/apps/files_sharing/js/PublicUploadView.js @@ -152,9 +152,10 @@ var view = new OCA.Sharing.PublicUploadView({ shareToken: $('#sharingToken').val() }); + $('#preview .uploadForm').append(view.$el); + view.render(); - $('#preview .uploadForm').append(view.$el); $('#uploadprogresswrapper .stop').on('click', function () { view.onUploadCancel(); }); From b2c56a40398a8697d55f88aa3fed46356a5fba28 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 12 Apr 2019 15:29:45 +0300 Subject: [PATCH 03/28] Filter unavailable users --- .../lib/Command/PollIncomingShares.php | 7 ++++++ .../tests/Command/PollIncomingSharesTest.php | 25 ++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/Command/PollIncomingShares.php b/apps/federatedfilesharing/lib/Command/PollIncomingShares.php index 4ad70b6d5dd3..c8e36635e0b3 100644 --- a/apps/federatedfilesharing/lib/Command/PollIncomingShares.php +++ b/apps/federatedfilesharing/lib/Command/PollIncomingShares.php @@ -82,6 +82,13 @@ public function execute(InputInterface $input, OutputInterface $output) { $cursor = $this->getCursor(); while ($data = $cursor->fetch()) { $user = $this->userManager->get($data['user']); + if ($user === null) { + $output->writeln( + "Skipping user \"{$data['user']}\". Reason: user manager was unable to resolve the uid into the user object" + ); + continue; + } + $userMounts = $this->externalMountProvider->getMountsForUser($user, $this->loader); /** @var \OCA\Files_Sharing\External\Mount $mount */ foreach ($userMounts as $mount) { diff --git a/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php b/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php index 3b9dd52bfc54..956a14ceed1a 100644 --- a/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php +++ b/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php @@ -114,7 +114,7 @@ public function testUnavailableStorage() { $userMock = $this->createMock(IUser::class); $this->userManager->expects($this->once())->method('get') ->with($uid)->willReturn($userMock); - + $storage = $this->createMock(\OCA\Files_Sharing\External\Storage::class); $storage->method('hasUpdated')->willThrowException(new StorageNotAvailableException('Ooops')); $storage->method('getRemote')->willReturn('example.org'); @@ -133,4 +133,27 @@ public function testUnavailableStorage() { $output ); } + + public function testNotExistingUser() { + $uid = 'foo'; + $exprBuilder = $this->createMock(IExpressionBuilder::class); + $statementMock = $this->createMock(Statement::class); + $statementMock->method('fetch')->willReturnOnConsecutiveCalls(['user' => $uid], false); + $qbMock = $this->createMock(IQueryBuilder::class); + $qbMock->method('selectDistinct')->willReturnSelf(); + $qbMock->method('from')->willReturnSelf(); + $qbMock->method('where')->willReturnSelf(); + $qbMock->method('expr')->willReturn($exprBuilder); + $qbMock->method('execute')->willReturn($statementMock); + + $this->externalMountProvider->expects($this->never())->method('getMountsForUser'); + + $this->dbConnection->method('getQueryBuilder')->willReturn($qbMock); + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + $this->assertContains( + 'Skipping user "foo". Reason: user manager was unable to resolve the uid into the user object', + $output + ); + } } From a8584cd0c5605bca05f9c4a19653491041d28b72 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Tue, 16 Apr 2019 20:22:07 +0300 Subject: [PATCH 04/28] Remove unavailable shares --- .../lib/AppInfo/Application.php | 3 + .../lib/Command/PollIncomingShares.php | 34 +++++++-- .../tests/Command/PollIncomingSharesTest.php | 76 ++++++++++++++++--- apps/files_sharing/lib/External/Manager.php | 23 +++++- 4 files changed, 119 insertions(+), 17 deletions(-) diff --git a/apps/federatedfilesharing/lib/AppInfo/Application.php b/apps/federatedfilesharing/lib/AppInfo/Application.php index d50dfc7c9ae5..e89305162cd7 100644 --- a/apps/federatedfilesharing/lib/AppInfo/Application.php +++ b/apps/federatedfilesharing/lib/AppInfo/Application.php @@ -164,8 +164,10 @@ function ($c) { function ($c) use ($server) { if ($server->getAppManager()->isEnabledForUser('files_sharing')) { $sharingApp = new \OCA\Files_Sharing\AppInfo\Application(); + $externalManager = $sharingApp->getContainer()->query('ExternalManager'); $externalMountProvider = $sharingApp->getContainer()->query('ExternalMountProvider'); } else { + $externalManager = null; $externalMountProvider = null; } @@ -173,6 +175,7 @@ function ($c) use ($server) { $server->getDatabaseConnection(), $server->getUserManager(), \OC\Files\Filesystem::getLoader(), + $externalManager, $externalMountProvider ); } diff --git a/apps/federatedfilesharing/lib/Command/PollIncomingShares.php b/apps/federatedfilesharing/lib/Command/PollIncomingShares.php index c8e36635e0b3..dfcbd19a2aeb 100644 --- a/apps/federatedfilesharing/lib/Command/PollIncomingShares.php +++ b/apps/federatedfilesharing/lib/Command/PollIncomingShares.php @@ -22,7 +22,11 @@ namespace OCA\FederatedFileSharing\Command; use OC\ServerNotAvailableException; +use OC\User\NoUserException; +use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\External\MountProvider; +use OCP\Files\Mount\IMountManager; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IStorageFactory; use OCP\Files\StorageInvalidException; @@ -44,7 +48,11 @@ class PollIncomingShares extends Command { /** @var IStorageFactory */ private $loader; + /** @var Manager */ + private $externalManager; + /** @var MountProvider | null */ + private $externalMountProvider; /** @@ -55,11 +63,12 @@ class PollIncomingShares extends Command { * @param MountProvider $externalMountProvider * @param IStorageFactory $loader */ - public function __construct(IDBConnection $dbConnection, IUserManager $userManager, IStorageFactory $loader, MountProvider $externalMountProvider = null) { + public function __construct(IDBConnection $dbConnection, IUserManager $userManager, IStorageFactory $loader, Manager $externalManager = null, MountProvider $externalMountProvider = null) { parent::__construct(); $this->dbConnection = $dbConnection; $this->userManager = $userManager; $this->loader = $loader; + $this->externalManager = $externalManager; $this->externalMountProvider = $externalMountProvider; } @@ -96,9 +105,22 @@ public function execute(InputInterface $input, OutputInterface $output) { /** @var Storage $storage */ $storage = $mount->getStorage(); $this->refreshStorageRoot($storage); + } catch (NoUserException $e) { + $shareData = $this->getExternalShareData($data['user'], $mount->getMountPoint()); + $entryId = $shareData['id']; + $remote = $shareData['remote']; + // uid was null so we need to set it + $this->externalManager->setUid($data['user']); + $this->externalManager->removeShare($mount->getMountPoint()); + // and now we need to reset uid back to null + $this->externalManager->setUid(null); + $output->writeln( + "Remote \"$remote\" reports that external share with id \"$entryId\" no longer exists. Removing it.." + ); } catch (\Exception $e) { - $entryId = $this->getExternalShareId($data['user'], $mount->getMountPoint()); - $remote = $storage->getRemote(); + $shareData = $this->getExternalShareData($data['user'], $mount->getMountPoint()); + $entryId = $shareData['id']; + $remote = $shareData['remote']; $reason = $e->getMessage(); $output->writeln( "Skipping external share with id \"$entryId\" from remote \"$remote\". Reason: \"$reason\"" @@ -137,13 +159,13 @@ protected function getCursor() { return $qb->execute(); } - protected function getExternalShareId(string $userId, string $mountPoint) { + protected function getExternalShareData(string $userId, string $mountPoint) { $relativeMountPoint =\rtrim( \substr($mountPoint, \strlen("/$userId/files")), '/' ); $qb = $this->dbConnection->getQueryBuilder(); - $qb->selectDistinct('id') + $qb->select('*') ->from('share_external') ->where( $qb->expr()->eq('user', @@ -155,6 +177,6 @@ protected function getExternalShareId(string $userId, string $mountPoint) { )); $result = $qb->execute(); $externalShare = $result->fetch(); - return $externalShare['id'] ?? 0; + return $externalShare; } } diff --git a/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php b/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php index 956a14ceed1a..2c146df53b9a 100644 --- a/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php +++ b/apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php @@ -22,9 +22,12 @@ namespace OCA\FederatedFileSharing\Tests\Command; use Doctrine\DBAL\Driver\Statement; +use OC\User\NoUserException; use OCA\FederatedFileSharing\Tests\TestCase; use OCA\FederatedFileSharing\Command\PollIncomingShares; +use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\External\MountProvider; +use OCA\Files_Sharing\External\Storage; use OCP\DB\QueryBuilder\IExpressionBuilder; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Storage\IStorageFactory; @@ -32,6 +35,7 @@ use OCP\IDBConnection; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Tester\CommandTester; /** @@ -44,25 +48,31 @@ class PollIncomingSharesTest extends TestCase { /** @var CommandTester */ private $commandTester; - /** @var IDBConnection | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IDBConnection | MockObject */ private $dbConnection; - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserManager | MockObject */ private $userManager; - /** @var MountProvider | \PHPUnit_Framework_MockObject_MockObject */ + /** @var MountProvider | MockObject */ private $externalMountProvider; - /** @var IStorageFactory | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IStorageFactory | MockObject */ private $loader; + /** + * @var Manager | MockObject + */ + private $externalManager; + protected function setUp() { parent::setUp(); $this->dbConnection = $this->createMock(IDBConnection::class); $this->userManager = $this->createMock(IUserManager::class); - $this->externalMountProvider = $this->createMock(MountProvider::class); $this->loader = $this->createMock(IStorageFactory::class); - $command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->externalMountProvider); + $this->externalManager = $this->createMock(Manager::class); + $this->externalMountProvider = $this->createMock(MountProvider::class); + $command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->externalManager, $this->externalMountProvider); $this->commandTester = new CommandTester($command); } @@ -92,7 +102,7 @@ public function testNoSharesPoll() { } public function testWithFilesSharingDisabled() { - $command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, null); + $command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->externalManager, null); $this->commandTester = new CommandTester($command); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); @@ -103,8 +113,13 @@ public function testUnavailableStorage() { $uid = 'foo'; $exprBuilder = $this->createMock(IExpressionBuilder::class); $statementMock = $this->createMock(Statement::class); - $statementMock->method('fetch')->willReturnOnConsecutiveCalls(['user' => $uid], false); + $statementMock->method('fetch')->willReturnOnConsecutiveCalls( + ['user' => $uid], + ['id' => 50, 'remote' => 'example.org'], + false + ); $qbMock = $this->createMock(IQueryBuilder::class); + $qbMock->method('select')->willReturnSelf(); $qbMock->method('selectDistinct')->willReturnSelf(); $qbMock->method('from')->willReturnSelf(); $qbMock->method('where')->willReturnSelf(); @@ -115,7 +130,7 @@ public function testUnavailableStorage() { $this->userManager->expects($this->once())->method('get') ->with($uid)->willReturn($userMock); - $storage = $this->createMock(\OCA\Files_Sharing\External\Storage::class); + $storage = $this->createMock(Storage::class); $storage->method('hasUpdated')->willThrowException(new StorageNotAvailableException('Ooops')); $storage->method('getRemote')->willReturn('example.org'); @@ -129,7 +144,7 @@ public function testUnavailableStorage() { $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); $this->assertContains( - 'Skipping external share with id "0" from remote "example.org". Reason: "Ooops"', + 'Skipping external share with id "50" from remote "example.org". Reason: "Ooops"', $output ); } @@ -156,4 +171,45 @@ public function testNotExistingUser() { $output ); } + + public function testPollingUnsharedMount() { + $uid = 'foo'; + $exprBuilder = $this->createMock(IExpressionBuilder::class); + $statementMock = $this->createMock(Statement::class); + $statementMock->method('fetch')->willReturnOnConsecutiveCalls( + ['user' => $uid], + ['id' => 50, 'remote' => 'example.org'], + false + ); + $qbMock = $this->createMock(IQueryBuilder::class); + $qbMock->method('select')->willReturnSelf(); + $qbMock->method('selectDistinct')->willReturnSelf(); + $qbMock->method('from')->willReturnSelf(); + $qbMock->method('where')->willReturnSelf(); + $qbMock->method('expr')->willReturn($exprBuilder); + $qbMock->method('execute')->willReturn($statementMock); + + $userMock = $this->createMock(IUser::class); + $this->userManager->expects($this->once())->method('get') + ->with($uid)->willReturn($userMock); + + $this->externalManager->expects($this->once())->method('removeShare'); + + $storage = $this->createMock(Storage::class); + $storage->method('hasUpdated')->willThrowException(new NoUserException('Ooops')); + + $mount = $this->createMock(\OCA\Files_Sharing\External\Mount::class); + $mount->method('getStorage')->willReturn($storage); + $mount->method('getMountPoint')->willReturn("/$uid/files/point"); + $this->externalMountProvider->expects($this->once())->method('getMountsForUser') + ->willReturn([$mount]); + + $this->dbConnection->method('getQueryBuilder')->willReturn($qbMock); + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + $this->assertContains( + 'Remote "example.org" reports that external share with id "50" no longer exists. Removing it..', + $output + ); + } } diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index db92d1d17886..6b2f23cddc4f 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -30,6 +30,7 @@ namespace OCA\Files_Sharing\External; use OC\Files\Filesystem; +use OC\User\NoUserException; use OCP\Files; use OCP\Notification\IManager; use OCP\Share\Events\AcceptShare; @@ -264,7 +265,7 @@ public function processNotification($remoteShare) { * @return string */ protected function stripPath($path) { - $prefix = '/' . $this->uid . '/files'; + $prefix = "/{$this->uid}/files"; return \rtrim(\substr($path, \strlen($prefix)), '/'); } @@ -315,7 +316,27 @@ public function setMountPoint($source, $target) { return $result; } + /** + * Explicitly set uid when the shares are managed in CLI + * + * @param string|null $uid + */ + public function setUid($uid) { + // FIXME: External manager should not depend on uid + $this->uid = $uid; + } + + /** + * @param $mountPoint + * @return bool + * + * @throws NoUserException + */ public function removeShare($mountPoint) { + if ($this->uid === null) { + throw new NoUserException(); + } + $mountPointObj = $this->mountManager->find($mountPoint); $id = $mountPointObj->getStorage()->getCache()->getId(''); From 05725ec997bf82e052b82ad1d6b6961a64329833 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Thu, 18 Apr 2019 12:57:22 +0530 Subject: [PATCH 05/28] [stable10] Backport of Remove share permission check from the file upload Remove share permission check from the file upload js file. Signed-off-by: Sujith H --- apps/files/js/file-upload.js | 6 +----- .../features/bootstrap/WebUIFilesContext.php | 17 +++++++++++++++++ .../shareByPublicLink.feature | 12 ++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 1f5137c6c768..e8e7b6c8c96e 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -829,11 +829,7 @@ OC.Uploader.prototype = _.extend({ return true; } var fileInfo = fileList.findFile(file.name); - var sharePermission = $("#sharePermission").val(); - if (sharePermission !== undefined) { - sharePermission &= (OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE); - } - if (fileInfo && (sharePermission !== (OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE))) { + if (fileInfo) { conflicts.push([ // original _.extend(fileInfo, { diff --git a/tests/acceptance/features/bootstrap/WebUIFilesContext.php b/tests/acceptance/features/bootstrap/WebUIFilesContext.php index 73c7e2b90e72..1a9c0f43596d 100644 --- a/tests/acceptance/features/bootstrap/WebUIFilesContext.php +++ b/tests/acceptance/features/bootstrap/WebUIFilesContext.php @@ -1045,6 +1045,23 @@ public function theUserChoosesToInTheUploadDialog($label) { $pageObject->waitForUploadProgressbarToFinish(); } + /** + * @When the user uploads file :name and clicks :label button :number_of times using webUI + * + * @param string $name + * @param string $label + * @param int $number_of + * + * @return void + */ + public function theUserClicksUploadAndCancelMultipleTimes($name, $label, $number_of) { + for ($i = 0; $i < $number_of; $i++) { + $this->theUserUploadsFileUsingTheWebUI($name); + $this->theUserChoosesToInTheUploadDialog($label); + \usleep(STANDARD_SLEEP_TIME_MICROSEC); + } + } + /** * @Then /^the (?:deleted|moved) elements should (not|)\s?be listed on the webUI$/ * diff --git a/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature b/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature index 69504ffc495c..9b1db1b9e487 100644 --- a/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature +++ b/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature @@ -371,6 +371,7 @@ Feature: Share by public link When the user creates a new public link for folder "simple-folder" using the webUI with | permission | upload-write-without-modify | And the public accesses the last created public link using the webUI + Then it should not be possible to delete file "lorem.txt" using the webUI Scenario: user edits the permission of an already existing public link from read-write to upload-write-without-overwrite Given the user has created a new public link for folder "simple-folder" using the webUI with @@ -381,6 +382,17 @@ Feature: Share by public link Then file "lorem.txt" should be listed on the webUI And file "lorem (2).txt" should be listed on the webUI + Scenario: user creates public link with view download and upload feature and uploads and cancels same file multiple times to verify the conflict dialog exits after clicking cancel button + Given the user has created a new public link for folder "simple-folder" using the webUI with + | permission | upload-write-without-modify | + And the public accesses the last created public link using the webUI + When the user uploads file "lorem.txt" and clicks "Cancel" button 10 times using webUI + Then no dialog should be displayed on the webUI + And no notification should be displayed on the webUI + And file "lorem.txt" should be listed on the webUI + And the content of "lorem.txt" should not have changed + And file "lorem (2).txt" should not be listed on the webUI + @issue-35177 Scenario: User renames a subfolder among subfolders with same names which are shared by public links Given user "user1" has created folder "nf1" From 103a23415c42b792cf770b2a010abc2a2df98fd4 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Mon, 22 Apr 2019 15:07:20 +0300 Subject: [PATCH 06/28] Respect 'writable' appdir flag on update --- lib/private/Installer.php | 10 +++--- lib/private/legacy/app.php | 16 ++++++++- tests/lib/InstallerTest.php | 68 ++++++++++++++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/lib/private/Installer.php b/lib/private/Installer.php index a0c7daae293a..164c8bdf041d 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -222,16 +222,18 @@ public static function updateApp($info= [], $isShipped=false) { $info = self::checkAppsIntegrity($info, $extractDir, $path, $isShipped); $currentDir = OC_App::getAppPath($info['id']); + if (\is_dir("$currentDir/.git")) { + throw new AppAlreadyInstalledException("App <{$info['id']}> is a git clone - it will not be updated."); + } + $basedir = OC_App::getInstallPath(); $basedir .= '/'; $basedir .= $info['id']; - if ($currentDir !== false && \is_writable($currentDir)) { + if ($currentDir !== false && OC_App::isAppDirWritable($info['id'])) { $basedir = $currentDir; } - if (\is_dir("$basedir/.git")) { - throw new AppAlreadyInstalledException("App <{$info['id']}> is a git clone - it will not be updated."); - } + if (\is_dir($basedir)) { OC_Helper::rmdirr($basedir); } diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 5cd34e9e2228..1a12c117622c 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -568,6 +568,20 @@ public static function getAppWebPath($appId) { */ public static function isAppDirWritable($appId) { $path = self::getAppPath($appId); + // Check if the parent directory is marked as writable in config.php + if ($path !== false) { + $appDir = \substr($path, 0, -\strlen("/$appId")); + foreach (OC::$APPSROOTS as $dir) { + if ($dir['path'] !== $appDir) { + continue; + } + if (!isset($dir['writable']) + || $dir['writable'] !== true + ) { + return false; + } + } + } return ($path !== false) ? \is_writable($path) : false; } @@ -942,6 +956,7 @@ public static function getAppVersions() { * @return bool */ public static function updateApp($appId) { + \OC::$server->getAppManager()->clearAppsCache(); $appPath = self::getAppPath($appId); if ($appPath === false) { return false; @@ -958,7 +973,6 @@ public static function updateApp($appId) { } self::executeRepairSteps($appId, $appData['repair-steps']['post-migration']); self::setupLiveMigrations($appId, $appData['repair-steps']['live-migration']); - \OC::$server->getAppManager()->clearAppsCache(); // run upgrade code if (\file_exists($appPath . '/appinfo/update.php')) { self::loadApp($appId, false); diff --git a/tests/lib/InstallerTest.php b/tests/lib/InstallerTest.php index 4c8eac34afa7..ebaa4ec9e1fe 100644 --- a/tests/lib/InstallerTest.php +++ b/tests/lib/InstallerTest.php @@ -17,11 +17,12 @@ protected function setUp() { parent::setUp(); Installer::removeApp(self::$appid); + \OC::$server->getConfig()->deleteAppValues(self::$appid); } protected function tearDown() { Installer::removeApp(self::$appid); - + \OC::$server->getConfig()->deleteAppValues(self::$appid); parent::tearDown(); } @@ -89,4 +90,69 @@ public function testUpdateApp() { $this->assertNotEquals($oldVersionNumber, $newVersionNumber); } + + /** + * Tests that update is installed into writable app dir if the original app dir is not writable + */ + public function testUpdateIntoWritableAppDir() { + $oldAppRoots = \OC::$APPSROOTS; + $relativePath = "/anotherdir"; + + // Install old version + $pathOfOldTestApp = __DIR__ . '/../data/testapp.zip'; + $oldTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); + \OC_Helper::copyr($pathOfOldTestApp, $oldTmp); + $oldData = [ + 'path' => $oldTmp, + 'source' => 'path', + 'appdata' => [ + 'id' => 'testapp', + 'level' => 100, + ] + ]; + $installResult = Installer::installApp($oldData); + $this->assertEquals('testapp', $installResult); + $oldAppPath = \OC_App::getAppPath(self::$appid); + + // Mark the first app as dir non-writable and create the second as writable + $firstAppDir = \array_shift(\OC::$APPSROOTS); + $firstAppDir['writable'] = false; + + $path = \dirname($firstAppDir['path']) . $relativePath; + \mkdir($path); + \clearstatcache(); + \OC::$APPSROOTS = [ + $firstAppDir, + [ + 'path' => $path, + 'url' => $relativePath, + 'writable' => true + ] + ]; + \OC::$server->getAppManager()->clearAppsCache(); + + // Update app + $pathOfNewTestApp = __DIR__ . '/../data/testapp2.zip'; + $newTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); + \OC_Helper::copyr($pathOfNewTestApp, $newTmp); + + $newData = [ + 'path' => $newTmp, + 'source' => 'path', + 'appdata' => [ + 'id' => 'testapp', + 'level' => 100, + ] + ]; + $updateResult = Installer::updateApp($newData); + $this->assertTrue($updateResult); + $newAppPath = \OC_App::getAppPath(self::$appid); + + $this->assertNotEquals($oldAppPath, $newAppPath); + $this->assertStringStartsWith($path, $newAppPath); + + \OC_Helper::rmdirr($path); + \OC::$APPSROOTS = $oldAppRoots; + \OC::$server->getAppManager()->clearAppsCache(); + } } From f75c4644cc2ba14396c775ddb76c0528742e6eda Mon Sep 17 00:00:00 2001 From: karakayasemi Date: Fri, 26 Apr 2019 14:44:42 +0300 Subject: [PATCH 07/28] add new capability privateLinksDetailsParam --- apps/files/lib/Capabilities.php | 1 + apps/files/tests/CapabilitiesTest.php | 51 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 apps/files/tests/CapabilitiesTest.php diff --git a/apps/files/lib/Capabilities.php b/apps/files/lib/Capabilities.php index 4d332ecba303..797756bd24ad 100644 --- a/apps/files/lib/Capabilities.php +++ b/apps/files/lib/Capabilities.php @@ -58,6 +58,7 @@ public function getCapabilities() { ], 'files' => [ 'privateLinks' => true, + 'privateLinksDetailsParam' => true, 'bigfilechunking' => true, 'blacklisted_files' => $this->config->getSystemValue('blacklisted_files', ['.htaccess']), ], diff --git a/apps/files/tests/CapabilitiesTest.php b/apps/files/tests/CapabilitiesTest.php new file mode 100644 index 000000000000..54b13e4e3f0a --- /dev/null +++ b/apps/files/tests/CapabilitiesTest.php @@ -0,0 +1,51 @@ + + * + * @copyright Copyright (c) 2019, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +class CapabilitiesTest extends TestCase { + + /** @var IConfig| MockObject */ + protected $config; + + /** @var Capabilities */ + protected $capabilities; + + protected function setUp() { + parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->capabilities = new Capabilities($this->config); + } + + public function testGetCapabilities() { + $result = $this->capabilities->getCapabilities(); + $this->assertArrayHasKey('checksums', $result); + $this->assertArrayHasKey('files', $result); + $this->assertArrayHasKey('privateLinksDetailsParam', $result['files']); + $this->assertTrue($result['files']['privateLinksDetailsParam']); + } +} From 1bbc3e993484564bdcfd9a610d4dae6ca09a4daa Mon Sep 17 00:00:00 2001 From: Ben RUBSON Date: Mon, 29 Apr 2019 11:11:35 +0200 Subject: [PATCH 08/28] Avoid headers set twice in some situations --- .htaccess | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.htaccess b/.htaccess index 4c026696921a..621eb9136a8f 100644 --- a/.htaccess +++ b/.htaccess @@ -15,22 +15,30 @@ # Add security and privacy related headers + Header unset X-Content-Type-Options Header always set X-Content-Type-Options "nosniff" + Header unset X-XSS-Protection Header always set X-XSS-Protection "1; mode=block" + Header unset X-Robots-Tag Header always set X-Robots-Tag "none" + Header unset X-Frame-Options Header always set X-Frame-Options "SAMEORIGIN" + Header unset X-Download-Options Header always set X-Download-Options "noopen" + Header unset X-Permitted-Cross-Domain-Policies Header always set X-Permitted-Cross-Domain-Policies "none" SetEnv modHeadersAvailable true # Let browsers cache CSS, JS files for half a year + Header unset Cache-Control Header always set Cache-Control "max-age=15778463" # Let browsers cache WOFF files for a week + Header unset Cache-Control Header always set Cache-Control "max-age=604800" @@ -84,4 +92,3 @@ Options -Indexes ModPagespeed Off - From f0e25a4c151edd4da5a06cc2b87539fbc5e5e713 Mon Sep 17 00:00:00 2001 From: Sergey Linnik Date: Tue, 9 Apr 2019 17:42:11 +0300 Subject: [PATCH 09/28] Add the requiredPermissions property for extra share attributes --- core/js/shareitemmodel.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index 021a8e1cf8de..456c4622ec9a 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -74,6 +74,7 @@ * @property {number[]} shareType * @property {number[]} incompatiblePermissions * @property {OC.Share.Types.ShareAttribute[]} incompatibleAttributes + * @property {number[]} requiredPermissions */ /** @@ -885,6 +886,11 @@ compatible = false; } } + for(var ii in attr.requiredPermissions) { + if (!this._hasPermission(permissions, attr.requiredPermissions[ii])) { + compatible = false; + } + } if (compatible) { filteredByPermissions.push(attr); From 9c87af1cfc13833139bd28352d37d91cb4a5b299 Mon Sep 17 00:00:00 2001 From: Piotr Mrowczynski Date: Sat, 13 Apr 2019 14:05:30 +0200 Subject: [PATCH 10/28] Add js tests for share attributes --- core/js/sharedialogshareelistview.js | 6 +- core/js/shareitemmodel.js | 87 +++-- .../tests/specs/sharedialogshareelistview.js | 175 ++++++++- core/js/tests/specs/shareitemmodelSpec.js | 332 +++++++++++++++++- 4 files changed, 553 insertions(+), 47 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 8dac9550c416..73254fbd652e 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -137,19 +137,19 @@ attributes.map(function(attribute) { // Check if the share attribute set for this file is still in // registered share attributes and get its label - var label = model.getRegisteredShareAttributeLabel( + var regAttr = model.getRegisteredShareAttribute( attribute.scope, attribute.key ); - if (label) { + if (regAttr && regAttr.label) { list.push({ cid: cid, shareWith: shareWith, enabled: attribute.enabled, scope: attribute.scope, name: attribute.key, - label: label + label: regAttr.label }); } else { OC.Notification.showTemporary(t('core', 'Share with ' + diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index 456c4622ec9a..05bb83d5da56 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -38,7 +38,7 @@ * @typedef {object} OC.Share.Types.ShareInfo * @property {number} share_type * @property {number} permissions - * @property {string} attributes + * @property {OC.Share.Types.ShareAttribute[]} attributes * @property {number} file_source optional * @property {number} item_source * @property {string} token @@ -73,8 +73,8 @@ * @property {string} description * @property {number[]} shareType * @property {number[]} incompatiblePermissions - * @property {OC.Share.Types.ShareAttribute[]} incompatibleAttributes * @property {number[]} requiredPermissions + * @property {OC.Share.Types.ShareAttribute[]} incompatibleAttributes */ /** @@ -86,6 +86,13 @@ 'storage', 'share_type', 'parent', 'stime' ]; + /** + * Properties which are json and need to be parsed + */ + var SHARE_RESPONSE_JSON_PROPS = [ + 'attributes' + ]; + /** * @class OCA.Share.ShareItemModel * @classdesc @@ -172,10 +179,11 @@ } properties.permissions = defaultPermissions & possiblePermissions; + // FIXME: simplify the logic by merging and then filtering // Set default attributes for this share based on registered // attributes (filtered by their incompatible permissions) var newShareAttributes = []; - var filteredRegisteredAttributes = this.filterRegisteredAttributes(properties.permissions); + var filteredRegisteredAttributes = this._filterRegisteredAttributes(properties.permissions); _.map(filteredRegisteredAttributes, function(filteredRegisteredAttribute) { var isCompatible = true; // Check if this attribute can be added due to its incompatible attributes @@ -234,12 +242,13 @@ var self = this; options = options || {}; + // FIXME: simplify the logic by merging and then filtering // Set share attributes for this share based on registered // attributes (filtered by their incompatible permissions // and incompatible attributes) var newShareAttributes = []; var filteredAttributes = []; - var filteredRegisteredAttributes = this.filterRegisteredAttributes(properties.permissions); + var filteredRegisteredAttributes = this._filterRegisteredAttributes(properties.permissions); _.map(filteredRegisteredAttributes, function(filteredRegisteredAttribute) { // Check if this allowed registered attribute // is on the list of currently set properties, @@ -786,9 +795,15 @@ // returns integers as string... var i; for (i = 0; i < SHARE_RESPONSE_INT_PROPS.length; i++) { - var prop = SHARE_RESPONSE_INT_PROPS[i]; - if (!_.isUndefined(share[prop])) { - share[prop] = parseInt(share[prop], 10); + var propInt = SHARE_RESPONSE_INT_PROPS[i]; + if (!_.isUndefined(share[propInt])) { + share[propInt] = parseInt(share[propInt], 10); + } + } + for (i = 0; i < SHARE_RESPONSE_JSON_PROPS.length; i++) { + var propJson = SHARE_RESPONSE_JSON_PROPS[i]; + if (!_.isUndefined(share[propJson])) { + share[propJson] = JSON.parse(share.attributes); } } return share; @@ -869,6 +884,25 @@ return _.uniq(result); }, + /** + * Returns share attributes for given share index + * + * @param shareIndex + * @returns OC.Share.Types.ShareAttribute[] + */ + getShareAttributes: function(shareIndex) { + /** @type OC.Share.Types.ShareInfo **/ + var share = this.get('shares')[shareIndex]; + if(!_.isObject(share)) { + throw "Unknown Share"; + } + + if (_.isNull(share.attributes) || _.isUndefined(share.attributes)) { + return []; + } + return share.attributes; + }, + /** * Filter registered attributes by current share permissions * @@ -876,7 +910,7 @@ * @returns {OC.Share.Types.RegisteredShareAttribute[]} * @private */ - filterRegisteredAttributes: function(permissions) { + _filterRegisteredAttributes: function(permissions) { var filteredByPermissions = []; for(var i in this._registeredAttributes) { var compatible = true; @@ -900,51 +934,30 @@ return filteredByPermissions; }, - /** - * Returns share attributes for given share index - * - * @param shareIndex - * @returns OC.Share.Types.ShareAttribute[] - */ - getShareAttributes: function(shareIndex) { - /** @type OC.Share.Types.ShareInfo **/ - var share = this.get('shares')[shareIndex]; - if(!_.isObject(share)) { - throw "Unknown Share"; - } - - if (_.isUndefined(share.attributes) || _.isUndefined(share.permissions)) { - return []; - } - - // Add attributes for this share - var currentAttributes = JSON.parse(share.attributes); - if (currentAttributes) { - return currentAttributes; - } - return []; - }, - /** * Returns share attribute label for given attribute scope and name. If * attribute does not exist, null is returned. * * @param scope * @param key - * @returns string|null + * @returns {OC.Share.Types.RegisteredShareAttribute} */ - getRegisteredShareAttributeLabel: function(scope, key) { + getRegisteredShareAttribute: function(scope, key) { for(var i in this._registeredAttributes) { if (this._registeredAttributes[i].scope === scope && this._registeredAttributes[i].key === key) { - return this._registeredAttributes[i].label; + return this._registeredAttributes[i]; } } return null; }, /** - * Apps can register default share attributes + * Apps can register default share attributes. The applications + * registering share attributes are required to follow the rules: + * attribute enabled -> functionality is added (e.g. can download) + * attribute disabled -> functionality is restricted + * incompatible attribute -> functionality is ignored * * @param {OC.Share.Types.RegisteredShareAttribute} $shareAttribute */ diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index 910dc93269c4..ca865e0b259c 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -45,14 +45,14 @@ describe('OC.Share.ShareDialogShareeListView', function () { sharePermissions: 31 }); - var attributes = { + var properties = { itemType: fileInfoModel.isDirectory() ? 'folder' : 'file', itemSource: fileInfoModel.get('id'), possiblePermissions: 31, permissions: 31 }; - shareModel = new OC.Share.ShareItemModel(attributes, { + shareModel = new OC.Share.ShareItemModel(properties, { configModel: configModel, fileInfoModel: fileInfoModel }); @@ -91,7 +91,8 @@ describe('OC.Share.ShareDialogShareeListView', function () { describe('rendering', function() { it('Renders shares', function() { - shareModel.set('shares', [{ + shareModel.set('shares', [ + { id: 100, item_source: 123, permissions: 1, @@ -99,10 +100,12 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_with: 'user1', share_with_displayname: 'User One', share_with_additional_info: 'user1@example.com' - }, { + }, + { id: 101, item_source: 123, permissions: 1, + attributes: [], share_type: OC.Share.SHARE_TYPE_GROUP, share_with: 'group1', share_with_displayname: 'Group One' @@ -122,9 +125,39 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect($li.find('.username').text()).toEqual('Group One (group)'); expect($li.find('.user-additional-info').length).toEqual(0); }); + + it('renders share attribute correctly', function () { + shareModel.registerShareAttribute({ + scope: "test", + key: "test-attribute", + default: true, + label: "test attribute", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [], + incompatibleAttributes: [] + }); + + shareModel.set('shares', [{ + id: 100, + item_source: '123', + permissions: 1, + attributes: [{ scope: 'test', key: 'test-attribute', enabled: true }], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }]); + + listView.render(); + var $li = listView.$('li').eq(0); + var input = $li.find("input[name='test-attribute']"); + expect(input.is(':checked')).toEqual(true); + expect(input.labels().text()).toEqual('test attribute'); + }); }); describe('Manages checkbox events correctly', function () { + it('Checks cruds boxes when edit box checked', function () { shareModel.set('shares', [{ id: 100, @@ -196,6 +229,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(listView.$el.find("input[name='mailNotification']").length).toEqual(0); }); + it('displays error if email notification not sent', function () { var notifStub = sinon.stub(OC.dialogs, 'alert'); shareModel.set('shares', [{ @@ -222,6 +256,139 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(listView.$el.find("input[name='mailNotification']").hasClass('hidden')).toEqual(false); }); + it('share attribute can be unchecked', function () { + shareModel.registerShareAttribute({ + scope: "test", + key: "test-attribute", + default: true, + label: "test attribute", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [], + incompatibleAttributes: [] + }); + + shareModel.set('shares', [{ + id: 100, + item_source: '123', + permissions: 1, + attributes: [{ scope: 'test', key: 'test-attribute', enabled: true }], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }]); + + listView.render(); + listView.$el.find("input[name='test-attribute']").click(); + expect(listView.$el.find("input[name='test-attribute']").is(':checked')).toEqual(false); + expect(updateShareStub.calledOnce).toEqual(true); + }); + + it('share attribute checkbox enabled by checking required permission', function () { + shareModel.registerShareAttribute({ + scope: "test", + key: "test-attribute", + default: true, + label: "test attribute", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [ OC.PERMISSION_UPDATE ], + incompatibleAttributes: [] + }); + + shareModel.set('shares', [{ + id: 100, + item_source: '123', + permissions: 1, + attributes: [], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }]); + + listView.render(); + + expect(listView.$el.find("input[name='test-attribute']").length > 0).toEqual(false); + + updateShareStub.callsFake(function() { + // Updated share permission should now enable the permission + shareModel.set('shares', [{ + id: 100, + item_source: '123', + permissions: 1, + attributes: [{ scope: 'test', key: 'test-attribute', enabled: true }], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }]); + }); + + // Click on update should cause attribute test-attribute + // to appear as requiredPermissions got satisfied + listView.$el.find("input[name='update']").click(); + expect(listView.$el.find("input[name='test-attribute']").is(':checked')).toEqual(true); + expect(updateShareStub.calledOnce).toEqual(true); + }); + + it('share attribute checkbox enabled by unchecking incompatible attribute', function () { + shareModel.registerShareAttribute({ + scope: "test", + key: "incompatible-attribute", + default: true, + label: "incompatible attribute", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [ ], + incompatibleAttributes: [] + }); + shareModel.registerShareAttribute({ + scope: "test", + key: "test-attribute", + default: true, + label: "test attribute", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [ ], + incompatibleAttributes: [{ scope: 'test', key: 'incompatible-attribute', enabled: true }] + }); + + shareModel.set('shares', [{ + id: 100, + item_source: '123', + permissions: 1, + attributes: [{ scope: 'test', key: 'incompatible-attribute', enabled: true }], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }]); + + listView.render(); + + expect(listView.$el.find("input[name='incompatible-attribute']").is(':checked')).toEqual(true); + expect(listView.$el.find("input[name='test-attribute']").length > 0).toEqual(false); + + updateShareStub.callsFake(function() { + // Updated share permission should now enable the permission + shareModel.set('shares', [{ + id: 100, + item_source: '123', + permissions: 1, + attributes: [ + { scope: 'test', key: 'incompatible-attribute', enabled: false }, + { scope: 'test', key: 'test-attribute', enabled: true } + ], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }]); + }); + + listView.$el.find("input[name='incompatible-attribute']").click(); + + expect(listView.$el.find("input[name='incompatible-attribute']").is(':checked')).toEqual(false); + expect(listView.$el.find("input[name='test-attribute']").is(':checked')).toEqual(true); + expect(updateShareStub.calledOnce).toEqual(true); + }); }); }); diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js index c71fec21700a..bd245305c900 100644 --- a/core/js/tests/specs/shareitemmodelSpec.js +++ b/core/js/tests/specs/shareitemmodelSpec.js @@ -47,13 +47,13 @@ describe('OC.Share.ShareItemModel', function() { sharePermissions: 31 }); - var attributes = { + var properties = { itemType: fileInfoModel.isDirectory() ? 'folder' : 'file', itemSource: fileInfoModel.get('id'), possiblePermissions: fileInfoModel.get('sharePermissions') }; configModel = new OC.Share.ShareConfigModel(); - model = new OC.Share.ShareItemModel(attributes, { + model = new OC.Share.ShareItemModel(properties, { configModel: configModel, fileInfoModel: fileInfoModel }); @@ -118,7 +118,7 @@ describe('OC.Share.ShareItemModel', function() { fetchReshareStub = null; }); - it('populates attributes with parsed response', function() { + it('populates properties with parsed response', function() { /* jshint camelcase: false */ fetchReshareDeferred.resolve(makeOcsResponse([ { @@ -133,6 +133,7 @@ describe('OC.Share.ShareItemModel', function() { id: 100, item_source: 123, permissions: 31, + attributes: '[{"scope":"test","key":"test","enabled":false}]', share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user1', share_with_displayname: 'User One' @@ -140,6 +141,7 @@ describe('OC.Share.ShareItemModel', function() { id: 101, item_source: 123, permissions: 31, + attributes: '[]', share_type: OC.Share.SHARE_TYPE_GROUP, share_with: 'group', share_with_displayname: 'group' @@ -147,6 +149,7 @@ describe('OC.Share.ShareItemModel', function() { id: 102, item_source: 123, permissions: 31, + attributes: '[]', share_type: OC.Share.SHARE_TYPE_REMOTE, share_with: 'foo@bar.com/baz', share_with_displayname: 'foo@bar.com/baz' @@ -200,6 +203,7 @@ describe('OC.Share.ShareItemModel', function() { expect(shares.length).toEqual(3); expect(shares[0].id).toEqual(100); expect(shares[0].permissions).toEqual(31); + expect(shares[0].attributes).toEqual([{ scope: 'test', key: 'test', enabled: false }]); expect(shares[0].share_type).toEqual(OC.Share.SHARE_TYPE_USER); expect(shares[0].share_with).toEqual('user1'); expect(shares[0].share_with_displayname).toEqual('User One'); @@ -587,6 +591,328 @@ describe('OC.Share.ShareItemModel', function() { }); }); + describe('share attributes', function() { + beforeEach(function() { + model.set({ + reshare: {}, + shares: [], + _registeredAttributes: [] + }); + }); + + /** + * Creates dummy attribute + * + * @return {OC.Share.Types.RegisteredShareAttribute} registered attribute + */ + function createRegisteredAttribute() { + return { + scope: "test", + key: "test", + default: true, + label: "test", + shareType : [ + OC.Share.SHARE_TYPE_GROUP, + OC.Share.SHARE_TYPE_USER + ], + incompatiblePermissions: [], + requiredPermissions: [], + incompatibleAttributes: [] + }; + } + + /** + * Parses attributes of share creation + * request (request send to the server) + * + * @return {OC.Share.Types.ShareAttribute[]} + */ + function parseLastRequestAttributes() { + var requestBody = OC.parseQueryString(fakeServer.requests[0].requestBody); + + var i = -1; + var attributes = []; + _.map(Object.keys(requestBody), function(key) { + if (key.indexOf('attributes') !== -1) { + if (key.indexOf('scope') !== -1) { + i = i + 1; + attributes.push({}); + attributes[i].scope = requestBody[key]; + } + if (key.indexOf('key') !== -1) { + attributes[i].key = requestBody[key]; + } + if (key.indexOf('enabled') !== -1) { + attributes[i].enabled = JSON.parse(requestBody[key]); + } + } + }); + + //console.log(requestBody); + return attributes; + } + + /** + * Tests sharing with the given possible attributes + * + * @param {int} permissionsToSet + * @param {OC.Share.Types.RegisteredShareAttribute[]} attributesToRegister + * @param {Object} sharePropertiesToUpdate + * @return {OC.Share.Types.ShareAttribute[]} + */ + function testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) { + model.set({ + permissions: permissionsToSet, + possiblePermissions: permissionsToSet + }); + + _.map(attributesToRegister, function (attributeToRegister) { + model.registerShareAttribute(attributeToRegister); + }); + + if (Object.keys(sharePropertiesToUpdate).length > 0) { + // if there are properties to update, update share + model.updateShare(123, sharePropertiesToUpdate, {}); + } { + // no share properties to update, so new share + model.addShare({ + shareType: OC.Share.SHARE_TYPE_USER, + shareWith: 'user2' + }); + } + + return parseLastRequestAttributes(); + } + + describe('new share', function() { + + it('no registered attributes', function () { + // define test + var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; + var attributesToRegister = []; + var sharePropertiesToUpdate = {}; + + // define expected result and test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual([]); + }); + + it('registered attributes become default attributes', function () { + // define test + var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; + var attr1 = createRegisteredAttribute(); + var attributesToRegister = [ + attr1 + ]; + var sharePropertiesToUpdate = {}; + + // define expected result + var attributesToExpect = [ + {scope: "test", key: "test", enabled: true} + ]; + + // test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual(attributesToExpect); + }); + + it('test registering attributes with all filters', function () { + var attr1 = createRegisteredAttribute(); + attr1.key = "requires-create"; + attr1.incompatiblePermissions = []; + attr1.requiredPermissions = [OC.PERMISSION_CREATE]; + attr1.incompatibleAttributes = []; + + var attr2 = createRegisteredAttribute(); + attr2.key = "requires-update"; + attr2.incompatiblePermissions = []; + attr2.requiredPermissions = [OC.PERMISSION_UPDATE]; + attr2.incompatibleAttributes = []; + + var attr3 = createRegisteredAttribute(); + attr3.key = "incompatible-create"; + attr3.incompatiblePermissions = [OC.PERMISSION_CREATE]; + attr3.requiredPermissions = []; + attr3.incompatibleAttributes = []; + + var attr4 = createRegisteredAttribute(); + attr4.key = "incompatible-update"; + attr4.incompatiblePermissions = [OC.PERMISSION_UPDATE]; + attr4.requiredPermissions = []; + attr4.incompatibleAttributes = []; + + var attr5 = createRegisteredAttribute(); + attr5.key = "incompatible-attribute-requires-update-true"; + attr5.incompatiblePermissions = []; + attr5.requiredPermissions = []; + attr5.incompatibleAttributes = [{ + scope: "test", + key: "requires-update", + enabled: true + }]; + + var attr6 = createRegisteredAttribute(); + attr6.key = "incompatible-attribute-requires-update-false"; + attr6.incompatiblePermissions = []; + attr6.requiredPermissions = []; + attr6.incompatibleAttributes = [{ + scope: "test", + key: "requires-update", + enabled: false + }]; + + // register attribute + var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; + var attributesToRegister = [ + attr1, attr2, attr3, attr4, attr5, attr6 + ]; + + // this test does not update existing share + var sharePropertiesToUpdate = {}; + + // expect that new created share will have following attributes + var attributesToExpect = [ + {scope: "test", key: "requires-update", enabled: true}, + {scope: "test", key: "incompatible-create", enabled: true}, + { + scope: "test", + key: "incompatible-attribute-requires-update-false", + enabled: true + } + ]; + + // test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual(attributesToExpect); + }); + }); + + describe('update share', function() { + it('update with new attribute but none registered (error handling scenario)', function() { + // define test + var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; + var attributesToRegister = []; + var sharePropertiesToUpdate = { + attributes: [ + { scope: "test", key: "test", enabled: false } + ], + permissions: permissionsToSet + }; + + // define expected result and test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual([]); + }); + + it('update existing default attribute with new enabled value', function() { + // define test + var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; + + var attr1 = createRegisteredAttribute(); + attr1.scope = "test"; + attr1.key = "test"; + attr1.default = true; + var attributesToRegister = [attr1]; + + var sharePropertiesToUpdate = { + attributes: [ + { scope: "test", key: "test", enabled: false } + ], + permissions: permissionsToSet + }; + + // define expected result + var attributesToExpect = [ + { scope: "test", key: "test", enabled: false } + ]; + + // test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual(attributesToExpect); + }); + + it('update permission with incompatible/required permissions filter', function() { + // define test + var permissionsToSet = OC.PERMISSION_READ; + + var attr1 = createRegisteredAttribute(); + attr1.key = "incompatible-create"; + attr1.incompatiblePermissions = [ OC.PERMISSION_CREATE ]; + attr1.requiredPermissions = []; + attr1.incompatibleAttributes = []; + var attr2 = createRegisteredAttribute(); + attr2.key = "required-create"; + attr2.incompatiblePermissions = []; + attr2.requiredPermissions = [ OC.PERMISSION_CREATE ]; + attr2.incompatibleAttributes = []; + + var attributesToRegister = [attr1, attr2]; + + var sharePropertiesToUpdate = { + permissions: OC.PERMISSION_READ | OC.PERMISSION_CREATE + }; + + // define expected result + var attributesToExpect = [ + { scope: "test", key: "required-create", enabled: true } + ]; + + // test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual(attributesToExpect); + }); + + it('update permissions with incompatible filter for previously enabled attribute', function() { + // define test + var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE; + + // test attribute which is enabled only without update permission + var attr1 = createRegisteredAttribute(); + attr1.key = "incompatible-update"; + attr1.incompatiblePermissions = [ OC.PERMISSION_UPDATE ]; + attr1.requiredPermissions = [ ]; + attr1.incompatibleAttributes = []; + + // test attribute which is not available when review attr is enabled + var attr2 = createRegisteredAttribute(); + attr2.key = "incompatible-with-attribute"; + attr2.incompatiblePermissions = []; + attr2.requiredPermissions = []; + attr2.incompatibleAttributes = [{ scope: "test", key: "incompatible-update", enabled: true }]; + + // test attribute that can always be registered + var attr3 = createRegisteredAttribute(); + attr3.key = "test-attribute"; + attr3.incompatiblePermissions = []; + attr3.requiredPermissions = []; + attr3.incompatibleAttributes = []; + + var attributesToRegister = [attr1, attr2, attr3]; + + var sharePropertiesToUpdate = { + permissions: OC.PERMISSION_READ + }; + + // define expected result - restricted-options should not appear + var attributesToExpect = [ + { scope: "test", key: "incompatible-update", enabled: true }, + { scope: "test", key: "test-attribute", enabled: true } + ]; + + // test + expect( + testShareWithAttributes(permissionsToSet, attributesToRegister, sharePropertiesToUpdate) + ).toEqual(attributesToExpect); + }); + }); + }); + describe('creating shares', function() { it('sends POST method to endpoint with passed values', function() { model.addShare({ From 6a4958e4d3c36f902b1fccbc7d9e3b1e6fda8d2f Mon Sep 17 00:00:00 2001 From: Piotr Mrowczynski Date: Mon, 29 Apr 2019 18:48:09 +0200 Subject: [PATCH 11/28] Refactor share attribute js tests --- .../tests/specs/sharedialogshareelistview.js | 20 +++++++++---------- core/js/tests/specs/shareitemmodelSpec.js | 14 ++++++------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index ca865e0b259c..1e0f8e0a8d03 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -140,7 +140,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { shareModel.set('shares', [{ id: 100, - item_source: '123', + item_source: 123, permissions: 1, attributes: [{ scope: 'test', key: 'test-attribute', enabled: true }], share_type: OC.Share.SHARE_TYPE_USER, @@ -152,7 +152,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { var $li = listView.$('li').eq(0); var input = $li.find("input[name='test-attribute']"); expect(input.is(':checked')).toEqual(true); - expect(input.labels().text()).toEqual('test attribute'); + expect($("label[for='" + input.attr('id') + "']").text()).toEqual('test attribute'); }); }); @@ -256,7 +256,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(listView.$el.find("input[name='mailNotification']").hasClass('hidden')).toEqual(false); }); - it('share attribute can be unchecked', function () { + it('unchecks share attribute when clicked on checked', function () { shareModel.registerShareAttribute({ scope: "test", key: "test-attribute", @@ -270,7 +270,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { shareModel.set('shares', [{ id: 100, - item_source: '123', + item_source: 123, permissions: 1, attributes: [{ scope: 'test', key: 'test-attribute', enabled: true }], share_type: OC.Share.SHARE_TYPE_USER, @@ -284,7 +284,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(updateShareStub.calledOnce).toEqual(true); }); - it('share attribute checkbox enabled by checking required permission', function () { + it('shows share attribute checkbox when checking required permission', function () { shareModel.registerShareAttribute({ scope: "test", key: "test-attribute", @@ -298,7 +298,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { shareModel.set('shares', [{ id: 100, - item_source: '123', + item_source: 123, permissions: 1, attributes: [], share_type: OC.Share.SHARE_TYPE_USER, @@ -314,7 +314,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { // Updated share permission should now enable the permission shareModel.set('shares', [{ id: 100, - item_source: '123', + item_source: 123, permissions: 1, attributes: [{ scope: 'test', key: 'test-attribute', enabled: true }], share_type: OC.Share.SHARE_TYPE_USER, @@ -330,7 +330,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(updateShareStub.calledOnce).toEqual(true); }); - it('share attribute checkbox enabled by unchecking incompatible attribute', function () { + it('shows share attribute checkbox when unchecking incompatible attribute', function () { shareModel.registerShareAttribute({ scope: "test", key: "incompatible-attribute", @@ -354,7 +354,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { shareModel.set('shares', [{ id: 100, - item_source: '123', + item_source: 123, permissions: 1, attributes: [{ scope: 'test', key: 'incompatible-attribute', enabled: true }], share_type: OC.Share.SHARE_TYPE_USER, @@ -371,7 +371,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { // Updated share permission should now enable the permission shareModel.set('shares', [{ id: 100, - item_source: '123', + item_source: 123, permissions: 1, attributes: [ { scope: 'test', key: 'incompatible-attribute', enabled: false }, diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js index bd245305c900..261408d42f11 100644 --- a/core/js/tests/specs/shareitemmodelSpec.js +++ b/core/js/tests/specs/shareitemmodelSpec.js @@ -686,7 +686,7 @@ describe('OC.Share.ShareItemModel', function() { describe('new share', function() { - it('no registered attributes', function () { + it('returns no attributes when no registered attributes', function () { // define test var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; var attributesToRegister = []; @@ -698,7 +698,7 @@ describe('OC.Share.ShareItemModel', function() { ).toEqual([]); }); - it('registered attributes become default attributes', function () { + it('uses registered attributes as default attributes', function () { // define test var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; var attr1 = createRegisteredAttribute(); @@ -718,7 +718,7 @@ describe('OC.Share.ShareItemModel', function() { ).toEqual(attributesToExpect); }); - it('test registering attributes with all filters', function () { + it('properly filters registered attributes', function () { var attr1 = createRegisteredAttribute(); attr1.key = "requires-create"; attr1.incompatiblePermissions = []; @@ -791,7 +791,7 @@ describe('OC.Share.ShareItemModel', function() { }); describe('update share', function() { - it('update with new attribute but none registered (error handling scenario)', function() { + it('returns no attributes when update with new attribute but none registered (error handling scenario)', function() { // define test var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; var attributesToRegister = []; @@ -808,7 +808,7 @@ describe('OC.Share.ShareItemModel', function() { ).toEqual([]); }); - it('update existing default attribute with new enabled value', function() { + it('updates attribute with new enabled value correctly', function() { // define test var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE; @@ -836,7 +836,7 @@ describe('OC.Share.ShareItemModel', function() { ).toEqual(attributesToExpect); }); - it('update permission with incompatible/required permissions filter', function() { + it('uses/hides attributes with permission filters of registered attributes', function() { // define test var permissionsToSet = OC.PERMISSION_READ; @@ -868,7 +868,7 @@ describe('OC.Share.ShareItemModel', function() { ).toEqual(attributesToExpect); }); - it('update permissions with incompatible filter for previously enabled attribute', function() { + it('uses/hides attributes with attribute filter of registered attributes when permission changes', function() { // define test var permissionsToSet = OC.PERMISSION_READ | OC.PERMISSION_UPDATE; From 57e2e0de232a122fd27e812cf2b1b9dc8e334c5d Mon Sep 17 00:00:00 2001 From: Piotr Mrowczynski Date: Sat, 27 Apr 2019 12:53:24 +0200 Subject: [PATCH 12/28] When reshared disallow changing share attributes --- core/js/sharedialogshareelistview.js | 6 +- .../tests/specs/sharedialogshareelistview.js | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 73254fbd652e..f0bf569a732e 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -69,7 +69,7 @@ '
' + '{{#each shareAttributes}}' + '' + - '' + + '' + '' + '' + '{{/each}}' + @@ -129,6 +129,9 @@ var cid = this.cid; var shareWith = model.getShareWith(shareIndex); + // Check if reshare, and if so disable the checkboxes + var isReshare = model.hasReshare(); + // Returns OC.Share.Types.ShareAttribute[] which were set for this // share (and stored in DB) var attributes = model.getShareAttributes(shareIndex); @@ -145,6 +148,7 @@ if (regAttr && regAttr.label) { list.push({ cid: cid, + isReshare: isReshare, shareWith: shareWith, enabled: attribute.enabled, scope: attribute.scope, diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index 1e0f8e0a8d03..ee2fc72b4bfe 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -389,6 +389,64 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(listView.$el.find("input[name='test-attribute']").is(':checked')).toEqual(true); expect(updateShareStub.calledOnce).toEqual(true); }); + + it('prevents checking/unchecking attribute when reshared', function () { + shareModel.registerShareAttribute({ + scope: "test", + key: "test-attribute-checked", + default: true, + label: "test attribute checked", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [], + incompatibleAttributes: [] + }); + shareModel.registerShareAttribute({ + scope: "test", + key: "test-attribute-unchecked", + default: false, + label: "test attribute unchecked", + shareType : [], + incompatiblePermissions: [], + requiredPermissions: [], + incompatibleAttributes: [] + }); + + shareModel.set('reshare', { + uid_owner: 100 + }); + shareModel.set('shares', [ + { + id: 100, + item_source: 123, + permissions: 1, + attributes: [{ scope: 'test', key: 'test-attribute-checked', enabled: true }], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user1', + share_with_displayname: 'User One' + }, + { + id: 101, + item_source: 123, + permissions: 1, + attributes: [{ scope: 'test', key: 'test-attribute-unchecked', enabled: false }], + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user2', + share_with_displayname: 'User Two' + } + ]); + + listView.render(); + + // Click should have no action + listView.$el.find("input[name='test-attribute-checked']").click(); + expect(listView.$el.find("input[name='test-attribute-checked']").is(':checked')).toEqual(true); + listView.$el.find("input[name='test-attribute-unchecked']").click(); + expect(listView.$el.find("input[name='test-attribute-unchecked']").is(':checked')).toEqual(false); + + // Update never called when clicked + expect(updateShareStub.called).toEqual(false); + }); }); }); From 8e5d3414d0a2f3949e2978a6d9dfc7562c7958d0 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Tue, 30 Apr 2019 19:13:36 +0530 Subject: [PATCH 13/28] [stable10] Backport of Aborted uploads are not cleared properly File uploads should also be checked if they are aborted or not. This would help to check if the file is aborted or not. If aborted then it would be cleared from the list. Signed-off-by: Sujith H --- apps/files/js/file-upload.js | 3 ++- apps/files/tests/js/fileUploadSpec.js | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index e8e7b6c8c96e..1b5825ec075c 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -682,6 +682,7 @@ OC.Uploader.prototype = _.extend({ this.log('canceling uploads'); jQuery.each(this._uploads, function(i, upload) { upload.abort(); + upload.aborted = true; }); this.clear(); }, @@ -691,7 +692,7 @@ OC.Uploader.prototype = _.extend({ clear: function() { var remainingUploads = {}; _.each(this._uploads, function(upload, key) { - if (!upload.isDone) { + if (!upload.isDone && !upload.aborted) { remainingUploads[key] = upload; } }); diff --git a/apps/files/tests/js/fileUploadSpec.js b/apps/files/tests/js/fileUploadSpec.js index 9854f62e0898..eae6ce796379 100644 --- a/apps/files/tests/js/fileUploadSpec.js +++ b/apps/files/tests/js/fileUploadSpec.js @@ -133,11 +133,13 @@ describe('OC.Upload tests', function() { it('clear leaves pending uploads', function() { uploader._uploads = { 'abc': {name: 'a job well done.txt', isDone: true}, - 'def': {name: 'whatevs.txt'} + 'def': {name: 'whatevs.txt'}, + 'ghi': {name: 'aborted.txt', aborted: true} }; uploader.clear(); + //This does verify that aborted upload(s) will not be available in the _uploads expect(uploader._uploads).toEqual({'def': {name: 'whatevs.txt'}}); }); }); From 8ea76aa8677c9c9de6137411947a647f668d6d79 Mon Sep 17 00:00:00 2001 From: karakayasemi Date: Tue, 30 Apr 2019 22:44:22 +0300 Subject: [PATCH 14/28] [10.2.0] Backport of dont show notification if federated share autoaccepted --- .../lib/FedShareManager.php | 33 ++++++++++--------- .../lib/FederatedShareProvider.php | 2 +- .../tests/FederatedShareProviderTest.php | 6 +--- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/apps/federatedfilesharing/lib/FedShareManager.php b/apps/federatedfilesharing/lib/FedShareManager.php index 7f904854598f..d290fc6b2618 100644 --- a/apps/federatedfilesharing/lib/FedShareManager.php +++ b/apps/federatedfilesharing/lib/FedShareManager.php @@ -130,8 +130,9 @@ public function createShare(Address $ownerAddress, $token ) { $owner = $ownerAddress->getUserId(); + $remote = $ownerAddress->getOrigin(); $shareId = $this->federatedShareProvider->addShare( - $ownerAddress->getOrigin(), $token, $name, $owner, $shareWith, $remoteId + $remote, $token, $name, $owner, $shareWith, $remoteId ); $this->eventDispatcher->dispatch( @@ -163,20 +164,22 @@ public function createShare(Address $ownerAddress, $sharedByAddress->getCloudId(), \trim($name, '/') ]; - $notification = $this->createNotification($shareWith); - $notification->setDateTime(new \DateTime()) - ->setObject('remote_share', $shareId) - ->setSubject('remote_share', $params) - ->setMessage('remote_share', $params); - $declineAction = $notification->createAction(); - $declineAction->setLabel('decline') - ->setLink($link, 'DELETE'); - $notification->addAction($declineAction); - $acceptAction = $notification->createAction(); - $acceptAction->setLabel('accept') - ->setLink($link, 'POST'); - $notification->addAction($acceptAction); - $this->notificationManager->notify($notification); + if (!$this->federatedShareProvider->getAccepted($remote, $shareWith)) { + $notification = $this->createNotification($shareWith); + $notification->setDateTime(new \DateTime()) + ->setObject('remote_share', $shareId) + ->setSubject('remote_share', $params) + ->setMessage('remote_share', $params); + $declineAction = $notification->createAction(); + $declineAction->setLabel('decline') + ->setLink($link, 'DELETE'); + $notification->addAction($declineAction); + $acceptAction = $notification->createAction(); + $acceptAction->setLabel('accept') + ->setLink($link, 'POST'); + $notification->addAction($acceptAction); + $this->notificationManager->notify($notification); + } } /** diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index cfe4db97fa92..6b217cd56c9f 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -1053,7 +1053,7 @@ public function addShare($remote, $token, $name, $owner, $shareWith, $remoteId) * * @return bool */ - protected function getAccepted($remote, $shareWith) { + public function getAccepted($remote, $shareWith) { $event = $this->eventDispatcher->dispatch( 'remoteshare.received', new GenericEvent('', ['remote' => $remote]) diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index e805b9afaa83..f5a7ddea4483 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -919,11 +919,7 @@ public function testGetAccepted($autoAddServers, $globalAutoAccept, $userAutoAcc ->with('remoteshare.received', $this->anything()) ->willReturn($event); - $shouldAutoAccept = $this->invokePrivate( - $this->provider, - 'getAccepted', - ['remote', 'user@server.com'] - ); + $shouldAutoAccept = $this->provider->getAccepted('remote', 'user@server.com'); $this->assertEquals($expected, $shouldAutoAccept); } From 89fa7854aaf40d107f82145a2248c3da0321e84d Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 2 May 2019 10:39:33 +0300 Subject: [PATCH 15/28] Set mail_send via old share API --- .../lib/Controller/Share20OcsController.php | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index f10585fe4bec..bf75a622b07b 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -924,12 +924,13 @@ public function declineShare($id) { * @NoAdminRequired * * @param int $itemSource + * @param string $itemType * @param int $shareType * @param string $recipient * * @return Result */ - public function notifyRecipients($itemSource, $shareType, $recipient) { + public function notifyRecipients($itemSource, $itemType, $shareType, $recipient) { $recipientList = []; if ($shareType === Share::SHARE_TYPE_USER) { $recipientList[] = $this->userManager->get($recipient); @@ -964,12 +965,8 @@ public function notifyRecipients($itemSource, $shareType, $recipient) { // if we were able to send to at least one recipient, mark as sent // allowing the user to resend would spam users who already got a notification if (\count($result) < \count($recipientList)) { - $items = $this->shareManager->getSharedWith($recipient, $shareType, $node); - if (\count($items) > 0) { - $share = $items[0]; - $share->setMailSend(true); - $this->shareManager->updateShare($share); - } + // FIXME: migrate to a new share API + Share::setSendMailStatus($itemType, $itemSource, $shareType, $recipient, true); } $message = empty($result) @@ -988,22 +985,15 @@ public function notifyRecipients($itemSource, $shareType, $recipient) { * @NoAdminRequired * * @param int $itemSource + * @param string $itemType * @param int $shareType * @param string $recipient * * @return Result */ - public function notifyRecipientsDisabled($itemSource, $shareType, $recipient) { - $userFolder = $this->rootFolder->getUserFolder($this->currentUser->getUID()); - $nodes = $userFolder->getById($itemSource, true); - $node = $nodes[0] ?? null; - - $items = $this->shareManager->getSharedWith($recipient, $shareType, $node); - if (\count($items) > 0) { - $share = $items[0]; - $share->setMailSend(true); - $this->shareManager->updateShare($share); - } + public function notifyRecipientsDisabled($itemSource, $itemType, $shareType, $recipient) { + // FIXME: migrate to a new share API + Share::setSendMailStatus($itemType, $itemSource, $shareType, $recipient, true); return new Result(); } From f68f12f38381af2adc0c78185559535536c0d26f Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 2 May 2019 11:49:01 +0200 Subject: [PATCH 16/28] Update changelog for 10.2.0 RC2 --- CHANGELOG.md | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72737c0d9b3e..2e4bc330d398 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,16 +4,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). -## [Unreleased] +## [10.2.0] ### Added +- Add new capability to advertise the availability of the detail parameter for private links - [#35104](https://github.com/owncloud/core/issues/35104) - Add background:queue:execute occ command for running cron jobs manually - [#34995](https://github.com/owncloud/core/issues/34995) - Adding background:queue commands: status and delete - [#34783](https://github.com/owncloud/core/issues/34783) -- Added new permissions option for public link - [#34983](https://github.com/owncloud/core/issues/34983) +- Added new permissions option for public link - [#34983](https://github.com/owncloud/core/issues/34983) [#35082](https://github.com/owncloud/core/issues/35082) - Support for extra share key-value attributes - [#34951](https://github.com/owncloud/core/issues/34951) -- Internal permission to prevent file download when set in share attribute, for "secure view" feature - [#34951](https://github.com/owncloud/core/issues/34951) -- Support for automatically accepting incoming federated shares from trusted servers - [#34206](https://github.com/owncloud/core/issues/34206) +- Internal permission to prevent file download when set in share attribute, for "secure view" feature - [#34951](https://github.com/owncloud/core/issues/34951) [#35095](https://github.com/owncloud/core/issues/35095) +- Support for automatically accepting incoming federated shares from trusted servers - [#34206](https://github.com/owncloud/core/issues/34206) [#35135](https://github.com/owncloud/core/issues/35135) - User option for automatically accepting incoming shares - [#34647](https://github.com/owncloud/core/pull/34647) [#34842](https://github.com/owncloud/core/pull/34842) [#34934](https://github.com/owncloud/core/issues/34934) - User option for automatically accepting incoming federated shares - [#34706](https://github.com/owncloud/core/issues/34706) - User option to opt-out autocomplete in share dialog - [#34942](https://github.com/owncloud/core/issues/34942) @@ -31,7 +32,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Allow admins to enable medial search on group and user - [#34779](https://github.com/owncloud/core/issues/34779) - Add composer cleaner - [#34784](https://github.com/owncloud/core/issues/34784) - Add events for user preference changes - [#34820](https://github.com/owncloud/core/issues/34820) -- Add occ command to poll incoming federated shares for updates - [#34933](https://github.com/owncloud/core/issues/34933) [#34959](https://github.com/owncloud/core/issues/34959) [#34993](https://github.com/owncloud/core/issues/34993) +- Add occ command to poll incoming federated shares for updates - [#34933](https://github.com/owncloud/core/issues/34933) [#34959](https://github.com/owncloud/core/issues/34959) [#34993](https://github.com/owncloud/core/issues/34993) [#35073](https://github.com/owncloud/core/issues/35073) ### Changed @@ -59,14 +60,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Bump phpseclib/phpseclib from 2.0.13 to 2.0.15 - [#34285](https://github.com/owncloud/core/issues/34285) [#34741](https://github.com/owncloud/core/issues/34741) - Bump pimple/pimple from 3.0.2 to 3.2.3 - [#31753](https://github.com/owncloud/core/issues/31753) - Bump sinon from 7.1.1 to 7.3.1 in /build - [#34881](https://github.com/owncloud/core/issues/34881) [#34943](https://github.com/owncloud/core/issues/34943) -- Bump symfony v3.4.20 => v3.4.24 - [#34042](https://github.com/owncloud/core/issues/34042) [#34663](https://github.com/owncloud/core/issues/34663) [#34954](https://github.com/owncloud/core/issues/34954) -- Bump symfony/process from 3.4.21 to 3.4.22 - [#34407](https://github.com/owncloud/core/issues/34407) -- Bump symfony/translation from 3.4.21 to 3.4.22 - [#34406](https://github.com/owncloud/core/issues/34406) -- Bump symfony/console from 3.4.21 to 3.4.22 - [#34404](https://github.com/owncloud/core/issues/34404) +- Bump symfony and modules to 3.4.26 - [#35062](https://github.com/owncloud/core/issues/35062) - Bump symfony/polyfill components from v1.10.0 to v1.11.0 - [#34882](https://github.com/owncloud/core/pull/34882) -- Bump symfony/routing from 3.4.21 to 3.4.22 - [#34408](https://github.com/owncloud/core/issues/34408) -- Bump symfony/event-dispatcher from 3.4.21 to 3.4.22 - [#34405](https://github.com/owncloud/core/issues/34405) -- Bump remaining symfony 3.4.22 components - [#34412](https://github.com/owncloud/core/issues/34412) - Bump deepdiver1975/tarstreamer from 0.1.0 to 0.1.1 - [#34615](https://github.com/owncloud/core/issues/34615) - Bump zendframework/zend-servicemanager from 3.3.2 to 3.4.0 - [#33971](https://github.com/owncloud/core/issues/33971) - Bump zendframework/zend-inputfilter from 2.9.0 to 2.9.1 - [#34145](https://github.com/owncloud/core/issues/34145) @@ -80,7 +75,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Add email footer with motto in email for changing password - [#34498](https://github.com/owncloud/core/issues/34498) - Change the styling of the active settings navigation menu item - [#34561](https://github.com/owncloud/core/issues/34561) - Added delay in search field - [#34613](https://github.com/owncloud/core/issues/34613) -- Tidy up code for notification by email - [#34786](https://github.com/owncloud/core/issues/34786) +- Tidy up code for notification by email - [#34786](https://github.com/owncloud/core/issues/34786) [#35137](https://github.com/owncloud/core/issues/35137) - Some code now made PHP 7 specific - [#34925](https://github.com/owncloud/core/issues/34925) ### Removed @@ -92,6 +87,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Fixed +- Wrong translation file referenced for accept & decline share - [#35063](https://github.com/owncloud/core/issues/35063) +- Respect 'writable' appdir flag on update - [#35097](https://github.com/owncloud/core/issues/35097) +- Aborted uploads in web UI are now properly cleared - [#35134](https://github.com/owncloud/core/issues/35134) +- Fix regression with missing progress bar in files drop view - [#35059](https://github.com/owncloud/core/issues/35059) - Log exception when background job class not found - [#34723](https://github.com/owncloud/core/issues/34723) - Prevent concurrent updates in group shares to avoid duplicate entries - [#34769](https://github.com/owncloud/core/issues/34769) - Calender invitation now uses actual sender name - [#34901](https://github.com/owncloud/core/issues/34901) @@ -135,7 +134,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fix reset confirmation mail from occ - [#34154](https://github.com/owncloud/core/issues/34154) - Correctly write Login failed entry in log when 2FA is enforced - [#34055](https://github.com/owncloud/core/issues/34055) - Center the logo and login fields - [#34057](https://github.com/owncloud/core/issues/34057) -- Fix Apache warnings by setting headers to "always" in htaccess - [#34089](https://github.com/owncloud/core/issues/34089) +- Fix Apache warnings by setting headers to "always" in htaccess - [#34089](https://github.com/owncloud/core/issues/34089) [#35118](https://github.com/owncloud/core/issues/35118) - Fix external storage advanced checkbox state issue - [#34168](https://github.com/owncloud/core/issues/34168) - Set permissions on log file creation instead of every write - [#34061](https://github.com/owncloud/core/issues/34061) - Images are again properly rotated now based on EXIF rotation - [#34356](https://github.com/owncloud/core/issues/34356) @@ -1023,7 +1022,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - provisioning API now also returns the user's home path: [#26850](https://github.com/owncloud/core/issues/26850) - web updater shows link to changelog in admin page: [#26796](https://github.com/owncloud/core/issues/26796) -[Unreleased]: https://github.com/owncloud/core/compare/v10.1.1...stable10 +[10.2.0]: https://github.com/owncloud/core/compare/v10.1.1...v10.2.0 [10.1.1]: https://github.com/owncloud/core/compare/v10.1.0...v10.1.1 [10.1.0]: https://github.com/owncloud/core/compare/v10.0.10...v10.1.0 [10.0.10]: https://github.com/owncloud/core/compare/v10.0.9...v10.0.10 From 40fe2a66673a5ecc1bfb52eeac5bf68e9b2d0a8b Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Thu, 2 May 2019 17:43:40 +0200 Subject: [PATCH 17/28] Set version to 10.2.0RC2 --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index c763517a5448..29edda16d211 100644 --- a/version.php +++ b/version.php @@ -25,10 +25,10 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [10, 2, 0, 1]; +$OC_Version = [10, 2, 0, 2]; // The human readable string -$OC_VersionString = '10.2.0 RC1'; +$OC_VersionString = '10.2.0 RC2'; $OC_VersionCanBeUpgradedFrom = [[8, 2, 11],[9, 0, 9],[9, 1]]; From 7d0c168e864443677e290d28fb4caf1a6c3c0506 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Mon, 6 May 2019 15:23:20 +0530 Subject: [PATCH 18/28] [10.2.0] Backport of Fix update share permissions for public link When user tries to change the permission from permission 15 to permission 7 ( i.e, View/Download/ Upload), the permissions are not updated properly. The reason for this is due to the overwriting of newpermission variable with permission 15. This changeset removes the overwriting to newpermission variable. Signed-off-by: Sujith H --- .../lib/Controller/Share20OcsController.php | 3 -- .../Controller/Share20OcsControllerTest.php | 14 +++++-- .../updateShare.feature | 37 ++++++++++++++++++- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index bf75a622b07b..0b2cde87615e 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -800,9 +800,6 @@ public function updateShare($id) { $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new Result(null, 400, $this->l->t('Public upload is only possible for publicly shared folders')); } - - // normalize to correct public upload permissions - $newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE; } // create-only (upload-only) diff --git a/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php b/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php index b725701e5baa..533418670803 100644 --- a/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php +++ b/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php @@ -1773,9 +1773,15 @@ public function testUpdateLinkShareEnablePublicUpload($params) { $this->shareManager->expects($this->once())->method('updateShare')->with( $this->callback(function (\OCP\Share\IShare $share) { - return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && - $share->getPassword() === 'password' && - $share->getExpirationDate() === null; + if ($share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE)) { + return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && + $share->getPassword() === 'password' && + $share->getExpirationDate() === null; + } else { + return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE) && + $share->getPassword() === 'password' && + $share->getExpirationDate() === null; + } }) )->will($this->returnArgument(0)); @@ -2052,7 +2058,7 @@ public function testUpdateLinkSharePermissions() { $this->shareManager->expects($this->once())->method('updateShare')->with( $this->callback(function (\OCP\Share\IShare $share) use ($date) { - return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) && + return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE) && $share->getPassword() === 'password' && $share->getExpirationDate() === $date; }) diff --git a/tests/acceptance/features/apiShareManagementBasic/updateShare.feature b/tests/acceptance/features/apiShareManagementBasic/updateShare.feature index 5a6afe829a29..39c301f04ec2 100644 --- a/tests/acceptance/features/apiShareManagementBasic/updateShare.feature +++ b/tests/acceptance/features/apiShareManagementBasic/updateShare.feature @@ -149,7 +149,7 @@ Feature: sharing When the user creates a public link share using the sharing API with settings | path | FOLDER | And the user updates the last share using the sharing API with - | permissions | 7 | + | permissions | 15 | And the user gets the info of the last share using the sharing API Then the OCS status code should be "" And the HTTP status code should be "200" @@ -175,6 +175,39 @@ Feature: sharing | 1 | 100 | | 2 | 200 | + @public_link_share-feature-required + Scenario Outline: Creating a new public link share, updating its permissions to view download and upload and getting its info + Given using OCS API version "" + And as user "user0" + When the user creates a public link share using the sharing API with settings + | path | FOLDER | + And the user updates the last share using the sharing API with + | permissions | 7 | + And the user gets the info of the last share using the sharing API + Then the OCS status code should be "" + And the HTTP status code should be "200" + And the fields of the last response should include + | id | A_NUMBER | + | item_type | folder | + | item_source | A_NUMBER | + | share_type | 3 | + | file_source | A_NUMBER | + | file_target | /FOLDER | + | permissions | 7 | + | stime | A_NUMBER | + | token | A_TOKEN | + | storage | A_NUMBER | + | mail_send | 0 | + | uid_owner | user0 | + | file_parent | A_NUMBER | + | displayname_owner | User Zero | + | url | AN_URL | + | mimetype | httpd/unix-directory | + Examples: + | ocs_api_version | ocs_status_code | + | 1 | 100 | + | 2 | 200 | + @public_link_share-feature-required Scenario Outline: Creating a new public link share, updating publicUpload option and getting its info Given using OCS API version "" @@ -406,4 +439,4 @@ Feature: sharing Examples: | ocs_api_version | ocs_status_code | | 1 | 100 | - | 2 | 200 | \ No newline at end of file + | 2 | 200 | From a06b142d42a07c3b88821430cfdac0eae09bea09 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Thu, 9 May 2019 22:50:37 +0530 Subject: [PATCH 19/28] [10.2.0] Backport of Fix permission and update behaviour for public link Remove update permission for Download/View/Upload option for public link. And hence the behaviour of public links created using this option has changed. 1. User cannot delete the files/folders inside this public link. 2. User cannot update the files which are already present in the public link 3. User cannot rename files. Signed-off-by: Sujith H --- apps/files/js/file-upload.js | 5 +++++ .../lib/Controller/Share20OcsController.php | 3 ++- core/js/sharedialoglinkshareview.js | 4 ++-- tests/TestHelpers/SharingHelper.php | 4 ++-- .../createShare.feature | 4 ++-- .../apiShareOperations/uploadToShare.feature | 5 +---- .../features/bootstrap/WebUIFilesContext.php | 6 ++---- .../shareByPublicLink.feature | 21 +++++++------------ 8 files changed, 24 insertions(+), 28 deletions(-) diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 1b5825ec075c..dfa4aa0b7dc5 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -831,6 +831,11 @@ OC.Uploader.prototype = _.extend({ } var fileInfo = fileList.findFile(file.name); if (fileInfo) { + var sharePermission = parseInt($("#sharePermission").val()); + if (sharePermission === (OC.PERMISSION_READ | OC.PERMISSION_CREATE)) { + OC.Notification.show(t('files', 'The file {file} already exists', {file: fileInfo.name}), {type: 'error'}); + return false; + } conflicts.push([ // original _.extend(fileInfo, { diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index 0b2cde87615e..d78fcfa9d903 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -461,7 +461,7 @@ public function createShare() { ); } elseif ($permissions === \OCP\Constants::PERMISSION_CREATE || $permissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) || - $permissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_CREATE)) { + $permissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE)) { $share->setPermissions($permissions); } else { // because when "publicUpload" is passed usually no permissions are set, @@ -776,6 +776,7 @@ public function updateShare($id) { if ($newPermissions !== null && $newPermissions !== Constants::PERMISSION_READ && $newPermissions !== Constants::PERMISSION_CREATE && + $newPermissions !== (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE) && // legacy $newPermissions !== (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE) && // correct diff --git a/core/js/sharedialoglinkshareview.js b/core/js/sharedialoglinkshareview.js index 12c837e72400..070aed725b19 100644 --- a/core/js/sharedialoglinkshareview.js +++ b/core/js/sharedialoglinkshareview.js @@ -265,8 +265,8 @@ publicUploadWriteLabel : t('core', 'Download / View / Upload'), publicUploadWriteDescription : t('core', 'Recipients can view, download and upload contents.'), - publicUploadWriteValue : OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE, - publicUploadWriteSelected : this.model.get('permissions') === (OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_CREATE), + publicUploadWriteValue : OC.PERMISSION_READ | OC.PERMISSION_CREATE, + publicUploadWriteSelected : this.model.get('permissions') === (OC.PERMISSION_READ | OC.PERMISSION_CREATE), publicReadWriteLabel : t('core', 'Download / View / Edit'), publicReadWriteDescription : t('core', 'Recipients can view, download and edit contents.'), diff --git a/tests/TestHelpers/SharingHelper.php b/tests/TestHelpers/SharingHelper.php index 1b65b90bbc84..e2a3a5bce04f 100644 --- a/tests/TestHelpers/SharingHelper.php +++ b/tests/TestHelpers/SharingHelper.php @@ -50,7 +50,7 @@ class SharingHelper { * 1 = read; 2 = update; 4 = create; * 8 = delete; 16 = share; 31 = all * 15 = change - * 7 = uploadwriteonly + * 5 = uploadwriteonly * (default: 31, for public shares: 1) * Pass either the (total) number, * or the keyword, @@ -171,7 +171,7 @@ public static function getPermissionSum($permissions) { 'read' => 1, 'update' => 2, 'create' => 4, - 'uploadwriteonly' => 7, + 'uploadwriteonly' => 5, 'delete' => 8, 'change' => 15, 'share' => 16, diff --git a/tests/acceptance/features/apiShareManagementBasic/createShare.feature b/tests/acceptance/features/apiShareManagementBasic/createShare.feature index d42ce09493bd..cfa92dd8cd76 100644 --- a/tests/acceptance/features/apiShareManagementBasic/createShare.feature +++ b/tests/acceptance/features/apiShareManagementBasic/createShare.feature @@ -224,13 +224,13 @@ Feature: sharing And user "user0" has created folder "/afolder" When user "user0" creates a public link share using the sharing API with settings | path | /afolder | - | permissions | 7 | + | permissions | 5 | Then the OCS status code should be "" And the HTTP status code should be "200" And the share fields of the last share should include | id | A_NUMBER | | share_type | 3 | - | permissions | 7 | + | permissions | 5 | Examples: | ocs_api_version | ocs_status_code | | 1 | 100 | diff --git a/tests/acceptance/features/apiShareOperations/uploadToShare.feature b/tests/acceptance/features/apiShareOperations/uploadToShare.feature index 1781c21a9f77..6a0f8902931c 100644 --- a/tests/acceptance/features/apiShareOperations/uploadToShare.feature +++ b/tests/acceptance/features/apiShareOperations/uploadToShare.feature @@ -328,8 +328,5 @@ Feature: sharing | path | FOLDER | | permissions | uploadwriteonly | When the public uploads file "test.txt" with content "test" using the public WebDAV API - And the public uploads file "test.txt" with content "test2" with autorename mode using the public WebDAV API - And the public uploads file "test.txt" with content "test3" with autorename mode using the public WebDAV API + And the public uploads file "test.txt" with content "test2" using the public WebDAV API Then the content of file "/FOLDER/test.txt" for user "user0" should be "test" - And the content of file "/FOLDER/test (2).txt" for user "user0" should be "test2" - And the content of file "/FOLDER/test (3).txt" for user "user0" should be "test3" diff --git a/tests/acceptance/features/bootstrap/WebUIFilesContext.php b/tests/acceptance/features/bootstrap/WebUIFilesContext.php index 1a9c0f43596d..7a7f71f270f5 100644 --- a/tests/acceptance/features/bootstrap/WebUIFilesContext.php +++ b/tests/acceptance/features/bootstrap/WebUIFilesContext.php @@ -1046,18 +1046,16 @@ public function theUserChoosesToInTheUploadDialog($label) { } /** - * @When the user uploads file :name and clicks :label button :number_of times using webUI + * @When the user uploads file :name :number_of times using webUI * * @param string $name - * @param string $label * @param int $number_of * * @return void */ - public function theUserClicksUploadAndCancelMultipleTimes($name, $label, $number_of) { + public function theUserClicksUploadAndCancelMultipleTimes($name, $number_of) { for ($i = 0; $i < $number_of; $i++) { $this->theUserUploadsFileUsingTheWebUI($name); - $this->theUserChoosesToInTheUploadDialog($label); \usleep(STANDARD_SLEEP_TIME_MICROSEC); } } diff --git a/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature b/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature index 9b1db1b9e487..494cba0399a1 100644 --- a/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature +++ b/tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature @@ -373,22 +373,17 @@ Feature: Share by public link And the public accesses the last created public link using the webUI Then it should not be possible to delete file "lorem.txt" using the webUI - Scenario: user edits the permission of an already existing public link from read-write to upload-write-without-overwrite - Given the user has created a new public link for folder "simple-folder" using the webUI with - | permission | read-write | - When the user changes the permission of the public link named "Public link" to "upload-write-without-modify" - And the public accesses the last created public link using the webUI - When the user uploads file "lorem.txt" keeping both new and existing files using the webUI - Then file "lorem.txt" should be listed on the webUI - And file "lorem (2).txt" should be listed on the webUI - - Scenario: user creates public link with view download and upload feature and uploads and cancels same file multiple times to verify the conflict dialog exits after clicking cancel button + Scenario: user creates public link with view download and upload feature and uploads same file name and verifies no auto-renamed file seen in UI Given the user has created a new public link for folder "simple-folder" using the webUI with | permission | upload-write-without-modify | And the public accesses the last created public link using the webUI - When the user uploads file "lorem.txt" and clicks "Cancel" button 10 times using webUI - Then no dialog should be displayed on the webUI - And no notification should be displayed on the webUI + When the user uploads file "lorem.txt" 5 times using webUI + Then notifications should be displayed on the webUI with the text + | The file lorem.txt already exists | + | The file lorem.txt already exists | + | The file lorem.txt already exists | + | The file lorem.txt already exists | + | The file lorem.txt already exists | And file "lorem.txt" should be listed on the webUI And the content of "lorem.txt" should not have changed And file "lorem (2).txt" should not be listed on the webUI From 1de4ca6cf323d114e49f76fca000f9f133332715 Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Fri, 10 May 2019 20:31:10 +0200 Subject: [PATCH 20/28] Set version to 10.2.0RC3 --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index 29edda16d211..20bb8fdd69bc 100644 --- a/version.php +++ b/version.php @@ -25,10 +25,10 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [10, 2, 0, 2]; +$OC_Version = [10, 2, 0, 3]; // The human readable string -$OC_VersionString = '10.2.0 RC2'; +$OC_VersionString = '10.2.0 RC3'; $OC_VersionCanBeUpgradedFrom = [[8, 2, 11],[9, 0, 9],[9, 1]]; From f5ef4add7f633f1f77f4dbf31122f8d7dae029b0 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Tue, 14 May 2019 13:24:06 +0300 Subject: [PATCH 21/28] Treat send attempt unsuccessful when there is a message. --- .../lib/Controller/Share20OcsController.php | 14 +++++++++----- core/js/sharedialogshareelistview.js | 2 +- core/js/tests/specs/sharedialogshareelistview.js | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index d78fcfa9d903..a1923cf981c1 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -967,13 +967,17 @@ public function notifyRecipients($itemSource, $itemType, $shareType, $recipient) Share::setSendMailStatus($itemType, $itemSource, $shareType, $recipient, true); } - $message = empty($result) - ? null - : $this->l->t( + if (empty($result)) { + $message = null; + $data = ['status' => 'success']; + } else { + $message = $this->l->t( "Couldn't send mail to following recipient(s): %s ", \implode(', ', $result) ); - return new Result([], 200, $message); + $data = ['status' => 'error']; + } + return new Result($data, 200, $message); } /** @@ -992,7 +996,7 @@ public function notifyRecipients($itemSource, $itemType, $shareType, $recipient) public function notifyRecipientsDisabled($itemSource, $itemType, $shareType, $recipient) { // FIXME: migrate to a new share API Share::setSendMailStatus($itemType, $itemSource, $shareType, $recipient, true); - return new Result(); + return new Result(['status' => 'success']); } /** diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index f0bf569a732e..29583d284a5a 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -369,7 +369,7 @@ $loading.removeClass('hidden'); this.model.sendNotificationForShare(shareType, shareWith, true).then(function(result) { - if (result.ocs.meta.status === 'ok') { + if (result.ocs.data.status === 'success') { OC.Notification.showTemporary(t('core', 'Email notification was sent!')); $target.remove(); } else { diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index ee2fc72b4bfe..05ce4405dbb7 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -221,7 +221,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(notificationStub.called).toEqual(true); notificationStub.restore(); - deferred.resolve({ ocs: { meta: {status: 'ok' }}}); + deferred.resolve({ ocs: { data : { status: 'success'}, meta: {message: null }}}); expect(notifStub.calledOnce).toEqual(true); notifStub.restore(); @@ -247,7 +247,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(notificationStub.called).toEqual(true); notificationStub.restore(); - deferred.resolve({ ocs: { meta: {status: 'error', message: 'message'}}}); + deferred.resolve({ ocs: { data: {status: 'error'}, meta: {message: 'message'}}}); expect(notifStub.calledOnce).toEqual(true); notifStub.restore(); From e9ee4927b54ed4a17e25ec9913525e315986f95d Mon Sep 17 00:00:00 2001 From: Sujith H Date: Wed, 15 May 2019 16:42:37 +0530 Subject: [PATCH 22/28] [10.2.0] Backport of The public link edit template was outside public upload check The public link edit template was outside public upload check. And hence for creating the public link for files the full permission option was shown. This change tries to address the issue. Signed-off-by: Sujith H --- core/js/sharedialoglinkshareview.js | 2 +- .../specs/sharedialoglinkshareviewSpec.js | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/core/js/sharedialoglinkshareview.js b/core/js/sharedialoglinkshareview.js index 070aed725b19..f7c9c8597a5f 100644 --- a/core/js/sharedialoglinkshareview.js +++ b/core/js/sharedialoglinkshareview.js @@ -27,12 +27,12 @@ '' + '

{{publicReadDescription}}

' + '
' + + '{{#if publicUploadPossible}}' + '
' + '' + '' + '

{{publicReadWriteDescription}}

' + '
' + - '{{#if publicUploadPossible}}' + '
' + '' + '' + diff --git a/core/js/tests/specs/sharedialoglinkshareviewSpec.js b/core/js/tests/specs/sharedialoglinkshareviewSpec.js index 08e536a230f3..42ccf9fed4e0 100644 --- a/core/js/tests/specs/sharedialoglinkshareviewSpec.js +++ b/core/js/tests/specs/sharedialoglinkshareviewSpec.js @@ -209,7 +209,54 @@ describe('OC.Share.ShareDialogLinkShareView', function() { }); view.render(); expect(view.$('.publicPermissions').length).toEqual(4); + expect(view.$('.publicPermissions').context.innerHTML).toContain('Download / View'); + expect(view.$('.publicPermissions').context.innerHTML).toContain('Download / View / Edit'); + expect(view.$('.publicPermissions').context.innerHTML).toContain('Download / View / Upload'); + expect(view.$('.publicPermissions').context.innerHTML).toContain('Upload only (File Drop)'); }); + it('renders listing radio buttons for file when public upload is allowed globally', function () { + fileInfoModel = new OCA.Files.FileInfoModel({ + id: '123', + name: 'file.txt', + path: '/subdir', + size: 100, + mimetype: 'text/plain', + permissions: 31, + sharePermissions: 31 + }); + itemModel = new OC.Share.ShareItemModel({ + itemType: 'file', + itemSource: 123, + permissions: 31 + }, { + configModel: configModel, + fileInfoModel: fileInfoModel + }); + + model = new OC.Share.ShareModel({ + id: 1, + name: 'first link', + token: 'tehtokenz', + shareType: OC.Share.SHARE_TYPE_LINK, + itemType: 'file', + stime: 1489657516, + permissions: OC.PERMISSION_READ + }); + + view = new OC.Share.ShareDialogLinkShareView({ + model: model, + itemModel: itemModel + }); + + model.set({ + permissions: OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE + }); + + view.render(); + expect(view.$('.publicPermissions').length).toEqual(1); + expect(view.$('.publicPermissions').context.innerHTML).toContain('Download / View'); + }); + it('renders checkbox disabled when public upload is disallowed by user', function() { publicUploadConfigStub.returns(true); model.set({ From f29eb61cba5ae43ad139c4d48be938cb08971ccd Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Thu, 16 May 2019 09:10:24 +0200 Subject: [PATCH 23/28] Update changelog for 10.2.0 RC4 Signed-off-by: Patrick Jahns --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e4bc330d398..864d0a5f3192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Add new capability to advertise the availability of the detail parameter for private links - [#35104](https://github.com/owncloud/core/issues/35104) - Add background:queue:execute occ command for running cron jobs manually - [#34995](https://github.com/owncloud/core/issues/34995) -- Adding background:queue commands: status and delete - [#34783](https://github.com/owncloud/core/issues/34783) -- Added new permissions option for public link - [#34983](https://github.com/owncloud/core/issues/34983) [#35082](https://github.com/owncloud/core/issues/35082) +- Adding background:queue commands: status and delete - [#34783](https://github.com/owncloud/core/issues/34783) [#35228](https://github.com/owncloud/core/pull/35228) +- Added new permissions option for public link - [#34983](https://github.com/owncloud/core/issues/34983) [#35082](https://github.com/owncloud/core/issues/35082)[#35159](https://github.com/owncloud/core/pull/35159)[#35197](https://github.com/owncloud/core/pull/35197)[#35238](https://github.com/owncloud/core/pull/35238) - Support for extra share key-value attributes - [#34951](https://github.com/owncloud/core/issues/34951) - Internal permission to prevent file download when set in share attribute, for "secure view" feature - [#34951](https://github.com/owncloud/core/issues/34951) [#35095](https://github.com/owncloud/core/issues/35095) - Support for automatically accepting incoming federated shares from trusted servers - [#34206](https://github.com/owncloud/core/issues/34206) [#35135](https://github.com/owncloud/core/issues/35135) @@ -100,7 +100,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Reduce sharing query size by properly reusing the query builder - [#34915](https://github.com/owncloud/core/issues/34915) - Tar download support for file names longer than 99 chars - [#34615](https://github.com/owncloud/core/issues/34615) - Fix Webdav error page, include CSP and message - [#34817](https://github.com/owncloud/core/issues/34817) -- Handle accept decline with invalid share id - [#34786](https://github.com/owncloud/core/issues/34786) +- Handle accept decline with invalid share id - [#34786](https://github.com/owncloud/core/issues/34786) [#35221](https://github.com/owncloud/core/pull/35221) - Normalize path when moving chunks to final destination - [#34777](https://github.com/owncloud/core/issues/34777) - Better support for international email addresses after swiftmailer update - [#34759](https://github.com/owncloud/core/issues/34759) - Fix first time login handling - [#34758](https://github.com/owncloud/core/issues/34758) From f025bf4477adc61c7ac719f85f9f78d635ae56c8 Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Thu, 16 May 2019 09:13:32 +0200 Subject: [PATCH 24/28] set version to 10.2.0RC4 Signed-off-by: Patrick Jahns --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index 20bb8fdd69bc..f93af68a6779 100644 --- a/version.php +++ b/version.php @@ -25,10 +25,10 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [10, 2, 0, 3]; +$OC_Version = [10, 2, 0, 4]; // The human readable string -$OC_VersionString = '10.2.0 RC3'; +$OC_VersionString = '10.2.0 RC4'; $OC_VersionCanBeUpgradedFrom = [[8, 2, 11],[9, 0, 9],[9, 1]]; From f43648704baa174fb21e9f298f9318d9944ca268 Mon Sep 17 00:00:00 2001 From: David Toledo Date: Thu, 16 May 2019 10:05:24 +0200 Subject: [PATCH 25/28] Update README.md file with 10.2 version --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b09776c3b75d..7b5d260bbfe7 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ A personal cloud which runs on your own server.** * ... ## Installation instructions -https://doc.owncloud.org/server/latest/admin_manual/installation/ +https://doc.owncloud.org/server/10.2/admin_manual/installation/ ## Contribution Guidelines https://owncloud.org/contribute/ @@ -38,4 +38,4 @@ Please submit translations via Transifex: https://www.transifex.com/projects/p/owncloud/ For detailed information about translations: -https://doc.owncloud.org/server/10.1/developer_manual/core/translation.html +https://doc.owncloud.org/server/10.2/developer_manual/core/translation.html From 27cf63d3bbcdd094ba4685b9cc30575e68ba8ddb Mon Sep 17 00:00:00 2001 From: paurakhsharma Date: Tue, 14 May 2019 09:45:14 +0545 Subject: [PATCH 26/28] Add webUI tests for share mail notification --- .drone.yml | 2 + tests/acceptance/config/behat.yml | 2 + .../bootstrap/WebUISharingContext.php | 38 +++++++++- .../lib/FilesPageElement/SharingDialog.php | 22 +++++- .../shareWithGroups.feature | 74 +++++++++++++++++++ .../shareWithUsers.feature | 49 +++++++++++- 6 files changed, 183 insertions(+), 4 deletions(-) diff --git a/.drone.yml b/.drone.yml index 82c91bf792cb..33ee1b18f179 100644 --- a/.drone.yml +++ b/.drone.yml @@ -1317,6 +1317,7 @@ matrix: CHOWN_SERVER: true OWNCLOUD_LOG: true INSTALL_TESTING_APP: true + USE_EMAIL: true - PHP_VERSION: 7.1 TEST_SUITE: selenium @@ -1329,6 +1330,7 @@ matrix: CHOWN_SERVER: true OWNCLOUD_LOG: true INSTALL_TESTING_APP: true + USE_EMAIL: true - PHP_VERSION: 7.1 TEST_SUITE: selenium diff --git a/tests/acceptance/config/behat.yml b/tests/acceptance/config/behat.yml index 8bb11299b492..c172c89f012d 100644 --- a/tests/acceptance/config/behat.yml +++ b/tests/acceptance/config/behat.yml @@ -429,6 +429,7 @@ default: - WebUIGeneralContext: - WebUILoginContext: - WebUISharingContext: + - EmailContext: - WebUIAdminSharingSettingsContext: webUISharingInternalUsers: @@ -440,6 +441,7 @@ default: - WebUIGeneralContext: - WebUILoginContext: - WebUISharingContext: + - EmailContext: - WebUIAdminSharingSettingsContext: - WebUIPersonalSharingSettingsContext: diff --git a/tests/acceptance/features/bootstrap/WebUISharingContext.php b/tests/acceptance/features/bootstrap/WebUISharingContext.php index 8f1943ef329e..b23731cada47 100644 --- a/tests/acceptance/features/bootstrap/WebUISharingContext.php +++ b/tests/acceptance/features/bootstrap/WebUISharingContext.php @@ -675,7 +675,7 @@ public function userReactsToShareOfferedByUsingWebUI( $fileRows = $this->sharedWithYouPage->findAllFileRowsByName( $share, $this->getSession() ); - + $found = false; foreach ($fileRows as $fileRow) { if ($offeredBy === $fileRow->getSharer()) { @@ -776,6 +776,40 @@ public function removesPublicLinkAtCertainPosition($number, $entryName) { $this->sharingDialog->removePublicLink($session, $number); } + /** + * @When the user sends the share notification by email using the webUI + * + * @return void + */ + public function theUserSendsTheShareNotificationByEmailUsingTheWebui() { + PHPUnit\Framework\Assert::assertNotNull( + $this->sharingDialog, "Sharing Dialog is not open" + ); + $this->sharingDialog->sendShareNotificationByEmail($this->getSession()); + } + + /** + * @Then the user should not be able to send the share notification by email using the webUI + * + * @return void + */ + public function theUserShouldNotBeAbleToSendTheShareNotificationByEmailUsingTheWebui() { + $errorMessage = ""; + PHPUnit\Framework\Assert::assertNotNull( + $this->sharingDialog, "Sharing Dialog is not open" + ); + try { + $this->sharingDialog->sendShareNotificationByEmail($this->getSession()); + } catch (Exception $e) { + $errorMessage = $e->getMessage(); + } + PHPUnit\Framework\Assert::assertContains( + "could not find notify by email button", + $errorMessage, + "User was not expected to be able to send the share notification by email but was" + ); + } + /** * @Then the public should not get access to the publicly shared file * @@ -988,7 +1022,7 @@ public function fileFolderShouldBeMarkedAsSharedBy( $this->filesPage->closeDetailsDialog(); } catch (Exception $e) { } - + $row = $this->filesPage->findFileRowByName($itemName, $this->getSession()); $sharingBtn = $row->findSharingButton(); PHPUnit\Framework\Assert::assertSame( diff --git a/tests/acceptance/features/lib/FilesPageElement/SharingDialog.php b/tests/acceptance/features/lib/FilesPageElement/SharingDialog.php index c3eafd9f5e16..3a655e1818ed 100644 --- a/tests/acceptance/features/lib/FilesPageElement/SharingDialog.php +++ b/tests/acceptance/features/lib/FilesPageElement/SharingDialog.php @@ -58,6 +58,7 @@ class SharingDialog extends OwncloudPage { private $noSharingMessageXpath = "//div[@class='noSharingPlaceholder']"; private $publicLinkRemoveBtnXpath = "//div[contains(@class, 'removeLink')]"; private $publicLinkTitleXpath = "//span[@class='link-entry--title']"; + private $notifyByEmailBtnXpath = "//input[@name='mailNotification']"; private $shareWithListXpath = "//ul[@id='shareWithList']/li"; private $userNameSpanXpath = "//span[contains(@class,'username')]"; @@ -209,7 +210,7 @@ public function shareWithUserOrGroup( $userElements = $autocompleteNodeElement->findAll( "xpath", $this->autocompleteItemsTextXpath ); - + $userFound = false; foreach ($userElements as $user) { if ($this->getTrimmedText($user) === $nameToMatch) { @@ -620,6 +621,25 @@ public function isUserPresentInShareWithList($username) { return false; } + /** + * send share notification by email to the sharee + * + * @param Session $session + * + * @return void + */ + public function sendShareNotificationByEmail($session) { + $notifyByEmailBtn = $this->find("xpath", $this->notifyByEmailBtnXpath); + $this->assertElementNotNull( + $notifyByEmailBtn, + __METHOD__ . + " xpath $this->notifyByEmailBtnXpath " . + "could not find notify by email button" + ); + $notifyByEmailBtn->click(); + $this->waitForAjaxCallsToStartAndFinish($session); + } + /** * waits for the dialog to appear * diff --git a/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature b/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature index 2c24dd7e7264..1b7c563e970c 100644 --- a/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature +++ b/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature @@ -130,3 +130,77 @@ Feature: Sharing files and folders with internal groups And the user types "system-group" in the share-with-field Then a tooltip with the text "No users or groups found for system-group" should be shown near the share-with-field on the webUI And the autocomplete list should not be displayed on the webUI + + @mailhog + Scenario: user should be able to send notification by email when allow share mail notification has been enabled + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And user "user3" has logged in using the webUI + And user "user3" has shared file "lorem.txt" with group "grp1" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI + Then a notification should be displayed on the webUI with the text "Email notification was sent!" + And the email address "user1@example.org" should have received an email with the body containing + """ + just letting you know that User Three shared lorem.txt with you. + """ + And the email address "user2@example.org" should have received an email with the body containing + """ + just letting you know that User Three shared lorem.txt with you. + """ + + @mailhog + Scenario: user should not be able to send notification by email more than once + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And user "user3" has logged in using the webUI + And user "user3" has shared file "lorem.txt" with group "grp1" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI + Then the user should not be able to send the share notification by email using the webUI + When the user reloads the current page of the webUI + And the user opens the share dialog for file "lorem.txt" + Then the user should not be able to send the share notification by email using the webUI + + Scenario: user should not be able to send notification by email when allow share mail notification has been disabled + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "no" + And user "user3" has logged in using the webUI + And user "user3" has shared file "lorem.txt" with group "grp1" + When the user opens the share dialog for file "lorem.txt" + Then the user should not be able to send the share notification by email using the webUI + + @mailhog + Scenario: user should not get an email notification if the user is added to the group after the mail notification was sent + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And user "user0" has been created with default attributes + And user "user3" has logged in using the webUI + And user "user3" has shared file "lorem.txt" with group "grp1" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI + Then a notification should be displayed on the webUI with the text "Email notification was sent!" + When the administrator adds user "user0" to group "grp1" using the provisioning API + Then the email address "user0@example.org" should not have received an email + + @mailhog @issue-35218 + Scenario: user should get an error message when trying to send notification by email to the group where some user have set up their email and others haven't + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And these users have been created: + | username | + | brand-new-user | + | off-brand-new-user | + And user "brand-new-user" has been added to group "grp1" + And user "off-brand-new-user" has been added to group "grp1" + And user "user3" has logged in using the webUI + And user "user3" has shared file "lorem.txt" with group "grp1" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI + Then a notification should be displayed on the webUI with the text "Email notification was sent!" +# Then dialog should be displayed on the webUI +# | title | content | +# | Email notification not sent | Couldn't send mail to following recipient(s): brand-new-user, off-brand-new-user | + And the email address "user1@example.org" should have received an email with the body containing + """ + just letting you know that User Three shared lorem.txt with you. + """ + And the email address "user2@example.org" should have received an email with the body containing + """ + just letting you know that User Three shared lorem.txt with you. + """ diff --git a/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature b/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature index 43993a7d3f13..3a64a82810d1 100644 --- a/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature +++ b/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature @@ -232,4 +232,51 @@ Feature: Sharing files and folders with internal users And the user deletes share with user "User Two" for the current file Then the user "User Two" should not be in share with user list And file "lorem.txt" should not be listed in shared-with-others page on the webUI - And as "user2" file "lorem (2).txt" should not exist \ No newline at end of file + And as "user2" file "lorem (2).txt" should not exist + + @mailhog + Scenario: user should be able to send notification by email when allow share mail notification has been enabled + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And user "user1" has logged in using the webUI + And user "user1" has shared file "lorem.txt" with user "user2" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI + Then a notification should be displayed on the webUI with the text "Email notification was sent!" + And the email address "user2@example.org" should have received an email with the body containing + """ + just letting you know that User One shared lorem.txt with you. + """ + + @mailhog @issue-35218 + Scenario: user should get and error message when trying to send notification by email to a user who has not setup their email + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And these users have been created: + |username|password| + |user0 |1234 | + And user "user1" has logged in using the webUI + And user "user1" has shared file "lorem.txt" with user "user0" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI +# Then dialog should be displayed on the webUI +# | title | content | +# | Email notification not sent | Couldn't send mail to following recipient(s): user0 | + Then a notification should be displayed on the webUI with the text "Email notification was sent!" + + @mailhog + Scenario: user should not be able to send notification by email more than once + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" + And user "user1" has logged in using the webUI + And user "user1" has shared file "lorem.txt" with user "user2" + And the user has opened the share dialog for file "lorem.txt" + When the user sends the share notification by email using the webUI + Then the user should not be able to send the share notification by email using the webUI + When the user reloads the current page of the webUI + And the user opens the share dialog for file "lorem.txt" + Then the user should not be able to send the share notification by email using the webUI + + Scenario: user should not be able to send notification by email when allow share mail notification has been disabled + Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "no" + And user "user1" has logged in using the webUI + And user "user1" has shared file "lorem.txt" with user "user2" + When the user opens the share dialog for file "lorem.txt" + Then the user should not be able to send the share notification by email using the webUI From 073cff924dce5cbb7330f88a3f6b7523defdf434 Mon Sep 17 00:00:00 2001 From: paurakhsharma Date: Thu, 16 May 2019 12:21:27 +0545 Subject: [PATCH 27/28] Unskip behat step and remove issue tag --- .../webUISharingInternalGroups/shareWithGroups.feature | 9 ++++----- .../webUISharingInternalUsers/shareWithUsers.feature | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature b/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature index 1b7c563e970c..016b8b89cbcd 100644 --- a/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature +++ b/tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature @@ -179,7 +179,7 @@ Feature: Sharing files and folders with internal groups When the administrator adds user "user0" to group "grp1" using the provisioning API Then the email address "user0@example.org" should not have received an email - @mailhog @issue-35218 + @mailhog Scenario: user should get an error message when trying to send notification by email to the group where some user have set up their email and others haven't Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" And these users have been created: @@ -192,10 +192,9 @@ Feature: Sharing files and folders with internal groups And user "user3" has shared file "lorem.txt" with group "grp1" And the user has opened the share dialog for file "lorem.txt" When the user sends the share notification by email using the webUI - Then a notification should be displayed on the webUI with the text "Email notification was sent!" -# Then dialog should be displayed on the webUI -# | title | content | -# | Email notification not sent | Couldn't send mail to following recipient(s): brand-new-user, off-brand-new-user | + Then dialog should be displayed on the webUI + | title | content | + | Email notification not sent | Couldn't send mail to following recipient(s): brand-new-user, off-brand-new-user | And the email address "user1@example.org" should have received an email with the body containing """ just letting you know that User Three shared lorem.txt with you. diff --git a/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature b/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature index 3a64a82810d1..453806f52018 100644 --- a/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature +++ b/tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature @@ -247,7 +247,7 @@ Feature: Sharing files and folders with internal users just letting you know that User One shared lorem.txt with you. """ - @mailhog @issue-35218 + @mailhog Scenario: user should get and error message when trying to send notification by email to a user who has not setup their email Given parameter "shareapi_allow_mail_notification" of app "core" has been set to "yes" And these users have been created: @@ -257,10 +257,9 @@ Feature: Sharing files and folders with internal users And user "user1" has shared file "lorem.txt" with user "user0" And the user has opened the share dialog for file "lorem.txt" When the user sends the share notification by email using the webUI -# Then dialog should be displayed on the webUI -# | title | content | -# | Email notification not sent | Couldn't send mail to following recipient(s): user0 | - Then a notification should be displayed on the webUI with the text "Email notification was sent!" + Then dialog should be displayed on the webUI + | title | content | + | Email notification not sent | Couldn't send mail to following recipient(s): user0 | @mailhog Scenario: user should not be able to send notification by email more than once From 7fe2d2cdcfe990d8b092cc2cc4e36b6e3f56bacf Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Thu, 16 May 2019 17:16:07 +0200 Subject: [PATCH 28/28] set version to 10.2.0 Signed-off-by: Patrick Jahns --- CHANGELOG.md | 2 +- version.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 864d0a5f3192..d10f35fac39a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). -## [10.2.0] +## [10.2.0] - 2019-05-16 ### Added diff --git a/version.php b/version.php index f93af68a6779..e5d4cdac9645 100644 --- a/version.php +++ b/version.php @@ -25,10 +25,10 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [10, 2, 0, 4]; +$OC_Version = [10, 2, 0, 5]; // The human readable string -$OC_VersionString = '10.2.0 RC4'; +$OC_VersionString = '10.2.0'; $OC_VersionCanBeUpgradedFrom = [[8, 2, 11],[9, 0, 9],[9, 1]];