Merge pull request #61312 from nextcloud/fix/files-external-edit-auth-mechanism

fix(files_external): Allow editing auth mechanism on saved mounts

Author: susnux

apps/files_external/src/components/AddExternalStorageDialog/AddExternalStorageDialog.vue

@@ -27,6 +27,7 @@ import ApplicableEntities from './ApplicableEntities.vue'
 import AuthMechanismConfiguration from './AuthMechanismConfiguration.vue'
 import BackendConfiguration from './BackendConfiguration.vue'
 import MountOptions from './MountOptions.vue'
+import { pruneUnusedAuthMechanismOptions } from '../../utils/externalStorageUtils.ts'
 
 const open = defineModel<boolean>('open', { default: true })
 
@@ -63,6 +64,14 @@ const authMechanism = computed({
 		return authMechanisms.value.find((a) => a.identifier === internalStorage.value.authMechanism)
 	},
 	set(value?: IAuthMechanism) {
+		const previous = authMechanisms.value.find((a) => a.identifier === internalStorage.value.authMechanism)
+		if (previous && previous.identifier !== value?.identifier && internalStorage.value.backendOptions) {
+			pruneUnusedAuthMechanismOptions(
+				internalStorage.value.backendOptions,
+				previous.configuration,
+				[value?.configuration, backend.value?.configuration],
+			)
+		}
 		internalStorage.value.authMechanism = value?.identifier
 	},
 })
