Skip to content

Commit d1d0070

Browse files
provokateurinprovokateurin
authored
Merge pull request #62464 from nextcloud/fix/sharing/expiration-date-required-and-default
 fix(Sharing): Compute required, default and max expiration date based on recipients
2 parents 412100f + 248b465 commit d1d0070

25 files changed

Lines changed: 384 additions & 190 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: 78 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -813,34 +813,6 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
813813
}
814814
}
815815

816-
foreach (array_keys($shares) as $id) {
817-
foreach ($registryPropertyTypes as $propertyTypeClass => $propertyType) {
818-
if (
819-
!isset($shares[$id]['properties'][$propertyTypeClass])
820-
&& isset($shareSourceTypeClasses[$id], $shareRecipientTypeClasses[$id])
821-
&& array_intersect($registryPropertyTypeCompatibleSourceTypeClasses[$propertyTypeClass], array_keys($shareSourceTypeClasses[$id])) !== []
822-
&& array_intersect($registryPropertyTypeCompatibleRecipientTypeClasses[$propertyTypeClass], array_keys($shareRecipientTypeClasses[$id])) !== []) {
823-
$value = $propertyType->getDefaultValue();
824-
825-
$timestamp = $this->manager->generateTimestamp();
826-
$this->setLastUpdated([(string)$id], $timestamp);
827-
828-
$qb = $this->connection->getQueryBuilder();
829-
$qb
830-
->insert('sharing_share_properties')
831-
->values([
832-
'share_id' => $qb->createNamedParameter($id),
833-
'property_class' => $qb->createNamedParameter($propertyTypeClass),
834-
'property_value' => $qb->createNamedParameter($propertyType instanceof ISharePropertyTypeModifyValue ? $propertyType->modifyValueOnSave(null, $value) : $value),
835-
])
836-
->executeStatement();
837-
838-
$shares[$id]['properties'][$propertyTypeClass] = new ShareProperty($propertyTypeClass, $value);
839-
$shares[$id]['last_updated'] = $timestamp;
840-
}
841-
}
842-
}
843-
844816
$registrySourceTypePermissionTypeClasses = $this->registry->getSourceTypePermissionTypeClasses();
845817
$registryGenericPermissionTypeClasses = $this->registry->getGenericPermissionTypeClasses();
846818

@@ -891,33 +863,6 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
891863
}
892864
}
893865

894-
$permissionTypes = $this->registry->getPermissionTypes();
895-
896-
foreach (array_keys($shares) as $id) {
897-
foreach (array_keys($shareCompatiblePermissionTypeClasses[$id]) as $permissionTypeClass) {
898-
$permissionType = $permissionTypes[$permissionTypeClass];
899-
if (!isset($shares[$id]['permissions'][$permissionTypeClass])) {
900-
$enabled = $permissionType->isEnabledByDefault();
901-
902-
$timestamp = $this->manager->generateTimestamp();
903-
$this->setLastUpdated([(string)$id], $timestamp);
904-
905-
$qb = $this->connection->getQueryBuilder();
906-
$qb
907-
->insert('sharing_share_permissions')
908-
->values([
909-
'share_id' => $qb->createNamedParameter($id),
910-
'permission_class' => $qb->createNamedParameter($permissionTypeClass),
911-
'permission_enabled' => $qb->createNamedParameter($enabled, IQueryBuilder::PARAM_BOOL),
912-
])
913-
->executeStatement();
914-
915-
$shares[$id]['permissions'][$permissionTypeClass] = new SharePermission($permissionTypeClass, $enabled);
916-
$shares[$id]['last_updated'] = $timestamp;
917-
}
918-
}
919-
}
920-
921866
$shares = array_map(static fn (array $share): Share => new Share(
922867
$share['id'],
923868
$share['owner'],
@@ -948,6 +893,84 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
948893
}
949894
}
950895

