fix: imported accounts are subject to the password lifetime - #884
Merged
blaipr merged 1 commit intoAug 26, 2026
Merged
Conversation
Every door that creates an account asks the preset service to clamp `passDateChange` to the policy's lifetime — the web form does it, and so do the API's create and edit-pass. The importers do not: `ImportBase::addAccount()` goes straight to `AccountService::create()`, which applies the permission and privacy presets but has never applied the password ones. So a fixed policy saying passwords expire after ninety days said nothing at all about the several thousand accounts that arrived through a CSV, a KeePass file or a sysPass export — which is exactly the population most likely to be carrying old credentials. The clamp is applied on import now, for all three importers, since they share this method. `checkPasswordPreset()` — the one that validates length and character classes — is deliberately *not* applied, and this is the judgement in the change rather than the code. An import is a migration of credentials that already exist somewhere else. Refusing the ones that predate the policy would mean not being able to migrate at all, and the import runs in a single transaction, so one weak password would take the whole file with it. The policy still bites the moment the account is next edited, and the lifetime clamp is what makes somebody get there rather than waiting for it to be noticed. The preset service reaches ImportBase through ImportHelper, which is how it already gets the account, category, client, tag and config services. Checked by dropping the clamp: the new test fails, asserting on the stored DTO rather than on the clamp having been called. The test needed the clamp stub to consult a per-test value rather than being re-stubbed in the test itself — the first stub registered is the one that answers, so a per-test override of a setUp stub silently does nothing. That is the second time that has caught me today.
blaipr
deleted the
fix/imported-accounts-are-subject-to-the-password-lifetime
branch
August 26, 2026 19:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every door that creates an account asks the preset service to clamp
passDateChangetothe policy's lifetime — the web form does it, and so do the API's create and edit-pass.
The importers do not:
ImportBase::addAccount()goes straight toAccountService::create(), which applies the permission and privacy presets but has neverapplied the password ones. So a fixed policy saying passwords expire after ninety days
said nothing at all about the several thousand accounts that arrived through a CSV, a
KeePass file or a sysPass export — which is exactly the population most likely to be
carrying old credentials.
The clamp is applied on import now, for all three importers, since they share this method.
checkPasswordPreset()— the one that validates length and character classes — isdeliberately not applied, and this is the judgement in the change rather than the code.
An import is a migration of credentials that already exist somewhere else. Refusing the
ones that predate the policy would mean not being able to migrate at all, and the import
runs in a single transaction, so one weak password would take the whole file with it. The
policy still bites the moment the account is next edited, and the lifetime clamp is what
makes somebody get there rather than waiting for it to be noticed.
The preset service reaches ImportBase through ImportHelper, which is how it already gets
the account, category, client, tag and config services.
Checked by dropping the clamp: the new test fails, asserting on the stored DTO rather than
on the clamp having been called.
The test needed the clamp stub to consult a per-test value rather than being re-stubbed in
the test itself — the first stub registered is the one that answers, so a per-test override
of a setUp stub silently does nothing. That is the second time that has caught me today.