Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions apps/api/plane/app/urls/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@
UserEndpoint.as_view({"get": "retrieve_user_settings"}),
name="users",
),
path(
"users/me/email/generate-code/",
UserEndpoint.as_view({"post": "generate_email_verification_code"}),
name="user-email-verify-code",
),
path(
"users/me/email/",
UserEndpoint.as_view({"patch": "update_email"}),
name="user-email-update",
),
# Profile
path("users/me/profile/", ProfileEndpoint.as_view(), name="accounts"),
# End profile
Expand Down
189 changes: 186 additions & 3 deletions apps/api/plane/app/views/user/base.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
# Python imports
import uuid
import json
import logging
import random
import string
import secrets

# Django imports
from django.db.models import Case, Count, IntegerField, Q, When
from django.contrib.auth import logout
from django.utils import timezone
from django.utils.decorators import method_decorator
from django.views.decorators.cache import cache_control
from django.views.decorators.vary import vary_on_cookie
from django.core.validators import validate_email
from django.core.cache import cache

# Third party imports
from rest_framework import status
Expand Down Expand Up @@ -36,9 +46,11 @@
from plane.authentication.utils.host import user_ip
from plane.bgtasks.user_deactivation_email_task import user_deactivation_email
from plane.utils.host import base_host
from django.utils.decorators import method_decorator
from django.views.decorators.cache import cache_control
from django.views.decorators.vary import vary_on_cookie
from plane.bgtasks.user_email_update_task import send_email_update_magic_code, send_email_update_confirmation
from plane.authentication.rate_limit import EmailVerificationThrottle


logger = logging.getLogger("plane")


class UserEndpoint(BaseViewSet):
Expand All @@ -49,6 +61,14 @@ class UserEndpoint(BaseViewSet):
def get_object(self):
return self.request.user

def get_throttles(self):
"""
Apply rate limiting to specific endpoints.
"""
if self.action == "generate_email_verification_code":
return [EmailVerificationThrottle()]
return super().get_throttles()

@method_decorator(cache_control(private=True, max_age=12))
@method_decorator(vary_on_cookie)
def retrieve(self, request):
Expand All @@ -69,6 +89,169 @@ def retrieve_instance_admin(self, request):
def partial_update(self, request, *args, **kwargs):
return super().partial_update(request, *args, **kwargs)

def _validate_new_email(self, user, new_email):
"""
Validate the new email address.

Args:
user: The User instance
new_email: The new email address to validate

Returns:
Response object with error if validation fails, None if validation passes
"""
if not new_email:
return Response(
{"error": "Email is required"},
status=status.HTTP_400_BAD_REQUEST,
)

# Validate email format
try:
validate_email(new_email)
except Exception:
return Response(
{"error": "Invalid email format"},
status=status.HTTP_400_BAD_REQUEST,
)

# Check if email is the same as current email
if new_email == user.email:
Copy link

Choose a reason for hiding this comment

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

Bug: Email case sensitivity causes validation inconsistency

The comparison new_email == user.email compares a lowercased email against the user's stored email which may have mixed case. If a user's current email is "User@Example.com" and they enter "user@example.com", the validation incorrectly allows this as a "different" email, but the database save at line 235 will fail due to the unique constraint since emails are typically case-insensitive in practice. The comparison needs to normalize both sides: new_email == user.email.lower().

Fix in Cursor Fix in Web

return Response(
{"error": "New email must be different from current email"},
status=status.HTTP_400_BAD_REQUEST,
)

# Check if email already exists in the User model
if User.objects.filter(email=new_email).exclude(id=user.id).exists():
return Response(
{"error": "An account with this email already exists"},
status=status.HTTP_400_BAD_REQUEST,
)

return None

def generate_email_verification_code(self, request):
"""
Generate and send a magic code to the new email address for verification.
Rate limited to 3 requests per hour per user (enforced by EmailVerificationThrottle).
Additional per-email cooldown of 60 seconds prevents rapid repeated requests.
"""
user = self.get_object()
new_email = request.data.get("email", "").strip().lower()

# Validate the new email
validation_error = self._validate_new_email(user, new_email)
if validation_error:
return validation_error

