fix(files_sharing): restore state when updating share failed

We need to save the previous state - here the password - so that if the
update fails we can revert the shown state.
This happens e.g. if you have the password policy app and try to add an
unsecure password.

To reproduce (with password policy):
1. Create new link share
2. enable password protection
3. use insecure password like `1234`
4. save share

Now you see that the update failed, but the password protection is still
enabled. This happened because `password` and `newPassword` were
misused. `password` was already set when `newPassword` was not saved so
we could not know to what we need to reset when the update failed.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
pull/54055/head
Ferdinand Thiessen 2025-07-23 12:57:14 +07:00
parent cf29ebb764
commit fb397e6d23
3 changed files with 37 additions and 27 deletions

@ -74,10 +74,10 @@
{{ config.enforcePasswordForPublicLink ? t('files_sharing', 'Password protection (enforced)') : t('files_sharing', 'Password protection') }}
</NcActionCheckbox>
<NcActionInput v-if="pendingEnforcedPassword || share.password"
<NcActionInput v-if="pendingEnforcedPassword || isPasswordProtected"
class="share-link-password"
:label="t('files_sharing', 'Enter a password')"
:value.sync="share.password"
:value.sync="share.newPassword"
:disabled="saving"
:required="config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink"
:minlength="isPasswordPolicyEnabled && config.passwordPolicy.minLength"
@ -115,7 +115,8 @@
</template>
</NcActionInput>
<NcActionButton @click.prevent.stop="onNewLinkShare(true)">
<NcActionButton :disabled="pendingEnforcedPassword && !share.newPassword"
@click.prevent.stop="onNewLinkShare(true)">
<template #icon>
<CheckIcon :size="20" />
</template>
@ -646,6 +647,7 @@ export default {
// create share & close menu
const share = new Share(shareDefaults)
share.newPassword = share.password
const component = await new Promise(resolve => {
this.$emit('add:share', share, resolve)
})
@ -838,7 +840,7 @@ export default {
*/
onPasswordSubmit() {
if (this.hasUnsavedPassword) {
this.share.password = this.share.newPassword.trim()
this.share.newPassword = this.share.newPassword.trim()
this.queueUpdate('password')
}
},
@ -853,7 +855,7 @@ export default {
*/
onPasswordProtectedByTalkChange() {
if (this.hasUnsavedPassword) {
this.share.password = this.share.newPassword.trim()
this.share.newPassword = this.share.newPassword.trim()
}
this.queueUpdate('sendPasswordByTalk', 'password')

@ -165,12 +165,12 @@ export default {
isPasswordProtected: {
get() {
return this.config.enforcePasswordForPublicLink
|| !!this.share.password
|| this.share.password !== ''
|| this.share.newPassword !== undefined
},
async set(enabled) {
if (enabled) {
this.share.password = await GeneratePassword(true)
this.$set(this.share, 'newPassword', this.share.password)
this.$set(this.share, 'newPassword', await GeneratePassword(true))
} else {
this.share.password = ''
this.$delete(this.share, 'newPassword')
@ -272,7 +272,7 @@ export default {
this.loading = true
this.open = false
await this.deleteShare(this.share.id)
console.debug('Share deleted', this.share.id)
logger.debug('Share deleted', { shareId: this.share.id })
const message = this.share.itemType === 'file'
? t('files_sharing', 'File "{path}" has been unshared', { path: this.share.path })
: t('files_sharing', 'Folder "{path}" has been unshared', { path: this.share.path })
@ -303,7 +303,12 @@ export default {
const properties = {}
// force value to string because that is what our
// share api controller accepts
propertyNames.forEach(name => {
for (const name of propertyNames) {
if (name === 'password') {
properties[name] = this.share.newPassword ?? this.share.password
continue
}
if (this.share[name] === null || this.share[name] === undefined) {
properties[name] = ''
} else if ((typeof this.share[name]) === 'object') {
@ -311,7 +316,7 @@ export default {
} else {
properties[name] = this.share[name].toString()
}
})
}
return this.updateQueue.add(async () => {
this.saving = true
@ -319,8 +324,9 @@ export default {
try {
const updatedShare = await this.updateShare(this.share.id, properties)
if (propertyNames.indexOf('password') >= 0) {
if (propertyNames.includes('password')) {
// reset password state after sync
this.share.password = this.share.newPassword ?? ''
this.$delete(this.share, 'newPassword')
// updates password expiration time after sync
@ -328,14 +334,18 @@ export default {
}
// clear any previous errors
this.$delete(this.errors, propertyNames[0])
for (const property of propertyNames) {
this.$delete(this.errors, property)
}
showSuccess(this.updateSuccessMessage(propertyNames))
} catch (error) {
logger.error('Could not update share', { error, share: this.share, propertyNames })
const { message } = error
if (message && message !== '') {
this.onSyncError(propertyNames[0], message)
for (const property of propertyNames) {
this.onSyncError(property, message)
}
showError(message)
} else {
// We do not have information what happened, but we should still inform the user
@ -384,6 +394,13 @@ export default {
* @param {string} message the error message
*/
onSyncError(property, message) {
if (property === 'password' && this.share.newPassword) {
if (this.share.newPassword === this.share.password) {
this.share.password = ''
}
this.$delete(this.share, 'newPassword')
}
// re-open menu if closed
this.open = true
switch (property) {

@ -128,7 +128,7 @@
</NcCheckboxRadioSwitch>
<NcPasswordField v-if="isPasswordProtected"
autocomplete="new-password"
:value="hasUnsavedPassword ? share.newPassword : ''"
:value="share.newPassword ?? ''"
:error="passwordError"
:helper-text="errorPasswordLabel || passwordHint"
:required="isPasswordEnforced && isNewShare"
@ -872,7 +872,6 @@ export default {
if (this.isNewShare) {
if ((this.config.enableLinkPasswordByDefault || this.isPasswordEnforced) && this.isPublicShare) {
this.$set(this.share, 'newPassword', await GeneratePassword(true))
this.$set(this.share, 'password', this.share.newPassword)
this.advancedSectionAccordionExpanded = true
}
/* Set default expiration dates if configured */
@ -973,10 +972,7 @@ export default {
this.share.note = ''
}
if (this.isPasswordProtected) {
if (this.hasUnsavedPassword && this.isValidShareAttribute(this.share.newPassword)) {
this.share.password = this.share.newPassword
this.$delete(this.share, 'newPassword')
} else if (this.isPasswordEnforced && this.isNewShare && !this.isValidShareAttribute(this.share.password)) {
if (this.isPasswordEnforced && this.isNewShare && !this.isValidShareAttribute(this.share.password)) {
this.passwordError = true
}
} else {
@ -1000,7 +996,7 @@ export default {
incomingShare.expireDate = this.hasExpirationDate ? this.share.expireDate : ''
if (this.isPasswordProtected) {
incomingShare.password = this.share.password
incomingShare.password = this.share.newPassword
}
let share
@ -1032,9 +1028,8 @@ export default {
this.$emit('add:share', this.share)
} else {
// Let's update after creation as some attrs are only available after creation
await this.queueUpdate(...permissionsAndAttributes)
this.$emit('update:share', this.share)
emit('update:share', this.share)
this.queueUpdate(...permissionsAndAttributes)
}
await this.getNode()
@ -1111,10 +1106,6 @@ export default {
* "sendPasswordByTalk".
*/
onPasswordProtectedByTalkChange() {
if (this.hasUnsavedPassword) {
this.share.password = this.share.newPassword.trim()
}
this.queueUpdate('sendPasswordByTalk', 'password')
},
isValidShareAttribute(value) {