Skip to content

Commit 9f061db

Browse files
Copilotskjnldsv
andcommitted
fix: scope password guard to public shares only and restore early return
Agent-Logs-Url: https://github.com/nextcloud/server/sessions/00e5a913-460e-4e60-9e3e-4551e87d905a Co-authored-by: skjnldsv <14975046+skjnldsv@users.noreply.github.com>
1 parent bce67a2 commit 9f061db

3 files changed

Lines changed: 75 additions & 152 deletions

File tree

apps/files_sharing/src/views/SharingDetailsTab.spec.ts

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -233,69 +233,95 @@ describe('SharingDetailsTab - Password State Management Logic', () => {
233233
return typeof attr === 'string' && attr.length > 0
234234
}
235235

236-
it('should set passwordError when isPasswordProtected but newPassword is empty for new share', () => {
237-
const isPasswordProtected = true
238-
const isNewShare = true
239-
const newPassword = ''
240-
let passwordError = false
241-
242-
if (isPasswordProtected && isNewShare && !isValidShareAttribute(newPassword)) {
243-
passwordError = true
236+
/**
237+
* Simulates the password guard in saveShare() – returns true when execution
238+
* should be blocked (passwordError set and early return triggered).
239+
*/
240+
function shouldBlock(state: {
241+
isPasswordProtected: boolean
242+
isPublicShare: boolean
243+
isNewShare: boolean
244+
newPassword: string | undefined
245+
}): boolean {
246+
if (state.isPasswordProtected) {
247+
if (state.isPublicShare && state.isNewShare && !isValidShareAttribute(state.newPassword)) {
248+
return true
249+
}
244250
}
251+
return false
252+
}
253+
254+
// --- New public share: password missing → should block ---
245255

246-
expect(passwordError).toBe(true)
256+
it('blocks new public share when isPasswordProtected but newPassword is empty', () => {
257+
expect(shouldBlock({
258+
isPasswordProtected: true,
259+
isPublicShare: true,
260+
isNewShare: true,
261+
newPassword: '',
262+
})).toBe(true)
247263
})
248264

249-
it('should set passwordError when isPasswordProtected but newPassword is undefined for new share', () => {
250-
const isPasswordProtected = true
251-
const isNewShare = true
252-
const newPassword = undefined
253-
let passwordError = false
265+
it('blocks new public share when isPasswordProtected but newPassword is undefined', () => {
266+
expect(shouldBlock({
267+
isPasswordProtected: true,
268+
isPublicShare: true,
269+
isNewShare: true,
270+
newPassword: undefined,
271+
})).toBe(true)
272+
})
254273

255-
if (isPasswordProtected && isNewShare && !isValidShareAttribute(newPassword)) {
256-
passwordError = true
257-
}
274+
// --- New public share: valid password → should NOT block ---
258275

259-
expect(passwordError).toBe(true)
276+
it('does not block new public share when password is valid', () => {
277+
expect(shouldBlock({
278+
isPasswordProtected: true,
279+
isPublicShare: true,
280+
isNewShare: true,
281+
newPassword: 'valid-password-123',
282+
})).toBe(false)
260283
})
261284

262-
it('should not set passwordError when password is valid for new share', () => {
263-
const isPasswordProtected = true
264-
const isNewShare = true
265-
const newPassword = 'valid-password-123'
266-
let passwordError = false
267-
268-
if (isPasswordProtected && isNewShare && !isValidShareAttribute(newPassword)) {
269-
passwordError = true
270-
}
285+
// --- Non-public (user/group) share → should NEVER block (regression for #59254) ---
271286

272-
expect(passwordError).toBe(false)
287+
it('does not block new non-public share even when isPasswordProtected and newPassword is empty', () => {
288+
expect(shouldBlock({
289+
isPasswordProtected: true,
290+
isPublicShare: false,
291+
isNewShare: true,
292+
newPassword: '',
293+
})).toBe(false)
273294
})
274295

275-
it('should not set passwordError when isPasswordProtected is false', () => {
276-
const isPasswordProtected = false
277-
const isNewShare = true
278-
const newPassword = ''
279-
let passwordError = false
296+
it('does not block new non-public share even when isPasswordProtected and newPassword is undefined', () => {
297+
expect(shouldBlock({
298+
isPasswordProtected: true,
299+
isPublicShare: false,
300+
isNewShare: true,
301+
newPassword: undefined,
302+
})).toBe(false)
303+
})
280304

281-
if (isPasswordProtected && isNewShare && !isValidShareAttribute(newPassword)) {
282-
passwordError = true
283-
}
305+
// --- Existing public share (update path) → should NOT block ---
284306

285-
expect(passwordError).toBe(false)
307+
it('does not block existing public share with empty newPassword (update path)', () => {
308+
expect(shouldBlock({
309+
isPasswordProtected: true,
310+
isPublicShare: true,
311+
isNewShare: false,
312+
newPassword: '',
313+
})).toBe(false)
286314
})
287315

288-
it('should not validate password for existing shares', () => {
289-
const isPasswordProtected = true
290-
const isNewShare = false
291-
const newPassword = ''
292-
let passwordError = false
316+
// --- isPasswordProtected false → should NOT block ---
293317

294-
if (isPasswordProtected && isNewShare && !isValidShareAttribute(newPassword)) {
295-
passwordError = true
296-
}
297-
298-
expect(passwordError).toBe(false)
318+
it('does not block when isPasswordProtected is false', () => {
319+
expect(shouldBlock({
320+
isPasswordProtected: false,
321+
isPublicShare: true,
322+
isNewShare: true,
323+
newPassword: '',
324+
})).toBe(false)
299325
})
300326
})
301327
})

apps/files_sharing/src/views/SharingDetailsTab.vue

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1091,8 +1091,9 @@ export default {
10911091
this.share.note = ''
10921092
}
10931093
if (this.isPasswordProtected) {
1094-
if (this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
1094+
if (this.isPublicShare && this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
10951095
this.passwordError = true
1096+
return
10961097
}
10971098
} else {
10981099
this.share.password = ''

build/frontend-legacy/package-lock.json

Lines changed: 0 additions & 104 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)