Skip to content

fix(sharebymail): Send readable password by mail instead of hash - #65576

Merged
DerDreschner merged 1 commit into
masterfrom
fix/send-clear-password-by-mail
Oct 16, 2026
Merged

DerDreschner merged 1 commit into
masterfrom
fix/send-clear-password-by-mail

Conversation

@DerDreschner

Copy link
Copy Markdown
Member

Summary

Since ee1feb8 (which was backported to stable35 with #64892 ), the sent passwords when using share-by-mail are hashes and not the plaintext passwords anymore. This is kinda intentional, as the providers themselves should now only contain hashed passwords. To fix that, the manager itself needs to keep the plaintext password and use that when mailing the receiver of a share.

This PR also fixes code that Claude detected as dead in later iterations, but as I plan to do a backport, I'll remove that in a separate PR.

Checklist

AI (if applicable)

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

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
@DerDreschner DerDreschner self-assigned this Oct 16, 2026
@DerDreschner
DerDreschner requested a review from a team as a code owner October 16, 2026 09:49
@DerDreschner
DerDreschner requested review from Altahrim, CarlSchwan, salmart-dev and sorbaugh and removed request for a team October 16, 2026 09:49
@DerDreschner

Copy link
Copy Markdown
Member Author

/backport to stable35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Static review found no blocking issues, and regression tests cover the changed behavior; tests were not executed.

0 open findings

What changed in this PR

Fixes #65479 by sending readable share-by-mail passwords while preserving hashed passwords in storage.

Changes:

  • Retains caller-provided plaintext passwords for email notifications.
  • Handles generated passwords and prevents mailing password hashes.
  • Adds regression tests for password storage and notification behavior.
File Description
tests/​lib/​Share20/​ManagerTest.php Tests plaintext notifications and hashed return values.
lib/​private/​Share20/​Manager.php Passes plaintext passwords through a notification-only share clone.
apps/​sharebymail/​tests/​ShareByMailProviderTest.php Tests supplied hashes, generated passwords, and hash suppression.
apps/​sharebymail/​lib/​ShareByMailProvider.php Preserves generated plaintext for notification and skips mailing hashes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@DerDreschner
DerDreschner merged commit 5430d62 into master Oct 16, 2026
219 of 221 checks passed
@DerDreschner
DerDreschner deleted the fix/send-clear-password-by-mail branch October 16, 2026 10:23
@joshtrichards joshtrichards added this to the Nextcloud 36 milestone Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Password-protected share email exposes database password hash instead of plaintext in 35.0.2

6 participants