Preparations for Unified Sharing Legacy Sync

apps/sharebymail/lib/ShareByMailProvider.php

@@ -7,6 +7,7 @@
 
 namespace OCA\ShareByMail;
 
+use DateTime;
 use OC\Share20\DefaultShareProvider;
 use OC\Share20\Exception\InvalidShare;
 use OC\Share20\Share;
@@ -40,6 +41,7 @@
 use OCP\User\Exceptions\UserNotFoundException;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
+use RuntimeException;
 
 /**
  * Class ShareByMail
@@ -98,14 +100,10 @@ public function create(IShare $share): IShare {
 
 		// if the admin enforces a password for all mail shares we create a
 		// random password and send it to the recipient
-		$password = $share->getPassword() ?: '';
-		$passwordEnforced = $this->shareManager->shareApiLinkEnforcePassword();
-		if ($passwordEnforced && empty($password)) {
+		$password = $share->getPassword();
+		if ($password === null && $this->shareManager->shareApiLinkEnforcePassword()) {
 			$password = $this->autoGeneratePassword($share);
-		}
-
-		if (!empty($password)) {
-			$share->setPassword($this->hasher->hash($password));
+			$share->setPasswordHash($this->hasher->hash($password));
 		}
 
 		$shareId = $this->createMailShare($share);
@@ -116,7 +114,7 @@ public function create(IShare $share): IShare {
 		// Temporary set the clear password again to send it by mail
 		// This need to be done after the share was created in the database
 		// as the password is hashed in between.
-		if (!empty($password)) {
+		if ($password !== null) {
 			$data['password'] = $password;
 		}
 
@@ -227,6 +225,11 @@ protected function createMailShare(IShare $share): string {
 		if ($share->getToken() === '') {
 			$share->setToken($this->generateToken());
 		}
+
+		if ($share->getPassword() !== null && !$share->isPasswordHashed()) {
+			throw new RuntimeException('The password must be hashed already.');
+		}
+
 		return $this->addShareToDB(
 			$share->getNodeId(),
 			$share->getNodeType(),
@@ -241,6 +244,7 @@ protected function createMailShare(IShare $share): string {
 			$share->getHideDownload(),
 			$share->getLabel(),
 			$share->getExpirationDate(),
+			$share->getShareTime(),
 			$share->getNote(),
 			$share->getAttributes(),
 			$share->getMailSend(),
@@ -699,6 +703,7 @@ protected function addShareToDB(
 		?bool $hideDownload,
 		?string $label,
 		?\DateTimeInterface $expirationTime,
+		?DateTime $shareTime,
 		?string $note = '',
 		?IAttributes $attributes = null,
 		?bool $mailSend = true,
@@ -717,7 +722,7 @@ protected function addShareToDB(
 			->setValue('password', $qb->createNamedParameter($password))
 			->setValue('password_expiration_time', $qb->createNamedParameter($passwordExpirationTime, IQueryBuilder::PARAM_DATETIME_MUTABLE))
 			->setValue('password_by_talk', $qb->createNamedParameter($sendPasswordByTalk, IQueryBuilder::PARAM_BOOL))
-			->setValue('stime', $qb->createNamedParameter(time()))
+			->setValue('stime', $qb->createNamedParameter($shareTime?->getTimestamp() ?? time()))
 			->setValue('hide_download', $qb->createNamedParameter((int)$hideDownload, IQueryBuilder::PARAM_INT))
 			->setValue('label', $qb->createNamedParameter($label))
 			->setValue('note', $qb->createNamedParameter($note))
@@ -767,6 +772,11 @@ public function update(IShare $share, ?string $plainTextPassword = null): IShare
 			$expiration = \DateTime::createFromInterface($expiration);
 			$expiration->setTimezone(new \DateTimeZone(date_default_timezone_get()));
 		}
+
+		if ($share->getPassword() !== null && !$share->isPasswordHashed()) {
+			throw new RuntimeException('The password must be hashed already.');
+		}
+
 		$qb->update('share')
 			->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
 			->set('item_source', $qb->createNamedParameter($share->getNodeId()))
@@ -1052,7 +1062,9 @@ protected function createShareObject(array $data): IShare {
 		$shareTime->setTimestamp((int)$data['stime']);
 		$share->setShareTime($shareTime);
 		$share->setSharedWith($data['share_with'] ?? '');
-		$share->setPassword($data['password']);
+		if (($password = $data['password']) !== null) {
+			$share->setPasswordHash($password);
+		}
 		$passwordExpirationTime = \DateTime::createFromFormat('Y-m-d H:i:s', $data['password_expiration_time'] ?? '');
 		$share->setPasswordExpirationTime($passwordExpirationTime !== false ? $passwordExpirationTime : null);
 		$share->setLabel($data['label'] ?? '');

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -59,7 +59,6 @@ class ShareByMailProviderTest extends TestCase {
 	private IShare&MockObject $share;
 	private IConfig&MockObject $config;
 	private IMailer&MockObject $mailer;
-	private IHasher&MockObject $hasher;
 	private Defaults&MockObject $defaults;
 	private IManager&MockObject $shareManager;
 	private LoggerInterface&MockObject $logger;
@@ -92,7 +91,6 @@ protected function setUp(): void {
 		$this->activityManager = $this->createMock('OCP\Activity\IManager');
 		$this->settingsManager = $this->createMock(SettingsManager::class);
 		$this->defaults = $this->createMock(Defaults::class);
-		$this->hasher = $this->createMock(IHasher::class);
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->shareManager = $this->createMock(IManager::class);
 
@@ -122,7 +120,7 @@ private function getInstance(array $mockedMethods = []) {
 					$this->activityManager,
 					$this->settingsManager,
 					$this->defaults,
-					$this->hasher,
+					Server::get(IHasher::class),
 					$this->eventDispatcher,
 					$this->shareManager,
 					$this->getEmailValidatorWithStrictEmailCheck(),
@@ -144,7 +142,7 @@ private function getInstance(array $mockedMethods = []) {
 			$this->activityManager,
 			$this->settingsManager,
 			$this->defaults,
-			$this->hasher,
+			Server::get(IHasher::class),
 			$this->eventDispatcher,
 			$this->shareManager,
 			$this->getEmailValidatorWithStrictEmailCheck(),
@@ -253,8 +251,8 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
 		$share->expects($this->any())->method('getNode')->willReturn($node);
 
 		$share->expects($this->any())->method('getPassword')->willReturn('password');
-		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
-		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->never())->method('setPasswordHash');
 
 		// The given password (but not the autogenerated password) should not be
 		// mailed to the receiver of the share because permanent passwords are not enforced.
@@ -300,8 +298,8 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
 		$share->expects($this->any())->method('getNode')->willReturn($node);
 
 		$share->expects($this->any())->method('getPassword')->willReturn('password');
-		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
-		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->never())->method('setPasswordHash');
 
 		// No password is generated, so no emails need to be sent
 		// aside from the main email notification.
@@ -336,8 +334,8 @@ public function testCreateSendPasswordToOwnerWhenSendPasswordByMailIsDisabled():
 		$share->method('getPassword')->willReturn('password');
 
 		$this->mailer->method('validateMailAddress')->willReturn(true);
-		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
-		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->never())->method('setPasswordHash');
 
 		$instance = $this->getInstance([
 			'getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject',
@@ -399,8 +397,8 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPe
 
 		// Initially not set, but will be set by the autoGeneratePassword method.
 		$share->expects($this->exactly(3))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword');
-		$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
-		$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->once())->method('setPasswordHash')->with($this->callback(fn (string $hash): bool => Server::get(IHasher::class)->verify('autogeneratedPassword', $hash)));
 
 		// The autogenerated password should be mailed to the receiver of the share because permanent passwords are enforced.
 		$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true);
@@ -478,8 +476,8 @@ public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordP
 		$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare);
 
 		$share->expects($this->exactly(3))->method('getPassword')->willReturn('password');
-		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
-		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->never())->method('setPasswordHash');
 
 		// The given password (but not the autogenerated password) should be
 		// mailed to the receiver of the share.
@@ -566,8 +564,8 @@ public function testCreateSendPasswordByTalkWithEnforcedPasswordProtectionWithPe
 		$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'autogeneratedPassword'])->willReturn($expectedShare);
 
 		$share->expects($this->exactly(3))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword');
-		$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
-		$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->once())->method('setPasswordHash')->with($this->callback(fn (string $hash): bool => Server::get(IHasher::class)->verify('autogeneratedPassword', $hash)));
 
 		// The autogenerated password should be mailed to the owner of the share.
 		$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true);
@@ -662,8 +660,8 @@ public function sendNotificationToMultipleEmails() {
 		$share->expects($this->any())->method('getNode')->willReturn($node);
 
 		$share->expects($this->any())->method('getPassword')->willReturn('password');
-		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
-		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+		$share->expects($this->never())->method('setPassword');
+		$share->expects($this->never())->method('setPasswordHash');
 
 		// The given password (but not the autogenerated password) should not be
 		// mailed to the receiver of the share because permanent passwords are not enforced.
@@ -777,7 +775,8 @@ public function testAddShareToDB(): void {
 				$sendPasswordByTalk,
 				$hideDownload,
 				$label,
-				$expiration
+				$expiration,
+				null,
 			]
 		);
 
@@ -855,14 +854,18 @@ public function testUpdate(): void {
 	}
 
 	public static function dataUpdateSendPassword(): array {
+		$hasher = Server::get(IHasher::class);
+		$hash = $hasher->hash('password');
+		$hashNew = $hasher->hash('password new');
+
 		return [
-			['password', 'hashed', 'hashed new', false, false, true],
-			['', 'hashed', 'hashed new', false, false, false],
-			[null, 'hashed', 'hashed new', false, false, false],
-			['password', 'hashed', 'hashed', false, false, false],
-			['password', 'hashed', 'hashed new', false, true, false],
-			['password', 'hashed', 'hashed new', true, false, true],
-			['password', 'hashed', 'hashed', true, false, true],
+			['password', $hash, $hashNew, false, false, true],
+			['', $hash, $hashNew, false, false, false],
+			[null, $hash, $hashNew, false, false, false],
+			['password', $hash, $hash, false, false, false],
+			['password', $hash, $hashNew, false, true, false],
+			['password', $hash, $hashNew, true, false, true],
+			['password', $hash, $hash, true, false, true],
 		];
 	}
 
@@ -884,6 +887,7 @@ public function testUpdateSendPassword(?string $plainTextPassword, string $origi
 		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
 		$share->expects($this->any())->method('getNode')->willReturn($node);
 		$share->expects($this->any())->method('getId')->willReturn('42');
+		$share->expects($this->any())->method('isPasswordHashed')->willReturn(true);
 		$share->expects($this->any())->method('getPassword')->willReturn($newPassword);
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk);
 

apps/sharing/appinfo/info.xml

@@ -7,8 +7,8 @@
 	  xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd">
 	<id>sharing</id>
 	<name>Sharing</name>
-	<summary>TODO</summary>
-	<description>TODO</description>
+	<summary>This app provides APIs and occ commands to manage shares.</summary>
+	<description>This app provides APIs and occ commands to manage shares.</description>
 	<version>2.0.0-dev.3</version>
 	<licence>AGPL-3.0-or-later</licence>
 	<author>Kate Döen</author>

apps/sharing/lib/Command/AddShareRecipient.php

@@ -16,7 +16,6 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
-// TODO: Initiator missing.
 final class AddShareRecipient extends SharingBase {
 	#[\Override]
 	public function configure(): void {

apps/sharing/openapi.json

@@ -3,7 +3,7 @@
     "info": {
         "title": "sharing",
         "version": "0.0.1",
-        "description": "TODO",
+        "description": "This app provides APIs and occ commands to manage shares.",
         "license": {
             "name": "AGPL-3.0-or-later"
         }

build/psalm-baseline.xml

@@ -20,7 +20,6 @@
   <file src="apps/cloud_federation_api/lib/Controller/RequestHandlerController.php">
     <DeprecatedMethod>
       <code><![CDATA[getFederationIdFromSharedSecret]]></code>
-      <code><![CDATA[getFederationIdFromSharedSecret]]></code>
     </DeprecatedMethod>
   </file>
   <file src="apps/comments/lib/Activity/Listener.php">
@@ -201,32 +200,17 @@
       <code><![CDATA[null]]></code>
     </NullableReturnStatement>
     <UndefinedMagicPropertyFetch>
-      <code><![CDATA[$component->CLASS]]></code>
-      <code><![CDATA[$component->CLASS]]></code>
       <code><![CDATA[$component->CLASS]]></code>
       <code><![CDATA[$component->DTEND]]></code>
       <code><![CDATA[$component->DTEND]]></code>
-      <code><![CDATA[$component->DTEND]]></code>
-      <code><![CDATA[$component->DTSTART]]></code>
       <code><![CDATA[$component->DTSTART]]></code>
-      <code><![CDATA[$component->DTSTART]]></code>
-      <code><![CDATA[$component->DUE]]></code>
       <code><![CDATA[$component->DUE]]></code>
       <code><![CDATA[$component->DUE]]></code>
       <code><![CDATA[$component->DURATION]]></code>
       <code><![CDATA[$component->DURATION]]></code>
-      <code><![CDATA[$component->DURATION]]></code>
-      <code><![CDATA[$component->RDATE]]></code>
-      <code><![CDATA[$component->RDATE]]></code>
       <code><![CDATA[$component->RDATE]]></code>
       <code><![CDATA[$component->RDATE]]></code>
-      <code><![CDATA[$component->RDATE]]></code>
-      <code><![CDATA[$component->RDATE]]></code>
-      <code><![CDATA[$component->RRULE]]></code>
       <code><![CDATA[$component->RRULE]]></code>
-      <code><![CDATA[$component->RRULE]]></code>
-      <code><![CDATA[$component->UID]]></code>
-      <code><![CDATA[$component->UID]]></code>
       <code><![CDATA[$component->UID]]></code>
     </UndefinedMagicPropertyFetch>
   </file>
@@ -2423,9 +2407,6 @@
       <code><![CDATA[deleteAppValue]]></code>
       <code><![CDATA[deleteAppValue]]></code>
       <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
       <code><![CDATA[setAppValue]]></code>
       <code><![CDATA[setAppValue]]></code>
     </DeprecatedMethod>

core/Sharing/Recipient/GroupShareRecipientType.php

@@ -20,7 +20,7 @@
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\EventDispatcher\IEventListener;
-use OCP\Group\Events\GroupDeletedEvent;
+use OCP\Group\Events\BeforeGroupDeletedEvent;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
 use OCP\Interaction\InteractionReceiver;
@@ -30,7 +30,7 @@
 use OCP\Share\IShare;
 
 /**
- * @template-implements IEventListener<GroupDeletedEvent>
+ * @template-implements IEventListener<BeforeGroupDeletedEvent>
  */
 final class GroupShareRecipientType extends AShareRecipientTypeSearchCollaborator implements IEventListener {
 	public function __construct(
@@ -39,7 +39,7 @@ public function __construct(
 		private readonly IGroupManager $groupManager,
 		private readonly ISharingManager $manager,
 	) {
-		$eventDispatcher->addServiceListener(GroupDeletedEvent::class, self::class);
+		$eventDispatcher->addServiceListener(BeforeGroupDeletedEvent::class, self::class);
 	}
 
 	#[\Override]

core/Sharing/Recipient/UserShareRecipientType.php

@@ -26,10 +26,10 @@
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Share\IShare;
-use OCP\User\Events\UserDeletedEvent;
+use OCP\User\Events\BeforeUserDeletedEvent;
 
 /**
- * @template-implements IEventListener<UserDeletedEvent>
+ * @template-implements IEventListener<BeforeUserDeletedEvent>
  */
 final class UserShareRecipientType extends AShareRecipientTypeSearchCollaborator implements IEventListener {
 
@@ -39,7 +39,7 @@ public function __construct(
 		private readonly IUserManager $userManager,
 		private readonly ISharingManager $manager,
 	) {
-		$eventDispatcher->addServiceListener(UserDeletedEvent::class, self::class);
+		$eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, self::class);
 	}
 
 	#[\Override]

lib/composer/composer/autoload_classmap.php

@@ -2366,7 +2366,6 @@
     'OC\\Share20\\UserRemovedListener' => $baseDir . '/lib/private/Share20/UserRemovedListener.php',
     'OC\\Share\\Constants' => $baseDir . '/lib/private/Share/Constants.php',
     'OC\\Sharing\\ClassMapper' => $baseDir . '/lib/private/Sharing/ClassMapper.php',
-    'OC\\Sharing\\ISharingLegacyBackend' => $baseDir . '/lib/private/Sharing/ISharingLegacyBackend.php',
     'OC\\Sharing\\SharingBackend' => $baseDir . '/lib/private/Sharing/SharingBackend.php',
     'OC\\Sharing\\SharingManager' => $baseDir . '/lib/private/Sharing/SharingManager.php',
     'OC\\Sharing\\SharingRegistry' => $baseDir . '/lib/private/Sharing/SharingRegistry.php',