Skip to content

feat: add tests for password reset functionality for retired users#38737

Merged
robrap merged 16 commits into
openedx:masterfrom
ktyagiapphelix2u:ktyagi/BOMS-626
Jul 1, 2026
Merged

feat: add tests for password reset functionality for retired users#38737
robrap merged 16 commits into
openedx:masterfrom
ktyagiapphelix2u:ktyagi/BOMS-626

Conversation

@ktyagiapphelix2u

@ktyagiapphelix2u ktyagiapphelix2u commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR addresses follow-up feedback from PR #38427 by replacing the retirement TODO with a permanent comment explaining why social accounts are retained during the cooling-off period, and by adding tests to verify that retired users cannot initiate or complete password reset flows.

Private Ticket

https://2u-internal.atlassian.net/browse/BOMS-626

@ktyagiapphelix2u ktyagiapphelix2u requested review from a team as code owners June 10, 2026 07:35

@robrap robrap 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.

Thanks.

Comment thread openedx/core/djangoapps/user_api/accounts/utils.py Outdated
Comment thread openedx/core/djangoapps/user_api/accounts/utils.py Outdated
Comment thread openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py Outdated
Comment thread openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py Outdated

@robrap robrap 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.

Thanks.

Comment thread openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py Outdated
Comment thread openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py Outdated
Comment thread openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py Outdated

@robrap robrap 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.

The following are very minor comments, and we can merge shortly. It don't think it needs to go through another round of internal review, but whatever you want.

Comment thread openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py Outdated
# Always return 200 OK to prevent user enumeration while leaving the password unchanged and unusable.
assert response.status_code == 200
self.user.refresh_from_db()
assert not self.user.has_usable_password()

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.

Would it make sense to add assert not self.user.is_active here, and add this assertions above, to simply show that there is no change to any of the assertions? The last assertion here probably isn't needed above, because you're copying out the password there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sounds resonable proving nothing changed is the whole point of these tests.

@robrap robrap enabled auto-merge (squash) July 1, 2026 13:18
@robrap robrap merged commit c097370 into openedx:master Jul 1, 2026
41 checks passed
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