Skip to content

Strengthen password validation - #1447

Open
krishnab917 wants to merge 1 commit into
RunestoneInteractive:mainfrom
krishnab917:feature/1307-stronger-passwords
Open

Strengthen password validation#1447
krishnab917 wants to merge 1 commit into
RunestoneInteractive:mainfrom
krishnab917:feature/1307-stronger-passwords

Conversation

@krishnab917

Copy link
Copy Markdown

Summary

  • Increase the minimum password length from 6 to 10 characters.
  • Enforce the shared password validator for instructor CSV student enrollment.
  • Preserve leading and trailing password spaces while trimming other CSV fields.
  • Update password validation tests for the new policy.

Testing

  • 30 password validation tests passed.
  • Ruff checks passed.
  • Ruff formatting check passed.
  • git diff --check passed.

Closes #1307

@krishnab917
krishnab917 requested a review from bnmnetp as a code owner August 28, 2026 21:10
@krishnab917

Copy link
Copy Markdown
Author

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.

  1. Increased the minimum password length from 6 to 10

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.

  1. Kept the password character policy permissive

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.

  1. Applied the same validation to instructor CSV enrollment

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.

  1. Preserved password whitespace during CSV processing

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 = [
field.strip() if index != 4 else field
for index, field in enumerate(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.

  1. Updated the test coverage

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.
A password of exactly 10 characters being accepted.
Longer passphrases being accepted.
Passwords containing numbers and special characters being accepted.
Passwords containing leading/trailing spaces being accepted.
Whitespace-only passwords continuing to be 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
30 tests passed
uv run ruff check ...
All checks passed
uv run ruff format --check ...
3 files already formatted
git diff --check
Passed
Python syntax compilation of the modified instructor router
Passed

The PR's GitHub Actions checks are also both passing:

Lint / lint: Done
Tests / crud-tests: Done
Scope / existing accounts

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.

Feature Request: Stronger passwords

1 participant