Feat/password policy - #9
Merged
Merged
Conversation
Plain configuration data with a validation method, not a pluggable interface like the rest of this package — there's nothing to swap out here, just numbers and booleans. Validate reports every violated rule at once (as stable, machine- readable string codes — the engine doesn't own UI copy/localization anywhere else, so it doesn't start here), not just the first one hit. DefaultPasswordPolicy (8 min, 72 max, no character-class requirements) follows NIST 800-63B guidance that length matters far more than forced complexity rules. MaxLength defaults to 72 specifically because that's bcrypt's own real limit — without this check, a longer password hits a raw bcrypt library error at hash time instead of a clean policy violation.
Covers: multiple violations reported together, a good password passing cleanly, MaxLength 0 meaning no upper bound, symbol detection, and the default policy's actual bounds (8 min, 72 max).
Both take a policy security.PasswordPolicy param now, checked BEFORE
the breach check (cheapest/local check first — no reason to make an
external call for a password that's already rejected). A violation
returns *ErrPasswordPolicyViolation{Violations []string}, every
broken rule at once.
New Config.PasswordPolicy field. Unlike every other optional feature
in this engine, this has NO 'unconfigured means off' state — leaving
it as the zero value applies security.DefaultPasswordPolicy instead
(detected via MaxLength == 0, which no real policy would intentionally
set). Public facade signatures for SignUp/ChangePassword are
unchanged.
Updates existing signup_test.go/account_test.go call sites for the
new param.
Covers: a policy violation rejecting SignUp/ChangePassword with every broken rule reported together, a satisfying password succeeding, the breach checker never being called when policy already rejected the password, and current-password verification running before the new password's policy check on ChangePassword.
That case needed security.PasswordPolicy, which didn't exist yet on the branch this smoke test was first written on — deferred there, added back now that password policy exists.
Runnable end-to-end check with no database dependency: go run ./cmd/smoketest/password-policy. Walks the default policy applying automatically when Config.PasswordPolicy is left unset, a short password rejected, a 73-byte password rejected (bcrypt's real limit is 72), a custom stricter policy reporting multiple violations together, and a password satisfying that custom policy succeeding.
applyDefaults detected "unset" via PasswordPolicy.MaxLength == 0 alone. A policy that set MinLength/character requirements but left MaxLength untouched — exactly what a caller would naturally write — looked unset and got silently overwritten by DefaultPasswordPolicy, which has none of those requirements. Comparing the whole struct against its zero value instead means only a truly untouched Config.PasswordPolicy gets defaulted; setting even one field counts as a real custom policy and is used as-is.
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
1 participant
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.
No description provided.