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.

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

Implement expiration date for federated shares #25320

Merged
merged 4 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 13 additions & 3 deletions apps/federatedfilesharing/lib/FederatedShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ public function create(IShare $share) {
$permissions = $share->getPermissions();
$sharedBy = $share->getSharedBy();
$shareType = $share->getShareType();
$expirationDate = $share->getExpirationDate();

if ($shareType === IShare::TYPE_REMOTE_GROUP &&
!$this->isOutgoingServer2serverGroupShareEnabled()
Expand Down Expand Up @@ -219,7 +220,7 @@ public function create(IShare $share) {
if ($remoteShare) {
try {
$ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']);
$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType);
$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
$share->setId($shareId);
[$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId);
// remote share was create successfully if we get a valid token as return
Expand Down Expand Up @@ -264,7 +265,8 @@ protected function createFederatedShare(IShare $share) {
$share->getShareOwner(),
$share->getPermissions(),
$token,
$share->getShareType()
$share->getShareType(),
$share->getExpirationDate()
);

$failure = false;
Expand Down Expand Up @@ -370,9 +372,10 @@ protected function getShareFromExternalShareTable(IShare $share) {
* @param int $permissions
* @param string $token
* @param int $shareType
* @param \DateTime $expirationDate
* @return int
*/
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType) {
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) {
$qb = $this->dbConnection->getQueryBuilder();
$qb->insert('share')
->setValue('share_type', $qb->createNamedParameter($shareType))
Expand All @@ -383,6 +386,7 @@ private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ui
->setValue('uid_owner', $qb->createNamedParameter($uidOwner))
->setValue('uid_initiator', $qb->createNamedParameter($sharedBy))
->setValue('permissions', $qb->createNamedParameter($permissions))
->setValue('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATE))
->setValue('token', $qb->createNamedParameter($token))
->setValue('stime', $qb->createNamedParameter(time()));

Expand Down Expand Up @@ -412,6 +416,7 @@ public function update(IShare $share) {
->set('permissions', $qb->createNamedParameter($share->getPermissions()))
->set('uid_owner', $qb->createNamedParameter($share->getShareOwner()))
->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy()))
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE))
->execute();

// send the updated permission to the owner/initiator, if they are not the same
Expand Down Expand Up @@ -910,6 +915,11 @@ private function createShareObject($data) {

$share->setProviderId($this->identifier());

if ($data['expiration'] !== null) {
$expiration = \DateTime::createFromFormat('Y-m-d H:i:s', $data['expiration']);
$share->setExpirationDate($expiration);
}

return $share;
}

Expand Down
29 changes: 22 additions & 7 deletions apps/federatedfilesharing/tests/FederatedShareProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,17 @@ protected function tearDown(): void {
parent::tearDown();
}

public function testCreate() {
public function dataTestCreate() {
return [
[null, null],
[new \DateTime('2020-03-01T01:02:03'), '2020-03-01 01:02:03'],
];
}

/**
* @dataProvider dataTestCreate
*/
public function testCreate($expirationDate, $expectedDataDate) {
$share = $this->shareManager->newShare();

/** @var File|MockObject $node */
Expand All @@ -158,6 +168,7 @@ public function testCreate() {
->setShareOwner('shareOwner')
->setPermissions(19)
->setShareType(IShare::TYPE_REMOTE)
->setExpirationDate($expirationDate)
->setNode($node);

$this->tokenHandler->method('generateToken')->willReturn('token');
Expand Down Expand Up @@ -209,6 +220,7 @@ public function testCreate() {
'permissions' => 19,
'accepted' => 0,
'token' => 'token',
'expiration' => $expectedDataDate,
];
foreach (array_keys($expected) as $key) {
$this->assertEquals($expected[$key], $data[$key], "Assert that value for key '$key' is the same");
Expand All @@ -223,6 +235,7 @@ public function testCreate() {
$this->assertEquals(42, $share->getNodeId());
$this->assertEquals(19, $share->getPermissions());
$this->assertEquals('token', $share->getToken());
$this->assertEquals($expirationDate, $share->getExpirationDate());
}

public function testCreateCouldNotFindServer() {
Expand Down Expand Up @@ -442,10 +455,9 @@ public function testCreateAlreadyShared() {
}

/**
* @dataProvider datatTestUpdate
*
* @dataProvider dataTestUpdate
*/
public function testUpdate($owner, $sharedBy) {
public function testUpdate($owner, $sharedBy, $expirationDate) {
$this->provider = $this->getMockBuilder('OCA\FederatedFileSharing\FederatedShareProvider')
->setConstructorArgs(
[
Expand Down Expand Up @@ -478,6 +490,7 @@ public function testUpdate($owner, $sharedBy) {
->setShareOwner($owner)
->setPermissions(19)
->setShareType(IShare::TYPE_REMOTE)
->setExpirationDate(new \DateTime('2019-02-01T01:02:03'))
->setNode($node);

$this->tokenHandler->method('generateToken')->willReturn('token');
Expand Down Expand Up @@ -512,17 +525,19 @@ public function testUpdate($owner, $sharedBy) {
$share = $this->provider->create($share);

$share->setPermissions(1);
$share->setExpirationDate($expirationDate);
$this->provider->update($share);

$share = $this->provider->getShareById($share->getId());

$this->assertEquals(1, $share->getPermissions());
$this->assertEquals($expirationDate, $share->getExpirationDate());
}

public function datatTestUpdate() {
public function dataTestUpdate() {
return [
['sharedBy', 'shareOwner'],
['shareOwner', 'shareOwner']
['sharedBy', 'shareOwner', new \DateTime('2020-03-01T01:02:03')],
['shareOwner', 'shareOwner', null],
];
}

Expand Down
6 changes: 3 additions & 3 deletions apps/files_sharing/js/dist/files_sharing_tab.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion apps/files_sharing/js/dist/files_sharing_tab.js.map

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion apps/files_sharing/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ public function getCapabilities() {
$public['expire_date_internal']['enforced'] = $this->config->getAppValue('core', 'shareapi_enforce_internal_expire_date', 'no') === 'yes';
}

$public['expire_date_remote'] = [];
$public['expire_date_remote']['enabled'] = $this->config->getAppValue('core', 'shareapi_default_remote_expire_date', 'no') === 'yes';
if ($public['expire_date_remote']['enabled']) {
$public['expire_date_remote']['days'] = $this->config->getAppValue('core', 'shareapi_remote_expire_after_n_days', '7');
$public['expire_date_remote']['enforced'] = $this->config->getAppValue('core', 'shareapi_enforce_remote_expire_date', 'no') === 'yes';
}

$public['send_mail'] = $this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no') === 'yes';
$public['upload'] = $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes') === 'yes';
$public['upload_files_drop'] = $public['upload'];
Expand All @@ -111,7 +118,10 @@ public function getCapabilities() {
$res['federation'] = [
'outgoing' => $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') === 'yes',
'incoming' => $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') === 'yes',
'expire_date' => ['enabled' => true]
// old bogus one, expire_date was not working before, keeping for compatibility
'expire_date' => ['enabled' => true],
// the real deal, signifies that expiration date can be set on federated shares
'expire_date_supported' => ['enabled' => true],
];

// Sharee searches
Expand Down
24 changes: 24 additions & 0 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -587,15 +587,39 @@ public function createShare(
throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType]));
}

if ($shareWith === null) {
throw new OCSNotFoundException($this->l->t('Please specify a valid federated user id'));
}

$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType]));
}

if ($shareWith === null) {
throw new OCSNotFoundException($this->l->t('Please specify a valid federated group id'));
}

$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
Comment on lines +601 to +620
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider using OCSBadRequestException instead?

Suggested change
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType]));
}
if ($shareWith === null) {
throw new OCSNotFoundException($this->l->t('Please specify a valid federated group id'));
}
$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
throw new OCSBadRequestException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType]));
}
if ($shareWith === null) {
throw new OCSBadRequestException($this->l->t('Please specify a valid federated group id'));
}
$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSBadRequestException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to change the existing semantics as it might affect clients and what status code they are reacting on.

