Skip to content

Commit 11fb0de

Browse files
nfebetobiasKaminsky
authored andcommitted
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 <fenn25.fn@gmail.com>
1 parent 27228a2 commit 11fb0de

4 files changed

Lines changed: 336 additions & 1 deletion

File tree

Lines changed: 331 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,331 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
vi.mock('../services/ConfigService.ts', () => ({
9+
default: vi.fn().mockImplementation(() => ({
10+
enableLinkPasswordByDefault: false,
11+
enforcePasswordForPublicLink: false,
12+
isPublicUploadEnabled: true,
13+
isDefaultExpireDateEnabled: false,
14+
isDefaultInternalExpireDateEnabled: false,
15+
isDefaultRemoteExpireDateEnabled: false,
16+
defaultExpirationDate: null,
17+
defaultInternalExpirationDate: null,
18+
defaultRemoteExpirationDateString: null,
19+
isResharingAllowed: true,
20+
excludeReshareFromEdit: false,
21+
showFederatedSharesAsInternal: false,
22+
defaultPermissions: 31,
23+
})),
24+
}))
25+
26+
vi.mock('../utils/GeneratePassword.ts', () => ({
27+
default: vi.fn().mockResolvedValue('generated-password-123'),
28+
}))
29+
30+
describe('SharingDetailsTab - Password State Management Logic', () => {
31+
describe('isPasswordProtected getter logic', () => {
32+
it('returns true when passwordProtectedState is explicitly true', () => {
33+
const passwordProtectedState: boolean | undefined = true
34+
const enforcePasswordForPublicLink = false
35+
const newPassword: string | undefined = undefined
36+
const password: string | undefined = undefined
37+
38+
const isPasswordProtected = (() => {
39+
if (enforcePasswordForPublicLink) {
40+
return true
41+
}
42+
if (passwordProtectedState !== undefined) {
43+
return passwordProtectedState
44+
}
45+
return typeof newPassword === 'string'
46+
|| typeof password === 'string'
47+
})()
48+
49+
expect(isPasswordProtected).toBe(true)
50+
})
51+
52+
it('returns false when passwordProtectedState is explicitly false', () => {
53+
const passwordProtectedState: boolean | undefined = false
54+
const enforcePasswordForPublicLink = false
55+
const newPassword: string | undefined = 'some-password'
56+
const password: string | undefined = undefined
57+
58+
const isPasswordProtected = (() => {
59+
if (enforcePasswordForPublicLink) {
60+
return true
61+
}
62+
if (passwordProtectedState !== undefined) {
63+
return passwordProtectedState
64+
}
65+
return typeof newPassword === 'string'
66+
|| typeof password === 'string'
67+
})()
68+
69+
expect(isPasswordProtected).toBe(false)
70+
})
71+
72+
it('returns true when enforcePasswordForPublicLink is true regardless of other state', () => {
73+
const passwordProtectedState: boolean | undefined = false
74+
const enforcePasswordForPublicLink = true
75+
const newPassword: string | undefined = undefined
76+
const password: string | undefined = undefined
77+
78+
const isPasswordProtected = (() => {
79+
if (enforcePasswordForPublicLink) {
80+
return true
81+
}
82+
if (passwordProtectedState !== undefined) {
83+
return passwordProtectedState
84+
}
85+
return typeof newPassword === 'string'
86+
|| typeof password === 'string'
87+
})()
88+
89+
expect(isPasswordProtected).toBe(true)
90+
})
91+
92+
it('falls back to inferring from password when passwordProtectedState is undefined', () => {
93+
const passwordProtectedState: boolean | undefined = undefined
94+
const enforcePasswordForPublicLink = false
95+
const newPassword: string | undefined = 'some-password'
96+
const password: string | undefined = undefined
97+
98+
const isPasswordProtected = (() => {
99+
if (enforcePasswordForPublicLink) {
100+
return true
101+
}
102+
if (passwordProtectedState !== undefined) {
103+
return passwordProtectedState
104+
}
105+
return typeof newPassword === 'string'
106+
|| typeof password === 'string'
107+
})()
108+
109+
expect(isPasswordProtected).toBe(true)
110+
})
111+
112+
it('returns false when passwordProtectedState is undefined and no passwords exist', () => {
113+
const passwordProtectedState: boolean | undefined = undefined
114+
const enforcePasswordForPublicLink = false
115+
const newPassword: string | undefined = undefined
116+
const password: string | undefined = undefined
117+
118+
const isPasswordProtected = (() => {
119+
if (enforcePasswordForPublicLink) {
120+
return true
121+
}
122+
if (passwordProtectedState !== undefined) {
123+
return passwordProtectedState
124+
}
125+
return typeof newPassword === 'string'
126+
|| typeof password === 'string'
127+
})()
128+
129+
expect(isPasswordProtected).toBe(false)
130+
})
131+
})
132+
133+
describe('initializeAttributes sets passwordProtectedState', () => {
134+
it('should set passwordProtectedState to true when enableLinkPasswordByDefault is true', async () => {
135+
const config = {
136+
enableLinkPasswordByDefault: true,
137+
enforcePasswordForPublicLink: false,
138+
}
139+
const isNewShare = true
140+
const isPublicShare = true
141+
let passwordProtectedState: boolean | undefined
142+
143+
if (isNewShare) {
144+
if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) {
145+
passwordProtectedState = true
146+
}
147+
}
148+
149+
expect(passwordProtectedState).toBe(true)
150+
})
151+
152+
it('should set passwordProtectedState to true when isPasswordEnforced is true', async () => {
153+
const config = {
154+
enableLinkPasswordByDefault: false,
155+
enforcePasswordForPublicLink: true,
156+
}
157+
const isNewShare = true
158+
const isPublicShare = true
159+
let passwordProtectedState: boolean | undefined
160+
161+
if (isNewShare) {
162+
if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) {
163+
passwordProtectedState = true
164+
}
165+
}
166+
167+
expect(passwordProtectedState).toBe(true)
168+
})
169+
170+
it('should not set passwordProtectedState for non-public shares', async () => {
171+
const config = {
172+
enableLinkPasswordByDefault: true,
173+
enforcePasswordForPublicLink: false,
174+
}
175+
const isNewShare = true
176+
const isPublicShare = false
177+
let passwordProtectedState: boolean | undefined
178+
179+
if (isNewShare) {
180+
if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) {
181+
passwordProtectedState = true
182+
}
183+
}
184+
185+
expect(passwordProtectedState).toBe(undefined)
186+
})
187+
188+
it('should not set passwordProtectedState for existing shares', async () => {
189+
const config = {
190+
enableLinkPasswordByDefault: true,
191+
enforcePasswordForPublicLink: false,
192+
}
193+
const isNewShare = false
194+
const isPublicShare = true
195+
let passwordProtectedState: boolean | undefined
196+
197+
if (isNewShare) {
198+
if ((config.enableLinkPasswordByDefault || config.enforcePasswordForPublicLink) && isPublicShare) {
199+
passwordProtectedState = true
200+
}
201+
}
202+
203+
expect(passwordProtectedState).toBe(undefined)
204+
})
205+
})
206+
207+
describe('saveShare validation blocks empty password', () => {
208+
const isValidShareAttribute = (attr: unknown) => {
209+
return typeof attr === 'string' && attr.length > 0
210+
}
211+
212+
it('should set passwordError when isPasswordProtected but newPassword is empty for new share', () => {
213+
const isPasswordProtected = true
214+
const isNewShare = true
215+
const newPassword = ''
216+
let passwordError = false
217+
218+
if (isPasswordProtected) {
219+
if (isNewShare && !isValidShareAttribute(newPassword)) {
220+
passwordError = true
221+
}
222+
}
223+
224+
expect(passwordError).toBe(true)
225+
})
226+
227+
it('should set passwordError when isPasswordProtected but newPassword is undefined for new share', () => {
228+
const isPasswordProtected = true
229+
const isNewShare = true
230+
const newPassword = undefined
231+
let passwordError = false
232+
233+
if (isPasswordProtected) {
234+
if (isNewShare && !isValidShareAttribute(newPassword)) {
235+
passwordError = true
236+
}
237+
}
238+
239+
expect(passwordError).toBe(true)
240+
})
241+
242+
it('should not set passwordError when password is valid for new share', () => {
243+
const isPasswordProtected = true
244+
const isNewShare = true
245+
const newPassword = 'valid-password-123'
246+
let passwordError = false
247+
248+
if (isPasswordProtected) {
249+
if (isNewShare && !isValidShareAttribute(newPassword)) {
250+
passwordError = true
251+
}
252+
}
253+
254+
expect(passwordError).toBe(false)
255+
})
256+
257+
it('should not set passwordError when isPasswordProtected is false', () => {
258+
const isPasswordProtected = false
259+
const isNewShare = true
260+
const newPassword = ''
261+
let passwordError = false
262+
263+
if (isPasswordProtected) {
264+
if (isNewShare && !isValidShareAttribute(newPassword)) {
265+
passwordError = true
266+
}
267+
}
268+
269+
expect(passwordError).toBe(false)
270+
})
271+
272+
it('should not validate password for existing shares', () => {
273+
const isPasswordProtected = true
274+
const isNewShare = false
275+
const newPassword = ''
276+
let passwordError = false
277+
278+
if (isPasswordProtected) {
279+
if (isNewShare && !isValidShareAttribute(newPassword)) {
280+
passwordError = true
281+
}
282+
}
283+
284+
expect(passwordError).toBe(false)
285+
})
286+
})
287+
288+
describe('checkbox persistence after clearing password', () => {
289+
it('checkbox remains checked when passwordProtectedState is explicitly true even if password is cleared', () => {
290+
let passwordProtectedState: boolean | undefined = true
291+
const enforcePasswordForPublicLink = false
292+
let newPassword: string | undefined = 'initial-password'
293+
294+
newPassword = ''
295+
296+
const isPasswordProtected = (() => {
297+
if (enforcePasswordForPublicLink) {
298+
return true
299+
}
300+
if (passwordProtectedState !== undefined) {
301+
return passwordProtectedState
302+
}
303+
return typeof newPassword === 'string'
304+
|| false
305+
})()
306+
307+
expect(isPasswordProtected).toBe(true)
308+
})
309+
310+
it('checkbox unchecks incorrectly if passwordProtectedState was never set (bug scenario)', () => {
311+
let passwordProtectedState: boolean | undefined = undefined
312+
const enforcePasswordForPublicLink = false
313+
let newPassword: string | undefined = 'initial-password'
314+
315+
newPassword = undefined
316+
317+
const isPasswordProtected = (() => {
318+
if (enforcePasswordForPublicLink) {
319+
return true
320+
}
321+
if (passwordProtectedState !== undefined) {
322+
return passwordProtectedState
323+
}
324+
return typeof newPassword === 'string'
325+
|| false
326+
})()
327+
328+
expect(isPasswordProtected).toBe(false)
329+
})
330+
})
331+
})

