Skip to content

chore(auth): Clean up identity linking quick inc fix #103215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 12, 2025
Merged

chore(auth): Clean up identity linking quick inc fix #103215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 0 additions & 7 deletions src/sentry/options/defaults.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3520,13 +3520,6 @@
flags=FLAG_AUTOMATOR_MODIFIABLE,
)

# Killswitch for linking identities for demo users
register(
"identity.prevent-link-identity-for-demo-users.enabled",
type=Bool,
default=False,
flags=FLAG_AUTOMATOR_MODIFIABLE,
)

register(
"sentry.send_onboarding_task_metrics",
Expand Down
46 changes: 1 addition & 45 deletions src/sentry/users/models/identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.db.models import Q, QuerySet
from django.utils import timezone

from sentry import analytics, options
from sentry import analytics
from sentry.backup.scopes import RelocationScope
from sentry.db.models import (
BoundedPositiveIntegerField,
Expand All @@ -19,7 +19,6 @@
control_silo_model,
)
from sentry.db.models.manager.base import BaseManager
from sentry.demo_mode.utils import is_demo_user
from sentry.integrations.types import ExternalProviders, IntegrationProviderSlug
from sentry.users.services.user import RpcUser

Expand Down Expand Up @@ -90,17 +89,6 @@ def link_identity(
the case where the user is linked to a different identity or the
identity is linked to a different user.
"""
# NOTE(vgrozdanic): temporary fix for #inc-1373 to stop the bleed
if is_demo_user(user) and options.get(
"identity.prevent-link-identity-for-demo-users.enabled"
):
logger.warning(
"Preventing link identity for demo user",
extra={"user_id": user.id, "idp_id": idp.id, "external_id": external_id},
stack_info=True,
)
return None

from sentry.integrations.slack.analytics import SlackIntegrationIdentityLinked

defaults = {
Comment on lines 89 to 94
Copy link

@sentry sentry bot Nov 12, 2025

Choose a reason for hiding this comment

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

Bug: Demo users can link identities via non-SSO paths due to insufficient is_demo_user checks.
Severity: CRITICAL | Confidence: 0.95

🔍 Detailed Analysis

The removal of the identity.prevent-link-identity-for-demo-users.enabled check allows demo users to link identities through non-SSO code paths. The new check in _handle_membership() only protects the SSO auth flow, leaving Identity Pipeline and Integration Pipeline vulnerable. These pipelines directly call Identity.objects.link_identity() or Identity.objects.update_external_id_and_defaults() without checking if the user is a demo user, potentially reintroducing the leak mentioned in issue #inc-1373.

💡 Suggested Fix

Re-implement a comprehensive is_demo_user check across all identity linking code paths, specifically in Identity Pipeline and Integration Pipeline, to prevent demo users from linking identities.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/users/models/identity.py#L89-L94

Potential issue: The removal of the
`identity.prevent-link-identity-for-demo-users.enabled` check allows demo users to link
identities through non-SSO code paths. The new check in `_handle_membership()` only
protects the SSO auth flow, leaving `Identity Pipeline` and `Integration Pipeline`
vulnerable. These pipelines directly call `Identity.objects.link_identity()` or
`Identity.objects.update_external_id_and_defaults()` without checking if the user is a
demo user, potentially reintroducing the leak mentioned in [issue
#inc-1373](https://sentry.sentry.io/issues/inc-1373).

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down Expand Up @@ -146,17 +134,6 @@ def create_identity(
user: User | RpcUser,
defaults: Mapping[str, Any],
) -> Identity | None:
# NOTE(vgrozdanic): temporary fix for #inc-1373 to stop the bleed
if is_demo_user(user) and options.get(
"identity.prevent-link-identity-for-demo-users.enabled"
):
logger.warning(
"Preventing creating identity for demo user",
extra={"user_id": user.id, "idp_id": idp.id, "external_id": external_id},
stack_info=True,
)
return None

identity_model = self.create(
idp_id=idp.id, user_id=user.id, external_id=external_id, **defaults
)
Expand All @@ -182,16 +159,6 @@ def reattach(
Removes identities under `idp` associated with either `external_id` or `user`
and creates a new identity linking them.
"""
if is_demo_user(user) and options.get(
"identity.prevent-link-identity-for-demo-users.enabled"
):
logger.warning(
"Preventing reattaching identity for demo user",
extra={"user_id": user.id, "idp_id": idp.id, "external_id": external_id},
stack_info=True,
)
return None

self.delete_identity(user=user, idp=idp, external_id=external_id)
return self.create_identity(user=user, idp=idp, external_id=external_id, defaults=defaults)

Expand All @@ -206,17 +173,6 @@ def update_external_id_and_defaults(
Updates the identity object for a given user and identity provider
with the new external id and other fields related to the identity status
"""
# NOTE(vgrozdanic): temporary fix for #inc-1373 to stop the bleed
if is_demo_user(user) and options.get(
"identity.prevent-link-identity-for-demo-users.enabled"
):
logger.warning(
"Preventing updating identity for demo user",
extra={"user_id": user.id, "idp_id": idp.id, "external_id": external_id},
stack_info=True,
)
return None

query = self.filter(user_id=user.id, idp=idp)
query.update(external_id=external_id, **defaults)
identity_model = query.get()
Expand Down
29 changes: 0 additions & 29 deletions tests/sentry/users/models/test_identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
from sentry.identity import register
from sentry.identity.providers.dummy import DummyProvider
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers import override_options
from sentry.testutils.silo import control_silo_test
from sentry.users.models.identity import Identity


@control_silo_test
Expand All @@ -21,30 +19,3 @@ def test_get_provider(self) -> None:
provider = identity_model.get_provider()
assert provider.name == "Dummy"
assert provider.key == "dummy"

def test_link_identity_for_demo_users(self) -> None:
user = self.create_user()
idp = self.create_identity_provider(type="dummy", external_id="1234")

# if the setting is disabled, the identity should be linked even for demo users
with override_options({"demo-mode.enabled": True, "demo-mode.users": [user.id]}):
assert Identity.objects.link_identity(user, idp, "1234") is not None

# if the setting is enabled, the identity should not be linked for demo users
with override_options(
{
"identity.prevent-link-identity-for-demo-users.enabled": True,
"demo-mode.enabled": True,
"demo-mode.users": [user.id],
}
):
assert Identity.objects.link_identity(user, idp, "1234") is None

# if the setting is enabled, the identity should still be linked for non-demo users
with override_options(
{
"identity.prevent-link-identity-for-demo-users.enabled": True,
"demo-mode.enabled": True,
}
):
assert Identity.objects.link_identity(user, idp, "1234") is not None
Loading