Various improvements related to the recent implementation of temporary passwords

for mail shares:

1- Changes style of "forgot password?" and "Back" button
2- Adds information about share password's expiration time in the emails sent.
3- Shows password expiration time in the Share menu
4- Fixes an issue when the message "Password expires..." would be shown for non email share types (which don't have temporary passswords)
5- At share's creation, password should only be sent when it's a permanent one

See also #31952

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>

Author: StCyr

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -279,7 +279,7 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array
 		} elseif ($share->getShareType() === IShare::TYPE_EMAIL) {
 			$result['share_with'] = $share->getSharedWith();
 			$result['password'] = $share->getPassword();
-			$result['password_expiration_time'] = $share->getPasswordExpirationTime();
+			$result['password_expiration_time'] = $share->getPasswordExpirationTime() !== null ? $share->getPasswordExpirationTime()->format(\DateTime::ATOM) : null;
 			$result['send_password_by_talk'] = $share->getSendPasswordByTalk();
 			$result['share_with_displayname'] = $this->getDisplayNameFromAddressBook($share->getSharedWith(), 'EMAIL');
 			$result['token'] = $share->getToken();

apps/files_sharing/src/components/SharingEntryLink.vue

@@ -192,6 +192,12 @@
 						@submit="onPasswordSubmit">
 						{{ t('files_sharing', 'Enter a password') }}
 					</ActionInput>
+					<ActionText v-if="isEmailShareType && passwordExpirationTime" icon="icon-info">
+						{{ t('files_sharing', 'Password expires {passwordExpirationTime}', {passwordExpirationTime}) }}
+					</ActionText>
+					<ActionText v-else-if="isEmailShareType && passwordExpirationTime !== null" icon="icon-error">
+						{{ t('files_sharing', 'Password expired') }}
+					</ActionText>
 
 					<!-- password protected by Talk -->
 					<ActionCheckbox v-if="isPasswordProtectedByTalkAvailable"
@@ -461,6 +467,20 @@ export default {
 			},
 		},
 
