fix(files_sharing): restore password guard early return scoped to public shares only

[Copilot]

PR #59262 removed the return from the saveShare() password validation guard to fix a regression (#59254) where non-public shares were incorrectly blocked. That fix was too broad — removing return entirely re-introduced the original "save twice" bug for public shares: passwordError would be set but execution continued, incomingShare.password = undefined, and the share was created without the password.

Changes

• apps/files_sharing/src/views/SharingDetailsTab.vue — add && this.isPublicShare to the guard condition and restore the return:

- if (this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
+ if (this.isPublicShare && this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
      this.passwordError = true
+     return
  }

Non-public shares never support passwords, so they should never be blocked by this guard. Public shares missing a required password are correctly blocked again.

• apps/files_sharing/src/views/SharingDetailsTab.spec.ts — replaced tests that didn't account for isPublicShare with coverage for:
  • New public share + empty/undefined newPassword → blocked (error shown, no API call)
  • New public share + valid password → not blocked
  • New non-public share (user/group) → never blocked regardless of isPasswordProtected (regression guard for [Bug]: Since NC 32.0.7 / 33.0.1 "Save share" Button without any function #59254)
  • Existing public share (update path) → not blocked

TODO

• ...

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

• The content of this PR was partly or fully generated using AI

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• download.cypress.io
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node dist/index.js --exec install /home/REDACTED/.cache/node-gyp/24.14.1/deps/zlib -I /home/REDACTED/.cache/node-gyp/24.14.1/deps/v8/include -I ../../../../../nan --64 -o Release/obj.target/sshcrypto/src/binding.o conf�� " "./Release/.deps/Release/obj.target/cpu_features/deps/cpu_features/src/" --global tnet/tools/git copilot.originalas (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

Problem

The "save twice" bug is back for public shares with a password. When a user creates a new public share and sets a password, the password is silently dropped on the first save — the share is created without the password — and the user must open the share settings and save a second time for the password to actually take effect.

Root cause

In apps/files_sharing/src/views/SharingDetailsTab.vue, inside saveShare(), there used to be a return guard that prevented saving when a new public share had isPasswordProtected enabled but share.newPassword was empty/invalid:

if (this.isPasswordProtected) {
    if (this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
        this.passwordError = true
        return  // ← was here
    }
} else {
    this.share.password = ''
}

PR #59262 removed the return to fix a regression introduced by PR #58226 where non-public shares (user/group shares) were incorrectly blocked from saving because isPasswordProtected was truthy for them too (due to passwordProtectedState state leaking). The fix was correct in intent but too broad — removing return entirely re-introduced the original "save twice" bug for public shares.

Without the return, on a new public share where newPassword is missing:

1. passwordError = true is set and shown in the UI — but execution continues
2. incomingShare.password = this.share.newPassword → undefined
3. addShare() → undefined is falsy → password is silently omitted from the createShare POST
4. Share is created without a password
5. User must save a second time for the password to apply

Fix

Scope the return guard to public shares only (isPublicShare). Non-public shares don't support passwords so they should never be blocked. Public shares that require a password should keep the hard block:

if (this.isPasswordProtected) {
    if (this.isPublicShare && this.isNewShare && !this.isValidShareAttribute(this.share.newPassword)) {
        this.passwordError = true
        return  // only block public shares missing a password
    }
} else {
    this.share.password = ''
}

This restores correct behaviour for public shares (block if password missing) while not regressing #59254 (non-public shares are no longer blocked since isPublicShare is checked first).

Files to change

• apps/files_sharing/src/views/SharingDetailsTab.vue — add && this.isPublicShare to the existing isNewShare && !isValidShareAttribute(newPassword) guard and restore the return

Tests to add

Look for the existing test files for SharingDetailsTab.vue. They are likely in apps/files_sharing/src/views/__tests__/ or apps/files_sharing/src/tests/. Add test cases covering:

1. New public share with password set → saveShare() should proceed (not return early), addShare should be called with the password
2. New public share with password protection enabled but newPassword empty/undefined → saveShare() should set passwordError = true and return early without calling addShare
3. New non-public (user/group) share → saveShare() should always proceed regardless of isPasswordProtected or newPassword state (regression test for [Bug]: Since NC 32.0.7 / 33.0.1 "Save share" Button without any function #59254)
4. Existing public share (update path) → saveShare() should call queueUpdate and not be blocked by the password guard (the guard only applies to isNewShare)

Use the existing test patterns and mocks already present in the test suite. Mock addShare, queueUpdate, createShare as needed.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: We've been having recurring issues with the sharing password sidebar on Server
@nextcloud/server/issues/57011
@nextcloud/server/pull/58226
@nextcloud/server/issues/59254
@nextcloud/server/pull/59267

Now after my last PR in 59262, it seems the save twice issue is back. Please check the PRs, commits and the edited file git history to understand the issues.
It used to work properly. Some users report 32.0.2 as latest working version.
Analyse this, find the root cause and offers a potential suggestion and explanation

Assistant: Now let me look at the current state of SharingDetailsTab.vue to understand the full saveShare flow:

Assistant: Now I have enough context to deliver a comprehensive analysis. Let me look at the critical saveShare method section.

Assistant: Let me now look at the saveShare method and ShareRequests.js to complete the picture:

Assistant: ## Root Cause Analysis

After tracing all the PRs and the current code, the root cause has been found. It's a pre-existing bug in addShare() that was never actually fixed, but got masked and then unmasked by the subsequent PRs.

---

The Core Bug — addShare() uses the wrong property

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

[skjnldsv]

/compile

[susnux]

code change makes sense but did not test it.

[susnux]

Those "tests" are a bit ridiculous (but not since this PR) testing your test code which (maybe) at some point is similar like the implementation 👀
Probably should rather be proper component tests or e2e tests.

[nfebe]

Blocking because of the tests.

[nfebe]

Moved to with better tests #59873