Merge pull request #21109 from nextcloud/backport/19793/stable18

[stable18] Fix resharing of federated shares that were created out of links
This commit is contained in:
Roeland Jago Douma 2020-05-28 16:37:28 +02:00 committed by GitHub
commit 876479799e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 45 additions and 43 deletions

View file

@ -48,6 +48,7 @@ use OCP\ISession;
use OCP\IUserSession;
use OCP\Share\IManager;
use OCP\Util;
use OCP\Share\IShare;
/**
* Class MountPublicLinkController
@ -161,6 +162,7 @@ class MountPublicLinkController extends Controller {
}
$share->setSharedWith($shareWith);
$share->setShareType(IShare::TYPE_REMOTE);
try {
$this->federatedShareProvider->create($share);

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View file

@ -490,16 +490,21 @@ class ShareAPIController extends OCSController {
throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders'));
}
$share->setPermissions(
Constants::PERMISSION_READ |
$permissions = Constants::PERMISSION_READ |
Constants::PERMISSION_CREATE |
Constants::PERMISSION_UPDATE |
Constants::PERMISSION_DELETE
);
Constants::PERMISSION_DELETE;
} else {
$share->setPermissions(Constants::PERMISSION_READ);
$permissions = Constants::PERMISSION_READ;
}
// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
if (($permissions & Constants::PERMISSION_READ) && $this->shareManager->outgoingServer2ServerSharesAllowed()) {
$permissions |= Constants::PERMISSION_SHARE;
}
$share->setPermissions($permissions);
// Set password
if ($password !== '') {
$share->setPassword($password);
@ -1030,6 +1035,11 @@ class ShareAPIController extends OCSController {
}
if ($newPermissions !== null) {
// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
if (($newPermissions & Constants::PERMISSION_READ) && $this->shareManager->outgoingServer2ServerSharesAllowed()) {
$newPermissions |= Constants::PERMISSION_SHARE;
}
$share->setPermissions($newPermissions);
$permissions = $newPermissions;
}

View file

@ -126,21 +126,21 @@
<template v-if="share.canEdit">
<!-- folder -->
<template v-if="isFolder && fileHasCreatePermission && config.isPublicUploadEnabled">
<ActionRadio :checked="share.permissions === publicUploadRValue"
<ActionRadio :checked="sharePermissions === publicUploadRValue"
:value="publicUploadRValue"
:name="randomId"
:disabled="saving"
@change="togglePermissions">
{{ t('files_sharing', 'Read only') }}
</ActionRadio>
<ActionRadio :checked="share.permissions === publicUploadRWValue"
<ActionRadio :checked="sharePermissions === publicUploadRWValue"
:value="publicUploadRWValue"
:disabled="saving"
:name="randomId"
@change="togglePermissions">
{{ t('files_sharing', 'Allow upload and editing') }}
</ActionRadio>
<ActionRadio :checked="share.permissions === publicUploadWValue"
<ActionRadio :checked="sharePermissions === publicUploadWValue"
:value="publicUploadWValue"
:disabled="saving"
:name="randomId"
@ -357,6 +357,15 @@ export default {
},
computed: {
/**
* Return the current share permissions
* We always ignore the SHARE permission as this is used for the
* federated sharing.
* @returns {number}
*/
sharePermissions() {
return this.share.permissions & ~OC.PERMISSION_SHARE
},
/**
* Generate a unique random id for this SharingEntryLink only
* This allows ActionRadios to have the same name prop

View file

@ -205,7 +205,9 @@ class ApiTest extends TestCase {
$ocs->cleanup();
$data = $result->getData();
$this->assertEquals(1, $data['permissions']);
$this->assertEquals(\OCP\Constants::PERMISSION_READ |
\OCP\Constants::PERMISSION_SHARE,
$data['permissions']);
$this->assertEmpty($data['expiration']);
$this->assertTrue(is_string($data['token']));
@ -230,7 +232,8 @@ class ApiTest extends TestCase {
\OCP\Constants::PERMISSION_READ |
\OCP\Constants::PERMISSION_CREATE |
\OCP\Constants::PERMISSION_UPDATE |
\OCP\Constants::PERMISSION_DELETE,
\OCP\Constants::PERMISSION_DELETE |
\OCP\Constants::PERMISSION_SHARE,
$data['permissions']
);
$this->assertEmpty($data['expiration']);
@ -1002,7 +1005,8 @@ class ApiTest extends TestCase {
\OCP\Constants::PERMISSION_READ |
\OCP\Constants::PERMISSION_CREATE |
\OCP\Constants::PERMISSION_UPDATE |
\OCP\Constants::PERMISSION_DELETE,
\OCP\Constants::PERMISSION_DELETE |
\OCP\Constants::PERMISSION_SHARE,
$share1->getPermissions()
);

View file

@ -91,7 +91,7 @@ Feature: sharing
And the HTTP status code should be "200"
And Share fields of last share match with
| id | A_NUMBER |
| permissions | 15 |
| permissions | 31 |
| expiration | +3 days |
| url | AN_URL |
| token | A_TOKEN |
@ -130,7 +130,7 @@ Feature: sharing
| share_type | 3 |
| file_source | A_NUMBER |
| file_target | /FOLDER |
| permissions | 1 |
| permissions | 17 |
| stime | A_NUMBER |
| expiration | +3 days |
| token | A_TOKEN |
@ -163,7 +163,7 @@ Feature: sharing
| share_type | 3 |
| file_source | A_NUMBER |
| file_target | /FOLDER |
| permissions | 1 |
| permissions | 17 |
| stime | A_NUMBER |
| token | A_TOKEN |
| storage | A_NUMBER |
@ -195,7 +195,7 @@ Feature: sharing
| share_type | 3 |
| file_source | A_NUMBER |
| file_target | /FOLDER |
| permissions | 15 |
| permissions | 31 |
| stime | A_NUMBER |
| token | A_TOKEN |
| storage | A_NUMBER |
@ -259,7 +259,7 @@ Feature: sharing
| share_type | 3 |
| file_source | A_NUMBER |
| file_target | /FOLDER |
| permissions | 15 |
| permissions | 31 |
| stime | A_NUMBER |
| token | A_TOKEN |
| storage | A_NUMBER |

View file

@ -626,11 +626,6 @@ class Manager implements IManager {
throw new \Exception('Link sharing is not allowed');
}
// Link shares by definition can't have share permissions
if ($share->getPermissions() & \OCP\Constants::PERMISSION_SHARE) {
throw new \InvalidArgumentException('Link shares cant have reshare permissions');
}
// Check if public upload is allowed
if (!$this->shareApiLinkAllowPublicUpload() &&
($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE))) {

View file

@ -1375,24 +1375,6 @@ class ManagerTest extends \Test\TestCase {
}
public function testLinkCreateChecksSharePermissions() {
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Link shares cant have reshare permissions');
$share = $this->manager->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_SHARE);
$this->config
->method('getAppValue')
->will($this->returnValueMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
]));
self::invokePrivate($this->manager, 'linkCreateChecks', [$share]);
}
public function testLinkCreateChecksNoPublicUpload() {
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Public upload is not allowed');