@@ -106,7 +115,7 @@ watch(authMechanisms, () => {
 		<NcSelect
 			v-model="authMechanism"
 			:options="authMechanisms"
-			:disabled="!internalStorage.backend || authMechanisms.length <= 1 || !!(internalStorage.id && internalStorage.authMechanism)"
+			:disabled="!internalStorage.backend || authMechanisms.length <= 1"
 			:inputLabel="t('files_external', 'Authentication')"
 			label="name"
 			required />

apps/files_external/src/utils/externalStorageUtils.spec.ts

@@ -5,7 +5,7 @@
 
 import { File, Folder, Permission } from '@nextcloud/files'
 import { describe, expect, test } from 'vitest'
-import { isNodeExternalStorage } from './externalStorageUtils.ts'
+import { isNodeExternalStorage, pruneUnusedAuthMechanismOptions } from './externalStorageUtils.ts'
 
 describe('Is node an external storage', () => {
 	test('A Folder with a backend and a valid scope is an external storage', () => {
@@ -78,3 +78,41 @@ describe('Is node an external storage', () => {
 		expect(isNodeExternalStorage(folder)).toBe(false)
 	})
 })
+
+describe('Prune unused authentication mechanism options', () => {
+	test('removes the previous mechanism options the new mechanism does not use', () => {
+		const backendOptions: Record<string, unknown> = { user: 'alice', password: 'secret', client_id: 'abc' }
+		pruneUnusedAuthMechanismOptions(
+			backendOptions,
+			{ user: {}, password: {} },
+			[{ client_id: {}, client_secret: {} }, {}],
+		)
+		expect(backendOptions).toEqual({ client_id: 'abc' })
+	})
+
+	test('keeps backend options when only the mechanism changes', () => {
+		const backendOptions: Record<string, unknown> = { host: 'h', root: '/r', user: 'alice', password: 'secret' }
+		pruneUnusedAuthMechanismOptions(
+			backendOptions,
+			{ user: {}, password: {} },
+			[{ token: {} }, { host: {}, root: {} }],
+		)
+		expect(backendOptions).toEqual({ host: 'h', root: '/r' })
+	})
+
+	test('keeps fields shared between the old and new mechanism', () => {
+		const backendOptions: Record<string, unknown> = { configured: true, user: 'alice' }
+		pruneUnusedAuthMechanismOptions(
+			backendOptions,
+			{ configured: {}, user: {} },
+			[{ configured: {} }, {}],
+		)
+		expect(backendOptions).toEqual({ configured: true })
+	})
+
+	test('does nothing when there is no previous configuration', () => {
+		const backendOptions: Record<string, unknown> = { user: 'alice' }
+		pruneUnusedAuthMechanismOptions(backendOptions, undefined, [{}, {}])
+		expect(backendOptions).toEqual({ user: 'alice' })
+	})
+})

apps/files_external/src/utils/externalStorageUtils.ts

@@ -28,3 +28,28 @@ export function isNodeExternalStorage(node: INode) {
 	// Specific markers that we're sure are ext storage only
 	return attributes.scope === 'personal' || attributes.scope === 'system'
 }
+
+/**
+ * Remove stored option values that belonged to the previous authentication
+ * mechanism and are not used by the newly selected mechanism or the backend.
+ *
+ * A mount keeps all backend and authentication values in a single map keyed by
+ * configuration field name. When the mechanism is changed on an existing mount,
+ * the previous mechanism's fields would otherwise linger as unused values.
+ *
+ * @param backendOptions - The stored option values, mutated in place
+ * @param previousConfiguration - Configuration of the previously selected mechanism
+ * @param keptConfigurations - Configurations whose field names must be preserved
+ */
+export function pruneUnusedAuthMechanismOptions(
+	backendOptions: Record<string, unknown>,
+	previousConfiguration: Record<string, unknown> | undefined,
+	keptConfigurations: Array<Record<string, unknown> | undefined>,
+): void {
+	const kept = new Set(keptConfigurations.flatMap((configuration) => Object.keys(configuration ?? {})))
+	for (const key of Object.keys(previousConfiguration ?? {})) {
+		if (!kept.has(key)) {
+			delete backendOptions[key]
+		}
+	}
+}

dist/ActivityCommentAction-C8NJ--_R.chunk.mjs

@@ -0,0 +1,2 @@
+import{a as t}from"./index-DL1yHC1K-orb-zzee.chunk.mjs";import{t as e}from"./translation-DoG5ZELJ-C5oC8Tcn.chunk.mjs";import{C as m,a}from"./CommentView-CO7XL0eV.chunk.mjs";import{l as p}from"./activity-B66lnZYB.chunk.mjs";import{b as i,r as s,o as n,c,m as u}from"./Web-ByHSuvRG.chunk.mjs";import{_ as l}from"./public-C1mLBHT3.chunk.mjs";import"./index-BRuD4Qrz.chunk.mjs";import"./NcModal-DUWLRm_F-C0gD58QY.chunk.mjs";import"./logger-D3RVzcfQ-B261d025.chunk.mjs";import"./createElementId-DhjFt1I9-CZ2eH1SD.chunk.mjs";import"./index-C6ey-Mhx.chunk.mjs";import"./TrashCanOutline-CW4_EEhq.chunk.mjs";import"./mdi-Ci0zJ0QG.chunk.mjs";import"./pinia-Bn5aG74F.chunk.mjs";import"./PencilOutline-DJX4SP_q.chunk.mjs";/* empty css                                          */import"./NcAvatar-M3-CbKbq-DbNpmr-b.chunk.mjs";import"./index-D-iKxf2E.chunk.mjs";import"./util-djQ-4xJ5.chunk.mjs";import"./ArrowRight-BgQTbtKu.chunk.mjs";import"./colors-BDeMBgfq-D1xNHBAd.chunk.mjs";import"./NcUserStatusIcon-DsviB2Cr-CJ45dJff.chunk.mjs";import"./NcDateTime.vue_vue_type_script_setup_true_lang-BJuPH7S7-BRczm9CK.chunk.mjs";import"./NcUserBubble-CDQa0hGy-DMYvkftD.chunk.mjs";import"./GetComments-DFpRzp64.chunk.mjs";import"./index-CI-5vlTq.chunk.mjs";const d=i({components:{Comment:a},mixins:[m],props:{reloadCallback:{type:Function,required:!0}},methods:{onNewComment(){try{this.reloadCallback()}catch(o){t(e("comments","Could not reload comments")),p.error("Could not reload comments",{error:o})}}}});function C(o,f,y,w,D,N){const r=s("Comment");return n(),c(r,u(o.editorData,{autoComplete:o.autoComplete,resourceType:o.resourceType,editor:!0,userData:o.userData,resourceId:o.resourceId,class:"comments-action",onNew:o.onNewComment}),null,16,["autoComplete","resourceType","userData","resourceId","onNew"])}const S=l(d,[["render",C],["__scopeId","data-v-29a1e244"]]);export{S as default};
+//# sourceMappingURL=ActivityCommentAction-CxhX-kOL.chunk.mjs.map

dist/ActivityCommentAction-CxhX-kOL.chunk.mjs

@@ -0,0 +1,2 @@
+import{t as s}from"./translation-DoG5ZELJ-C5oC8Tcn.chunk.mjs";import{C as p,a}from"./CommentView-CO7XL0eV.chunk.mjs";import{_ as i}from"./public-C1mLBHT3.chunk.mjs";import{r as n,o as c,c as u,m as l}from"./Web-ByHSuvRG.chunk.mjs";import"./index-C6ey-Mhx.chunk.mjs";import"./pinia-Bn5aG74F.chunk.mjs";import"./PencilOutline-DJX4SP_q.chunk.mjs";import"./logger-D3RVzcfQ-B261d025.chunk.mjs";import"./createElementId-DhjFt1I9-CZ2eH1SD.chunk.mjs";import"./NcModal-DUWLRm_F-C0gD58QY.chunk.mjs";/* empty css                                          */import"./NcAvatar-M3-CbKbq-DbNpmr-b.chunk.mjs";import"./index-D-iKxf2E.chunk.mjs";import"./util-djQ-4xJ5.chunk.mjs";import"./ArrowRight-BgQTbtKu.chunk.mjs";import"./colors-BDeMBgfq-D1xNHBAd.chunk.mjs";import"./NcUserStatusIcon-DsviB2Cr-CJ45dJff.chunk.mjs";import"./NcDateTime.vue_vue_type_script_setup_true_lang-BJuPH7S7-BRczm9CK.chunk.mjs";import"./TrashCanOutline-CW4_EEhq.chunk.mjs";import"./NcUserBubble-CDQa0hGy-DMYvkftD.chunk.mjs";import"./index-DL1yHC1K-orb-zzee.chunk.mjs";import"./index-BRuD4Qrz.chunk.mjs";import"./mdi-Ci0zJ0QG.chunk.mjs";import"./activity-B66lnZYB.chunk.mjs";import"./GetComments-DFpRzp64.chunk.mjs";import"./index-CI-5vlTq.chunk.mjs";const d={name:"ActivityCommentEntry",components:{Comment:a},mixins:[p],props:{comment:{type:Object,required:!0},reloadCallback:{type:Function,required:!0}},data(){return{commentMessage:""}},watch:{comment(){this.commentMessage=this.comment.props.message}},mounted(){this.commentMessage=this.comment.props.message},methods:{t:s}};function g(t,e,o,f,m,C){const r=n("Comment");return c(),u(r,l({ref:"comment",tag:"li"},o.comment.props,{autoComplete:t.autoComplete,resourceType:t.resourceType,message:m.commentMessage,resourceId:t.resourceId,userData:t.genMentionsData(o.comment.props.mentions),class:"comments-activity",onDelete:e[0]||(e[0]=y=>o.reloadCallback())}),null,16,["autoComplete","resourceType","message","resourceId","userData"])}const Q=i(d,[["render",g],["__scopeId","data-v-afc310f1"]]);export{Q as default};
+//# sourceMappingURL=ActivityCommentEntry-DFdL49TV.chunk.mjs.map