Skip to content

KC-1423: fix validation for passwords and passphrase in record creation and updation - #2330

Closed
sshrushanth-ks wants to merge 5 commits into
releasefrom
passphrase-policy-validation-fix
Closed

sshrushanth-ks wants to merge 5 commits into
releasefrom
passphrase-policy-validation-fix

Conversation

@sshrushanth-ks

@sshrushanth-ks sshrushanth-ks commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Apply password complexity rules to generated passwords, passphrase rules to generated passphrases, and matching domain-specific policies to manually entered passwords and passphrases. Manual entries are not checked against generator defaults when no domain policy matches.

Changes

  • keepercommander/enforcement.py: Added domain policy matching, generator policy selection, and manual password or allowed passphrase validation.
  • keepercommander/generator.py: Applied domain passphrase settings during generation.
  • keepercommander/commands/record_edit.py: Applied policy selection to typed record add and update flows; prioritized the main URL field.
  • keepercommander/commands/recordv2.py: Updated legacy add and edit policy handling, including the --force override.
  • keepercommander/commands/recordv3.py: Applied domain-aware validation to v3 record operations.
  • keepercommander/commands/nested_share_folder/record_commands.py: Added policy validation to NSF record operations and update retries.
  • unit-tests/: Added coverage for domain matching, generated and manual password/passphrase validation, URL selection, legacy overrides, and NSF retries.

@sshrushanth-ks sshrushanth-ks self-assigned this Aug 28, 2026
@sshrushanth-ks
sshrushanth-ks marked this pull request as ready for review August 28, 2026 11:17

@craiglurey craiglurey 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.

I don’t agree with this. The whole point of enterprise enforcements is to ensure that users can’t type or generate a weak password. The functionality of Commander needs to be the same as the user on the vault. I'd like to understand where this requirement came from.

@amangalampalli-ks

Copy link
Copy Markdown
Contributor

I don’t agree with this. The whole point of enterprise enforcements is to ensure that users can’t type or generate a weak password. The functionality of Commander needs to be the same as the user on the vault. I'd like to understand where this requirement came from.

We identified a bug in Commander: when both password and passphrase policies were applied, it incorrectly validated constraints across fields - for example, displaying a passphrase policy error while a user was entering a password.
After reviewing the Vault’s behavior, we found that it enforces these policies only in the generator, not during manual field entry. We therefore planned to update Commander to align with the Vault’s current behavior.

@sshrushanth-ks
sshrushanth-ks force-pushed the passphrase-policy-validation-fix branch from ed58f2e to 3321146 Compare October 15, 2026 05:49
@sshrushanth-ks
sshrushanth-ks marked this pull request as draft October 15, 2026 05:50
@sshrushanth-ks
sshrushanth-ks requested review from craiglurey and removed request for craiglurey and sk-keeper October 15, 2026 05:50
…on and updation (#2314)

* fix validation for passwords and passphrase in record creation and updation

* Fix generated password policy validation after review

- Track generated key-pair passphrases
- Centralize v3 add/edit policy validation
- Preserve random-password diagnostics
- Keep manual passwords exempt from generated-password policy
- Add regression coverage across record and NSF commands

* Validate generated passwords at generation site instead of post-hoc via
instance flags. This fixes two issues:

1. v3 add/edit --generate now validates (was silently bypassed)
2. No cross-field contamination in multi-field records

Manual passwords remain unvalidated (handled by BreachWatch).

* updated failing test
…on and updation, and apply domain-specific policies to manual passwords and passphrases (#2453)

* fix validation for passwords and passphrase in record creation and updation

* Apply domain-specific policies to generated and manual passwords

Select generator policies by record URL and validate generated passwords
and passphrases with their respective rules. Apply explicit domain
policies to manually entered passwords and passphrases, while leaving
manual entries unchecked when no domain policy matches.
Cover record add and update flows, including legacy and nested records.
Add regression tests for policy selection and manual password validation.

* Handle Unicode passphrases in domain policy validation

Normalize passphrase words to NFC and accept Unicode letters during
manual passphrase validation. Remove the unused NSF field resolver and
add regression coverage for domain-policy enforcement in NSF updates.

* Apply domain password rules to manual entries without inferring
passphrase intent from separators. Keep passphrase rules for generated
passphrases and add regression coverage for hyphenated passwords.

* Addressed review comments:

Use matching domain settings for passphrase generation and allow manual
entries that satisfy the domain’s password or permitted passphrase rules.
Prefer the primary record URL, add a policy override for legacy edits,
and revalidate manual passwords on NSF update retries.

Add regression coverage for policy selection, URL precedence, overrides,
and retry validation.
@sshrushanth-ks
sshrushanth-ks force-pushed the passphrase-policy-validation-fix branch from 0465a0a to e0fd975 Compare October 30, 2026 13:44
@sshrushanth-ks

Copy link
Copy Markdown
Contributor Author

Latest PR:#2482

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants