Skip to content

fix(security): throttle failed student password reset answer attempts#324

Open
Isaries wants to merge 2 commits into
WISE-Community:developfrom
Isaries:fix/student-reset-throttle
Open

fix(security): throttle failed student password reset answer attempts#324
Isaries wants to merge 2 commits into
WISE-Community:developfrom
Isaries:fix/student-reset-throttle

Conversation

@Isaries

@Isaries Isaries commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

The student password reset compared the security answer in plain text with no limit on attempts. Because these answers are low entropy, an attacker could brute force the answer and take over a student account. This adds throttling that matches the approach the teacher password reset flow already uses.

Changes

  • Block the reset after a small number of failed answers within a short window (5 attempts within 10 minutes) on both the security answer step and the password change step.
  • Reuse the existing failed verification attempt fields on the user details, so there is no database migration.

Paired change

This is paired with a WISE-Client change (branch fix/student-reset-throttle) that displays a message for the new throttling response code.

Testing

  • Builds on JDK 17 (mvnw package).

The student password reset compared the security answer in plain text with
no attempt limit, so the low-entropy answer could be brute forced to take
over any student account. The change endpoint had neither a limit nor a
captcha, unlike the teacher flow which locks out after repeated failures.

Lock the reset after too many failed answer attempts within a short window,
for both the answer check and the password change endpoints, matching the
teacher flow. The counter reuses the existing failed password reset
verification fields, so no schema change is needed.
@Isaries

Isaries commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Paired client change: WISE-Community/WISE-Client#2325

@Aaron-Detre Aaron-Detre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made one minor suggestion, but the feature works as described, and the code looks good.

…dent/StudentForgotAccountAPIController.java

Co-authored-by: Aaron Detre <aarondetre@gmail.com>
@Isaries

Isaries commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review @Aaron-Detre. I have applied your suggestion for isTooManyFailedAnswerAttempts, and the build is green, so this should be ready to merge.

Two notes on the design, for the record:

  • The counter reuses the teacher verification-code fields (recentFailedVerificationCodeAttemptTime / numberOfRecentFailedVerificationCodeAttempts). This is safe because students never obtain a verification code (that path is teacher-only and requires an emailed code), so the two flows write these fields for disjoint sets of accounts, and it avoids a migration. The counter is also cleared on a correct answer and on a successful password change, so a legitimate reset leaves no stale throttle state.

  • One intentional deviation from the teacher flow: the student step checks isTooManyFailedAnswerAttempts before recording the attempt (check-then-record), whereas the teacher verification-code step increments before checking. The practical effect is that the student flow allows one more failed guess (blocks on the 6th vs the teacher's 5th) before the same 5-attempt / 10-minute window applies. Happy to align it to increment-first if you would prefer exact parity.

Paired client change: WISE-Community/WISE-Client#2325

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.

3 participants