From 8e7a5f18c7cd4f7ab6c75682aeba9b83fedbe88d Mon Sep 17 00:00:00 2001 From: sohailfatima <23100065@lums.edu.pk> Date: Tue, 21 May 2024 08:06:11 +0500 Subject: [PATCH 1/3] feat: added tracking events for password reset initiation --- .../management/commands/recover_account.py | 11 +++++++++++ .../core/djangoapps/user_authn/views/login.py | 10 ++++++++++ .../user_authn/views/password_reset.py | 17 +++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/common/djangoapps/student/management/commands/recover_account.py b/common/djangoapps/student/management/commands/recover_account.py index 78e45cdbb3f6..1cb1249ff65e 100644 --- a/common/djangoapps/student/management/commands/recover_account.py +++ b/common/djangoapps/student/management/commands/recover_account.py @@ -16,6 +16,7 @@ from django.utils.http import int_to_base36 from edx_ace import ace from edx_ace.recipient import Recipient +from eventtracking import tracker from common.djangoapps.student.models import AccountRecoveryConfiguration from openedx.core.djangoapps.user_authn.toggles import should_redirect_to_authn_microfrontend @@ -27,6 +28,7 @@ from openedx.core.lib.celery.task_utils import emulate_http_request logger = logging.getLogger(__name__) # pylint: disable=invalid-name +PASSWORD_RESET_INITIATED = 'edx.user.passwordreset.initiated' class Command(BaseCommand): @@ -84,6 +86,15 @@ def handle(self, *args, **options): user = get_user_model().objects.get(Q(username__iexact=username) | Q(email__iexact=current_email)) user.email = desired_email user.save() + tracker.emit( + PASSWORD_RESET_INITIATED, + { + "email": user.email, + "user_id": user.id, + "old_email": current_email, + "source": "Account Recovery Management Command", + } + ) self.send_password_reset_email(user, site) successful_updates.append(desired_email) except Exception as exc: # pylint: disable=broad-except diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 7d049871266e..6c4eaf0bbddd 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -24,6 +24,7 @@ from django.views.decorators.http import require_http_methods from django_ratelimit.decorators import ratelimit from edx_django_utils.monitoring import set_custom_attribute +from eventtracking import tracker from openedx_events.learning.data import UserData, UserPersonalData from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED from openedx_filters.learning.filters import StudentLoginRequested @@ -61,6 +62,7 @@ log = logging.getLogger("edx.student") AUDIT_LOG = logging.getLogger("audit") USER_MODEL = get_user_model() +PASSWORD_RESET_INITIATED = 'edx.user.passwordreset.initiated' def _do_third_party_auth(request): @@ -194,6 +196,14 @@ def _enforce_password_policy_compliance(request, user): # lint-amnesty, pylint: LoginFailures.increment_lockout_counter(user) AUDIT_LOG.info("Password reset initiated for email %s.", user.email) + tracker.emit( + PASSWORD_RESET_INITIATED, + { + "email": user.email, + "user_id": user.id, + "source": "Policy Compliance", + } + ) send_password_reset_email_for_user(user, request) # Prevent the login attempt. diff --git a/openedx/core/djangoapps/user_authn/views/password_reset.py b/openedx/core/djangoapps/user_authn/views/password_reset.py index f8049d4d914c..57bde03c4544 100644 --- a/openedx/core/djangoapps/user_authn/views/password_reset.py +++ b/openedx/core/djangoapps/user_authn/views/password_reset.py @@ -52,6 +52,7 @@ POST_EMAIL_KEY = 'openedx.core.djangoapps.util.ratelimit.request_post_email' REAL_IP_KEY = 'openedx.core.djangoapps.util.ratelimit.real_ip' SETTING_CHANGE_INITIATED = 'edx.user.settings.change_initiated' +PASSWORD_RESET_INITIATED = 'edx.user.passwordreset.initiated' # Maintaining this naming for backwards compatibility. log = logging.getLogger("edx.student") @@ -289,6 +290,14 @@ def password_reset(request): user = request.user # Prefer logged-in user's email email = user.email if user.is_authenticated else request.POST.get('email') + tracker.emit( + PASSWORD_RESET_INITIATED, + { + "email": email, + "user_id": user.id, + "source": "Logistration Page", + } + ) AUDIT_LOG.info("Password reset initiated for email %s.", email) if getattr(request, 'limited', False): @@ -608,6 +617,14 @@ def password_change_request_handler(request): # Prefer logged-in user's email email = user.email if user.is_authenticated else request.POST.get('email') AUDIT_LOG.info("Password reset initiated for email %s.", email) + tracker.emit( + PASSWORD_RESET_INITIATED, + { + "email": email, + "user_id": user.id, + "source": "Account API", + } + ) if getattr(request, 'limited', False) and not request_from_support_tools: AUDIT_LOG.warning("Password reset rate limit exceeded for email %s.", email) From 582b41694f59c49be4ef2117d7f0a696317ca32b Mon Sep 17 00:00:00 2001 From: sohailfatima <23100065@lums.edu.pk> Date: Tue, 21 May 2024 14:44:52 +0500 Subject: [PATCH 2/3] fix: failing password reset tests --- .../user_authn/views/tests/test_reset_password.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py index 7e7202750465..b89b458ed1ae 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py @@ -33,7 +33,7 @@ from openedx.core.djangoapps.user_api.tests.test_views import UserAPITestCase from openedx.core.djangoapps.user_api.accounts import EMAIL_MAX_LENGTH, EMAIL_MIN_LENGTH from openedx.core.djangoapps.user_authn.views.password_reset import ( - SETTING_CHANGE_INITIATED, password_reset, LogistrationPasswordResetView, + SETTING_CHANGE_INITIATED, PASSWORD_RESET_INITIATED, password_reset, LogistrationPasswordResetView, PasswordResetConfirmWrapper, password_change_request_handler) from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from common.djangoapps.student.tests.factories import TEST_PASSWORD, UserFactory @@ -114,7 +114,7 @@ def test_user_bad_password_reset(self): assert bad_pwd_resp.status_code == 200 obj = json.loads(bad_pwd_resp.content.decode('utf-8')) assert obj == {'success': True, 'value': "('registration/password_reset_done.html', [])"} - self.assert_no_events_were_emitted() + self.assert_event_emission_count(PASSWORD_RESET_INITIATED, 1) @patch( 'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string', @@ -134,7 +134,7 @@ def test_nonexist_email_password_reset(self): assert bad_email_resp.status_code == 200 obj = json.loads(bad_email_resp.content.decode('utf-8')) assert obj == {'success': True, 'value': "('registration/password_reset_done.html', [])"} - self.assert_no_events_were_emitted() + self.assert_event_emission_count(PASSWORD_RESET_INITIATED, 1) @patch( 'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string', @@ -146,7 +146,6 @@ def test_password_reset_ratelimited_for_non_existing_user(self): for non-existing user. """ self.assert_password_reset_ratelimitted('thisdoesnotexist@foo.com', AnonymousUser()) - self.assert_no_events_were_emitted() @patch( 'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string', From 17f48adb49c6d3d72c89078d2840a8f3b5cd1e2e Mon Sep 17 00:00:00 2001 From: sohailfatima <23100065@lums.edu.pk> Date: Mon, 27 May 2024 15:15:48 +0500 Subject: [PATCH 3/3] feat: remove unnecessary PII --- .../djangoapps/student/management/commands/recover_account.py | 2 -- openedx/core/djangoapps/user_authn/views/login.py | 1 - openedx/core/djangoapps/user_authn/views/password_reset.py | 2 -- 3 files changed, 5 deletions(-) diff --git a/common/djangoapps/student/management/commands/recover_account.py b/common/djangoapps/student/management/commands/recover_account.py index 1cb1249ff65e..7b156abdc510 100644 --- a/common/djangoapps/student/management/commands/recover_account.py +++ b/common/djangoapps/student/management/commands/recover_account.py @@ -89,9 +89,7 @@ def handle(self, *args, **options): tracker.emit( PASSWORD_RESET_INITIATED, { - "email": user.email, "user_id": user.id, - "old_email": current_email, "source": "Account Recovery Management Command", } ) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 6c4eaf0bbddd..2ad3c0ff18a0 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -199,7 +199,6 @@ def _enforce_password_policy_compliance(request, user): # lint-amnesty, pylint: tracker.emit( PASSWORD_RESET_INITIATED, { - "email": user.email, "user_id": user.id, "source": "Policy Compliance", } diff --git a/openedx/core/djangoapps/user_authn/views/password_reset.py b/openedx/core/djangoapps/user_authn/views/password_reset.py index 57bde03c4544..321101fb9a18 100644 --- a/openedx/core/djangoapps/user_authn/views/password_reset.py +++ b/openedx/core/djangoapps/user_authn/views/password_reset.py @@ -293,7 +293,6 @@ def password_reset(request): tracker.emit( PASSWORD_RESET_INITIATED, { - "email": email, "user_id": user.id, "source": "Logistration Page", } @@ -620,7 +619,6 @@ def password_change_request_handler(request): tracker.emit( PASSWORD_RESET_INITIATED, { - "email": email, "user_id": user.id, "source": "Account API", }