Secure Password Change & UI Enhancement (closes #4939) by dineshyr29-04 · Pull Request #5777 · learning-unlimited/ESP-Website

dineshyr29-04 · 2026-04-29T09:00:35Z

Overview

This PR implements critical security enhancements for the password change functionality and adds a premium visibility toggler for better UX.

Changes

🛡️ Security Fixes

Identical Password Rejection: Users can no longer set a new password that is the same as their current one.
Strict Complexity Enforcement: New passwords must now be:
- 8+ characters long
- Include Uppercase, Lowercase, Number, and Special Character.

✨ UI Enhancements

Visibility Toggler: Added a sleek eye-icon toggle to all password fields.
Premium Styling: Added modern SVG icons and refined alignment for a professional look.

Verification

Verified that identical passwords are rejected.
Verified complexity rules.
Tested toggler functionality in multiple visibility states.

…nts and ProgramModule field validations

…aints

…constraint handling

… validation logic

…r29-04/ESP-Website into class-constraints-module

Copilot

Pull request overview

This PR tightens password-change validation in the MyESP account workflow and adds UI affordances for password field visibility. It also introduces a new “Class Constraints” management module (plus supporting schedule-token model/migration/admin wiring) for program scheduling constraints.

Changes:

Add server-side password rules: reject identical current/new password and enforce complexity requirements.
Update the change-password template to add per-field “show/hide password” toggles with inline styling/script.
Add a new Class Constraints module (templates/handler/tests) and a new ScheduleTestSubject token type to support subject-based schedule constraints.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
esp/templates/users/passwd.html	Adds password visibility toggles and styling to the change-password page.
esp/esp/users/forms/password_reset.py	Adds new-password validation (same-as-current + complexity).
esp/esp/program/modules/handlers/classconstraintsmodule.py	New admin-only module to create/delete schedule constraints using subject tokens.
esp/templates/program/modules/classconstraintsmodule/main.html	UI for listing/creating/deleting class constraints.
esp/esp/program/modules/handlers/tests/test_classconstraintsmodule.py	Tests for the new module’s constraint creation/evaluation/deletion behaviors.
esp/esp/program/models/init.py	Adds `ScheduleTestSubject` token model and cache invalidation dependencies.
esp/esp/program/migrations/0038_scheduletestsubject.py	Creates DB table for `ScheduleTestSubject`.
esp/esp/program/admin.py	Registers `ScheduleTestSubject` in Django admin.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T09:05:39Z

+        outline: none !important;
+    }
+    .toggle-password:hover {
+        color: #444;
+    }


.toggle-password forces outline: none !important;, which removes the focus indicator and is an accessibility regression. If you need custom styling, replace it with a visible focus style instead of disabling outlines.

Suggested change

outline: none !important;

}

.toggle-password:hover {

color: #444;

}

}

.toggle-password:hover {

color: #444;

}

.toggle-password:focus-visible {

outline: 2px solid #444;

outline-offset: 2px;

border-radius: 2px;

}

Copilot · 2026-04-29T09:05:40Z

+   {% if form.password.errors %}
+     <span class="form_error">{{ form.password.errors|join:", " }}</span>
   {% endif %}


The password field error rendering dropped role="alert"/live region behavior from the previous markup, so screen readers may not announce validation errors. Consider restoring role="alert" (or aria-live="polite") on the error container spans.

Copilot · 2026-04-29T09:05:40Z

+                subject_exps = ScheduleTestSubject.objects.values_list('exp_id', flat=True)
+                context['constraints'] = ScheduleConstraint.objects.filter(program=prog, condition_id__in=subject_exps)
+                return render_to_response(self.baseDir() + 'main.html', request, context)


subject_exps = ScheduleTestSubject.objects.values_list('exp_id', flat=True) is unscoped and may pull IDs for all programs; combined with condition_id__in=... this can produce a very large IN (...) list. Filter by the current program (e.g., subject__parent_program=prog) and distinct() the IDs, or use a join-based filter instead.

Copilot · 2026-04-29T09:05:40Z

+        try:
+            subject_exps = ScheduleTestSubject.objects.values_list('exp_id', flat=True)
+            constraint = ScheduleConstraint.objects.get(id=constraint_id, program=prog, condition_id__in=subject_exps)
+        except ScheduleConstraint.DoesNotExist:


Same unscoped values_list('exp_id') issue in deletion: it may scan/return exp IDs across all programs. Filter ScheduleTestSubject by subject__parent_program=prog (and ideally distinct()) to keep the lookup small and predictable.

Copilot · 2026-04-29T09:05:40Z

+            elif 'delete' in request.POST:
+                raw_id = request.POST.get('constraint_id', '')
+                # Fix: validate/cast before querying to avoid ValueError 500s.
+                try:
+                    constraint_id = int(raw_id)
+                except (ValueError, TypeError):
+                    return HttpResponseBadRequest("Invalid constraint ID.")
+                self._delete_constraint(constraint_id, prog)


