Security improvements for email tokens

[hlbmtc]

https://www.notion.so/metaculus/Expire-Password-Reset-Links-on-Email-Change-f1613821497f4e33bbbe503b9a059dad?v=2f76aaf4f1018099baa0000cdc0a6471&source=copy_link

Summary by CodeRabbit

• Tests

  • Added comprehensive unit tests for token flows: email-change and password-reset success, expiry/invalid/mismatch cases, multi-token and cross-token invalidation, and login-triggered invalidation.

• Refactor

  • Redesigned email-change token generation/validation for more robust, signed tokens and unified error handling to improve reliability.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new unit test module exercising token lifecycles and refactors EmailChangeTokenGenerator to inherit from Django's PasswordResetTokenGenerator, introducing signed token creation and validation methods used by email-change flows.

Changes

Cohort / File(s)	Summary
Token Invalidation Tests tests/unit/test_auth/test_token_invalidation.py	New comprehensive unit tests covering email-change and password-reset token generation, validation, expiration, malformed tokens, user/token mismatches, email-in-use races, multi-token invalidation, cross-invalidation between token types, and login-driven invalidation.
Token Generation Refactor users/services/common.py	EmailChangeTokenGenerator now subclasses PasswordResetTokenGenerator; adds key_salt, make_token(user, new_email) (signed payload) and check_token(user, token, max_age) (verify signer + embedded Django token); generate_email_change_token and change_email_from_token updated to use these methods and raise ValidationError on invalid/expired tokens.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Client
participant Service as EmailChangeService
participant Signer as TimestampSigner
participant DjangoToken as PasswordResetTokenGenerator
participant DB as UserStore

Client->>Service: request generate token(user, new_email)
Service->>DjangoToken: generate validation_token(user)
DjangoToken-->>Service: validation_token
Service->>Signer: sign {user_id, new_email, validation_token}
Signer-->>Client: token

Client->>Service: submit token to change email
Service->>Signer: unsign and validate age
Signer-->>Service: payload {user_id, new_email, validation_token}
Service->>DB: fetch user by id
DB-->>Service: user
Service->>DjangoToken: check_token(user, validation_token)
DjangoToken-->>Service: valid/invalid
Service->>DB: update user.email (if valid)
DB-->>Service: updated user
Service-->>Client: success / ValidationError

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Global tokens invalidation #4199 — Modifies users/services/common.py and touches email-change/password-reset token handling, directly related to the refactor here.

Suggested reviewers

• elisescu
• cemreinanc
• lsabor
• ncarazon

Poem

🐇 I signed the token, sealed with care,
A tiny hop through code and air.
Emails switch and passwords mend,
Two tokens race — one wins in the end.
🥕 Happy hops for tests well-run, hooray!

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Security improvements for email tokens' directly aligns with the main changes: enhanced email token validation, expiration handling, and cross-invalidation logic across token types.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/email-token-improvements

---

🧹 Recent nitpick comments

users/services/common.py (2)

109-133: Avoid colon-delimited payload parsing.
split(":", 2) will break if new_email ever contains : (or if you add fields later). A structured payload avoids delimiter collisions and is more extensible.

♻️ Suggested refactor (JSON payload)

+import json
@@
-        payload = f"{user.id}:{new_email}:{validation_token}"
-        return self.signer.sign(payload)
+        payload = json.dumps(
+            {"uid": user.pk, "new_email": new_email, "token": validation_token},
+            separators=(",", ":"),
+        )
+        return self.signer.sign(payload)
@@
-            user_id, new_email, validation_token = payload.split(":", 2)
-
-            if int(user_id) != user.pk:
-                return None
+            try:
+                data = json.loads(payload)
+            except json.JSONDecodeError:
+                return None
+
+            if str(data.get("uid")) != str(user.pk):
+                return None
+
+            new_email = data.get("new_email")
+            validation_token = data.get("token")
+            if not new_email or not validation_token:
+                return None

---

103-108: Use the defined salt when instantiating TimestampSigner.
The key_salt constant is defined but not passed to TimestampSigner(). Without a salt parameter, tokens signed by this generator use Django's default (effectively SECRET_KEY only), which could allow cross-context token reuse if other parts of the system also use TimestampSigner without a salt. Scoping the signer with key_salt prevents token reuse across different purposes and aligns with Django's namespacing best practice.

🔧 Suggested change

-        self.signer = TimestampSigner()
+        self.signer = TimestampSigner(salt=self.key_salt)

Note: The same pattern exists in authentication/services.py (SignupInviteService); consider applying this fix consistently across the codebase.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 831c1d4 and a9f6ce6.

📒 Files selected for processing (1)

• users/services/common.py

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-01-15T19:29:58.940Z

Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4075
File: authentication/urls.py:24-26
Timestamp: 2026-01-15T19:29:58.940Z
Learning: In this codebase, DRF is configured to use IsAuthenticated as the default in REST_FRAMEWORK['DEFAULT_PERMISSION_CLASSES'] within metaculus_web/settings.py. Therefore, explicit permission_classes([IsAuthenticated]) decorators are unnecessary on DRF views unless a view needs to override the default. When reviewing Python files, verify that views relying on the default are not redundantly decorated, and flag cases where permissions are being over-specified or when a non-default permission is explicitly required.

Applied to files:

• users/services/common.py

🪛 Ruff (0.14.14)

users/services/common.py

[warning] 132-132: Consider moving this statement to an else block

(TRY300)

---

[warning] 139-139: Avoid specifying long messages outside the exception class

(TRY003)

---

[warning] 147-147: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build Docker Image / Build Docker Image
• GitHub Check: integration-tests
• GitHub Check: Frontend Checks
• GitHub Check: Backend Checks

🔇 Additional comments (1)

users/services/common.py (1)

137-147: LGTM on integrating the new token generator into the email-change flow.
Centralized validation + consistent error handling looks good.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🚀 Preview Environment

Your preview environment is ready!

Resource	Details
🌐 Preview URL	https://metaculus-pr-4202-feat-email-token-improvements-preview.mtcl.cc
📦 Docker Image	ghcr.io/metaculus/metaculus:feat-email-token-improvements-a9f6ce6
🗄️ PostgreSQL	NeonDB branch preview/pr-4202-feat-email-token-improvements
⚡ Redis	Fly Redis mtc-redis-pr-4202-feat-email-token-improvements

Details

• Commit: 8d136c4d91de5c4774744c81404d368e922e5906
• Branch: feat/email-token-improvements
• Fly App: metaculus-pr-4202-feat-email-token-improvements

---

ℹ️ Preview Environment Info

Isolation:

• PostgreSQL and Redis are fully isolated from production
• Each PR gets its own database branch and Redis instance
• Changes pushed to this PR will trigger a new deployment

Limitations:

• Background workers and cron jobs are not deployed in preview environments
• If you need to test background jobs, use Heroku staging environments

Cleanup:

• This preview will be automatically destroyed when the PR is closed