Skip to content

Commit

Permalink
Various improvements related to the recent implementation of temporar…
Browse files Browse the repository at this point in the history 
…y 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>
  • Loading branch information
@StCyr
StCyr committed Jun 8, 2022
1 parent b7bce42 commit e49ef59
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 33 deletions.
2 changes: 1 addition & 1 deletion apps/files_sharing/lib/Controller/ShareAPIController.php
View file
Expand Up @@ -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();
Expand Down
20 changes: 20 additions & 0 deletions apps/files_sharing/src/components/SharingEntryLink.vue
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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?
*
Expand Down
3 changes: 2 additions & 1 deletion apps/files_sharing/src/mixins/ShareRequests.js
View file
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion apps/files_sharing/src/mixins/SharesMixin.js
View file
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions apps/files_sharing/src/models/Share.js
View file
Expand Up @@ -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
*
Expand Down
18 changes: 17 additions & 1 deletion apps/sharebymail/lib/ShareByMailProvider.php
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
35 changes: 15 additions & 20 deletions apps/sharebymail/tests/ShareByMailProviderTest.php
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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');

Expand All @@ -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);
Expand All @@ -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');

Expand Down
15 changes: 6 additions & 9 deletions core/templates/publicshareauth.php
View file
Expand Up @@ -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>

0 comments on commit e49ef59

Please sign in to comment.