The module adds explicit handling for non-integer constraint_id values (returning 400), but the test suite in this PR doesn’t cover that branch. Add a request-level test (or a unit test around the POST handler) that posts a non-integer constraint_id and asserts a 400 response.

Copilot · 2026-04-29T09:05:41Z

+    def clean_newpasswd(self):
+        new_passwd = self.cleaned_data.get('newpasswd')
+        current_passwd = self.cleaned_data.get('password')
+
+        if new_passwd == current_passwd:
+            raise forms.ValidationError("New password cannot be the same as the current password.")
+
+        if len(new_passwd) < 8:
+            raise forms.ValidationError("Password must be at least 8 characters long.")


clean_newpasswd() assumes new_passwd is always present; if the field fails its own validation (e.g., missing/too long), Django will omit it from cleaned_data and len(new_passwd) will raise TypeError, turning a normal validation error into a 500. Guard for None (and return early) before running length/regex checks.

Copilot · 2026-04-29T09:05:41Z

+        context['form'] = ClassConstraintsForm(prog)
+        subject_exps = ScheduleTestSubject.objects.values_list('exp_id', flat=True)
+        context['constraints'] = ScheduleConstraint.objects.filter(program=prog, condition_id__in=subject_exps)
+        return render_to_response(self.baseDir() + 'main.html', request, context)


Same as above: this unfiltered values_list('exp_id') can generate an unnecessarily large IN (...) for constraints listing. Scope the ScheduleTestSubject query to prog and distinct() it, or use an ORM join to avoid materializing IDs.

Copilot · 2026-04-29T09:05:41Z

+class ClassConstraintsModule(ProgramModuleObj, CoreModule):
+    """
+    Module for managing complex class constraints like prerequisites and
+    mutual exclusions. Constraints are represented as ScheduleConstraints
+    using BooleanExpressions composed of ScheduleTestSubject tokens.
+    """


The PR description/linked issue is about password change security & UI, but this file adds an entire new Class Constraints management module plus related model/migration changes. Please either update the PR description to cover this scope or split these changes into a separate PR to keep review and risk manageable.

Copilot · 2026-04-29T09:05:42Z

+- Fix ClassSubject.objects.create() — requires category, grade_min, grade_max.
+- Use ScheduleMap for evaluation tests instead of a raw dict.
+- Add test for class_a == class_b form validation.
+- Add test for invalid constraint_id (non-integer) handling.


This test module docstring claims a test was added for invalid (non-integer) constraint_id handling, but no such test exists in the file. Either add the test or update/remove the docstring bullet to match what’s actually covered.

Suggested change

- Add test for invalid constraint_id (non-integer) handling.

Copilot · 2026-04-29T09:05:42Z

+     <button type="button" class="toggle-password" tabindex="-1" aria-label="Toggle password visibility">
+       <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="eye-icon"><path d="M1 12s4-8 11-8 11 8 11 8-4 8-11 8-11-8-11-8z"></path><circle cx="12" cy="12" r="3"></circle></svg>
+     </button>


The visibility toggle button is removed from the tab order (tabindex="-1"), which makes it unusable for keyboard-only users. Since this directly affects password entry UX, keep it focusable (default) and ensure it can be triggered via keyboard.

Oval17 · 2026-05-08T15:00:36Z

Hi @dineshyr29-04
Can you look at the review comments by copilot once.
Thanks

dineshyr29-04 · 2026-05-09T04:34:41Z

ok i will see once and i will get it done.

dineshyr29-04 added 10 commits April 14, 2026 21:27

Implement Class Constraints Module and ScheduleTestSubject token

f145e04

The changes in the classconstraintsmodule and the bugs are fixed

abd2275

fix: update test fixtures to match correct model fields

1b25567

feat: add ScheduleTestSubject model and update Program model constrai…

339b29c

…nts and ProgramModule field validations

fix: scope ClassConstraintsModule to only manage subject-based constr…

668f38e

…aints

fix: ensure ScheduleTestSubject references are correctly filtered in …

e44bb9b

…constraint handling

Merge branch 'main' into class-constraints-module

4e1c6a0

feat: implement secure password reset form with visibility toggle and…

588637d

… validation logic

Merge branch 'class-constraints-module' of https://github.com/dineshy…

d2fb2fb

…r29-04/ESP-Website into class-constraints-module

feat: add password change template with visibility toggle functionality

4ca498a

Copilot AI review requested due to automatic review settings April 29, 2026 09:00

github-actions Bot added tests frontend backend database programs users labels Apr 29, 2026

Copilot started reviewing on behalf of dineshyr29-04 April 29, 2026 09:01 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Merge branch 'main' into password-change-work

b143b30

Merge branch 'main' into password-change-work

fe27cc3

Uh oh!

Conversation

dineshyr29-04 commented Apr 29, 2026

Overview

Changes

🛡️ Security Fixes

✨ UI Enhancements

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Oval17 commented May 8, 2026

Uh oh!

dineshyr29-04 commented May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants