feat: logout other sessions on email change #33846

syedsajjadkazmii · 2023-11-30T08:07:24Z

Logout of other sessions on email change.

openedx/core/djangoapps/user_authn/views/login.py

robrap

I haven’t done a detailed review yet, but wanted to pass on some higher level thoughts. Thank you.

openedx/core/djangoapps/safe_sessions/middleware.py

robrap

Thanks. Some additional comments.

openedx/core/djangoapps/safe_sessions/middleware.py

openedx/core/djangoapps/user_authn/views/login.py

common/djangoapps/student/signals/receivers.py

syedsajjadkazmii · 2023-12-07T11:47:25Z

Related, you can see some of the exemptions of SafeSessions by searching for uses of mark_user_change_as_expected. Also see notes about some of the exemptions. Additionally, masquerading needed exemptions. This code for email check may not run into the same issues, but a little testing and a safety net toggle won't hurt.
Please confirm what types of manual testing you have also done, including if you test that masquerading is not affected. Thank you.

I manually tested the following scenarios

With the ENFORCE_SESSION_EMAIL_MATCH flag set to false, I logged into both Chrome and Safari using the same email. After changing the email in Safari, I observed that I remained logged in to both browsers.
With the ENFORCE_SESSION_EMAIL_MATCH flag set to true, I again logged into both Chrome and Safari with the same email. Following an email change in Safari, I observed that I remained logged in to Safari, but the session was invalidated in Chrome.

Regarding masquerading, I interpreted it as the ability to view the platform as other users in the dashboard. I tested it by masquerading as other users in the dashboard, and everything worked as expected. However, if there's a different interpretation or specific aspects related to masquerading that require testing, please provide clarification. I would be happy to test that too.

robrap · 2023-12-11T21:50:40Z

Regarding masquerading, I interpreted it as the ability to view the platform as other users in the dashboard. I tested it by masquerading as other users in the dashboard, and everything worked as expected. However, if there's a different interpretation or specific aspects related to masquerading that require testing, please provide clarification. I would be happy to test that too.

Yes. Just making sure masquerading still works appropriately when ENFORCE_SESSION_EMAIL_MATCH is enabled. I think masquerading messes with the session, but maybe it does so outside of the scope of your check, or maybe not in the way I think. So, as long as it isn't broken, that sounds good.

robrap

I added a variety of thoughts. Good luck. You are welcome to reach out to others if you want to make steady progress before January.

openedx/core/djangoapps/safe_sessions/middleware.py

cms/djangoapps/contentstore/tests/tests.py

common/djangoapps/student/views/management.py

openedx/core/djangoapps/safe_sessions/middleware.py

openedx/core/djangoapps/safe_sessions/tests/test_middleware.py

openedx/core/djangoapps/user_authn/cookies.py

openedx/core/djangoapps/user_authn/views/login.py

robrap

Thanks for the updates. Looks good. Just minor requests and thoughts. Thanks.

openedx/core/djangoapps/safe_sessions/middleware.py

openedx/core/djangoapps/user_authn/views/login.py

common/djangoapps/student/views/management.py

openedx/core/djangoapps/safe_sessions/middleware.py

robrap

Thank you. I'm going to see what you decide about some of my comments before trying to see if we have all the test cases covered.

lms/envs/common.py

openedx/core/djangoapps/safe_sessions/tests/test_middleware.py

robrap

Thank you. A minor comment (I hope). This is just about ready.

Lastly, once you make any final updates, if you could do one more manual check of masquerading, that would be great.

openedx/core/djangoapps/safe_sessions/tests/test_middleware.py

openedx/core/djangoapps/user_authn/views/tests/test_login.py

openedx/core/djangoapps/safe_sessions/tests/test_middleware.py

lms/envs/common.py

openedx/core/djangoapps/safe_sessions/middleware.py

robrap

Nice. Thank you.

robrap · 2024-01-25T20:44:47Z

openedx/core/djangoapps/safe_sessions/middleware.py

+            if request.session.get('email', None) is None:
+                # .. custom_attribute_name: session_with_no_email_found
+                # .. custom_attribute_description: Indicates that user's email was not
+                #      stored in the user's session.


Nit: Since we are updating the session.

Suggested change

# stored in the user's session.

# yet stored in the user's session.

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: syedsajjadkazmii <[email protected]>

edx-pipeline-bot · 2024-01-29T11:25:31Z

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot · 2024-01-29T14:41:20Z

2U Release Notice: This PR has been deployed to the edX production environment.

syedsajjadkazmii commented Nov 30, 2023

View reviewed changes

openedx/core/djangoapps/user_authn/views/login.py Outdated Show resolved Hide resolved

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch 8 times, most recently from 6248964 to 1d02808 Compare December 1, 2023 09:19

robrap reviewed Dec 4, 2023

View reviewed changes

openedx/core/djangoapps/safe_sessions/middleware.py Outdated Show resolved Hide resolved

robrap reviewed Dec 5, 2023

View reviewed changes

robrap assigned syedsajjadkazmii Dec 5, 2023

robrap added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Dec 5, 2023

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch 7 times, most recently from 3bb06de to a86486b Compare December 7, 2023 11:24

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch from a86486b to e3f2c63 Compare December 20, 2023 12:45

robrap reviewed Dec 20, 2023

View reviewed changes

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch 5 times, most recently from fdecb1f to 8806707 Compare January 8, 2024 12:46

robrap reviewed Jan 8, 2024

View reviewed changes

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch from 99f30ff to 8115c06 Compare January 18, 2024 14:10

robrap reviewed Jan 18, 2024

View reviewed changes

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch 5 times, most recently from 1d32cec to 7f12636 Compare January 19, 2024 07:20

robrap reviewed Jan 19, 2024

View reviewed changes

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch 5 times, most recently from da7804b to 2b22484 Compare January 25, 2024 14:50

robrap approved these changes Jan 25, 2024

View reviewed changes

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch from 2b22484 to 323a9f8 Compare January 26, 2024 05:14

syedsajjadkazmii added 8 commits January 26, 2024 10:15

feat: logout other sessions on email change

619107d

fix: updated the approach for session invalidation

14fa1f5

fix: update and add tests

dca0a78

fix: update tests with descriptive comments

efcb9e3

feat: add integration tests

169147f

fix: store email in session update

093e8ed

fix: add setting for tests

eaeaa12

fix: fix tests

14e52bd

syedsajjadkazmii force-pushed the sajjad/VAN-1694 branch from 323a9f8 to 14e52bd Compare January 26, 2024 05:15

github-actions bot and others added 2 commits January 29, 2024 14:17

feat: Upgrade Python dependency edx-drf-extensions (#34135)

82dc03d

Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: syedsajjadkazmii <[email protected]>

Merge branch 'master' into sajjad/VAN-1694

f3c96e9

syedsajjadkazmii merged commit cb2a34e into master Jan 29, 2024
64 checks passed

syedsajjadkazmii deleted the sajjad/VAN-1694 branch January 29, 2024 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: logout other sessions on email change #33846

feat: logout other sessions on email change #33846

syedsajjadkazmii commented Nov 30, 2023

robrap left a comment

robrap left a comment

syedsajjadkazmii commented Dec 7, 2023

robrap commented Dec 11, 2023

robrap left a comment

robrap left a comment

robrap left a comment

robrap left a comment

robrap left a comment

robrap Jan 25, 2024

edx-pipeline-bot commented Jan 29, 2024

edx-pipeline-bot commented Jan 29, 2024

	# stored in the user's session.
	# yet stored in the user's session.

feat: logout other sessions on email change #33846

feat: logout other sessions on email change #33846

Conversation

syedsajjadkazmii commented Nov 30, 2023

robrap left a comment

Choose a reason for hiding this comment

robrap left a comment

Choose a reason for hiding this comment

syedsajjadkazmii commented Dec 7, 2023

robrap commented Dec 11, 2023

robrap left a comment

Choose a reason for hiding this comment

robrap left a comment

Choose a reason for hiding this comment

robrap left a comment

Choose a reason for hiding this comment

robrap left a comment

Choose a reason for hiding this comment

robrap left a comment

Choose a reason for hiding this comment

robrap Jan 25, 2024

Choose a reason for hiding this comment

edx-pipeline-bot commented Jan 29, 2024

edx-pipeline-bot commented Jan 29, 2024