Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable17] Fix resharing of federated shares that were created out of links #21110

Merged
merged 7 commits into from
May 28, 2020
Merged

[stable17] Fix resharing of federated shares that were created out of links #21110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
703fca8
Set proper share type when converting link shares to federated shares
juliusknorr Mar 5, 2020
f50bf10
Link shares have reshare permission if outgoing federated shares are …
juliusknorr Mar 5, 2020
1808cf9
Remove unneeded test since links have resharing permissions by default
juliusknorr Apr 9, 2020
6941bbe
Adjust integration tests to new permissions
juliusknorr Apr 23, 2020
b48df49
Have share permissions on link shares if it is enabled
rullzer May 6, 2020
cccd0d8
Update public link share permission code
rullzer May 6, 2020
3584797
Fix share update test
juliusknorr May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
use OCP\IUserSession;
use OCP\Share\IManager;
use OCP\Util;
use OCP\Share\IShare;

/**
* Class MountPublicLinkController
Expand Down Expand Up @@ -161,6 +162,7 @@ public function createFederatedShare($shareWith, $token, $password = '') {
}

$share->setSharedWith($shareWith);
$share->setShareType(IShare::TYPE_REMOTE);

try {
$this->federatedShareProvider->create($share);
Expand Down
20 changes: 15 additions & 5 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,16 +470,21 @@ public function createShare(
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);
Expand Down Expand Up @@ -903,6 +908,11 @@ public function updateShare(
}

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;
}
Expand Down
10 changes: 7 additions & 3 deletions apps/files_sharing/tests/ApiTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ public function testCreateShareLink() {
$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']));

Expand All @@ -228,7 +230,8 @@ public function testCreateShareLinkPublicUpload() {
\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']);
Expand Down Expand Up @@ -974,7 +977,8 @@ function testUpdateShareUpload() {
\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()
);

Expand Down
10 changes: 5 additions & 5 deletions build/integration/features/sharing-v1.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,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 |
Expand Down Expand Up @@ -128,7 +128,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 |
Expand Down Expand Up @@ -160,7 +160,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 |
Expand Down Expand Up @@ -191,7 +191,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 |
Expand Down Expand Up @@ -253,7 +253,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 |
Expand Down
5 changes: 0 additions & 5 deletions lib/private/Share20/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,11 +537,6 @@ protected function linkCreateChecks(\OCP\Share\IShare $share) {
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 can’t 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))) {
Expand Down
18 changes: 0 additions & 18 deletions tests/lib/Share20/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,24 +1361,6 @@ public function testLinkCreateChecksNoLinkSharesAllowed() {
self::invokePrivate($this->manager, 'linkCreateChecks', [$share]);
}

/**
* @expectedException Exception
* @expectedExceptionMessage Link shares can’t have reshare permissions
*/
public function testLinkCreateChecksSharePermissions() {
$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]);
}

/**
* @expectedException Exception
* @expectedExceptionMessage Public upload is not allowed
Expand Down