Merge pull request #19793 from nextcloud/bugfix/noid/link-to-federated-reshare

Fix resharing of federated shares that were created out of links
This commit is contained in:
Morris Jobke 2020-04-27 11:05:34 +02:00 committed by GitHub
commit 1738e17e20
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 18 additions and 33 deletions

View File

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

View File

@ -492,15 +492,18 @@ class ShareAPIController extends OCSController {
throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders')); throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders'));
} }
$share->setPermissions( $permissions = Constants::PERMISSION_READ |
Constants::PERMISSION_READ |
Constants::PERMISSION_CREATE | Constants::PERMISSION_CREATE |
Constants::PERMISSION_UPDATE | Constants::PERMISSION_UPDATE |
Constants::PERMISSION_DELETE Constants::PERMISSION_DELETE;
);
} else { } 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 ($this->shareManager->outgoingServer2ServerSharesAllowed()) {
$permissions |= Constants::PERMISSION_SHARE;
}
$share->setPermissions($permissions);
// Set password // Set password
if ($password !== '') { if ($password !== '') {

View File

@ -203,7 +203,9 @@ class ApiTest extends TestCase {
$ocs->cleanup(); $ocs->cleanup();
$data = $result->getData(); $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->assertEmpty($data['expiration']);
$this->assertTrue(is_string($data['token'])); $this->assertTrue(is_string($data['token']));
@ -228,7 +230,8 @@ class ApiTest extends TestCase {
\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_READ |
\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_CREATE |
\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_UPDATE |
\OCP\Constants::PERMISSION_DELETE, \OCP\Constants::PERMISSION_DELETE |
\OCP\Constants::PERMISSION_SHARE,
$data['permissions'] $data['permissions']
); );
$this->assertEmpty($data['expiration']); $this->assertEmpty($data['expiration']);

View File

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

View File

@ -625,11 +625,6 @@ class Manager implements IManager {
throw new \Exception('Link sharing is not allowed'); 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 // Check if public upload is allowed
if (!$this->shareApiLinkAllowPublicUpload() && if (!$this->shareApiLinkAllowPublicUpload() &&
($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE))) { ($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE))) {

View File

@ -1374,24 +1374,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')
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
]);
self::invokePrivate($this->manager, 'linkCreateChecks', [$share]);
}
public function testLinkCreateChecksNoPublicUpload() { public function testLinkCreateChecksNoPublicUpload() {
$this->expectException(\Exception::class); $this->expectException(\Exception::class);
$this->expectExceptionMessage('Public upload is not allowed'); $this->expectExceptionMessage('Public upload is not allowed');