diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index b96a61288517f..3a41fc790fb98 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/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'] ?? ''); diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index c87748f6657ce..59971ce925324 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/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); diff --git a/apps/sharing/appinfo/info.xml b/apps/sharing/appinfo/info.xml index 405780fba970c..e6bf3b24f2cb9 100644 --- a/apps/sharing/appinfo/info.xml +++ b/apps/sharing/appinfo/info.xml @@ -7,8 +7,8 @@ xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd"> sharing Sharing - TODO - TODO + This app provides APIs and occ commands to manage shares. + This app provides APIs and occ commands to manage shares. 1.0.4 AGPL-3.0-or-later Kate Döen diff --git a/apps/sharing/lib/Command/AddShareRecipient.php b/apps/sharing/lib/Command/AddShareRecipient.php index 5742b0816087b..e45a347943a1d 100644 --- a/apps/sharing/lib/Command/AddShareRecipient.php +++ b/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 { diff --git a/apps/sharing/openapi.json b/apps/sharing/openapi.json index 089195d6b4879..6914cedb1a7b0 100644 --- a/apps/sharing/openapi.json +++ b/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" } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 3376b25694767..3fb2f9f109cab 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -25,7 +25,6 @@ ['uid' => &$uid] )]]> - @@ -206,32 +205,17 @@ - CLASS]]> - CLASS]]> CLASS]]> DTEND]]> DTEND]]> - DTEND]]> - DTSTART]]> DTSTART]]> - DTSTART]]> - DUE]]> DUE]]> DUE]]> DURATION]]> DURATION]]> - DURATION]]> - RDATE]]> - RDATE]]> RDATE]]> RDATE]]> - RDATE]]> - RDATE]]> - RRULE]]> RRULE]]> - RRULE]]> - UID]]> - UID]]> UID]]> @@ -2460,9 +2444,6 @@ - - - diff --git a/core/Sharing/Recipient/GroupShareRecipientType.php b/core/Sharing/Recipient/GroupShareRecipientType.php index d2539c72399b2..01490505dcce4 100644 --- a/core/Sharing/Recipient/GroupShareRecipientType.php +++ b/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 + * @template-implements IEventListener */ 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] diff --git a/core/Sharing/Recipient/UserShareRecipientType.php b/core/Sharing/Recipient/UserShareRecipientType.php index 3c69a7ab804b1..61927b0b1f8c0 100644 --- a/core/Sharing/Recipient/UserShareRecipientType.php +++ b/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 + * @template-implements IEventListener */ 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] diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index d80e425b285e4..86172e2a0c79f 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2356,7 +2356,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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index ee95df28b9974..b605621ba9b77 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2397,7 +2397,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Share20\\UserRemovedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserRemovedListener.php', 'OC\\Share\\Constants' => __DIR__ . '/../../..' . '/lib/private/Share/Constants.php', 'OC\\Sharing\\ClassMapper' => __DIR__ . '/../../..' . '/lib/private/Sharing/ClassMapper.php', - 'OC\\Sharing\\ISharingLegacyBackend' => __DIR__ . '/../../..' . '/lib/private/Sharing/ISharingLegacyBackend.php', 'OC\\Sharing\\SharingBackend' => __DIR__ . '/../../..' . '/lib/private/Sharing/SharingBackend.php', 'OC\\Sharing\\SharingManager' => __DIR__ . '/../../..' . '/lib/private/Sharing/SharingManager.php', 'OC\\Sharing\\SharingRegistry' => __DIR__ . '/../../..' . '/lib/private/Sharing/SharingRegistry.php', diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php index 7499db4046f07..a27c4ef89c536 100644 --- a/lib/private/Group/Group.php +++ b/lib/private/Group/Group.php @@ -98,11 +98,6 @@ public function setDisplayName(string $displayName): bool { return false; } - /** - * get all users in the group - * - * @return array - */ #[\Override] public function getUsers(): array { if ($this->usersLoaded) { diff --git a/lib/private/Security/Hasher.php b/lib/private/Security/Hasher.php index 418c8b88a58b8..2bbcd2fa6fa22 100644 --- a/lib/private/Security/Hasher.php +++ b/lib/private/Security/Hasher.php @@ -36,6 +36,8 @@ class Hasher implements IHasher { private array $options = []; /** Salt used for legacy passwords */ private ?string $legacySalt = null; + /** Only used for testing */ + private ?string $forcedAlgorithm = null; public function __construct( private IConfig $config, @@ -72,15 +74,13 @@ public function __construct( public function hash(string $message): string { $alg = $this->getPrefferedAlgorithm(); - if (\defined('PASSWORD_ARGON2ID') && $alg === PASSWORD_ARGON2ID) { - return 3 . '|' . password_hash($message, PASSWORD_ARGON2ID, $this->options); - } - - if (\defined('PASSWORD_ARGON2I') && $alg === PASSWORD_ARGON2I) { - return 2 . '|' . password_hash($message, PASSWORD_ARGON2I, $this->options); - } + $version = match ($alg) { + PASSWORD_ARGON2ID => 3, + PASSWORD_ARGON2I => 2, + PASSWORD_BCRYPT => 1, + }; - return 1 . '|' . password_hash($message, PASSWORD_BCRYPT, $this->options); + return $version . '|' . password_hash($message, $alg, $this->options); } /** @@ -182,6 +182,10 @@ private function needsRehash(string $hash): bool { } private function getPrefferedAlgorithm(): string { + if ($this->forcedAlgorithm !== null) { + return $this->forcedAlgorithm; + } + $default = PASSWORD_BCRYPT; if (\defined('PASSWORD_ARGON2I')) { $default = PASSWORD_ARGON2I; diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index c7a47188f587b..0fff3fd0644bc 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -46,6 +46,7 @@ use OCP\Share\IShareProviderWithNotification; use OCP\Util; use Psr\Log\LoggerInterface; +use RuntimeException; use function str_starts_with; use function strlen; @@ -110,7 +111,10 @@ public function create(IShare $share) { if ($share->getShareType() === IShare::TYPE_USER) { //Set the UID of the user we share with $qb->setValue('share_with', $qb->createNamedParameter($share->getSharedWith())); - $qb->setValue('accepted', $qb->createNamedParameter(IShare::STATUS_PENDING)); + if ($share->getStatus() === null) { + $share->setStatus(IShare::STATUS_PENDING); + } + $qb->setValue('accepted', $qb->createNamedParameter($share->getStatus())); //If an expiration date is set store it if ($expirationDate !== null) { @@ -133,8 +137,11 @@ public function create(IShare $share) { $qb->setValue('token', $qb->createNamedParameter($share->getToken())); //If a password is set store it - if ($share->getPassword() !== null) { - $qb->setValue('password', $qb->createNamedParameter($share->getPassword())); + if (($password = $share->getPassword()) !== null) { + if (!$share->isPasswordHashed()) { + throw new RuntimeException('The password must be hashed already.'); + } + $qb->setValue('password', $qb->createNamedParameter($password)); } $qb->setValue('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)); @@ -185,9 +192,9 @@ public function create(IShare $share) { $qb->setValue('note', $qb->createNamedParameter($share->getNote())); } - // Set the time this share was created - $shareTime = $this->timeFactory->now(); + $shareTime = $share->getShareTime() ?? \DateTime::createFromImmutable($this->timeFactory->now()); $qb->setValue('stime', $qb->createNamedParameter($shareTime->getTimestamp())); + $share->setShareTime($shareTime); // insert the data and fetch the id of the share $qb->executeStatement(); @@ -197,8 +204,6 @@ public function create(IShare $share) { $share->setId((string)$id); $share->setProviderId($this->identifier()); - $share->setShareTime(\DateTime::createFromImmutable($shareTime)); - $mailSendValue = $share->getMailSend(); $share->setMailSend(($mailSendValue === null) ? true : $mailSendValue); @@ -287,10 +292,15 @@ public function update(IShare $share) { ->set('attributes', $qb->createNamedParameter($shareAttributes)) ->executeStatement(); } elseif ($share->getShareType() === IShare::TYPE_LINK) { + $password = $share->getPassword(); + if ($password !== null && !$share->isPasswordHashed()) { + throw new RuntimeException('The password must be hashed already.'); + } + $qb = $this->dbConn->getQueryBuilder(); $qb->update('share') ->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId()))) - ->set('password', $qb->createNamedParameter($share->getPassword())) + ->set('password', $qb->createNamedParameter($password)) ->set('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL)) ->set('uid_owner', $qb->createNamedParameter($share->getShareOwner())) ->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy())) @@ -1129,7 +1139,9 @@ private function createShare($data): IShare { $share->setSharedWith($data['share_with']); $share->setSharedWithDisplayNameCallback(fn (IShare $share) => $this->groupManager->getDisplayName($share->getSharedWith())); } elseif ($share->getShareType() === IShare::TYPE_LINK) { - $share->setPassword($data['password']); + if (($password = $data['password']) !== null) { + $share->setPasswordHash($password); + } $share->setSendPasswordByTalk((bool)$data['password_by_talk']); $share->setToken($data['token']); } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 48b7e700133e1..32e9f4a51f47a 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -83,6 +83,7 @@ use OCP\Util; use Override; use Psr\Log\LoggerInterface; +use RuntimeException; /** * This class is the communication hub for all sharing related operations. @@ -581,9 +582,8 @@ public function createShare(IShare $share): IShare { $this->verifyPassword($share->getPassword()); // If a password is set. Hash it! - if ($share->getShareType() === IShare::TYPE_LINK - && $share->getPassword() !== null) { - $share->setPassword($this->hasher->hash($share->getPassword())); + if (($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL) && $share->getPassword() !== null && !$share->isPasswordHashed()) { + $share->setPasswordHash($this->hasher->hash($share->getPassword())); } } @@ -602,9 +602,11 @@ public function createShare(IShare $share): IShare { } } - $target = $shareFolder . '/' . $share->getNode()->getName(); - $target = Filesystem::normalizePath($target); - $share->setTarget($target); + if ($share->getTarget() === null) { + $target = $shareFolder . '/' . $share->getNode()->getName(); + $target = Filesystem::normalizePath($target); + $share->setTarget($target); + } // Pre share event $event = new BeforeShareCreatedEvent($share); @@ -846,7 +848,7 @@ private function updateSharePasswordIfNeeded(IShare $share, IShare $originalShar // If a password is set. Hash it! if (!empty($share->getPassword())) { - $share->setPassword($this->hasher->hash($share->getPassword())); + $share->setPasswordHash($this->hasher->hash($share->getPassword())); if ($share->getShareType() === IShare::TYPE_EMAIL) { // Shares shared by email have temporary passwords $this->setSharePasswordExpirationTime($share); @@ -864,7 +866,12 @@ private function updateSharePasswordIfNeeded(IShare $share, IShare $originalShar } else { // Reset the password to the original one, as it is either the same // as the "new" password or a hashed version of it. - $share->setPassword($originalShare->getPassword()); + $password = $originalShare->getPassword(); + if ($password !== null && $originalShare->isPasswordHashed()) { + $share->setPasswordHash($password); + } else { + $share->setPassword($password); + } } return false; @@ -1487,13 +1494,17 @@ public function checkPassword(IShare $share, ?string $password): bool { return false; } + if (!$share->isPasswordHashed()) { + throw new RuntimeException('The password must be hashed already.'); + } + $newHash = ''; if (!$this->hasher->verify($password, $share->getPassword(), $newHash)) { return false; } if (!empty($newHash)) { - $share->setPassword($newHash); + $share->setPasswordHash($newHash); $provider = $this->factory->getProviderForType($share->getShareType()); $provider->update($share); } diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index 3ea440a885a68..35c70a1464bfc 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -16,6 +16,7 @@ use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\IUserManager; +use OCP\Security\IHasher; use OCP\Server; use OCP\Share\Exceptions\IllegalIDChangeException; use OCP\Share\IAttributes; @@ -57,6 +58,7 @@ class Share implements IShare { private $expireDate; /** @var string */ private $password; + private bool $isPasswordHashed = false; private ?\DateTimeInterface $passwordExpirationTime = null; /** @var bool */ private $sendPasswordByTalk = false; @@ -501,6 +503,18 @@ public function getShareOwner() { #[\Override] public function setPassword($password) { $this->password = $password; + $this->isPasswordHashed = false; + return $this; + } + + #[\Override] + public function setPasswordHash(string $passwordHash): IShare { + if (!Server::get(IHasher::class)->validate($passwordHash)) { + throw new \InvalidArgumentException(); + } + + $this->password = $passwordHash; + $this->isPasswordHashed = true; return $this; } @@ -512,6 +526,11 @@ public function getPassword() { return $this->password; } + #[\Override] + public function isPasswordHashed(): bool { + return $this->isPasswordHashed; + } + /** * @inheritdoc */ diff --git a/lib/private/Sharing/ISharingLegacyBackend.php b/lib/private/Sharing/ISharingLegacyBackend.php deleted file mode 100644 index 5aa9f45271e75..0000000000000 --- a/lib/private/Sharing/ISharingLegacyBackend.php +++ /dev/null @@ -1,59 +0,0 @@ -> - */ - public function getCompatibleSourceTypes(): array; - - /** - * @return list> - */ - public function getCompatibleRecipientTypes(): array; - - /** - * Update a share. - */ - public function updateShare(Share $share): void; - - /** - * Delete a share. - * - * @throws ShareNotFoundException - */ - public function deleteShare(string $id): void; - - /** - * Get a share. - * - * @throws ShareNotFoundException - */ - public function getShare(ShareAccessContext $accessContext, string $id): Share; - - /** - * Get multiple shares. - * - * @param ?class-string $filterSourceTypeClass - * @param ?positive-int $limit - * @return list - */ - public function getShares(ShareAccessContext $accessContext, ?string $filterSourceTypeClass, ?string $filterSourceTypeValue, ?string $lastShareID, ?int $limit): array; -} diff --git a/lib/private/Sharing/SharingManager.php b/lib/private/Sharing/SharingManager.php index 4b035e55741f2..0190d1408864a 100644 --- a/lib/private/Sharing/SharingManager.php +++ b/lib/private/Sharing/SharingManager.php @@ -50,8 +50,6 @@ use Random\Randomizer; use RuntimeException; -// TODO: Add accept/reject -// TODO: Add permission masking (reshares) // TODO: Test sharing to federated users, groups and circles // TODO: Implement share transfers // TODO: Cache share owner @@ -188,14 +186,7 @@ public function onOwnerDeleted(ShareAccessContext $accessContext, ShareUser $own // No need to update the last updated timestamp, because the share will be deleted anyway. - $ids = $this->backend->onOwnerDeleted($owner); - - $legacyBackend = $this->registry->getLegacyBackend(); - if ($legacyBackend instanceof ISharingLegacyBackend) { - foreach ($ids as $id) { - $legacyBackend->deleteShare($id); - } - } + $this->backend->onOwnerDeleted($owner); } #[\Override] @@ -759,11 +750,6 @@ public function deleteShare(ShareAccessContext $accessContext, Share $share): vo $this->validateShareEditPermissions($accessContext, $share); $this->backend->deleteShare($share->id); - - $legacyBackend = $this->registry->getLegacyBackend(); - if ($legacyBackend instanceof ISharingLegacyBackend) { - $legacyBackend->deleteShare($share->id); - } } #[\Override] @@ -988,27 +974,6 @@ private function processShareUpdates(array $shares): array { ); } } - - $legacyBackend = $this->registry->getLegacyBackend(); - if ($legacyBackend instanceof ISharingLegacyBackend) { - $compatibleSourceTypes = array_fill_keys($legacyBackend->getCompatibleSourceTypes(), true); - foreach ($share->sources as $source) { - if (!isset($compatibleSourceTypes[$source->class])) { - throw new RuntimeException('The legacy backend ' . $legacyBackend::class . ' does not support this source type: ' . $source->class); - } - } - - $compatibleRecipientTypes = array_fill_keys($legacyBackend->getCompatibleRecipientTypes(), true); - foreach ($share->recipients as $recipient) { - if (!isset($compatibleRecipientTypes[$recipient->class])) { - throw new RuntimeException( - 'The legacy backend ' . $legacyBackend::class . ' does not support this recipient type: ' . $recipient->class - ); - } - } - - $legacyBackend->updateShare($share); - } } return $shares; diff --git a/lib/private/Sharing/SharingRegistry.php b/lib/private/Sharing/SharingRegistry.php index d22e447f2c69a..d2c15b5ce9150 100644 --- a/lib/private/Sharing/SharingRegistry.php +++ b/lib/private/Sharing/SharingRegistry.php @@ -17,10 +17,7 @@ use NCU\Sharing\Source\IShareSourceType; use RuntimeException; -// TODO: Maybe add validate method to run all checks before using the manager final class SharingRegistry implements ISharingRegistry { - private ?ISharingLegacyBackend $legacyBackend = null; - /** @var array, IShareSourceType> */ private array $sourceTypes = []; @@ -59,7 +56,6 @@ final class SharingRegistry implements ISharingRegistry { #[\Override] public function clear(): void { - $this->legacyBackend = null; $this->sourceTypes = []; $this->recipientTypes = []; $this->propertyTypes = []; @@ -74,20 +70,6 @@ public function clear(): void { $this->permissionPresetCompatiblePermissionTypes = []; } - #[\Override] - public function registerLegacyBackend(ISharingLegacyBackend $legacyBackend): void { - if ($this->legacyBackend instanceof ISharingLegacyBackend) { - throw new RuntimeException('A sharing legacy backend is already registered'); - } - - $this->legacyBackend = $legacyBackend; - } - - #[\Override] - public function getLegacyBackend(): ?ISharingLegacyBackend { - return $this->legacyBackend; - } - #[\Override] public function registerSourceType(IShareSourceType $sourceType): void { $class = $sourceType::class; diff --git a/lib/private/Snowflake/SnowflakeGenerator.php b/lib/private/Snowflake/SnowflakeGenerator.php index 6e07bd72d6f61..a73253d246c7f 100644 --- a/lib/private/Snowflake/SnowflakeGenerator.php +++ b/lib/private/Snowflake/SnowflakeGenerator.php @@ -9,6 +9,7 @@ namespace OC\Snowflake; +use DateTimeImmutable; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IServerInfo; use OCP\Snowflake\ISnowflakeGenerator; @@ -31,9 +32,12 @@ public function __construct( } #[Override] - public function nextId(): string { + public function nextId(?DateTimeImmutable $timestamp = null): string { + $timestamp ??= $this->timeFactory->now(); + // Relative time - [$seconds, $milliseconds] = $this->getCurrentTime(); + $seconds = $timestamp->getTimestamp() - self::TS_OFFSET; + $milliseconds = (int)$timestamp->format('v'); $serverId = $this->serverInfo->getServerId(); $isCli = (int)$this->isCli(); // 1 bit @@ -124,14 +128,6 @@ private function convertToDecimal(array $bytes): string { return $digits; } - private function getCurrentTime(): array { - $time = $this->timeFactory->now(); - return [ - $time->getTimestamp() - self::TS_OFFSET, - (int)$time->format('v'), - ]; - } - private function isCli(): bool { return PHP_SAPI === 'cli'; } diff --git a/lib/public/IGroup.php b/lib/public/IGroup.php index 448b9f4c8f8ce..33da553d1ee38 100644 --- a/lib/public/IGroup.php +++ b/lib/public/IGroup.php @@ -41,7 +41,7 @@ public function setDisplayName(string $displayName): bool; /** * get all users in the group * - * @return IUser[] + * @return array * @since 8.0.0 */ public function getUsers(): array; diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 5e5647f52aa10..6c92f30ef0ccd 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -453,6 +453,14 @@ public function getShareOwner(); */ public function setPassword($password); + /** + * Sets the password for the shared, but in it's already hashed form. + * Use {@see isPasswordHashed} to check if the return value of {@see getPassword} is already hashed. + * + * @since 35.0.0 + */ + public function setPasswordHash(string $passwordHash): IShare; + /** * Get the password of this share. * If this share is obtained via a shareprovider the password is @@ -463,6 +471,13 @@ public function setPassword($password); */ public function getPassword(); + /** + * Returns whether the return value of {@see getPassword} is already hashed. + * + * @since 35.0.0 + */ + public function isPasswordHashed(): bool; + /** * Returns whether the share is password protected by any means (e.g. password or OTP) * @return bool diff --git a/lib/public/Snowflake/ISnowflakeGenerator.php b/lib/public/Snowflake/ISnowflakeGenerator.php index ee9a02bc486fa..475d376788313 100644 --- a/lib/public/Snowflake/ISnowflakeGenerator.php +++ b/lib/public/Snowflake/ISnowflakeGenerator.php @@ -9,6 +9,7 @@ namespace OCP\Snowflake; +use DateTimeImmutable; use OCP\AppFramework\Attribute\Consumable; /** @@ -39,11 +40,12 @@ interface ISnowflakeGenerator { * * Each call to this method is guaranteed to return a different ID. * + * @param ?DateTimeImmutable $timestamp Generate the Snowflake ID for a specific time. This should only be used in very special cases. * @return non-empty-string * * @since 33.0 */ - public function nextId(): string; + public function nextId(?DateTimeImmutable $timestamp = null): string; /** * Return the smallest possible Snowflake ID for a given timestamp diff --git a/lib/unstable/Sharing/ISharingRegistry.php b/lib/unstable/Sharing/ISharingRegistry.php index 7e56c194f00c1..77bb566bdc59f 100644 --- a/lib/unstable/Sharing/ISharingRegistry.php +++ b/lib/unstable/Sharing/ISharingRegistry.php @@ -14,7 +14,6 @@ use NCU\Sharing\Property\ISharePropertyType; use NCU\Sharing\Recipient\IShareRecipientType; use NCU\Sharing\Source\IShareSourceType; -use OC\Sharing\ISharingLegacyBackend; use OCP\AppFramework\Attribute\Consumable; /** @@ -27,16 +26,6 @@ interface ISharingRegistry { */ public function clear(): void; - /** - * @experimental 35.0.0 - */ - public function registerLegacyBackend(ISharingLegacyBackend $legacyBackend): void; - - /** - * @experimental 35.0.0 - */ - public function getLegacyBackend(): ?ISharingLegacyBackend; - /** * @experimental 35.0.0 */ diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 8917f0abaa290..89b5ea544b746 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -30,6 +30,7 @@ use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Mail\IMailer; +use OCP\Security\IHasher; use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager as IShareManager; @@ -405,12 +406,14 @@ public function testGetShareByIdUserGroupShare(): void { } public function testGetShareByIdLinkShare(): void { + $passwordHash = Server::get(IHasher::class)->hash('password'); + $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ 'share_type' => $qb->expr()->literal(IShare::TYPE_LINK), - 'password' => $qb->expr()->literal('password'), + 'password' => $qb->expr()->literal($passwordHash), 'password_by_talk' => $qb->expr()->literal(true), 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), @@ -440,7 +443,8 @@ public function testGetShareByIdLinkShare(): void { $this->assertEquals($id, $share->getId()); $this->assertEquals(IShare::TYPE_LINK, $share->getShareType()); $this->assertNull($share->getSharedWith()); - $this->assertEquals('password', $share->getPassword()); + $this->assertTrue($share->isPasswordHashed()); + $this->assertEquals($passwordHash, $share->getPassword()); $this->assertEquals(true, $share->getSendPasswordByTalk()); $this->assertEquals('sharedBy', $share->getSharedBy()); $this->assertEquals('shareOwner', $share->getShareOwner()); @@ -832,6 +836,8 @@ public function testCreateGroupShare(): void { } public function testCreateLinkShare(): void { + $passwordHash = Server::get(IHasher::class)->hash('password'); + $share = new Share($this->rootFolder, $this->userManager); $shareOwner = $this->createMock(IUser::class); @@ -862,7 +868,7 @@ public function testCreateLinkShare(): void { $share->setShareOwner('shareOwner'); $share->setNode($path); $share->setPermissions(1); - $share->setPassword('password'); + $share->setPasswordHash($passwordHash); $share->setSendPasswordByTalk(true); $share->setToken('token'); $expireDate = new \DateTime(); @@ -880,19 +886,22 @@ public function testCreateLinkShare(): void { $this->assertSame('/target', $share2->getTarget()); $this->assertLessThanOrEqual(new \DateTime(), $share2->getShareTime()); $this->assertSame($path, $share2->getNode()); - $this->assertSame('password', $share2->getPassword()); + $this->assertTrue($share2->isPasswordHashed()); + $this->assertSame($passwordHash, $share2->getPassword()); $this->assertSame(true, $share2->getSendPasswordByTalk()); $this->assertSame('token', $share2->getToken()); $this->assertEquals($expireDate->getTimestamp(), $share2->getExpirationDate()->getTimestamp()); } public function testGetShareByToken(): void { + $passwordHash = Server::get(IHasher::class)->hash('password'); + $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ 'share_type' => $qb->expr()->literal(IShare::TYPE_LINK), - 'password' => $qb->expr()->literal('password'), + 'password' => $qb->expr()->literal($passwordHash), 'password_by_talk' => $qb->expr()->literal(true), 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), @@ -916,7 +925,8 @@ public function testGetShareByToken(): void { $this->assertSame('shareOwner', $share->getShareOwner()); $this->assertSame('sharedBy', $share->getSharedBy()); $this->assertSame('secrettoken', $share->getToken()); - $this->assertSame('password', $share->getPassword()); + $this->assertTrue($share->isPasswordHashed()); + $this->assertSame($passwordHash, $share->getPassword()); $this->assertSame('the label', $share->getLabel()); $this->assertSame(true, $share->getSendPasswordByTalk()); $this->assertSame(null, $share->getSharedWith()); @@ -927,12 +937,14 @@ public function testGetShareByToken(): void { * as types on IShare, a string and not null */ public function testGetShareByTokenNullLabel(): void { + $passwordHash = Server::get(IHasher::class)->hash('password'); + $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ 'share_type' => $qb->expr()->literal(IShare::TYPE_LINK), - 'password' => $qb->expr()->literal('password'), + 'password' => $qb->expr()->literal($passwordHash), 'password_by_talk' => $qb->expr()->literal(true), 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), @@ -1907,6 +1919,8 @@ function ($userId) use ($users) { } public function testUpdateLink(): void { + $passwordHash = Server::get(IHasher::class)->hash('password'); + $id = $this->addShareToDB(IShare::TYPE_LINK, null, 'user1', 'user2', 'file', 42, 'target', 31, null, null); @@ -1940,7 +1954,7 @@ function ($userId) use ($users) { $share = $this->provider->getShareById($id); - $share->setPassword('password'); + $share->setPasswordHash($passwordHash); $share->setSendPasswordByTalk(true); $share->setSharedBy('user4'); $share->setShareOwner('user5'); @@ -1950,7 +1964,8 @@ function ($userId) use ($users) { $share2 = $this->provider->update($share); $this->assertEquals($id, $share2->getId()); - $this->assertEquals('password', $share2->getPassword()); + $this->assertTrue($share2->isPasswordHashed()); + $this->assertEquals($passwordHash, $share2->getPassword()); $this->assertSame(true, $share2->getSendPasswordByTalk()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); @@ -1959,7 +1974,8 @@ function ($userId) use ($users) { $share2 = $this->provider->getShareById($id); $this->assertEquals($id, $share2->getId()); - $this->assertEquals('password', $share2->getPassword()); + $this->assertTrue($share2->isPasswordHashed()); + $this->assertEquals($passwordHash, $share2->getPassword()); $this->assertSame(true, $share2->getSendPasswordByTalk()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); @@ -1967,13 +1983,15 @@ function ($userId) use ($users) { } public function testUpdateLinkRemovePassword(): void { + $passwordHash = Server::get(IHasher::class)->hash('password'); + $id = $this->addShareToDB(IShare::TYPE_LINK, 'foo', 'user1', 'user2', 'file', 42, 'target', 31, null, null); $qb = $this->dbConn->getQueryBuilder(); $qb->update('share'); $qb->where($qb->expr()->eq('id', $qb->createNamedParameter($id))); - $qb->set('password', $qb->createNamedParameter('password')); + $qb->set('password', $qb->createNamedParameter($passwordHash)); $this->assertEquals(1, $qb->executeStatement()); $users = []; @@ -2015,6 +2033,7 @@ function ($userId) use ($users) { $share2 = $this->provider->update($share); $this->assertEquals($id, $share2->getId()); + $this->assertFalse($share2->isPasswordHashed()); $this->assertEquals(null, $share2->getPassword()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); @@ -2023,6 +2042,7 @@ function ($userId) use ($users) { $share2 = $this->provider->getShareById($id); $this->assertEquals($id, $share2->getId()); + $this->assertFalse($share2->isPasswordHashed()); $this->assertEquals(null, $share2->getPassword()); $this->assertSame('user4', $share2->getSharedBy()); $this->assertSame('user5', $share2->getShareOwner()); diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 0e9cf12bbe402..11dbd74750b92 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -89,7 +89,6 @@ class ManagerTest extends \Test\TestCase { protected LoggerInterface&MockObject $logger; protected IConfig&MockObject $config; protected ISecureRandom&MockObject $secureRandom; - protected IHasher&MockObject $hasher; protected IShareProvider&MockObject $defaultProvider; protected IMountManager&MockObject $mountManager; protected IGroupManager&MockObject $groupManager; @@ -114,7 +113,6 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->config = $this->createMock(IConfig::class); $this->secureRandom = $this->createMock(ISecureRandom::class); - $this->hasher = $this->createMock(IHasher::class); $this->mountManager = $this->createMock(IMountManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); @@ -178,7 +176,7 @@ private function createManager(IProviderFactory $factory): Manager { $this->logger, $this->config, $this->secureRandom, - $this->hasher, + Server::get(IHasher::class), $this->mountManager, $this->groupManager, $this->l10nFactory, @@ -204,7 +202,7 @@ private function createManagerMock(): MockBuilder { $this->logger, $this->config, $this->secureRandom, - $this->hasher, + Server::get(IHasher::class), $this->mountManager, $this->groupManager, $this->l10nFactory, @@ -3501,11 +3499,6 @@ public function testCreateShareLink(): void { ->method('setLinkParent') ->with($share); - $this->hasher->expects($this->once()) - ->method('hash') - ->with('password') - ->willReturn('hashed'); - $this->secureRandom->method('generate') ->willReturn('token'); @@ -3549,7 +3542,8 @@ public function testCreateShareLink(): void { $this->assertEquals('/target', $share->getTarget()); $this->assertSame($date, $share->getExpirationDate()); $this->assertEquals('token', $share->getToken()); - $this->assertEquals('hashed', $share->getPassword()); + $this->assertTrue($share->isPasswordHashed()); + $this->assertNotEmpty($share->getPassword()); } public function testCreateShareMail(): void { @@ -4347,46 +4341,106 @@ public function testCheckPasswordNoPassword(): void { } public function testCheckPasswordInvalidPassword(): void { - $share = $this->createMock(IShare::class); - $share->method('getShareType')->willReturn(IShare::TYPE_LINK); - $share->method('getPassword')->willReturn('password'); - $share->method('isPasswordProtected')->willReturn(true); + $passwordHash = Server::get(IHasher::class)->hash('password'); - $this->hasher->method('verify')->with('invalidpassword', 'password', '')->willReturn(false); + $share = $this->manager->newShare() + ->setShareType(IShare::TYPE_LINK) + ->setPasswordHash($passwordHash); $this->assertFalse($this->manager->checkPassword($share, 'invalidpassword')); } public function testCheckPasswordValidPassword(): void { - $share = $this->createMock(IShare::class); - $share->method('getShareType')->willReturn(IShare::TYPE_LINK); - $share->method('getPassword')->willReturn('passwordHash'); - $share->method('isPasswordProtected')->willReturn(true); + $passwordHash = Server::get(IHasher::class)->hash('password'); - $this->hasher->method('verify')->with('password', 'passwordHash', '')->willReturn(true); + $share = $this->manager->newShare() + ->setShareType(IShare::TYPE_LINK) + ->setPasswordHash($passwordHash); $this->assertTrue($this->manager->checkPassword($share, 'password')); } - public function testCheckPasswordUpdateShare(): void { - $share = $this->manager->newShare(); - $share->setShareType(IShare::TYPE_LINK) - ->setPassword('passwordHash'); + public static function dataHasherAlgorithm(): array { + $algorithms = [ + PASSWORD_BCRYPT, + ]; - $this->hasher->method('verify')->with('password', 'passwordHash', '') - ->willReturnCallback(function ($pass, $hash, &$newHash) { - $newHash = 'newHash'; + if (\defined('PASSWORD_ARGON2I')) { + $algorithms[] = PASSWORD_ARGON2I; + } - return true; - }); + if (\defined('PASSWORD_ARGON2ID')) { + $algorithms[] = PASSWORD_ARGON2ID; + } + + return array_map(static fn (string $algorithm): array => [$algorithm], $algorithms); + } + + #[DataProvider('dataHasherAlgorithm')] + public function testCheckPasswordUpdateShareNoRehash(string $algorithm): void { + $hasher = Server::get(IHasher::class); + $this->invokePrivate($hasher, 'forcedAlgorithm', [$algorithm]); + + $passwordHash = $hasher->hash('password'); + $this->assertEquals((match ($algorithm) { + PASSWORD_ARGON2ID => 3, + PASSWORD_ARGON2I => 2, + PASSWORD_BCRYPT => 1, + }), (int)(explode('|', $passwordHash)[0])); + + $share = $this->manager->newShare() + ->setShareType(IShare::TYPE_LINK) + ->setPasswordHash($passwordHash); + + $this->defaultProvider->expects($this->never())->method('update'); + + $this->assertTrue($this->manager->checkPassword($share, 'password')); + + $this->invokePrivate($hasher, 'forcedAlgorithm', [null]); + } + + #[DataProvider('dataHasherAlgorithm')] + public function testCheckPasswordUpdateShare(string $algorithm): void { + $hasher = Server::get(IHasher::class); + $this->invokePrivate($hasher, 'forcedAlgorithm', [$algorithm]); + + $passwordHash = $hasher->hash('password'); + $this->assertEquals((match ($algorithm) { + PASSWORD_ARGON2ID => 3, + PASSWORD_ARGON2I => 2, + PASSWORD_BCRYPT => 1, + }), (int)(explode('|', $passwordHash)[0])); + + $previousHasherOptions = $this->invokePrivate($hasher, 'options'); + + // Make sure the hasher wants to rehash the password + $this->invokePrivate( + $hasher, + 'options', + [ + [ + 'threads' => $previousHasherOptions['threads'] ?? 1, + 'memory_cost' => ($previousHasherOptions['memory_cost'] ?? PASSWORD_ARGON2_DEFAULT_MEMORY_COST) + 1, + 'time_cost' => ($previousHasherOptions['time_cost'] ?? PASSWORD_ARGON2_DEFAULT_TIME_COST) + 1, + 'cost' => ($previousHasherOptions['cost'] ?? PASSWORD_BCRYPT_DEFAULT_COST) + 1, + ], + ], + ); + + $share = $this->manager->newShare() + ->setShareType(IShare::TYPE_LINK) + ->setPasswordHash($passwordHash); $this->defaultProvider->expects($this->once()) ->method('update') - ->with($this->callback(function (IShare $share) { - return $share->getPassword() === 'newHash'; + ->with($this->callback(function (IShare $share) use ($passwordHash) { + return $share->getPassword() !== null && $share->isPasswordHashed() && $share->getPassword() !== $passwordHash; })); $this->assertTrue($this->manager->checkPassword($share, 'password')); + + $this->invokePrivate($hasher, 'options', [$previousHasherOptions]); + $this->invokePrivate($hasher, 'forcedAlgorithm', [null]); } public function testUpdateShareCantChangeShareType(): void { @@ -4611,11 +4665,6 @@ public function testUpdateShareLink(): void { $manager->expects($this->once())->method('validateExpirationDateLink')->with($share); $manager->expects($this->once())->method('verifyPassword')->with('password'); - $this->hasher->expects($this->once()) - ->method('hash') - ->with('password') - ->willReturn('hashed'); - $this->defaultProvider->expects($this->once()) ->method('update') ->with($share) @@ -4691,9 +4740,6 @@ public function testUpdateShareLinkEnableSendPasswordByTalkWithNoPassword(): voi $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); @@ -4752,11 +4798,6 @@ public function testUpdateShareMail(): void { $manager->expects($this->once())->method('pathCreateChecks')->with($file); $manager->expects($this->once())->method('validateExpirationDateLink'); - $this->hasher->expects($this->once()) - ->method('hash') - ->with('password') - ->willReturn('hashed'); - $this->defaultProvider->expects($this->once()) ->method('update') ->with($share, 'password') @@ -4831,11 +4872,6 @@ public function testUpdateShareMailEnableSendPasswordByTalk(): void { $manager->expects($this->once())->method('pathCreateChecks')->with($file); $manager->expects($this->once())->method('validateExpirationDateLink'); - $this->hasher->expects($this->once()) - ->method('hash') - ->with('password') - ->willReturn('hashed'); - $this->defaultProvider->expects($this->once()) ->method('update') ->with($share, 'password') @@ -4910,16 +4946,6 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithDifferentPassword $manager->expects($this->once())->method('pathCreateChecks')->with($file); $manager->expects($this->once())->method('validateExpirationDateLink'); - $this->hasher->expects($this->once()) - ->method('verify') - ->with('password', 'anotherPasswordHash') - ->willReturn(false); - - $this->hasher->expects($this->once()) - ->method('hash') - ->with('password') - ->willReturn('hashed'); - $this->defaultProvider->expects($this->once()) ->method('update') ->with($share, 'password') @@ -4997,10 +5023,6 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword(): voi $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - // If the password is empty, we have nothing to hash - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); @@ -5065,10 +5087,6 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPassword(): v $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - // If the password is empty, we have nothing to hash - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); @@ -5133,10 +5151,6 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPasswordWithE $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - // If the password is empty, we have nothing to hash - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); @@ -5201,12 +5215,6 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithPreviousPassword( $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - // If the old & new passwords are the same, we don't do anything - $this->hasher->expects($this->never()) - ->method('verify'); - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); @@ -5271,12 +5279,6 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - // If the old & new passwords are the same, we don't do anything - $this->hasher->expects($this->never()) - ->method('verify'); - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); @@ -5341,12 +5343,6 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithoutChangingPassw $manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('validateExpirationDateLink'); - // If the old & new passwords are the same, we don't do anything - $this->hasher->expects($this->never()) - ->method('verify'); - $this->hasher->expects($this->never()) - ->method('hash'); - $this->defaultProvider->expects($this->never()) ->method('update'); diff --git a/tests/lib/Sharing/AbstractSharingManagerTests.php b/tests/lib/Sharing/AbstractSharingManagerTests.php index 3b96003476921..349de677d0bfd 100644 --- a/tests/lib/Sharing/AbstractSharingManagerTests.php +++ b/tests/lib/Sharing/AbstractSharingManagerTests.php @@ -244,9 +244,10 @@ public function setUp(): void { #[\Override] protected function tearDown(): void { + $openTransaction = false; if ($this->dbConnection->inTransaction()) { $this->dbConnection->rollBack(); - $this->fail('Open transaction was not committed.'); + $openTransaction = true; } $accessContext = new ShareAccessContext(overrideChecks: true); @@ -257,11 +258,13 @@ protected function tearDown(): void { $this->manager->deleteShare($accessContext, $share); } + $this->dbConnection->commit(); + $this->owner->delete(); $this->user1->delete(); $this->user2->delete(); - $this->dbConnection->commit(); + $this->registry->clear(); foreach ([ 'sharing_share', @@ -279,9 +282,11 @@ protected function tearDown(): void { $this->assertEquals(0, $qb->executeQuery()->fetchOne(), $table); } - $this->registry->clear(); - parent::tearDown(); + + if ($openTransaction) { + $this->fail('Open transaction was not committed.'); + } } private function reloadShare(ShareAccessContext $accessContext, Share $share): Share { diff --git a/tests/lib/Snowflake/GeneratorTest.php b/tests/lib/Snowflake/GeneratorTest.php index 3323773105a0d..a3d60f7b93efe 100644 --- a/tests/lib/Snowflake/GeneratorTest.php +++ b/tests/lib/Snowflake/GeneratorTest.php @@ -99,6 +99,18 @@ public function testGeneratorWithFixedTime(string $date, int $expectedSeconds, i $this->assertEquals($this->serverInfo->getServerId(), $data->getServerId()); } + #[DataProvider('provideSnowflakeData')] + public function testGeneratorWithTimestampParameter(string $date, int $expectedSeconds, int $expectedMilliseconds): void { + $dt = new \DateTimeImmutable($date); + + $generator = new SnowflakeGenerator(new TimeFactory(), $this->sequence, $this->serverInfo); + $data = $this->decoder->decode($generator->nextId($dt)); + + $this->assertEquals($expectedSeconds, $data->getCreatedAt()->format('U') - ISnowflakeGenerator::TS_OFFSET); + $this->assertEquals($expectedMilliseconds, (int)$data->getCreatedAt()->format('v')); + $this->assertEquals($this->serverInfo->getServerId(), $data->getServerId()); + } + public static function provideSnowflakeData(): array { $tests = [ ['2025-10-01 00:00:00.000000', 0, 0],