Skip to content

Commit 098973c

Browse files
committed
fix(Sharing): Compute required, default and max expiration date based on recipients
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent b829350 commit 098973c

25 files changed

Lines changed: 307 additions & 136 deletions

apps/files/lib/Sharing/Property/NodeGridViewSharePropertyType.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCA\Files\AppInfo\Application;
1313
use OCP\L10N\IFactory;
1414
use OCP\Sharing\Property\ABooleanSharePropertyType;
15+
use OCP\Sharing\Share;
1516

1617
final class NodeGridViewSharePropertyType extends ABooleanSharePropertyType {
1718
#[\Override]
@@ -35,12 +36,12 @@ public function isAdvanced(): bool {
3536
}
3637

3738
#[\Override]
38-
public function isRequired(): bool {
39+
public function isRequired(Share $share): bool {
3940
return false;
4041
}
4142

4243
#[\Override]
43-
public function getDefaultValue(): string {
44+
public function getDefaultValue(Share $share): string {
4445
return 'false';
4546
}
4647
}

apps/sharing/lib/SharingBackend.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
901901
&& isset($shareSourceTypeClasses[$id], $shareRecipientTypeClasses[$id])
902902
&& array_intersect($registryPropertyTypeCompatibleSourceTypeClasses[$propertyTypeClass], array_keys($shareSourceTypeClasses[$id])) !== []
903903
&& array_intersect($registryPropertyTypeCompatibleRecipientTypeClasses[$propertyTypeClass], array_keys($shareRecipientTypeClasses[$id])) !== []) {
904-
$value = $propertyType->getDefaultValue();
904+
$value = $propertyType->getDefaultValue($share);
905905

906906
$timestamp = $this->manager->generateTimestamp();
907907
$this->setLastUpdated([(string)$id], $timestamp);

core/Sharing/Property/ExpirationDateSharePropertyType.php

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use DateTimeImmutable;
1414
use DateTimeInterface;
1515
use OC\Core\AppInfo\Application;
16+
use OC\Core\Sharing\Recipient\EmailShareRecipientType;
17+
use OC\Core\Sharing\Recipient\TokenShareRecipientType;
1618
use OCP\L10N\IFactory;
1719
use OCP\Share\IManager;
1820
use OCP\Sharing\Property\ADateSharePropertyType;
@@ -52,51 +54,50 @@ public function isAdvanced(): bool {
5254
}
5355

5456
#[\Override]
55-
public function isRequired(): bool {
56-
if ($this->legacyManager->shareApiLinkDefaultExpireDateEnforced()) {
57+
public function isRequired(Share $share): bool {
58+
if ($this->hasTokenOrEmailRecipient($share) && $this->legacyManager->shareApiLinkDefaultExpireDateEnforced()) {
5759
return true;
5860
}
5961

60-
if ($this->legacyManager->shareApiRemoteDefaultExpireDateEnforced()) {
62+
if ($this->hasRemoteRecipient($share) && $this->legacyManager->shareApiRemoteDefaultExpireDateEnforced()) {
6163
return true;
6264
}
63-
64-
return $this->legacyManager->shareApiInternalDefaultExpireDateEnforced();
65+
return $this->hasLocalNonTokenAndEmailRecipient($share) && $this->legacyManager->shareApiInternalDefaultExpireDateEnforced();
6566
}
6667

6768
#[\Override]
68-
public function getDefaultValue(): ?string {
69-
return $this->getMaxExpirationDate()?->format(DateTimeInterface::ATOM);
69+
public function getDefaultValue(Share $share): ?string {
70+
return $this->getMaxExpirationDate($share)?->format(DateTimeInterface::ATOM);
7071
}
7172

7273
#[\Override]
73-
public function getMinDate(): \DateTimeImmutable {
74+
public function getMinDate(Share $share): \DateTimeImmutable {
7475
// Ensure the expiration date is in the future.
7576
return $this->now->add(new DateInterval('PT5M'));
7677
}
7778

7879
#[\Override]
79-
public function getMaxDate(): ?DateTimeImmutable {
80-
if ($this->isRequired()) {
80+
public function getMaxDate(Share $share): ?DateTimeImmutable {
81+
if ($this->isRequired($share)) {
8182
// Allow some time to pass between the user getting the max date and saving the date, as the time will shift in between.
82-
return $this->getMaxExpirationDate()?->add(new DateInterval('PT5M'));
83+
return $this->getMaxExpirationDate($share)?->add(new DateInterval('PT5M'));
8384
}
8485

8586
return null;
8687
}
8788

88-
private function getMaxExpirationDate(): ?DateTimeImmutable {
89-
// We do not have any distinction between link/remote/internal, so we just apply the lowest expiration days count to be safe.
89+
private function getMaxExpirationDate(Share $share): ?DateTimeImmutable {
9090
$days = INF;
91-
if ($this->legacyManager->shareApiLinkDefaultExpireDate()) {
91+
92+
if ($this->hasTokenOrEmailRecipient($share) && $this->legacyManager->shareApiLinkDefaultExpireDate()) {
9293
$days = min($days, $this->legacyManager->shareApiLinkDefaultExpireDays());
9394
}
9495

95-
if ($this->legacyManager->shareApiRemoteDefaultExpireDate()) {
96+
if ($this->hasRemoteRecipient($share) && $this->legacyManager->shareApiRemoteDefaultExpireDate()) {
9697
$days = min($days, $this->legacyManager->shareApiRemoteDefaultExpireDays());
9798
}
9899

99-
if ($this->legacyManager->shareApiInternalDefaultExpireDate()) {
100+
if ($this->hasLocalNonTokenAndEmailRecipient($share) && $this->legacyManager->shareApiInternalDefaultExpireDate()) {
100101
$days = min($days, $this->legacyManager->shareApiInternalDefaultExpireDays());
101102
}
102103

@@ -120,4 +121,34 @@ public function isFiltered(ShareAccessContext $accessContext, Share $share): boo
120121

121122
return false;
122123
}
124+
125+
private function hasTokenOrEmailRecipient(Share $share): bool {
126+
foreach ($share->recipients as $recipient) {
127+
if ($recipient->class === TokenShareRecipientType::class || $recipient->class === EmailShareRecipientType::class) {
128+
return true;
129+
}
130+
}
131+
132+
return false;
133+
}
134+
135+
private function hasRemoteRecipient(Share $share): bool {
136+
foreach ($share->recipients as $recipient) {
137+
if ($recipient->instance !== null) {
138+
return true;
139+
}
140+
}
141+
142+
return false;
143+
}
144+
145+
private function hasLocalNonTokenAndEmailRecipient(Share $share): bool {
146+
foreach ($share->recipients as $recipient) {
147+
if ($recipient->instance === null && $recipient->class !== TokenShareRecipientType::class && $recipient->class !== EmailShareRecipientType::class) {
148+
return true;
149+
}
150+
}
151+
152+
return false;
153+
}
123154
}

core/Sharing/Property/LabelSharePropertyType.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OC\Core\AppInfo\Application;
1313
use OCP\L10N\IFactory;
1414
use OCP\Sharing\Property\AStringSharePropertyType;
15+
use OCP\Sharing\Share;
1516

1617
final class LabelSharePropertyType extends AStringSharePropertyType {
1718
#[\Override]
@@ -35,12 +36,12 @@ public function isAdvanced(): bool {
3536
}
3637

3738
#[\Override]
38-
public function isRequired(): bool {
39+
public function isRequired(Share $share): bool {
3940
return false;
4041
}
4142

4243
#[\Override]
43-
public function getDefaultValue(): ?string {
44+
public function getDefaultValue(Share $share): ?string {
4445
return null;
4546
}
4647

core/Sharing/Property/NoteSharePropertyType.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OC\Core\AppInfo\Application;
1313
use OCP\L10N\IFactory;
1414
use OCP\Sharing\Property\AStringSharePropertyType;
15+
use OCP\Sharing\Share;
1516

1617
final class NoteSharePropertyType extends AStringSharePropertyType {
1718
#[\Override]
@@ -35,12 +36,12 @@ public function isAdvanced(): bool {
3536
}
3637

3738
#[\Override]
38-
public function isRequired(): bool {
39+
public function isRequired(Share $share): bool {
3940
return false;
4041
}
4142

4243
#[\Override]
43-
public function getDefaultValue(): ?string {
44+
public function getDefaultValue(Share $share): ?string {
4445
return null;
4546
}
4647

core/Sharing/Property/PasswordSharePropertyType.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ public function isAdvanced(): bool {
5656
}
5757

5858
#[\Override]
59-
public function isRequired(): bool {
59+
public function isRequired(Share $share): bool {
6060
// TODO: Enable group memberships check based on the owner.
6161
return $this->legacyManager->shareApiLinkEnforcePassword(false);
6262
}
6363

6464
#[\Override]
65-
public function getDefaultValue(): ?string {
66-
if (!$this->isRequired()) {
65+
public function getDefaultValue(Share $share): ?string {
66+
if (!$this->isRequired($share)) {
6767
return null;
6868
}
6969

lib/private/Sharing/SharingManager.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,11 @@ public function updateShareProperty(ShareAccessContext $accessContext, string $i
400400
throw new RuntimeException('The property is not registered: ' . $property->class);
401401
}
402402

403-
if ($property->value !== null && ($message = $propertyType->validateValue($this->l10nFactory, $property->value)) !== true) {
404-
throw new ShareInvalidException('Invalid property value: ' . $property->value . ' ' . $property->class, $message);
403+
if ($property->value !== null) {
404+
$share = $this->getShare($accessContext, $id);
405+
if (($message = $propertyType->validateValue($this->l10nFactory, $share, $property->value)) !== true) {
406+
throw new ShareInvalidException('Invalid property value: ' . $property->value . ' ' . $property->class, $message);
407+
}
405408
}
406409

407410
$backend->updateShareProperty($id, $property);
@@ -547,6 +550,7 @@ private function getBackend(?string $id): ISharingBackend {
547550
}
548551

549552
// TODO: Support IShareOwnerlessMount
553+
550554
/**
551555
* @throws ShareOperationForbiddenException
552556
*/
@@ -668,7 +672,7 @@ private function assertShareCanBeActive(Share $share): void {
668672
$propertyTypes = $this->registry->getPropertyTypes();
669673
foreach ($share->properties as $propertyTypeClass => $property) {
670674
$propertyType = $propertyTypes[$propertyTypeClass];
671-
if ($property->value === null && $propertyType->isRequired()) {
675+
if ($property->value === null && $propertyType->isRequired($share)) {
672676
throw new ShareInvalidException('Missing value for required property: ' . $propertyTypeClass, $this->l10n->t('You need to set a value for the %s', [$propertyType->getDisplayName($this->l10nFactory)]));
673677
}
674678
}

lib/public/Sharing/Property/ABooleanSharePropertyType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ abstract class ABooleanSharePropertyType implements ISharePropertyType {
2525
* @since 35.0.0
2626
*/
2727
#[\Override]
28-
public function validateValue(IFactory $l10nFactory, string $value): true|string {
28+
public function validateValue(IFactory $l10nFactory, Share $share, string $value): true|string {
2929
if ($value === 'true' || $value === 'false') {
3030
return true;
3131
}
@@ -39,7 +39,7 @@ public function validateValue(IFactory $l10nFactory, string $value): true|string
3939
* @since 35.0.0
4040
*/
4141
#[\Override]
42-
public function format(array $property): array {
42+
public function format(Share $share, array $property): array {
4343
$property['type'] = 'boolean';
4444
return $property;
4545
}

lib/public/Sharing/Property/ADateSharePropertyType.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ abstract class ADateSharePropertyType implements ISharePropertyType {
2727
/**
2828
* @since 35.0.0
2929
*/
30-
abstract public function getMinDate(): ?DateTimeImmutable;
30+
abstract public function getMinDate(Share $share): ?DateTimeImmutable;
3131

3232
/**
3333
* @since 35.0.0
3434
*/
35-
abstract public function getMaxDate(): ?DateTimeImmutable;
35+
abstract public function getMaxDate(Share $share): ?DateTimeImmutable;
3636

3737
/**
3838
* @since 35.0.0
3939
*/
4040
#[\Override]
41-
public function validateValue(IFactory $l10nFactory, string $value): true|string {
41+
public function validateValue(IFactory $l10nFactory, Share $share, string $value): true|string {
4242
try {
4343
$date = DateTimeImmutable::createFromFormat(DateTimeInterface::ATOM, $value);
4444
} catch (Exception) {
@@ -49,11 +49,11 @@ public function validateValue(IFactory $l10nFactory, string $value): true|string
4949
return $l10nFactory->get(Application::APP_ID)->t('Invalid ISO date: %s', [$value]);
5050
}
5151

52-
if (($minDate = $this->getMinDate()) instanceof DateTimeImmutable && $date->diff($minDate)->invert === 0) {
52+
if (($minDate = $this->getMinDate($share)) instanceof DateTimeImmutable && $date->diff($minDate)->invert === 0) {
5353
return $l10nFactory->get(Application::APP_ID)->t('Date needs to be after %1$s: %2$s', [$minDate->format(DateTimeInterface::ATOM), $value]);
5454
}
5555

56-
if (($maxDate = $this->getMaxDate()) instanceof DateTimeImmutable && $date->diff($maxDate)->invert === 1) {
56+
if (($maxDate = $this->getMaxDate($share)) instanceof DateTimeImmutable && $date->diff($maxDate)->invert === 1) {
5757
return $l10nFactory->get(Application::APP_ID)->t('Date needs to be before %1$s: %2$s', [$maxDate->format(DateTimeInterface::ATOM), $value]);
5858
}
5959

@@ -66,10 +66,10 @@ public function validateValue(IFactory $l10nFactory, string $value): true|string
6666
* @since 35.0.0
6767
*/
6868
#[\Override]
69-
public function format(array $property): array {
69+
public function format(Share $share, array $property): array {
7070
$property['type'] = 'date';
71-
$property['min_date'] = $this->getMinDate()?->format(DateTimeInterface::ATOM);
72-
$property['max_date'] = $this->getMaxDate()?->format(DateTimeInterface::ATOM);
71+
$property['min_date'] = $this->getMinDate($share)?->format(DateTimeInterface::ATOM);
72+
$property['max_date'] = $this->getMaxDate($share)?->format(DateTimeInterface::ATOM);
7373
return $property;
7474
}
7575
}

lib/public/Sharing/Property/AEnumSharePropertyType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ abstract public function getValidValues(): array;
3131
* @since 35.0.0
3232
*/
3333
#[\Override]
34-
public function validateValue(IFactory $l10nFactory, string $value): true|string {
34+
public function validateValue(IFactory $l10nFactory, Share $share, string $value): true|string {
3535
$validValues = $this->getValidValues();
3636
if (in_array($value, $validValues, true)) {
3737
return true;
@@ -46,7 +46,7 @@ public function validateValue(IFactory $l10nFactory, string $value): true|string
4646
* @since 35.0.0
4747
*/
4848
#[\Override]
49-
public function format(array $property): array {
49+
public function format(Share $share, array $property): array {
5050
$property['type'] = 'enum';
5151
$property['valid_values'] = $this->getValidValues();
5252
return $property;

0 commit comments

Comments
 (0)