From 4bf7be88afb8c2147e7892cc2ff16a8847b22693 Mon Sep 17 00:00:00 2001 From: nfebe Date: Tue, 10 Feb 2026 16:59:42 +0100 Subject: [PATCH] fix(sharing): Prevent empty password when checkbox is enabled Set passwordProtectedState explicitly when initializing shares with default passwords. This ensures the checkbox state is tracked independently of the password value, preventing it from unchecking when the password field is cleared. Also block saving new shares when password protection is enabled but no password is entered, regardless of enforcement settings. Added passWithNoTests to vitest configs to handle Vue 2/3 dual frontend test runs gracefully. Fixes: #57732, #57011 Signed-off-by: nfebe --- .../src/views/SharingDetailsTab.spec.ts | 331 ++++++++++++++++++ .../src/views/SharingDetailsTab.vue | 4 +- build/frontend-legacy/vitest.config.mts | 1 + build/frontend/vitest.config.ts | 1 + 4 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 apps/files_sharing/src/views/SharingDetailsTab.spec.ts diff --git a/apps/files_sharing/src/views/SharingDetailsTab.spec.ts b/apps/files_sharing/src/views/SharingDetailsTab.spec.ts new file mode 100644 index 0000000000000..07aacb2e36510 --- /dev/null +++ b/apps/files_sharing/src/views/SharingDetailsTab.spec.ts @@ -0,0 +1,331 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../services/ConfigService.ts', () => ({ + default: vi.fn().mockImplementation(() => ({ + enableLinkPasswordByDefault: false, + enforcePasswordForPublicLink: false, + isPublicUploadEnabled: true, + isDefaultExpireDateEnabled: false, + isDefaultInternalExpireDateEnabled: false, + isDefaultRemoteExpireDateEnabled: false, + defaultExpirationDate: null, + defaultInternalExpirationDate: null, + defaultRemoteExpirationDateString: null, + isResharingAllowed: true, + excludeReshareFromEdit: false, + showFederatedSharesAsInternal: false, + defaultPermissions: 31, + })), +})) + +vi.mock('../utils/GeneratePassword.ts', () => ({ + default: vi.fn().mockResolvedValue('generated-password-123'), +})) + +describe('SharingDetailsTab - Password State Management Logic', () => { + describe('isPasswordProtected getter logic', () => { + it('returns true when passwordProtectedState is explicitly true', () => { + const passwordProtectedState: boolean | undefined = true + const enforcePasswordForPublicLink = false + const newPassword: string | undefined = undefined + const password: string | undefined = undefined + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || typeof password === 'string' + })() + + expect(isPasswordProtected).toBe(true) + }) + + it('returns false when passwordProtectedState is explicitly false', () => { + const passwordProtectedState: boolean | undefined = false + const enforcePasswordForPublicLink = false + const newPassword: string | undefined = 'some-password' + const password: string | undefined = undefined + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || typeof password === 'string' + })() + + expect(isPasswordProtected).toBe(false) + }) + + it('returns true when enforcePasswordForPublicLink is true regardless of other state', () => { + const passwordProtectedState: boolean | undefined = false + const enforcePasswordForPublicLink = true + const newPassword: string | undefined = undefined + const password: string | undefined = undefined + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || typeof password === 'string' + })() + + expect(isPasswordProtected).toBe(true) + }) + + it('falls back to inferring from password when passwordProtectedState is undefined', () => { + const passwordProtectedState: boolean | undefined = undefined + const enforcePasswordForPublicLink = false + const newPassword: string | undefined = 'some-password' + const password: string | undefined = undefined + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || typeof password === 'string' + })() + + expect(isPasswordProtected).toBe(true) + }) + + it('returns false when passwordProtectedState is undefined and no passwords exist', () => { + const passwordProtectedState: boolean | undefined = undefined + const enforcePasswordForPublicLink = false + const newPassword: string | undefined = undefined + const password: string | undefined = undefined + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || typeof password === 'string' + })() + + expect(isPasswordProtected).toBe(false) + }) + }) + + describe('initializeAttributes sets passwordProtectedState', () => { + it('should set passwordProtectedState to true when enableLinkPasswordByDefault is true', async () => { + const config = { + enableLinkPasswordByDefault: true, + enforcePasswordForPublicLink: false, + } + const isNewShare = true + const isPublicShare = true + let passwordProtectedState: boolean | undefined + + if (isNewShare) { + if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) { + passwordProtectedState = true + } + } + + expect(passwordProtectedState).toBe(true) + }) + + it('should set passwordProtectedState to true when isPasswordEnforced is true', async () => { + const config = { + enableLinkPasswordByDefault: false, + enforcePasswordForPublicLink: true, + } + const isNewShare = true + const isPublicShare = true + let passwordProtectedState: boolean | undefined + + if (isNewShare) { + if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) { + passwordProtectedState = true + } + } + + expect(passwordProtectedState).toBe(true) + }) + + it('should not set passwordProtectedState for non-public shares', async () => { + const config = { + enableLinkPasswordByDefault: true, + enforcePasswordForPublicLink: false, + } + const isNewShare = true + const isPublicShare = false + let passwordProtectedState: boolean | undefined + + if (isNewShare) { + if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) { + passwordProtectedState = true + } + } + + expect(passwordProtectedState).toBe(undefined) + }) + + it('should not set passwordProtectedState for existing shares', async () => { + const config = { + enableLinkPasswordByDefault: true, + enforcePasswordForPublicLink: false, + } + const isNewShare = false + const isPublicShare = true + let passwordProtectedState: boolean | undefined + + if (isNewShare) { + if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) { + passwordProtectedState = true + } + } + + expect(passwordProtectedState).toBe(undefined) + }) + }) + + describe('saveShare validation blocks empty password', () => { + const isValidShareAttribute = (attr: unknown) => { + return typeof attr === 'string' && attr.length > 0 + } + + it('should set passwordError when isPasswordProtected but newPassword is empty for new share', () => { + const isPasswordProtected = true + const isNewShare = true + const newPassword = '' + let passwordError = false + + if (isPasswordProtected) { + if (isNewShare && !isValidShareAttribute(newPassword)) { + passwordError = true + } + } + + expect(passwordError).toBe(true) + }) + + it('should set passwordError when isPasswordProtected but newPassword is undefined for new share', () => { + const isPasswordProtected = true + const isNewShare = true + const newPassword = undefined + let passwordError = false + + if (isPasswordProtected) { + if (isNewShare && !isValidShareAttribute(newPassword)) { + passwordError = true + } + } + + expect(passwordError).toBe(true) + }) + + it('should not set passwordError when password is valid for new share', () => { + const isPasswordProtected = true + const isNewShare = true + const newPassword = 'valid-password-123' + let passwordError = false + + if (isPasswordProtected) { + if (isNewShare && !isValidShareAttribute(newPassword)) { + passwordError = true + } + } + + expect(passwordError).toBe(false) + }) + + it('should not set passwordError when isPasswordProtected is false', () => { + const isPasswordProtected = false + const isNewShare = true + const newPassword = '' + let passwordError = false + + if (isPasswordProtected) { + if (isNewShare && !isValidShareAttribute(newPassword)) { + passwordError = true + } + } + + expect(passwordError).toBe(false) + }) + + it('should not validate password for existing shares', () => { + const isPasswordProtected = true + const isNewShare = false + const newPassword = '' + let passwordError = false + + if (isPasswordProtected) { + if (isNewShare && !isValidShareAttribute(newPassword)) { + passwordError = true + } + } + + expect(passwordError).toBe(false) + }) + }) + + describe('checkbox persistence after clearing password', () => { + it('checkbox remains checked when passwordProtectedState is explicitly true even if password is cleared', () => { + let passwordProtectedState: boolean | undefined = true + const enforcePasswordForPublicLink = false + let newPassword: string | undefined = 'initial-password' + + newPassword = '' + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || false + })() + + expect(isPasswordProtected).toBe(true) + }) + + it('checkbox unchecks incorrectly if passwordProtectedState was never set (bug scenario)', () => { + let passwordProtectedState: boolean | undefined = undefined + const enforcePasswordForPublicLink = false + let newPassword: string | undefined = 'initial-password' + + newPassword = undefined + + const isPasswordProtected = (() => { + if (enforcePasswordForPublicLink) { + return true + } + if (passwordProtectedState !== undefined) { + return passwordProtectedState + } + return typeof newPassword === 'string' + || false + })() + + expect(isPasswordProtected).toBe(false) + }) + }) +}) diff --git a/apps/files_sharing/src/views/SharingDetailsTab.vue b/apps/files_sharing/src/views/SharingDetailsTab.vue index 9830fe88a6aca..e9668d78a01f2 100644 --- a/apps/files_sharing/src/views/SharingDetailsTab.vue +++ b/apps/files_sharing/src/views/SharingDetailsTab.vue @@ -974,6 +974,7 @@ export default { async initializeAttributes() { if (this.isNewShare) { if ((this.config.enableLinkPasswordByDefault || this.isPasswordEnforced) && this.isPublicShare) { + this.passwordProtectedState = true this.$set(this.share, 'newPassword', await GeneratePassword(true)) this.advancedSectionAccordionExpanded = true } @@ -1087,8 +1088,9 @@ export default { this.share.note = '' } if (this.isPasswordProtected) { - if (this.isPasswordEnforced && this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) { + if (this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) { this.passwordError = true + return } } else { this.share.password = '' diff --git a/build/frontend-legacy/vitest.config.mts b/build/frontend-legacy/vitest.config.mts index 066a5594c7424..5a480c80fdc5c 100644 --- a/build/frontend-legacy/vitest.config.mts +++ b/build/frontend-legacy/vitest.config.mts @@ -48,6 +48,7 @@ export default defineConfig({ }, test: { include: ['./{apps,core}/**/*.{test,spec}.?(c|m)[jt]s?(x)'], + passWithNoTests: true, environment: 'jsdom', environmentOptions: { jsdom: { diff --git a/build/frontend/vitest.config.ts b/build/frontend/vitest.config.ts index a7078a2356f17..09b3fe709f8d8 100644 --- a/build/frontend/vitest.config.ts +++ b/build/frontend/vitest.config.ts @@ -38,6 +38,7 @@ export default defineConfig({ }, test: { include: ['apps/**/*.{test,spec}.?(c|m)[jt]s?(x)'], + passWithNoTests: true, env: { LANG: 'en_US', TZ: 'UTC',