try:
# Generate magic code for email verification
# Use a special key prefix to distinguish from regular magic signin
# Include user ID to bind the code to the specific user
cache_key = f"magic_email_update_{user.id}_{new_email}"
## Generate a random token
token = (
"".join(secrets.choice(string.ascii_lowercase) for _ in range(4))
+ "-"
+ "".join(secrets.choice(string.ascii_lowercase) for _ in range(4))
+ "-"
+ "".join(secrets.choice(string.ascii_lowercase) for _ in range(4))
)
# Store in cache with 10 minute expiration
cache_data = json.dumps({"token": token})
cache.set(cache_key, cache_data, timeout=600)

# Send magic code to the new email
send_email_update_magic_code.delay(new_email, token)

Copy link

Choose a reason for hiding this comment

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

Bug: Cache entry persists after email sending failure

If send_email_update_magic_code.delay(new_email, token) fails or the exception handler is triggered after cache.set() executes, the cache entry remains for 10 minutes even though no verification email was sent. Users cannot retry immediately because the cache key exists, but they never received the code. The cache should be cleaned up in the exception handler or the cache should only be set after successful email dispatch.

Fix in Cursor Fix in Web

return Response(
{"message": "Verification code sent to email"},
status=status.HTTP_200_OK,
)
except Exception as e:
logger.error("Failed to generate verification code: %s", str(e), exc_info=True)
return Response(
{"error": "Failed to generate verification code. Please try again."},
status=status.HTTP_400_BAD_REQUEST,
)

def update_email(self, request):
"""
Verify the magic code and update the user's email address.
This endpoint verifies the code and updates the existing user record
without creating a new user, ensuring the user ID remains unchanged.
"""
user = self.get_object()
new_email = request.data.get("email", "").strip().lower()
code = request.data.get("code", "").strip()

# Validate the new email
validation_error = self._validate_new_email(user, new_email)
if validation_error:
return validation_error

if not code:
return Response(
{"error": "Verification code is required"},
status=status.HTTP_400_BAD_REQUEST,
)

# Verify the magic code
try:
cache_key = f"magic_email_update_{user.id}_{new_email}"
cached_data = cache.get(cache_key)

if not cached_data:
logger.warning("Cache key not found: %s. Code may have expired or was never generated.", cache_key)
return Response(
{"error": "Verification code has expired or is invalid"},
status=status.HTTP_400_BAD_REQUEST,
)

data = json.loads(cached_data)
stored_token = data.get("token")

if str(stored_token) != str(code):
return Response(
{"error": "Invalid verification code"},
status=status.HTTP_400_BAD_REQUEST,
)

except Exception as e:
return Response(
{"error": "Failed to verify code. Please try again."},
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

The error message "Failed to verify code. Please try again." (line 216) is too generic and doesn't help users understand what went wrong. This could be returned for various technical failures (JSON parsing errors, Redis connection issues, etc.). Consider providing more specific error messages or at least logging the detailed error for administrators while showing a user-friendly message.

Suggested change
{"error": "Failed to verify code. Please try again."},
{"error": "A technical error occurred during code verification. Please try again later or contact support if the problem persists."},

Copilot uses AI. Check for mistakes.
status=status.HTTP_400_BAD_REQUEST,
)
Copy link

Choose a reason for hiding this comment

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

Bug: Exception swallows error details during code verification

The exception handler catches all exceptions during code verification but doesn't log the actual error or provide specific feedback. This makes debugging difficult when legitimate errors occur, such as JSON parsing failures or cache connection issues. The caught exception e is never logged or used.

Fix in Cursor Fix in Web


# Final check: ensure email is still available (might have been taken between code generation and update)
if User.objects.filter(email=new_email).exclude(id=user.id).exists():
return Response(
{"error": "An account with this email already exists"},
status=status.HTTP_400_BAD_REQUEST,
)
old_email = user.email
# Update the email - this updates the existing user record without creating a new user
user.email = new_email
# Reset email verification status when email is changed
user.is_email_verified = False
user.save()
Copy link

Choose a reason for hiding this comment

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

Bug: Race condition in email update causes database error

A race condition exists between checking email availability and saving the user. After the final User.objects.filter(email=new_email).exclude(id=user.id).exists() check at line 228, another user could claim the same email before user.save() executes at line 238. This creates a window where two concurrent requests could pass the validation check but one would fail with a database integrity error when attempting to save a duplicate email.

Fix in Cursor Fix in Web


# delete the cache
cache.delete(cache_key)

