Merge pull request #31975 from nextcloud/bugfix/31952/fix-mail-link-share-password-expiration-settings

Adjust settings for mail link password

apps/sharebymail/lib/ShareByMailProvider.php

@@ -198,7 +198,7 @@ class ShareByMailProvider implements IShareProvider {
 
 		// Sends share password to receiver when it's a permanent one (otherwise she will have to request it via the showShare UI)
 		// or to owner when the password shall be given during a Talk session
-		if ($this->config->getSystemValue('allow_mail_share_permanent_password', true) || $share->getSendPasswordByTalk()) {
+		if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true || $share->getSendPasswordByTalk()) {
 			$send = $this->sendPassword($share, $password);
 			if ($passwordEnforced && $send === false) {
 				$this->sendPasswordToOwner($share, $password);

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -277,7 +277,7 @@ class ShareByMailProviderTest extends TestCase {
 		// The given password (but not the autogenerated password) should not be
 		// mailed to the receiver of the share because permanent passwords are not enforced.
 		$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(false);
-		$this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(false);
+		$this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(false);
 		$instance->expects($this->never())->method('autoGeneratePassword');
 
 		$this->assertSame('shareObject',
@@ -310,7 +310,7 @@ class ShareByMailProviderTest extends TestCase {
 		// The given password (but not 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(false);
-		$this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(true);
+		$this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(true);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 		$instance->expects($this->never())->method('autoGeneratePassword');
 
@@ -363,7 +363,7 @@ class ShareByMailProviderTest extends TestCase {
 
 		// 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);
-		$this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(true);
+		$this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(true);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 
 		$message = $this->createMock(IMessage::class);
@@ -408,7 +408,7 @@ class ShareByMailProviderTest extends TestCase {
 		// The given password (but not the autogenerated password) should be
 		// mailed to the receiver of the share.
 		$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true);
-		$this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(true);
+		$this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(true);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 		$instance->expects($this->never())->method('autoGeneratePassword');
 
@@ -453,7 +453,7 @@ class ShareByMailProviderTest extends TestCase {
 
 		// The autogenerated password should be mailed to the owner of the share.
 		$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(true);
-		$this->config->expects($this->once())->method('getSystemValue')->with('allow_mail_share_permanent_password')->willReturn(false);
+		$this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(false);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 		$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword');
 

config/config.sample.php

@@ -1540,6 +1540,18 @@ $CONFIG = [
  */
 'sharing.managerFactory' => '\OC\Share20\ProviderFactory',
 
+/**
+ * Enables expiration for link share passwords sent by email (sharebymail).
+ * The passwords will expire after the configured interval, the users can
+ * still request a new one in the public link page.
+ */
+'sharing.enable_mail_link_password_expiration' => false,
+
+/**
+ * Expiration interval for passwords, in seconds.
+ */
+'sharing.mail_link_password_expiration_interval' => 3600,
+
 /**
  * Define max number of results returned by the search for auto-completion of
  * users, groups, etc. The value must not be lower than 0 (for unlimited).

lib/private/Share20/Manager.php

@@ -1175,29 +1175,17 @@ class Manager implements IManager {
 	 * Set the share's password expiration time
 	 */
 	private function setSharePasswordExpirationTime(IShare $share): void {
-		if ($this->config->getSystemValue('allow_mail_share_permanent_password', true)) {
+		if (!$this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false)) {
 			// Sets password expiration date to NULL
 			$share->setPasswordExpirationTime();
 			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'
-			\OC::$server->getLogger()->logException($e, [
-				'message' => 'The \'share_temporary_password_expiration_interval\' system setting does not respect the DateInterval::__construct() format. Setting it to \'P0DT15M\''
-			]);
-			$expirationTime = $now->add(new \DateInterval('P0DT15M'));
-		} finally {
-			$share->setPasswordExpirationTime($expirationTime);
-		}
+		$now = new \DateTime();
+		$expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600);
+		$expirationTime = $now->add(new \DateInterval('PT' . $expirationInterval . 'S'));
+		$share->setPasswordExpirationTime($expirationTime);
 	}