Skip to content

Commit

Permalink
chore(auth) Remove require email verification feature (#77776)
Browse files Browse the repository at this point in the history
This behavior was added back in 2021 and has never been activated.
However, the serializer code if used will trigger SiloLimit errors and
not work. Instead of patching up an unused feature, removing it solves
the problem and makes maintenance easier in the future.

Fixes SENTRY-3F4E
  • Loading branch information
markstory authored Sep 24, 2024
1 parent 619a459 commit c0c1066
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 204 deletions.
24 changes: 0 additions & 24 deletions src/sentry/api/endpoints/organization_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,13 @@
OrganizationSlugCollisionException,
organization_provisioning_service,
)
from sentry.users.models.useremail import UserEmail
from sentry.users.services.user.serial import serialize_generic_user
from sentry.utils.audit import create_audit_entry

ERR_DEFAULT_ORG = "You cannot remove the default organization."
ERR_NO_USER = "This request requires an authenticated user."
ERR_NO_2FA = "Cannot require two-factor authentication without personal two-factor enabled."
ERR_SSO_ENABLED = "Cannot require two-factor authentication with SSO enabled"
ERR_EMAIL_VERIFICATION = "Cannot require email verification before verifying your email address."
ERR_3RD_PARTY_PUBLISHED_APP = "Cannot delete an organization that owns a published integration. Contact support if you need assistance."
ERR_PLAN_REQUIRED = "A paid plan is required to enable this feature."

Expand Down Expand Up @@ -273,7 +271,6 @@ class OrganizationSerializer(BaseOrganizationSerializer):
aggregatedDataConsent = serializers.BooleanField(required=False)
genAIConsent = serializers.BooleanField(required=False)
require2FA = serializers.BooleanField(required=False)
requireEmailVerification = serializers.BooleanField(required=False)
trustedRelays = serializers.ListField(child=TrustedRelaySerializer(), required=False)
allowJoinRequests = serializers.BooleanField(required=False)
relayPiiConfig = serializers.CharField(required=False, allow_blank=True, allow_null=True)
Expand Down Expand Up @@ -332,13 +329,6 @@ def validate_require2FA(self, value):
raise serializers.ValidationError(ERR_SSO_ENABLED)
return value

def validate_requireEmailVerification(self, value):
user = self.context["user"]
has_verified = UserEmail.objects.get_primary_email(user).is_verified
if value and not has_verified:
raise serializers.ValidationError(ERR_EMAIL_VERIFICATION)
return value

def validate_trustedRelays(self, value):
from sentry import features

Expand Down Expand Up @@ -439,8 +429,6 @@ def save_trusted_relays(self, incoming, changed_data, organization):
return incoming

def save(self):
from sentry import features

org = self.context["organization"]
changed_data = {}
if not hasattr(org, "__data"):
Expand Down Expand Up @@ -485,11 +473,6 @@ def save(self):
org.flags.codecov_access = data["codecovAccess"]
if "require2FA" in data:
org.flags.require_2fa = data["require2FA"]
if (
features.has("organizations:required-email-verification", org)
and "requireEmailVerification" in data
):
org.flags.require_email_verification = data["requireEmailVerification"]
if "allowMemberProjectCreation" in data:
org.flags.disable_member_project_creation = not data["allowMemberProjectCreation"]
if "allowSuperuserAccess" in data:
Expand Down Expand Up @@ -541,11 +524,6 @@ def save(self):
)
if data.get("require2FA") is True:
org.handle_2fa_required(self.context["request"])
if (
features.has("organizations:required-email-verification", org)
and data.get("requireEmailVerification") is True
):
org.handle_email_verification_required(self.context["request"])
return org, changed_data