# Logout the user
logout(request)

# Send confirmation email to the new email address
send_email_update_confirmation.delay(new_email)
# send the email to the old email address
send_email_update_confirmation.delay(old_email)

# Return updated user data
serialized_data = UserMeSerializer(user).data
return Response(serialized_data, status=status.HTTP_200_OK)

def deactivate(self, request):
# Check all workspace user is active
user = self.get_object()
Expand Down
21 changes: 20 additions & 1 deletion apps/api/plane/authentication/rate_limit.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Third party imports
from rest_framework.throttling import AnonRateThrottle
from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
from rest_framework import status
from rest_framework.response import Response

Expand All @@ -22,3 +22,22 @@ def throttle_failure_view(self, request, *args, **kwargs):
)
except AuthenticationException as e:
return Response(e.get_error_dict(), status=status.HTTP_429_TOO_MANY_REQUESTS)


class EmailVerificationThrottle(UserRateThrottle):
"""
Throttle for email verification code generation.
Limits to 3 requests per hour per user to prevent abuse.
"""

rate = "3/hour"
scope = "email_verification"

def throttle_failure_view(self, request, *args, **kwargs):
try:
raise AuthenticationException(
error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"],
error_message="RATE_LIMIT_EXCEEDED",
)
except AuthenticationException as e:
return Response(e.get_error_dict(), status=status.HTTP_429_TOO_MANY_REQUESTS)
110 changes: 110 additions & 0 deletions apps/api/plane/bgtasks/user_email_update_task.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# Python imports
import logging

# Third party imports
from celery import shared_task

# Django imports
from django.core.mail import EmailMultiAlternatives, get_connection
from django.template.loader import render_to_string
from django.utils.html import strip_tags

# Module imports
from plane.license.utils.instance_value import get_email_configuration
from plane.utils.exception_logger import log_exception


@shared_task
def send_email_update_magic_code(email, token):
try:
(
EMAIL_HOST,
EMAIL_HOST_USER,
EMAIL_HOST_PASSWORD,
EMAIL_PORT,
EMAIL_USE_TLS,
EMAIL_USE_SSL,
EMAIL_FROM,
) = get_email_configuration()

# Send the mail
subject = "Verify your new email address"
context = {"code": token, "email": email}

html_content = render_to_string("emails/auth/magic_signin.html", context)
text_content = strip_tags(html_content)

connection = get_connection(
host=EMAIL_HOST,
port=int(EMAIL_PORT),
username=EMAIL_HOST_USER,
password=EMAIL_HOST_PASSWORD,
use_tls=EMAIL_USE_TLS == "1",
use_ssl=EMAIL_USE_SSL == "1",
)

msg = EmailMultiAlternatives(
subject=subject,
body=text_content,
from_email=EMAIL_FROM,
to=[email],
connection=connection,
)
msg.attach_alternative(html_content, "text/html")
msg.send()
logging.getLogger("plane.worker").info("Email sent successfully.")
return
except Exception as e:
log_exception(e)
return


@shared_task
def send_email_update_confirmation(email):
"""
Send a confirmation email to the user after their email address has been successfully updated.

Args:
email: The new email address that was successfully updated
"""
try:
(
EMAIL_HOST,
EMAIL_HOST_USER,
EMAIL_HOST_PASSWORD,
EMAIL_PORT,
EMAIL_USE_TLS,
EMAIL_USE_SSL,
EMAIL_FROM,
) = get_email_configuration()

# Send the confirmation email
subject = "Plane email address successfully updated"
context = {"email": email}

html_content = render_to_string("emails/user/email_updated.html", context)
text_content = strip_tags(html_content)

connection = get_connection(
host=EMAIL_HOST,
port=int(EMAIL_PORT),
username=EMAIL_HOST_USER,
password=EMAIL_HOST_PASSWORD,
use_tls=EMAIL_USE_TLS == "1",
use_ssl=EMAIL_USE_SSL == "1",
)

msg = EmailMultiAlternatives(
subject=subject,
body=text_content,
from_email=EMAIL_FROM,
to=[email],
connection=connection,
)
msg.attach_alternative(html_content, "text/html")
msg.send()
logging.getLogger("plane.worker").info(f"Email update confirmation sent successfully to {email}.")
return
except Exception as e:
log_exception(e)
return
Loading
Loading