Skip to content

Commit 56fdf0e

Browse files
nfebeskjnldsv
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 010e49c commit 56fdf0e

File tree

2 files changed

+334
-1
lines changed

2 files changed

+334
-1
lines changed
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
@@ -900,6 +900,7 @@ export default {
900900
901901
if (this.isNewShare) {
902902
if ((this.config.enableLinkPasswordByDefault || this.isPasswordEnforced) && this.isPublicShare) {
903+
this.passwordProtectedState = true
903904
this.$set(this.share, 'newPassword', await GeneratePassword(true))
904905
this.advancedSectionAccordionExpanded = true
905906
}
@@ -1005,8 +1006,9 @@ export default {
10051006
this.share.note = ''
10061007
}
10071008
if (this.isPasswordProtected) {
1008-
if (this.isPasswordEnforced && this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
1009+
if (this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
10091010
this.passwordError = true
1011+
return
10101012
}
10111013
} else {
10121014
this.share.password = ''

0 commit comments

Comments
 (0)