+		passwordExpirationTime() {
+			if (this.share.passwordExpirationTime === null) {
+				return null
+			}
+
+			const expirationTime = moment(this.share.passwordExpirationTime)
+
+			if (expirationTime.diff(moment()) < 0) {
+				return false
+			}
+
+			return expirationTime.fromNow()
+		},
+
 		/**
 		 * Is Talk enabled?
 		 *

apps/files_sharing/src/mixins/ShareRequests.js

@@ -103,8 +103,9 @@ export default {
 				const request = await axios.put(shareUrl + `/${id}`, properties)
 				if (!request?.data?.ocs) {
 					throw request
+				} else {
+					return request.data.ocs.data
 				}
-				return true
 			} catch (error) {
 				console.error('Error while updating share', error)
 				if (error.response.status !== 400) {

apps/files_sharing/src/mixins/SharesMixin.js

@@ -235,11 +235,14 @@ export default {
 					this.saving = true
 					this.errors = {}
 					try {
-						await this.updateShare(this.share.id, properties)
+						const updatedShare = await this.updateShare(this.share.id, properties)
 
 						if (propertyNames.indexOf('password') >= 0) {
 							// reset password state after sync
 							this.$delete(this.share, 'newPassword')
+
+							// updates password expiration time after sync
+							this.share.passwordExpirationTime = updatedShare.password_expiration_time
 						}
 
 						// clear any previous errors

apps/files_sharing/src/models/Share.js

@@ -358,6 +358,27 @@ export default class Share {
 		this._share.password = password
 	}
 
+	/**
+	 * Password expiration time
+	 *
+	 * @return {string}
+	 * @readonly
+	 * @memberof Share
+	 */
+	get passwordExpirationTime() {
+		return this._share.password_expiration_time
+	}
+
+	/**
+	 * Password expiration time
+	 *
+	 * @param {string} password exipration time
+	 * @memberof Share
+	 */
+	set passwordExpirationTime(passwordExpirationTime) {
+		this._share.password_expiration_time = passwordExpirationTime
+	}
+
 	/**
 	 * Password protection by Talk of the share
 	 *

apps/sharebymail/lib/ShareByMailProvider.php

@@ -198,7 +198,7 @@ public function create(IShare $share) {
 
 		// 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('sharing.enable_mail_link_password_expiration', false) === true || $share->getSendPasswordByTalk()) {
+		if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === false || $share->getSendPasswordByTalk()) {
 			$send = $this->sendPassword($share, $password);
 			if ($passwordEnforced && $send === false) {
 				$this->sendPasswordToOwner($share, $password);
@@ -503,6 +503,13 @@ protected function sendPassword(IShare $share, $password) {
 		$emailTemplate->addBodyText($this->l->t('It is protected with the following password:'));
 		$emailTemplate->addBodyText($password);
 
+		if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true) {
+			$expirationTime = new \DateTime();
+			$expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600);
+			$expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S'));
+			$emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')]));
+		}
+
 		// The "From" contains the sharers name
 		$instanceName = $this->defaults->getName();
 		$senderName = $instanceName;
@@ -627,7 +634,16 @@ protected function sendPasswordToOwner(IShare $share, $password) {
 		$emailTemplate->addBodyText($bodyPart);
 		$emailTemplate->addBodyText($this->l->t('This is the password:'));
 		$emailTemplate->addBodyText($password);
+
+		if ($this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false) === true) {
+			$expirationTime = new \DateTime();
+			$expirationInterval = $this->config->getSystemValue('sharing.mail_link_password_expiration_interval', 3600);
+			$expirationTime = $expirationTime->add(new \DateInterval('PT' . $expirationInterval . 'S'));
+			$emailTemplate->addBodyText($this->l->t('This password will expire at %s', [$expirationTime->format('r')]));
+		}
+
 		$emailTemplate->addBodyText($this->l->t('You can choose a different password at any time in the share dialog.'));
+
 		$emailTemplate->addFooter();
 
 		$instanceName = $this->defaults->getName();

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -252,7 +252,7 @@ public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection()
 		);
 	}
 
-	public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtection() {
+	public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtectionWithPermanentPassword() {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
 		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
@@ -285,7 +285,7 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
 		);
 	}
 
-	public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtectionWithPermanentPassword() {
+	public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtectionWithoutPermanentPassword() {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
 		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
@@ -310,21 +310,16 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
 		// 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('sharing.enable_mail_link_password_expiration')->willReturn(true);
-		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 		$instance->expects($this->never())->method('autoGeneratePassword');
-
-		$message = $this->createMock(IMessage::class);
-		$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
-		$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
-		$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
-			'filename' => 'filename',
-			'password' => 'password',
-			'initiator' => 'owner',
-			'initiatorEmail' => null,
-			'shareWith' => 'receiver@example.com',
-		]);
-		$this->mailer->expects($this->once())->method('send');
+		$this->config->expects($this->any())->method('getSystemValue')->withConsecutive(
+			['sharing.enable_mail_link_password_expiration'],
+			['sharing.enable_mail_link_password_expiration'],
+			['sharing.mail_link_password_expiration_interval']
+		)->willReturnOnConsecutiveCalls(
+			true,
+			true,
+			3600
+		);
 
 		$this->assertSame('shareObject',
 			$instance->create($share)
@@ -363,7 +358,7 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPe
 
 		// 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('sharing.enable_mail_link_password_expiration')->willReturn(true);
+		$this->config->expects($this->any())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(false);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 
 		$message = $this->createMock(IMessage::class);
@@ -408,7 +403,7 @@ public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordP
 		// 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('sharing.enable_mail_link_password_expiration')->willReturn(true);
+		$this->config->expects($this->any())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(false);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
 		$instance->expects($this->never())->method('autoGeneratePassword');
 
@@ -429,7 +424,7 @@ public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordP
 		);
 	}
 
-	public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
+	public function testCreateSendPasswordByTalkWithEnforcedPasswordProtectionWithPermanentPassword() {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
 		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(true);
@@ -453,7 +448,7 @@ public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
 
 		// 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('sharing.enable_mail_link_password_expiration')->willReturn(false);
+		$this->config->expects($this->any())->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');
 

core/templates/publicshareauth.php

@@ -61,23 +61,20 @@ class="svg icon-confirm input-button-inline" value="" disabled="disabled" />
 
 <!-- request password button -->
 <?php if (!isset($_['identityOk']) && $_['share']->getShareType() === $_['share']::TYPE_EMAIL && !$_['share']->getSendPasswordByTalk()): ?>
-	<input type="button"
-		id="request-password-button-not-talk"
-		value="<?php p($l->t('Request password')); ?>"
-		class="primary" />
+	<a id="request-password-button-not-talk"><?php p($l->t('Forgot password?')); ?></a>
 <?php endif; ?>
 
 <!-- back to showShare button -->
 <form method="get">
 	<fieldset>
-		<input type="submit"
+		<a
+			href=""
 			id="request-password-back-button"
-			value="<?php p($l->t('Back')); ?>"
-			class="primary"
 <?php if (isset($_['identityOk'])): ?>
-			style="display:block;" />
+			style="display:block;">
 <?php else: ?>
-			style="display:none;" />
+			style="display:none;">
 <?php endif; ?>
+			<?php p($l->t('Back')); ?></a>
 	</fieldset>
 </form>