Expand Down Expand Up @@ -600,7 +578,6 @@ def post_org_pending_deletion(
exclude_fields=[
"accountRateLimit",
"projectRateLimit",
"requireEmailVerification",
"apdexThreshold",
"genAIConsent",
"metricsActivatePercentiles",
Expand Down Expand Up @@ -809,7 +786,6 @@ class OrganizationDetailsPutSerializer(serializers.Serializer):
projectRateLimit = serializers.IntegerField(
min_value=PROJECT_RATE_LIMIT_DEFAULT, required=False
)
requireEmailVerification = serializers.BooleanField(required=False)
apdexThreshold = serializers.IntegerField(required=False)

# TODO: publish when GA'd
Expand Down
20 changes: 3 additions & 17 deletions src/sentry/api/invite_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.http.request import HttpRequest
from django.utils.crypto import constant_time_compare

from sentry import audit_log, features
from sentry import audit_log
from sentry.models.authidentity import AuthIdentity
from sentry.models.authprovider import AuthProvider
from sentry.organizations.services.organization import (
Expand All @@ -16,7 +16,6 @@
)
from sentry.signals import member_joined
from sentry.users.models.user import User
from sentry.users.models.useremail import UserEmail
from sentry.utils import metrics
from sentry.utils.audit import create_audit_entry

Expand Down Expand Up @@ -275,22 +274,9 @@ def _needs_2fa(self) -> bool:
not self.request.user.is_authenticated or not self.request.user.has_2fa()
)

def _needs_email_verification(self) -> bool:
organization = self.invite_context.organization
if not (
features.has("organizations:required-email-verification", organization)
and organization.flags.require_email_verification
):
return False

user = self.request.user
primary_email_is_verified = (
isinstance(user, User) and UserEmail.objects.get_primary_email(user).is_verified
)
return not primary_email_is_verified

def get_onboarding_steps(self) -> dict[str, bool]:
return {
"needs2fa": self._needs_2fa(),
"needsEmailVerification": self._needs_email_verification(),
# needs email verification is being removed
"needsEmailVerification": False,
}
14 changes: 4 additions & 10 deletions src/sentry/api/serializers/models/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,6 @@ def get_feature_set(
def serialize(
self, obj: Organization, attrs: Mapping[str, Any], user: User, **kwargs: Any
) -> OrganizationSerializerResponse:
from sentry import features

if attrs.get("avatar"):
avatar = {
"avatarType": attrs["avatar"].get_avatar_type_display(),
Expand All @@ -347,10 +345,8 @@ def serialize(
"dateCreated": obj.date_added,
"isEarlyAdopter": bool(obj.flags.early_adopter),
"require2FA": bool(obj.flags.require_2fa),
"requireEmailVerification": bool(
features.has("organizations:required-email-verification", obj)
and obj.flags.require_email_verification
),
# requireEmailVerification has been deprecated
"requireEmailVerification": False,
"avatar": avatar,
"allowMemberInvite": not obj.flags.disable_member_invite,
"allowMemberProjectCreation": not obj.flags.disable_member_project_creation,
Expand Down Expand Up @@ -531,10 +527,8 @@ def serialize( # type: ignore[explicit-override, override]
),
"openMembership": bool(obj.flags.allow_joinleave),
"require2FA": bool(obj.flags.require_2fa),
"requireEmailVerification": bool(
features.has("organizations:required-email-verification", obj)
and obj.flags.require_email_verification
),
# The requireEmailVerification feature has been removed, this field is deprecated.
"requireEmailVerification": False,
"allowSharedIssues": not obj.flags.disable_shared_issues,
"enhancedPrivacy": bool(obj.flags.enhanced_privacy),
"dataScrubber": bool(
Expand Down
1 change: 0 additions & 1 deletion src/sentry/features/temporary.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,6 @@ def register_temporary_features(manager: FeatureManager):
manager.add("organizations:replay-play-from-replay-tab", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Enable version 2 of reprocessing (completely distinct from v1)
manager.add("organizations:reprocessing-v2", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
manager.add("organizations:required-email-verification", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Enable resolve in upcoming release
# TODO(steve): Remove when we remove the feature from the UI
manager.add("organizations:resolve-in-upcoming-release", OrganizationFeature, FeatureHandlerStrategy.OPTIONS, api_expose=True)
Expand Down
12 changes: 2 additions & 10 deletions src/sentry/models/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from django.utils.functional import cached_property

from bitfield import TypedClassBitField
from sentry import features, roles
from sentry import roles
from sentry.app import env
from sentry.backup.dependencies import PrimaryKeyMap
from sentry.backup.helpers import ImportFlags
Expand Down Expand Up @@ -189,7 +189,7 @@ class flags(TypedClassBitField):
# Temporarily opt out of new visibility features and ui
disable_new_visibility_features: bool

# Require and enforce email verification for all members.
# Require and enforce email verification for all members. (deprecated, not in use)
require_email_verification: bool

# Enable codecov integration.
Expand Down Expand Up @@ -442,14 +442,6 @@ def handle_2fa_required(self, request):

self._handle_requirement_change(request, remove_2fa_non_compliant_members)

def handle_email_verification_required(self, request):
from sentry.tasks.auth import remove_email_verification_non_compliant_members

if features.has("organizations:required-email-verification", self):
self._handle_requirement_change(
request, remove_email_verification_non_compliant_members
)

@staticmethod
def get_url_viewname() -> str:
"""
Expand Down
66 changes: 1 addition & 65 deletions src/sentry/tasks/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@

from django.db import router
from django.db.models import F
from django.urls import reverse

from sentry import audit_log, features, options
from sentry import audit_log, options
from sentry.auth import manager
from sentry.auth.exceptions import ProviderNotRegistered
from sentry.models.organization import Organization
Expand All @@ -17,13 +16,10 @@
from sentry.silo.safety import unguarded_write
from sentry.tasks.base import instrumented_task, retry
from sentry.types.region import RegionMappingNotFound
from sentry.users.models.user import User
from sentry.users.models.useremail import UserEmail
from sentry.users.services.user import RpcUser
from sentry.users.services.user.service import user_service
from sentry.utils.audit import create_audit_entry_from_user
from sentry.utils.email import MessageBuilder
from sentry.utils.http import absolute_uri

logger = logging.getLogger("sentry.auth")

Expand Down Expand Up @@ -196,63 +192,3 @@ def remove_2fa_non_compliant_members(org_id, actor_id=None, actor_key_id=None, i
TwoFactorComplianceTask().remove_non_compliant_members(
org_id, actor_id, actor_key_id, ip_address
)


class VerifiedEmailComplianceTask(OrganizationComplianceTask):
log_label = "verified email"

def is_compliant(self, user: RpcUser) -> bool:
if user:
return UserEmail.objects.get_primary_email(user).is_verified
return False

def call_to_action(self, org: Organization, user: RpcUser, member: OrganizationMember):
import django.contrib.auth.models

if isinstance(user, django.contrib.auth.models.User):
# TODO(RyanSkonnord): Add test to repro this case (or delete check if unable)
logger.warning(
"Could not send verified email compliance notification (non-Sentry User model)"
)
return
elif not isinstance(user, User):
raise TypeError(user)

email = UserEmail.objects.get_primary_email(user)
email_context = {
"confirm_url": absolute_uri(
reverse("sentry-account-confirm-email", args=[user.id, email.validation_hash])
),
"invite_url": member.get_invite_link(),
"email": email.email,
"organization": org,
}
subject = "{} {} Mandatory: Verify Email Address".format(
options.get("mail.subject-prefix"), org.name.capitalize()
)
message = MessageBuilder(
subject=subject,
template="sentry/emails/setup_email.txt",
html_template="sentry/emails/setup_email.html",
type="user.setup_email",
context=email_context,
)
message.send_async([email])


@instrumented_task(
name="sentry.tasks.remove_email_verification_non_compliant_members",
queue="auth",
default_retry_delay=60 * 5,
max_retries=5,
silo_mode=SiloMode.REGION,
)
@retry
def remove_email_verification_non_compliant_members(
org_id, actor_id=None, actor_key_id=None, ip_address=None
):
org = Organization.objects.get_from_cache(id=org_id)
if features.has("organizations:required-email-verification", org):
VerifiedEmailComplianceTask().remove_non_compliant_members(
org_id, actor_id, actor_key_id, ip_address
)
23 changes: 0 additions & 23 deletions src/sentry/templates/sentry/emails/setup_email.html

This file was deleted.

13 changes: 0 additions & 13 deletions src/sentry/templates/sentry/emails/setup_email.txt

This file was deleted.

15 changes: 0 additions & 15 deletions static/app/data/forms/organizationSecurityAndPrivacyGroups.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,6 @@ const formGroups: JsonFormObject[] = [
),
},
},
{
name: 'requireEmailVerification',
type: 'boolean',
label: t('Require Email Verification'),
help: t('Require and enforce email address verification for all members'),
visible: ({features}) => features.has('required-email-verification'),
confirm: {
true: t(
'This will remove all members whose email addresses are not verified from your organization. It will also send them an email to verify their address and reinstate their access and settings. Do you want to continue?'
),
false: t(
'Are you sure you want to allow users to access your organization without verifying their email address?'
),
},
},
{
name: 'allowSharedIssues',
type: 'boolean',
Expand Down
Loading

0 comments on commit c0c1066

Please sign in to comment.