Strengthen password validation - #1447
Conversation
|
I wanted to provide a more detailed explanation of the implementation and the reasoning behind the changes in this PR. What changed This PR strengthens Runestone’s password policy while keeping the existing validation architecture intact.
The existing validate_password() helper in components/rsptx/validation/fields.py accepted passwords with a minimum length of 6 characters. I changed the default minimum to 10 characters: def validate_password(password: str, min_length: int = 10) -> str | None: I kept this as a default parameter rather than hard-coding the value throughout the application so that the password policy remains centralized in one validation helper. The existing whitespace-only protection remains unchanged. A password containing only whitespace is still rejected, while legitimate spaces remain allowed.
I did not introduce requirements such as “must contain an uppercase letter, lowercase letter, number, and special character.” The current validator already treats the password as an opaque string and only validates whether it is non-empty/meaningful and meets the minimum length. I preserved that behavior so that users can use passphrases and other strong password formats without imposing arbitrary character-class requirements. There is also no new password maximum-length restriction in this PR. This avoids unnecessarily limiting long passphrases and is consistent with the intent of the issue to support passwords of at least 64 characters.
The instructor interface allows students to be registered through a CSV containing: username,email,first_name,last_name,password,course The enroll_students endpoint in bases/rsptx/admin_server_api/routers/instructor.py previously created users from this CSV without calling the shared password validator. That meant the regular registration flow and instructor-created accounts could have different password policies. This PR imports the existing validate_password() helper into the instructor router and validates row[4], which is the password field, before creating a new user. When a password is invalid, the row is rejected and reported through the existing CSV enrollment results mechanism rather than creating the account.
While tracing the CSV path, I also found that the entire CSV row was previously processed with: row = [field.strip() for field in row] That unintentionally stripped leading and trailing whitespace from passwords. The password validator explicitly does not strip passwords because spaces can legitimately be part of a password/passphrase. I therefore changed the CSV normalization so that all fields except the password are stripped: row = [ This preserves the exact password supplied in the CSV while retaining the existing whitespace cleanup for usernames, names, email addresses, and the course field.
The password validation tests in test/components/rsptx/validation/test_fields.py were updated to reflect the new policy. The tests now cover: Passwords shorter than 10 characters being rejected. This keeps the tests focused on the actual policy rather than enforcing arbitrary password composition rules. Validation performed I ran the following checks locally: uv run pytest test/components/rsptx/validation/test_fields.py The PR's GitHub Actions checks are also both passing: Lint / lint: Done |
Summary
Testing
Closes #1307