Sharing from review

Signed-off-by: Vincent Petry <[email protected]>
Co-authored-by: Carl Schwan <[email protected]>

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -1523,30 +1523,27 @@ private function parseDate(string $expireDate): \DateTime {
 
 	/**
 	 * Set the share's password expiration time
-	 *
-	 * @param IShare $share
-	 *
 	 */
-	private function setSharePasswordExpirationTime($share) {
+	private function setSharePasswordExpirationTime(IShare $share): void {
 		if ($this->config->getSystemValue('allow_mail_share_permanent_password')) {
 			// Sets password expiration date to NULL
 			$share->setPasswordExpirationTime();
-		} else {
-			// Sets password expiration date
-			$expirationTime = null;
-			try {
-				$now = new \DateTime();
-				$expirationInterval = $this->config->getSystemValue('share_temporary_password_expiration_interval');
-				if ($expirationInterval === '' || is_null($expirationInterval)) {
-					$expirationInterval = 'P0DT15M';
-				}
-				$expirationTime = $now->add(new \DateInterval($expirationInterval));
-			} catch (\Exception $e) {
-				// Catches invalid format for system value 'share_temporary_password_expiration_interval'
-				$expirationTime = $now->add(new \DateInterval('P0DT15M'));
-			} finally {
-				$share->setPasswordExpirationTime($expirationTime);
+			return;
+		}
+		// Sets password expiration date
+		$expirationTime = null;
+		try {
+			$now = new \DateTime();
+			$expirationInterval = $this->config->getSystemValue('share_temporary_password_expiration_interval');
+			if ($expirationInterval === '' || is_null($expirationInterval)) {
+				$expirationInterval = 'P0DT15M';
 			}
+			$expirationTime = $now->add(new \DateInterval($expirationInterval));
+		} catch (\Exception $e) {
+			// Catches invalid format for system value 'share_temporary_password_expiration_interval'
+			$expirationTime = $now->add(new \DateInterval('P0DT15M'));
+		} finally {
+			$share->setPasswordExpirationTime($expirationTime);
 		}
 	}
 

apps/files_sharing/lib/Controller/ShareController.php

@@ -85,56 +85,21 @@
  * @package OCA\Files_Sharing\Controllers
  */
 class ShareController extends AuthPublicShareController {
+	protected IConfig $config;
+	protected IUserManager $userManager;
+	protected ILogger $logger;
+	protected \OCP\Activity\IManager $activityManager;
+	protected IPreview $previewManager;
+	protected IRootFolder $rootFolder;
+	protected FederatedShareProvider $federatedShareProvider;
+	protected IAccountManager $accountManager;
+	protected IEventDispatcher $eventDispatcher;
+	protected IL10N $l10n;
+	protected Defaults $defaults;
+	protected ShareManager $shareManager;
+	protected ISecureRandom $secureRandom;
+	protected ?Share\IShare $share = null;
 
-	/** @var IConfig */
-	protected $config;
-	/** @var IUserManager */
-	protected $userManager;
-	/** @var ILogger */
-	protected $logger;
-	/** @var \OCP\Activity\IManager */
-	protected $activityManager;
-	/** @var IPreview */
-	protected $previewManager;
-	/** @var IRootFolder */
-	protected $rootFolder;
-	/** @var FederatedShareProvider */
-	protected $federatedShareProvider;
-	/** @var IAccountManager */
-	protected $accountManager;
-	/** @var IEventDispatcher */
-	protected $eventDispatcher;
-	/** @var IL10N */
-	protected $l10n;
-	/** @var Defaults */
-	protected $defaults;
-	/** @var ShareManager */
-	protected $shareManager;
-	/** @var ISecureRandom */
-	protected $secureRandom;
-
-	/** @var Share\IShare */
-	protected $share;
-
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IConfig $config
-	 * @param IURLGenerator $urlGenerator
-	 * @param IUserManager $userManager
-	 * @param ILogger $logger
-	 * @param \OCP\Activity\IManager $activityManager
-	 * @param \OCP\Share\IManager $shareManager
-	 * @param ISession $session
-	 * @param IPreview $previewManager
-	 * @param IRootFolder $rootFolder
-	 * @param FederatedShareProvider $federatedShareProvider
-	 * @param IAccountManager $accountManager
-	 * @param IEventDispatcher $eventDispatcher
-	 * @param IL10N $l10n
-	 * @param ISecureRandom $secureRandom
-	 * @param Defaults $defaults
-	 */
 	public function __construct(string $appName,
 								IRequest $request,
 								IConfig $config,
@@ -237,10 +202,10 @@ protected function showIdentificationResult(bool $success = false): TemplateResp
 	/**
 	 * Validate the identity token of a public share
 	 *
-	 * @param string $identityToken
+	 * @param ?string $identityToken
 	 * @return bool
 	 */
-	protected function validateIdentity(string $identityToken = null): bool {
+	protected function validateIdentity(?string $identityToken = null): bool {
 
 		if ($this->share->getShareType() !== IShare::TYPE_EMAIL) {
 			return false;
@@ -250,25 +215,19 @@ protected function validateIdentity(string $identityToken = null): bool {
 			return false;
 		}
 
-		if ($identityToken !== $this->share->getSharedWith()) {
-			return false;
-		}
-
-		return true;
-
+		return $identityToken === $this->share->getSharedWith();
 	}
 
 	/**
 	 * Generates a password for the share, respecting any password policy defined
 	 */
-	protected function generatePassword() {
+	protected function generatePassword(): void {
 		$event = new \OCP\Security\Events\GenerateSecurePasswordEvent();
 		$this->eventDispatcher->dispatchTyped($event);
 		$password = $event->getPassword() ?? $this->secureRandom->generate(20);
 
 		$this->share->setPassword($password);
 		$this->shareManager->updateShare($this->share);
-		return;
 	}
 
 	protected function verifyPassword(string $password): bool {

apps/sharebymail/lib/ShareByMailProvider.php

@@ -76,8 +76,7 @@
  */
 class ShareByMailProvider implements IShareProvider {
 
-	/** @var IConfig */
-	private $config;
+	private IConfig $config;
 
 	/** @var  IDBConnection */
 	private $dbConnection;
@@ -684,23 +683,24 @@ public function getChildren(IShare $parent) {
 	}
 
 	/**
-	 * add share to the database and return the ID
-	 *
-	 * @param int $itemSource
-	 * @param string $itemType
-	 * @param string $shareWith
-	 * @param string $sharedBy
-	 * @param string $uidOwner
-	 * @param int $permissions
-	 * @param string $token
-	 * @param string $password
-	 * @param bool $sendPasswordByTalk
-	 * @param bool $hideDownload
-	 * @param string $label
-	 * @param \DateTime|null $expirationTime
-	 * @return int
+	 * Add share to the database and return the ID
 	 */
-	protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $passwordExpirationTime, $sendPasswordByTalk, $hideDownload, $label, $expirationTime, $note = ''): int {
+	protected function addShareToDB(
+		?int $itemSource,
+		?string $itemType,
+		?string $shareWith,
+		?string $sharedBy,
+		?string $uidOwner,
+		?int $permissions,
+		?string $token,
+		?string $password,
+		?\DateTimeInterface $passwordExpirationTime,
+		?bool $sendPasswordByTalk,
+		?bool $hideDownload,
+		?string $label,
+		?\DateTimeInterface $expirationTime,
+		?string $note = ''
+	): int {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->insert('share')
 			->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
@@ -1026,7 +1026,8 @@ protected function createShareObject($data) {
 		$share->setShareTime($shareTime);
 		$share->setSharedWith($data['share_with']);
 		$share->setPassword($data['password']);
-		$share->setPasswordExpirationTime($data['password_expiration_time']);
+		$passwordExpirationTime = \DateTime::createFromFormat('Y-m-d H:i:s', $data['password_expiration_time']);
+		$share->setPasswordExpirationTime($passwordExpirationTime !== false? $passwordExpirationTime : null);
 		$share->setLabel($data['label']);
 		$share->setSendPasswordByTalk((bool)$data['password_by_talk']);
 		$share->setHideDownload((bool)$data['hide_download']);

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -476,6 +476,7 @@ public function testAddShareToDB() {
 		$hideDownload = true;
 		$label = 'label';
 		$expiration = new \DateTime();
+		$passwordExpirationTime = new \DateTime();
 
 
 		$instance = $this->getInstance();
@@ -491,7 +492,7 @@ public function testAddShareToDB() {
 				$permissions,
 				$token,
 				$password,
-				null,
+				$passwordExpirationTime,
 				$sendPasswordByTalk,
 				$hideDownload,
 				$label,
@@ -518,6 +519,7 @@ public function testAddShareToDB() {
 		$this->assertSame($permissions, (int)$result[0]['permissions']);
 		$this->assertSame($token, $result[0]['token']);
 		$this->assertSame($password, $result[0]['password']);
+		$this->assertSame($passwordExpirationTime->getTimestamp(), \DateTime::createFromFormat('Y-m-d H:i:s', $result[0]['password_expiration_time'])->getTimestamp());
 		$this->assertSame($sendPasswordByTalk, (bool)$result[0]['password_by_talk']);
 		$this->assertSame($hideDownload, (bool)$result[0]['hide_download']);
 		$this->assertSame($label, $result[0]['label']);

lib/private/Share20/Manager.php

@@ -1554,12 +1554,8 @@ public function checkPassword(IShare $share, $password) {
 
 		// Makes sure password hasn't expired
 		$expirationTime = $share->getPasswordExpirationTime();
-		if ($expirationTime !== null) {
-			$expirationDateTime = new \DateTime($expirationTime);
-			$now = new \DateTime();
-			if ($expirationDateTime < $now) {
-				return false;
-			}
+		if ($expirationTime !== null && $expirationTime < new \DateTime()) {
+			return false;
 		}
 
 		$newHash = '';

lib/private/Share20/Share.php

@@ -73,8 +73,7 @@ class Share implements IShare {
 	private $expireDate;
 	/** @var string */
 	private $password;
-	/** @var string */
-	private $passwordExpirationTime;
+	private ?\DateTimeInterface $passwordExpirationTime = null;
 	/** @var bool */
 	private $sendPasswordByTalk = false;
 	/** @var string */
@@ -466,15 +465,15 @@ public function getPassword() {
 	/**
 	 * @inheritdoc
 	 */
-	public function setPasswordExpirationTime($passwordExpirationTime = null) {
+	public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): IShare {
 		$this->passwordExpirationTime = $passwordExpirationTime;
 		return $this;
 	}
 
 	/**
 	 * @inheritdoc
 	 */
-	public function getPasswordExpirationTime() {
+	public function getPasswordExpirationTime(): ?\DateTimeInterface {
 		return $this->passwordExpirationTime;
 	}
 

lib/public/AppFramework/AuthPublicShareController.php

@@ -98,21 +98,26 @@ protected function showIdentificationResult(bool $success): TemplateResponse {
 	 *
 	 * @since 24.0.0
 	 */
-	abstract protected function validateIdentity(string $identityToken): bool;
+	protected function validateIdentity(?string $identityToken = null): bool {
+		return false;
+	};
 
 	/**
 	 * Generates a password
 	 *
-	 * @since 14.0.0
+	 * @since 24.0.0
 	 */
-	abstract protected function generatePassword();
+	protected function generatePassword(): void {
+	};
 
 	/**
 	 * Verify the password
 	 *
-	 * @since 14.0.0
+	 * @since 24.0.0
 	 */
-	abstract protected function verifyPassword(string $password): bool;
+	protected function verifyPassword(string $password): bool {
+		return false;
+	};
 
 	/**
 	 * Function called after failed authentication

lib/public/Share/IShare.php

@@ -451,16 +451,16 @@ public function getPassword();
 	/**
 	 * Set the password's expiration time of this share.
 	 *
-	 * @return \OCP\Share\IShare The modified object
+	 * @return self The modified object
+	 * @since 24.0.0
 	 */
-	public function setPasswordExpirationTime($passwordExpirationTime = null);
+	public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): IShare;
 
 	/**
 	 * Get the password's expiration time of this share.
-	 *
-	 * @return string
+	 * @since 24.0.0
 	 */
-	public function getPasswordExpirationTime();
+	public function getPasswordExpirationTime(): ?\DateTimeInterface;
 
 	/**
 	 * Set if the recipient can start a conversation with the owner to get the