Skip to content

refactor(files_sharing): clarify password generation logic#61414

Draft
joshtrichards wants to merge 3 commits into
masterfrom
jtr/filesSharingPasswordGeneration
Draft

refactor(files_sharing): clarify password generation logic#61414
joshtrichards wants to merge 3 commits into
masterfrom
jtr/filesSharingPasswordGeneration

Conversation

@joshtrichards

@joshtrichards joshtrichards commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

Clean up the file sharing GeneratePassword.ts module to make the server-side and local password generation paths clearer, safer to read, and easier to reason about:

  • extract local password generation logic into generateLocalPassword()
  • rename the random byte helper to fillRandomValues()
  • simplify password policy API access by extracting generateUrl and password
  • simplify OCS response access with optional chaining
  • clarify the local fallback behavior and related JSDoc/comments
  • refactor character selection to use an explicit Math.floor() byte-to-index calculation based on the full 256-value byte range (0..255), which makes the bounds and intent clearer and avoids relying on charAt() implicit float coercion

This is intended as a readability and maintainability cleanup rather than a meaningful behavior change. In particular, the byte-to-index refactor is a small correctness/readability improvement, not the sort of change that meaningfully alters the user-facing strength of generated passwords.

Notes

  • Local password generation still falls back to Math.random() as a last resort when the crypto API is unavailable. This remains intentionally compatible with the current behavior, but it is not cryptographically secure.
  • Success toast behavior is unchanged: a success toast is currently shown for server-side generation, but not for local fallback generation. I believe this is a bug, but kept it out-of-scope for now.
  • Logging on insecure fallback was considered, but is not included in this change to keep the runtime behavior and log surface unchanged.

While working on nextcloud-libraries/nextcloud-auth#950, I looked for other places where a similar cleanup would be relevant, which led to this PR.

Checklist

AI (if applicable)

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

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone Jun 18, 2026
@joshtrichards joshtrichards added 2. developing Work in progress feature: sharing ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jun 18, 2026
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards

Copy link
Copy Markdown
Member Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress feature: sharing ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants