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>

[skip ci]

Author: susnux

apps/files_sharing/src/components/SharingEntryLink.vue

@@ -67,10 +67,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"
@@ -108,7 +108,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>
@@ -630,6 +631,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)
 				})
@@ -822,7 +824,7 @@ export default {
 		 */
 		onPasswordSubmit() {
 			if (this.hasUnsavedPassword) {
-				this.share.password = this.share.newPassword.trim()
+				this.share.newPassword = this.share.newPassword.trim()
 				this.queueUpdate('password')
 			}
 		},
@@ -837,7 +839,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')

apps/files_sharing/src/mixins/SharesMixin.js

@@ -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,39 +303,49 @@ 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') {
 						properties[name] = JSON.stringify(this.share[name])
 					} else {
 						properties[name] = this.share[name].toString()
 					}
-				})
+				}
 
 				return this.updateQueue.add(async () => {
 					this.saving = true
 					this.errors = {}
 					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
 							this.share.passwordExpirationTime = updatedShare.password_expiration_time
 						}
 
 						// 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) {

apps/files_sharing/src/views/SharingDetailsTab.vue

@@ -114,7 +114,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"
@@ -803,7 +803,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 */
@@ -901,10 +900,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 {
@@ -928,7 +924,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
@@ -960,9 +956,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()
@@ -1039,10 +1034,6 @@ export default {
 		 * "sendPasswordByTalk".
 		 */
 		onPasswordProtectedByTalkChange() {
-			if (this.hasUnsavedPassword) {
-				this.share.password = this.share.newPassword.trim()
-			}
-
 			this.queueUpdate('sendPasswordByTalk', 'password')
 		},
 		isValidShareAttribute(value) {