896+
foreach (array_keys($shares) as $id) {
897+
foreach ($registryPropertyTypes as $propertyTypeClass => $propertyType) {
898+
$share = $shares[$id];
899+
if (
900+
!isset($share->properties[$propertyTypeClass])
901+
&& isset($shareSourceTypeClasses[$id], $shareRecipientTypeClasses[$id])
902+
&& array_intersect($registryPropertyTypeCompatibleSourceTypeClasses[$propertyTypeClass], array_keys($shareSourceTypeClasses[$id])) !== []
903+
&& array_intersect($registryPropertyTypeCompatibleRecipientTypeClasses[$propertyTypeClass], array_keys($shareRecipientTypeClasses[$id])) !== []) {
904+
$value = $propertyType->getDefaultValue($share);
905+
906+
$timestamp = $this->manager->generateTimestamp();
907+
$this->setLastUpdated([(string)$id], $timestamp);
908+
909+
$qb = $this->connection->getQueryBuilder();
910+
$qb
911+
->insert('sharing_share_properties')
912+
->values([
913+
'share_id' => $qb->createNamedParameter($id),
914+
'property_class' => $qb->createNamedParameter($propertyTypeClass),
915+
'property_value' => $qb->createNamedParameter($propertyType instanceof ISharePropertyTypeModifyValue ? $propertyType->modifyValueOnSave(null, $value) : $value),
916+
])
917+
->executeStatement();
918+
919+
$properties = $share->properties;
920+
$properties[$propertyTypeClass] = new ShareProperty($propertyTypeClass, $value);
921+
922+
$shares[$id] = new Share(
923+
$share->id,
924+
$share->owner,
925+
$timestamp,
926+
$share->state,
927+
$share->sources,
928+
$share->recipients,
929+
$properties,
930+
$share->permissions,
931+
);
932+
}
933+
}
934+
}
935+
936+
$permissionTypes = $this->registry->getPermissionTypes();
937+
foreach (array_keys($shares) as $id) {
938+
foreach (array_keys($shareCompatiblePermissionTypeClasses[$id]) as $permissionTypeClass) {
939+
$share = $shares[$id];
940+
if (!isset($share->permissions[$permissionTypeClass])) {
941+
$permissionType = $permissionTypes[$permissionTypeClass];
942+
$enabled = $permissionType->isEnabledByDefault();
943+
944+
$timestamp = $this->manager->generateTimestamp();
945+
$this->setLastUpdated([(string)$id], $timestamp);
946+
947+
$qb = $this->connection->getQueryBuilder();
948+
$qb
949+
->insert('sharing_share_permissions')
950+
->values([
951+
'share_id' => $qb->createNamedParameter($id),
952+
'permission_class' => $qb->createNamedParameter($permissionTypeClass),
953+
'permission_enabled' => $qb->createNamedParameter($enabled, IQueryBuilder::PARAM_BOOL),
954+
])
955+
->executeStatement();
956+
957+
$permissions = $share->permissions;
958+
$permissions[$permissionTypeClass] = new SharePermission($permissionTypeClass, $enabled);
959+
960+
$shares[$id] = new Share(
961+
$share->id,
962+
$share->owner,
963+
$timestamp,
964+
$share->state,
965+
$share->sources,
966+
$share->recipients,
967+
$share->properties,
968+
$permissions,
969+
);
970+
}
971+
}
972+
}
973+
951974
return array_values($shares);
952975
}
953976
}

‎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
@@ -404,8 +404,11 @@ public function updateShareProperty(ShareAccessContext $accessContext, string $i
404404
throw new RuntimeException('The property is not registered: ' . $property->class);
405405
}
406406

407-
if ($property->value !== null && ($message = $propertyType->validateValue($this->l10nFactory, $property->value)) !== true) {
408-
throw new ShareInvalidException('Invalid property value: ' . $property->value . ' ' . $property->class, $message);
407+
if ($property->value !== null) {
408+
$share = $this->getShare($accessContext, $id);
409+
if (($message = $propertyType->validateValue($this->l10nFactory, $share, $property->value)) !== true) {
410+
throw new ShareInvalidException('Invalid property value: ' . $property->value . ' ' . $property->class, $message);
411+
}
409412
}
410413

411414
$backend->updateShareProperty($id, $property);
@@ -551,6 +554,7 @@ private function getBackend(?string $id): ISharingBackend {
551554
}
552555

553556
// TODO: Support IShareOwnerlessMount
557+
554558
/**
555559
* @throws ShareOperationForbiddenException
556560
*/
@@ -672,7 +676,7 @@ private function assertShareCanBeActive(Share $share): void {
672676
$propertyTypes = $this->registry->getPropertyTypes();
673677
foreach ($share->properties as $propertyTypeClass => $property) {
674678
$propertyType = $propertyTypes[$propertyTypeClass];
675-
if ($property->value === null && $propertyType->isRequired()) {
679+
if ($property->value === null && $propertyType->isRequired($share)) {
676680
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)]));
677681
}
678682
}

0 commit comments

Comments
 (0)