Skip to content

fix(user): enforce every password character class on signup - #299

Open
tunglambk wants to merge 1 commit into
benavlabs:mainfrom
tunglambk:fix/password-character-classes
Open

tunglambk wants to merge 1 commit into
benavlabs:mainfrom
tunglambk:fix/password-character-classes

Conversation

@tunglambk

Copy link
Copy Markdown

Refs #298 — this is the interim option from the issue; the move to a crudauth PasswordPolicy still follows once #292 lands.

UserCreate.password declared pattern=r"^.{8,}|[0-9]+|[A-Z]+|[a-z]+|[^a-zA-Z0-9]+$". Because that is a top-level alternation, a password only has to match one branch, so abcdefgh, aaaaaaaaaaaa and eight spaces were all accepted while the description promised a number, an uppercase letter, a lowercase letter and a special character.

A field_validator now rejects a password that is missing any of the four classes, naming the one that is missing. min_length=8 stays as it was. A corrected pattern cannot express this: pydantic compiles pattern with the Rust regex crate, which has no lookahead.

Tests cover each character class, the documented example, and the length boundary. pytest tests/unit/modules/user/test_schemas.py — 10 passed; 7 of the 10 fail without the change. The full tests/unit suite is 260 passed, ruff check backend/src cli/src backend/tests is clean, and mypy src reports only the three pre-existing errors in the redis backends.

The declared pattern was an alternation, so any of its branches was enough: abcdefgh, aaaaaaaaaaaa and eight spaces were accepted while the description promised a number, an uppercase letter, a lowercase letter and a special character. A field_validator now rejects a password missing any of the four classes, and min_length stays. Pydantic compiles pattern with the Rust regex crate, which has no lookahead, so the check cannot be expressed as a corrected pattern.

Refs benavlabs#298
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.

1 participant