apps/files_sharing/src/views/SharingDetailsTab.vue

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,7 @@ export default {
974974
async initializeAttributes() {
975975
if (this.isNewShare) {
976976
if ((this.config.enableLinkPasswordByDefault || this.isPasswordEnforced) && this.isPublicShare) {
977+
this.passwordProtectedState = true
977978
this.$set(this.share, 'newPassword', await GeneratePassword(true))
978979
this.advancedSectionAccordionExpanded = true
979980
}
@@ -1087,8 +1088,9 @@ export default {
10871088
this.share.note = ''
10881089
}
10891090
if (this.isPasswordProtected) {
1090-
if (this.isPasswordEnforced && this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
1091+
if (this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
10911092
this.passwordError = true
1093+
return
10921094
}
10931095
} else {
10941096
this.share.password = ''

build/frontend-legacy/vitest.config.mts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export default defineConfig({
4848
},
4949
test: {
5050
include: ['./{apps,core}/**/*.{test,spec}.?(c|m)[jt]s?(x)'],
51+
passWithNoTests: true,
5152
environment: 'jsdom',
5253
environmentOptions: {
5354
jsdom: {

build/frontend/vitest.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export default defineConfig({
3838
},
3939
test: {
4040
include: ['apps/**/*.{test,spec}.?(c|m)[jt]s?(x)'],
41+
passWithNoTests: true,
4142
env: {
4243
LANG: 'en_US',
4344
TZ: 'UTC',

0 commit comments

Comments
 (0)