cc @tobiasKaminsky

}
}
} elseif ($shareType === IShare::TYPE_CIRCLE) {
if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled'));
Expand Down
23 changes: 11 additions & 12 deletions apps/files_sharing/src/components/SharingEntry.vue
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,14 @@
</ActionCheckbox>

<!-- expiration date -->
<ActionCheckbox
v-if="canHaveExpirationDate"
:checked.sync="hasExpirationDate"
<ActionCheckbox :checked.sync="hasExpirationDate"
:disabled="config.isDefaultInternalExpireDateEnforced || saving"
@uncheck="onExpirationDisable">
{{ config.isDefaultInternalExpireDateEnforced
? t('files_sharing', 'Expiration date enforced')
: t('files_sharing', 'Set expiration date') }}
</ActionCheckbox>
<ActionInput v-if="canHaveExpirationDate && hasExpirationDate"
<ActionInput v-if="hasExpirationDate"
ref="expireDate"
v-tooltip.auto="{
content: errors.expireDate,
Expand Down Expand Up @@ -224,14 +222,10 @@ export default {
},

canHaveNote() {
return !this.isRemoteShare
},

canHaveExpirationDate() {
return !this.isRemoteShare
return !this.isRemote
},

isRemoteShare() {
isRemote() {
return this.share.type === this.SHARE_TYPES.SHARE_TYPE_REMOTE
|| this.share.type === this.SHARE_TYPES.SHARE_TYPE_REMOTE_GROUP
},
Expand Down Expand Up @@ -358,8 +352,13 @@ export default {
},

dateMaxEnforced() {
return this.config.isDefaultInternalExpireDateEnforced
&& moment().add(1 + this.config.defaultInternalExpireDate, 'days')
if (!this.isRemote) {
return this.config.isDefaultInternalExpireDateEnforced
&& moment().add(1 + this.config.defaultInternalExpireDate, 'days')
} else {
return this.config.isDefaultRemoteExpireDateEnforced
&& moment().add(1 + this.config.defaultRemoteExpireDate, 'days')
}
},

/**
Expand Down
40 changes: 40 additions & 0 deletions apps/files_sharing/src/services/ConfigService.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,24 @@ export default class Config {
return expireDateString
}

/**
* Get the default remote expiration date as string
*
* @returns {string}
* @readonly
* @memberof Config
*/
get defaultRemoteExpirationDateString() {
let expireDateString = ''
if (this.isDefaultRemoteExpireDateEnabled) {
const date = window.moment.utc()
const expireAfterDays = this.defaultRemoteExpireDate
date.add(expireAfterDays, 'days')
expireDateString = date.format('YYYY-MM-DD')
}
return expireDateString
}

/**
* Are link shares password-enforced ?
*
Expand Down Expand Up @@ -150,6 +168,17 @@ export default class Config {
return OC.appConfig.core.defaultInternalExpireDateEnforced === true
}

/**
* Is remote shares expiration enforced ?
*
* @returns {boolean}
* @readonly
* @memberof Config
*/
get isDefaultRemoteExpireDateEnforced() {
return OC.appConfig.core.defaultRemoteExpireDateEnforced === true
}

/**
* Is there a default expiration date for new internal shares ?
*
Expand Down Expand Up @@ -209,6 +238,17 @@ export default class Config {
return OC.appConfig.core.defaultInternalExpireDate
}

/**
* Get the default days to remote shares expiration
*
* @returns {int}
* @readonly
* @memberof Config
*/
get defaultRemoteExpireDate() {
return OC.appConfig.core.defaultRemoteExpireDate
}

/**
* Is resharing allowed ?
*
Expand Down
7 changes: 7 additions & 0 deletions apps/files_sharing/tests/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,11 @@ public function testFederatedSharingNoOutgoing() {
$this->assertArrayHasKey('federation', $result);
$this->assertFalse($result['federation']['outgoing']);
}

public function testFederatedSharingExpirationDate() {
$result = $this->getResults([]);
$this->assertArrayHasKey('federation', $result);
$this->assertEquals(['enabled' => true], $result['federation']['expire_date']);
$this->assertEquals(['enabled' => true], $result['federation']['expire_date_supported']);
}
}
Loading