feat(email): verified email-change flow by FrankApiyo · Pull Request #3155 · onaio/onadata

FrankApiyo · 2026-06-30T19:09:29Z

Changes / Features implemented

Verified (verify-before-apply) email change: an OTP is sent to the new address and the change is applied only on confirmation. Endpoints, throttling and expiry behavior are documented in docs/profiles.rst.

Steps taken to verify this change does what is intended

Unit tests: OTP TTL / attempt cap / hashing, request throttling, confirm applies + invalidates the cached profile, race-on-confirm and replay both rejected
Manual end-to-end from the SPA (request → OTP email → confirm)

Side effects of implementing this change

Schema change: new PendingEmailChange table, with a management command to purge expired rows

Before submitting this PR for review, please make sure you have:

Included tests
Updated documentation

Closes ONADATA-1411

kelvin-muchiri · 2026-07-09T08:22:50Z

@@ -0,0 +1,32 @@
+# Generated by Django 5.2.11 on 2026-06-29 10:32
+


Do we need a new table for this? I think all this info can be encoded and signed within the link we send to the user. Can something like this suffice?

class EmailChangeTokenGenerator(PasswordResetTokenGenerator): """Token generator for change in email link""" def _make_hash_value(self, user, timestamp): return ( six.text_type(user.pk) + six.text_type(timestamp) + six.text_type(user.email) )

But I now see we need a place to store the new email.

Yes, we need to keep the new email in the DB and also keep track of the number of attempts the user has taken to reset the email.

Also, I don't think it would be secure to allow users to have access to the hashed OTP in the email alongside the OTP.

linear · 2026-07-15T06:48:41Z

ONADATA-1411

kelvin-muchiri · 2026-07-15T07:38:21Z

+
+    def test_request_email_change_happy_path(self):
+        """Happy path: returns 200, creates PendingEmailChange, sends 1 email."""
+        cache.clear()  # start with a clean send-throttle counter


I think it would be better to have cache.clear in tearDown instead, example

kelvin-muchiri · 2026-07-15T07:39:23Z

+
+    def test_request_email_change_bad_password(self):
+        """Wrong password returns 400 with a 'password' error key."""
+        cache.clear()  # clean send-throttle counter


Same as above, let's move cache.clear to tearDown

kelvin-muchiri · 2026-07-15T07:41:13Z

+        )
+        response = view(request, user="bob")
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("password", response.data)


Can we assert more info in the response data other than just password?

kelvin-muchiri · 2026-07-15T07:41:38Z

+        )
+        response = view(request, user="bob")
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("new_email", response.data)


Can we assert more in the response data other than just new_email?

kelvin-muchiri · 2026-07-15T07:42:04Z

+
+    def test_request_email_change_throttled(self):
+        """OTP send is rate-limited so it can't spam the target address."""
+        from onadata.apps.api.viewsets.user_profile_viewset import (


Can we move this import to the top?

Instead of importing MAX_EMAIL_CHANGE_REQUESTS can we use override_settings decorator?

kelvin-muchiri · 2026-07-15T07:42:36Z

+            MAX_EMAIL_CHANGE_REQUESTS,
+        )
+
+        cache.clear()


Let's move cache.clear to tearDown

kelvin-muchiri · 2026-07-15T07:45:12Z

+        """The rate limit gates entry, not just the send: even calls that fail
+        the password check count against the budget, so the password/uniqueness
+        checks can't be exercised as unbounded oracles."""
+        from onadata.apps.api.viewsets.user_profile_viewset import (


Let's use override_settings instead of importing from the viewset

kelvin-muchiri · 2026-07-15T07:45:51Z

+            MAX_EMAIL_CHANGE_REQUESTS,
+        )
+
+        cache.clear()


Let's move cache.clear to tearDown

kelvin-muchiri · 2026-07-15T07:48:56Z

+        view = UserProfileViewSet.as_view({"post": "request_email_change"})
+
+        # Exhaust the budget with wrong-password calls (each returns 400).
+        for _ in range(MAX_EMAIL_CHANGE_REQUESTS):


This is ok but I think for this test we can just set the cache value directly to simulate exhausted retry limit

kelvin-muchiri · 2026-07-15T07:50:51Z

+        request = self.factory.post("/", data={"otp": "000000"}, **self.extra)
+        response = view_confirm(request, user="bob")
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("otp", response.data)


We can assert more info in the response data other than just "otp"

kelvin-muchiri · 2026-07-15T07:51:28Z

+        request = self.factory.post("/", data={"otp": "123456"}, **self.extra)
+        response = view_confirm(request, user="bob")
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("otp", response.data)


We can assert more info from the response data other just "otp"

kelvin-muchiri · 2026-07-15T07:54:31Z

+    def test_confirm_invalidates_profile_cache(self):
+        """The cached profile response must be cleared so the new email is
+        served immediately, not stale until the cache TTL expires."""
+        from onadata.libs.utils.cache_tools import (


Let's move the import to the top. I think to avoid coupling between implementation and tests, we can use the literal string for the key e.g cache_key = f"foo-{self.user.username}{self.user.username}" and use standard cache.get and cache.set

kelvin-muchiri · 2026-07-15T07:58:41Z

+    ttl_seconds = getattr(settings, "EMAIL_CHANGE_OTP_TTL_SECONDS", 300)
+    ttl_minutes = max(1, ttl_seconds // 60)
+    subject = "Verify your new email address"
+    body = (


Should we have a HTML template like the rest of the emails?

kelvin-muchiri · 2026-07-15T08:01:32Z

+User = get_user_model()
+
+
+class PendingEmailChangeTests(TestCase):


Could we add docstrings for the test methods?

kelvin-muchiri · 2026-07-15T08:05:14Z


        return Response(status=status.HTTP_200_OK, data=data)

+    @action(methods=["POST"], detail=True, url_path="request_email_change")


The standard we've used is hyphens instead of underscors for URLs i. e request-email-change over request_email_change

We also need to add documentation for the new endpoint

kelvin-muchiri · 2026-07-15T08:06:36Z

+
+        return Response({"sent": True}, status=status.HTTP_200_OK)
+
+    @action(methods=["POST"], detail=True, url_path="confirm_email_change")


The standard we've used is hyphens instead of underscors for URLs i. e confirm-email-change over confirm_email_change

We also need to add documentation for the new endpoint

kelvin-muchiri · 2026-07-15T08:08:23Z

+        request = self.factory.post("/", data={"otp": code}, **self.extra)
+        response = view_confirm(request, user="bob")
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("new_email", response.data)


Can we assert more from the response other than new_email?

Add a two-step, OTP-verified email change to the profiles API: - PendingEmailChange model keeps the pending address and a keyed hash of a short-lived 6-digit code, with attempt caps and a management command to purge expired rows - POST profiles/{user}/request-email-change validates the password and address uniqueness, then emails the code (HTML + text templates); rate-limited per user via a shared bump_attempts cache primitive - POST profiles/{user}/confirm-email-change verifies the code, applies the address, marks it verified and invalidates the cached profile - normalize emails and enforce case-insensitive uniqueness - document both endpoints in docs/profiles.rst

FrankApiyo mentioned this pull request Jun 30, 2026

feat(email): verified email-change with Keycloak sync (+ login-resilience fixes) #3153

Closed

4 tasks

github-code-quality Bot found potential problems Jun 30, 2026

View reviewed changes

Comment thread onadata/apps/main/models/password_history.py Fixed

FrankApiyo force-pushed the feat/verified-email-change branch from f3822cd to a3a9322 Compare July 2, 2026 06:39

FrankApiyo requested review from kelvin-muchiri and ukanga July 7, 2026 11:35

kelvin-muchiri reviewed Jul 9, 2026

View reviewed changes

FrankApiyo requested a review from kelvin-muchiri July 15, 2026 05:20

This was referenced Jul 15, 2026

fix(auth): per-user PasswordHistory uniqueness (login lockout) #3173

Merged

feat(auth): graceful CSRF-failure retry on login #3174

Open

FrankApiyo changed the title ~~feat(email): verified email-change flow (+ login-resilience fixes)~~ feat(email): verified email-change flow Jul 15, 2026

FrankApiyo mentioned this pull request Jul 15, 2026

chore(deps): bump ona-oidc for the account proxy (blocked on ona-oidc#124) #3176

Closed

2 tasks

FrankApiyo mentioned this pull request Jul 15, 2026

chore(migrations): normalize autodetected MetaData index name #3177

Open

2 tasks

kelvin-muchiri reviewed Jul 15, 2026

View reviewed changes

FrankApiyo force-pushed the feat/verified-email-change branch from 673ce0a to 2a3ea7f Compare July 15, 2026 08:27

FrankApiyo force-pushed the feat/verified-email-change branch from 2a3ea7f to 908922b Compare July 15, 2026 08:31

FrankApiyo requested a review from kelvin-muchiri July 15, 2026 08:33

		@@ -0,0 +1,32 @@
		# Generated by Django 5.2.11 on 2026-06-29 10:32

		User = get_user_model()


		class PendingEmailChangeTests(TestCase):


		return Response(status=status.HTTP_200_OK, data=data)

		@action(methods=["POST"], detail=True, url_path="request_email_change")


		return Response({"sent": True}, status=status.HTTP_200_OK)

		@action(methods=["POST"], detail=True, url_path="confirm_email_change")

Uh oh!

Conversation

FrankApiyo commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes / Features implemented

Steps taken to verify this change does what is intended

Side effects of implementing this change

Uh oh!

Uh oh!

kelvin-muchiri Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kelvin-muchiri Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear Bot commented Jul 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kelvin-muchiri Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FrankApiyo commented Jun 30, 2026 •

edited

Loading

kelvin-muchiri Jul 9, 2026 •

edited

Loading

kelvin-muchiri Jul 9, 2026 •

edited

Loading

kelvin-muchiri Jul 15, 2026 •

edited

Loading