[stable35] fix: show custom permissions when share is removed from edit bundle

apps/files_sharing/src/components/SharingEntryQuickShareSelect.vue

@@ -39,10 +39,7 @@ import IconPencil from 'vue-material-design-icons/PencilOutline.vue'
 import IconFileUpload from 'vue-material-design-icons/TrayArrowUp.vue'
 import DropdownIcon from 'vue-material-design-icons/TriangleSmallDown.vue'
 import IconTune from 'vue-material-design-icons/Tune.vue'
-import {
-	ATOMIC_PERMISSIONS,
-	getBundledPermissions,
-} from '../lib/SharePermissionsToolBox.js'
+import { getBundledPermissions } from '../lib/SharePermissionsToolBox.js'
 import ShareDetails from '../mixins/ShareDetails.js'
 import SharesMixin from '../mixins/SharesMixin.js'
 
@@ -98,18 +95,17 @@ export default {
 		},
 
 		preSelectedOption() {
-			// We remove the share permission for the comparison as it is not relevant for bundled permissions.
-			const permissionsWithoutShare = this.share.permissions & ~ATOMIC_PERMISSIONS.SHARE
-			const basePermissions = getBundledPermissions(true)
-			if (permissionsWithoutShare === basePermissions.READ_ONLY) {
-				return this.canViewText
-			} else if (permissionsWithoutShare === basePermissions.ALL || permissionsWithoutShare === basePermissions.ALL_FILE) {
-				return this.canEditText
-			} else if (permissionsWithoutShare === basePermissions.FILE_DROP) {
-				return this.fileDropText
+			switch (this.permissionsBundle) {
+				case 'READ_ONLY':
+					return this.canViewText
+				case 'ALL':
+				case 'ALL_FILE':
+					return this.canEditText
+				case 'FILE_DROP':
+					return this.fileDropText
+				default:
+					return this.customPermissionsText
 			}
-
-			return this.customPermissionsText
 		},
 
 		options() {

apps/files_sharing/src/lib/SharePermissionsToolBox.js

@@ -122,3 +122,33 @@ export function togglePermissions(initialPermissionSet, permissionsToToggle) {
 export function canTogglePermissions(permissionSet, permissionsToToggle) {
 	return permissionsSetIsValid(togglePermissions(permissionSet, permissionsToToggle))
 }
+
+/**
+ * The permission bundles the share editor offers, in the order they are matched.
+ *
+ * @type {string[]}
+ */
+const EDITOR_BUNDLES = ['READ_ONLY', 'ALL', 'ALL_FILE', 'FILE_DROP']
+
+/**
+ * Find the permission bundle a share's permissions correspond to.
+ *
+ * Link and email shares carry the SHARE permission whenever federation on
+ * public shares is enabled: the server adds it on top of whatever bundle was
+ * picked, so it must be ignored when matching those shares against a bundle.
+ *
+ * @param {number} permissions - the share permissions.
+ * @param {object} [options] - matching options.
+ * @param {boolean} [options.isPublicShare] - whether the share is a link or email share.
+ * @param {boolean} [options.excludeReshareFromEdit] - whether SHARE is excluded from the editing bundles.
+ *
+ * @return {string|null} the name of the matching bundle, or `null` for custom permissions.
+ */
+export function matchBundledPermissions(permissions, { isPublicShare = false, excludeReshareFromEdit = false } = {}) {
+	const bundles = getBundledPermissions(isPublicShare || excludeReshareFromEdit)
+	const comparablePermissions = isPublicShare
+		? subtractPermissions(permissions, ATOMIC_PERMISSIONS.SHARE)
+		: permissions
+
+	return EDITOR_BUNDLES.find((bundle) => bundles[bundle] === comparablePermissions) ?? null
+}

apps/files_sharing/src/lib/SharePermissionsToolBox.spec.js

@@ -9,6 +9,7 @@ import {
 	canTogglePermissions,
 	getBundledPermissions,
 	hasPermissions,
+	matchBundledPermissions,
 	permissionsSetIsValid,
 	subtractPermissions,
 	togglePermissions,
@@ -144,4 +145,51 @@ describe('SharePermissionsToolBox', () => {
 		// BUNDLED_PERMISSIONS.ALL_FILE already includes SHARE
 		expect(BUNDLED_PERMISSIONS.ALL_FILE).toBe(permissionsWithShare.ALL_FILE)
 	})
+
+	describe('Matching bundled permissions', () => {
+		const { READ, UPDATE, CREATE, DELETE, SHARE } = ATOMIC_PERMISSIONS
+
+		test('matches the bundles of an internal share', () => {
+			expect(matchBundledPermissions(READ)).toBe('READ_ONLY')
+			expect(matchBundledPermissions(CREATE)).toBe('FILE_DROP')
+			expect(matchBundledPermissions(READ | UPDATE | CREATE | DELETE | SHARE)).toBe('ALL')
+			expect(matchBundledPermissions(READ | UPDATE | SHARE)).toBe('ALL_FILE')
+		})
+
+		test('reports permissions outside of a bundle as custom', () => {
+			expect(matchBundledPermissions(READ | UPDATE)).toBe(null)
+			expect(matchBundledPermissions(READ | CREATE)).toBe(null)
+			expect(matchBundledPermissions(ATOMIC_PERMISSIONS.NONE)).toBe(null)
+		})
+
+		test('matches the editing bundle without SHARE when resharing is excluded from editing', () => {
+			const options = { excludeReshareFromEdit: true }
+			expect(matchBundledPermissions(READ | UPDATE | CREATE | DELETE, options)).toBe('ALL')
+			expect(matchBundledPermissions(READ | UPDATE, options)).toBe('ALL_FILE')
+			// With resharing excluded, a share that grants it is no longer the editing bundle
+			expect(matchBundledPermissions(READ | UPDATE | CREATE | DELETE | SHARE, options)).toBe(null)
+		})
+
+		test('ignores the SHARE permission the server adds to public shares', () => {
+			const options = { isPublicShare: true }
+			// Link and email shares carry SHARE for federation, whatever bundle was picked
+			expect(matchBundledPermissions(READ | SHARE, options)).toBe('READ_ONLY')
+			expect(matchBundledPermissions(CREATE | SHARE, options)).toBe('FILE_DROP')
+			expect(matchBundledPermissions(READ | UPDATE | CREATE | DELETE | SHARE, options)).toBe('ALL')
+			expect(matchBundledPermissions(READ | UPDATE | SHARE, options)).toBe('ALL_FILE')
+		})
+
+		test('matches public shares the same way with resharing excluded from editing', () => {
+			const options = { isPublicShare: true, excludeReshareFromEdit: true }
+			expect(matchBundledPermissions(READ | SHARE, options)).toBe('READ_ONLY')
+			expect(matchBundledPermissions(READ | UPDATE | CREATE | DELETE | SHARE, options)).toBe('ALL')
+			expect(matchBundledPermissions(READ | UPDATE | CREATE | DELETE, options)).toBe('ALL')
+		})
+
+		test('still reports custom permissions on a public share', () => {
+			const options = { isPublicShare: true }
+			expect(matchBundledPermissions(READ | CREATE | SHARE, options)).toBe(null)
+			expect(matchBundledPermissions(READ | UPDATE | DELETE | SHARE, options)).toBe(null)
+		})
+	})
 })

apps/files_sharing/src/mixins/SharesMixin.js

@@ -10,10 +10,7 @@ import { ShareType } from '@nextcloud/sharing'
 import debounce from 'debounce'
 import PQueue from 'p-queue'
 import { fetchNode } from '../../../files/src/services/WebdavClient.ts'
-import {
-	ATOMIC_PERMISSIONS,
-	getBundledPermissions,
-} from '../lib/SharePermissionsToolBox.js'
+import { matchBundledPermissions } from '../lib/SharePermissionsToolBox.js'
 import Share from '../models/Share.ts'
 import Config from '../services/ConfigService.ts'
 import logger from '../services/logger.ts'
@@ -137,16 +134,14 @@ export default {
 			}
 			return this.config.isDefaultInternalExpireDateEnforced
 		},
+		permissionsBundle() {
+			return matchBundledPermissions(this.share.permissions, {
+				isPublicShare: this.isPublicShare,
+				excludeReshareFromEdit: this.config.excludeReshareFromEdit,
+			})
+		},
 		hasCustomPermissions() {
-			const basePermissions = getBundledPermissions(true)
-			const bundledPermissions = [
-				basePermissions.ALL,
-				basePermissions.ALL_FILE,
-				basePermissions.READ_ONLY,
-				basePermissions.FILE_DROP,
-			]
-			const permissionsWithoutShare = this.share.permissions & ~ATOMIC_PERMISSIONS.SHARE
-			return !bundledPermissions.includes(permissionsWithoutShare)
+			return this.permissionsBundle === null
 		},
 		maxExpirationDateEnforced() {
 			if (this.isExpiryDateEnforced) {

apps/files_sharing/src/views/SharingDetailsTab.vue

@@ -1051,12 +1051,14 @@ export default {
 		handleDefaultPermissions() {
 			if (this.isNewShare) {
 				const defaultPermissions = this.config.defaultPermissions
-				const permissionsWithoutShare = defaultPermissions & ~ATOMIC_PERMISSIONS.SHARE
-				const basePermissions = getBundledPermissions(true)
-				if (permissionsWithoutShare === basePermissions.READ_ONLY
-					|| permissionsWithoutShare === basePermissions.ALL
-					|| permissionsWithoutShare === basePermissions.ALL_FILE) {
-					this.sharingPermission = permissionsWithoutShare.toString()
+				const basePermissions = this.bundledPermissions
+				if (defaultPermissions === basePermissions.READ_ONLY) {
+					this.sharingPermission = basePermissions.READ_ONLY.toString()
+				} else if (defaultPermissions === basePermissions.ALL
+					|| defaultPermissions === basePermissions.ALL_FILE) {
+					this.sharingPermission = this.allPermissions
+				} else if (defaultPermissions === basePermissions.FILE_DROP) {
+					this.sharingPermission = basePermissions.FILE_DROP.toString()
 				} else {
 					this.sharingPermission = 'custom'
 					this.share.permissions = defaultPermissions

dist/1691-1691.js.map.license

@@ -0,0 +1 @@
+1691-1691.js.license