From 1c2f6ae42d42b37c1a87f95944a1d5ae74218e20 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 13:38:32 +0100 Subject: [PATCH 01/37] feat(replay): add model to allow per-user access control for replays --- .../api/serializers/models/organization.py | 27 +++++- .../core/endpoints/organization_details.py | 84 +++++++++++++++++++ .../1012_organizationmember_replay_access.py | 64 ++++++++++++++ src/sentry/models/__init__.py | 1 + .../models/organizationmemberreplayaccess.py | 34 ++++++++ src/sentry/testutils/helpers/backups.py | 8 ++ 6 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 src/sentry/migrations/1012_organizationmember_replay_access.py create mode 100644 src/sentry/models/organizationmemberreplayaccess.py diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 6be0c32f1eca40..b124cb2e6ed79c 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -76,6 +76,7 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationaccessrequest import OrganizationAccessRequest +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationonboardingtask import OrganizationOnboardingTask from sentry.models.project import Project from sentry.models.team import Team, TeamStatus @@ -87,7 +88,10 @@ if TYPE_CHECKING: from sentry.api.serializers.models.project import OrganizationProjectResponse - from sentry.users.api.serializers.user import UserSerializerResponse, UserSerializerResponseSelf + from sentry.users.api.serializers.user import ( + UserSerializerResponse, + UserSerializerResponseSelf, + ) # This cut-off date ensures that only new organizations created after this date go # through the logic that checks for the 'onboarding:complete' key in OrganizationOption. @@ -563,6 +567,7 @@ class DetailedOrganizationSerializerResponse(_DetailedOrganizationSerializerResp autoEnableCodeReview: bool autoOpenPrs: bool defaultCodeReviewTriggers: list[str] + replayAccessMembers: list[int] class DetailedOrganizationSerializer(OrganizationSerializer): @@ -747,6 +752,26 @@ def serialize( # type: ignore[override] "isDynamicallySampled": is_dynamically_sampled, } + if features.has("organizations:granular-replay-permissions", obj): + # The organization option can be missing, False, or null. + # Only set hasGranularReplayPermissions to True if the option exists and is truthy. + permissions_enabled = ( + OrganizationOption.objects.filter( + organization=obj, key="sentry:granular-replay-permissions" + ) + .values_list("value", flat=True) + .first() + ) + context["hasGranularReplayPermissions"] = bool(permissions_enabled) + context["replayAccessMembers"] = list( + OrganizationMemberReplayAccess.objects.filter(organization=obj).values_list( + "organizationmember_id", flat=True + ) + ) + else: + context["hasGranularReplayPermissions"] = False + context["replayAccessMembers"] = [] + if has_custom_dynamic_sampling(obj, actor=user): context["targetSampleRate"] = float( obj.get_option("sentry:target_sample_rate", TARGET_SAMPLE_RATE_DEFAULT) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index e9541d8f22bbc3..9f203a614096b3 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -94,6 +94,7 @@ from sentry.models.options.organization_option import OrganizationOption from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.project import Project from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import ( @@ -369,6 +370,12 @@ class OrganizationSerializer(BaseOrganizationSerializer): ingestThroughTrustedRelaysOnly = serializers.ChoiceField( choices=[("enabled", "enabled"), ("disabled", "disabled")], required=False ) + replayAccessMembers = serializers.ListField( + child=serializers.IntegerField(), + required=False, + allow_null=True, + help_text="List of organization member IDs that have access to replay data. Only modifiable by owners and managers.", + ) def _has_sso_enabled(self): org = self.context["organization"] @@ -589,6 +596,73 @@ def save(self, **kwargs): if trusted_relay_info is not None: self.save_trusted_relays(trusted_relay_info, changed_data, org) + if "hasGranularReplayPermissions" in data or "replayAccessMembers" in data: + if not features.has("organizations:granular-replay-permissions", org): + raise serializers.ValidationError( + { + "hasGranularReplayPermissions": "This feature is not enabled for your organization." + } + ) + + if not self.context["request"].access.has_scope("org:admin"): + raise serializers.ValidationError( + { + "hasGranularReplayPermissions": "You do not have permission to modify granular replay permissions." + } + ) + + if "hasGranularReplayPermissions" in data: + option_key = "sentry:granular-replay-permissions" + new_value = data["hasGranularReplayPermissions"] + + option_inst, created = OrganizationOption.objects.update_or_create( + organization=org, key=option_key, defaults={"value": new_value} + ) + + if new_value or created: + changed_data["hasGranularReplayPermissions"] = f"to {new_value}" + + if "replayAccessMembers" in data: + member_ids = data["replayAccessMembers"] + + if member_ids is None: + member_ids = [] + + current_member_ids = set( + OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( + "organizationmember_id", flat=True + ) + ) + new_member_ids = set(member_ids) + + to_add = new_member_ids - current_member_ids + to_remove = current_member_ids - new_member_ids + + if to_add: + OrganizationMemberReplayAccess.objects.bulk_create( + [ + OrganizationMemberReplayAccess( + organization=org, organizationmember_id=member_id + ) + for member_id in to_add + ] + ) + + if to_remove: + OrganizationMemberReplayAccess.objects.filter( + organization=org, organizationmember_id__in=to_remove + ).delete() + + if to_add or to_remove: + changes = [] + if to_add: + changes.append(f"added {len(to_add)} member(s)") + if to_remove: + changes.append(f"removed {len(to_remove)} member(s)") + changed_data["replayAccessMembers"] = ( + f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" + ) + if "openMembership" in data: org.flags.allow_joinleave = data["openMembership"] if "allowSharedIssues" in data: @@ -809,6 +883,16 @@ class OrganizationDetailsPutSerializer(serializers.Serializer): help_text="The role required to download debug information files, ProGuard mappings and source maps.", required=False, ) + hasGranularReplayPermissions = serializers.BooleanField( + help_text="Specify `true` to enable granular replay permissions, allowing per-member access control for replay data.", + required=False, + ) + replayAccessMembers = serializers.ListField( + child=serializers.IntegerField(), + help_text="A list of organization member IDs who have permission to access replay data. When empty, all members have access. Requires the granular-replay-permissions feature flag.", + required=False, + allow_null=True, + ) # avatar avatarType = serializers.ChoiceField( diff --git a/src/sentry/migrations/1012_organizationmember_replay_access.py b/src/sentry/migrations/1012_organizationmember_replay_access.py new file mode 100644 index 00000000000000..9dba851c1e91ea --- /dev/null +++ b/src/sentry/migrations/1012_organizationmember_replay_access.py @@ -0,0 +1,64 @@ +# Generated by Django 5.2.8 on 2025-12-02 12:31 + +import django.db.models.deletion +import django.utils.timezone +from django.db import migrations, models + +import sentry.db.models.fields.bounded +import sentry.db.models.fields.foreignkey +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "1011_update_oc_integration_cascade_to_null"), + ] + + operations = [ + migrations.CreateModel( + name="OrganizationMemberReplayAccess", + fields=[ + ( + "id", + sentry.db.models.fields.bounded.BoundedBigAutoField( + primary_key=True, serialize=False + ), + ), + ("date_added", models.DateTimeField(default=django.utils.timezone.now)), + ( + "organization", + sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="replay_access_set", + to="sentry.organization", + ), + ), + ( + "organizationmember", + sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="replay_access", + to="sentry.organizationmember", + ), + ), + ], + options={ + "db_table": "sentry_organizationmemberreplayaccess", + "unique_together": {("organization", "organizationmember")}, + }, + ), + ] diff --git a/src/sentry/models/__init__.py b/src/sentry/models/__init__.py index c34615e484f620..bccfd75ddb3a82 100644 --- a/src/sentry/models/__init__.py +++ b/src/sentry/models/__init__.py @@ -74,6 +74,7 @@ from .organizationmember import * # NOQA from .organizationmemberinvite import * # NOQA from .organizationmembermapping import * # NOQA +from .organizationmemberreplayaccess import * # NOQA from .organizationmemberteam import * # NOQA from .organizationmemberteamreplica import * # NOQA from .organizationonboardingtask import * # NOQA diff --git a/src/sentry/models/organizationmemberreplayaccess.py b/src/sentry/models/organizationmemberreplayaccess.py new file mode 100644 index 00000000000000..bb928aea29e7b6 --- /dev/null +++ b/src/sentry/models/organizationmemberreplayaccess.py @@ -0,0 +1,34 @@ +from __future__ import annotations + +from django.db import models +from django.utils import timezone + +from sentry.backup.scopes import RelocationScope +from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr + + +@region_silo_model +class OrganizationMemberReplayAccess(Model): + """ + Tracks which organization members have permission to access replay data. + + When no records exist for an organization, all members have access (default). + When records exist, only members with a record can access replays. + """ + + __relocation_scope__ = RelocationScope.Organization + + organization = FlexibleForeignKey("sentry.Organization", related_name="replay_access_set") + organizationmember = FlexibleForeignKey( + "sentry.OrganizationMember", + on_delete=models.CASCADE, + related_name="replay_access", + ) + date_added = models.DateTimeField(default=timezone.now) + + class Meta: + app_label = "sentry" + db_table = "sentry_organizationmemberreplayaccess" + unique_together = (("organization", "organizationmember"),) + + __repr__ = sane_repr("organization_id", "organizationmember_id") diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index 9e3993638a436b..43e98615197943 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -472,6 +472,14 @@ def create_exhaustive_organization( organization=org, key="sentry:scrape_javascript", value=True ) + # OrganizationMemberReplayAccess - add for the owner member + from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess + + owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) + OrganizationMemberReplayAccess.objects.create( + organization=org, organizationmember=owner_member + ) + # Team team = self.create_team(name=f"test_team_in_{slug}", organization=org) self.create_team_membership(user=owner, team=team) From 4ec9071d59ef970378407a8956df081c4441aabb Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 14:47:18 +0100 Subject: [PATCH 02/37] add tests --- .../core/endpoints/organization_details.py | 4 +- .../api/serializers/test_organization.py | 67 ++++++ .../endpoints/test_organization_details.py | 197 ++++++++++++++++++ 3 files changed, 266 insertions(+), 2 deletions(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 9f203a614096b3..e66e35de28d99f 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -370,6 +370,7 @@ class OrganizationSerializer(BaseOrganizationSerializer): ingestThroughTrustedRelaysOnly = serializers.ChoiceField( choices=[("enabled", "enabled"), ("disabled", "disabled")], required=False ) + hasGranularReplayPermissions = serializers.BooleanField(required=False) replayAccessMembers = serializers.ListField( child=serializers.IntegerField(), required=False, @@ -619,8 +620,7 @@ def save(self, **kwargs): organization=org, key=option_key, defaults={"value": new_value} ) - if new_value or created: - changed_data["hasGranularReplayPermissions"] = f"to {new_value}" + changed_data["hasGranularReplayPermissions"] = f"to {new_value}" if "replayAccessMembers" in data: member_ids = data["replayAccessMembers"] diff --git a/tests/sentry/api/serializers/test_organization.py b/tests/sentry/api/serializers/test_organization.py index 16ea1fcb20b95e..60a7c05a311bbf 100644 --- a/tests/sentry/api/serializers/test_organization.py +++ b/tests/sentry/api/serializers/test_organization.py @@ -17,6 +17,7 @@ from sentry.models.deploy import Deploy from sentry.models.environment import Environment from sentry.models.options.organization_option import OrganizationOption +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationonboardingtask import ( OnboardingTask, OnboardingTaskStatus, @@ -162,6 +163,72 @@ def test_detailed(self) -> None: assert isinstance(result["teamRoleList"], list) assert result["requiresSso"] == acc.requires_sso + def test_granular_replay_permissions_disabled_without_feature(self) -> None: + user = self.create_user() + organization = self.create_organization(owner=user) + acc = access.from_user(user, organization) + + serializer = DetailedOrganizationSerializer() + result = serialize(organization, user, serializer, access=acc) + + assert result["hasGranularReplayPermissions"] is False + assert result["replayAccessMembers"] == [] + + def test_granular_replay_permissions_flag_with_feature(self) -> None: + user = self.create_user() + organization = self.create_organization(owner=user) + acc = access.from_user(user, organization) + + with self.feature("organizations:granular-replay-permissions"): + serializer = DetailedOrganizationSerializer() + result = serialize(organization, user, serializer, access=acc) + assert result["hasGranularReplayPermissions"] is False + assert result["replayAccessMembers"] == [] + + OrganizationOption.objects.set_value( + organization=organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + result = serialize(organization, user, serializer, access=acc) + assert result["hasGranularReplayPermissions"] is True + assert result["replayAccessMembers"] == [] + + def test_replay_access_members_serialized(self) -> None: + user = self.create_user() + organization = self.create_organization(owner=user) + member1 = self.create_member( + organization=organization, user=self.create_user(), role="member" + ) + member2 = self.create_member( + organization=organization, user=self.create_user(), role="member" + ) + OrganizationMemberReplayAccess.objects.create( + organization=organization, organizationmember=member1 + ) + OrganizationMemberReplayAccess.objects.create( + organization=organization, organizationmember=member2 + ) + acc = access.from_user(user, organization) + + with self.feature("organizations:granular-replay-permissions"): + serializer = DetailedOrganizationSerializer() + result = serialize(organization, user, serializer, access=acc) + + assert set(result["replayAccessMembers"]) == {member1.id, member2.id} + + def test_replay_access_members_empty_when_none_set(self) -> None: + user = self.create_user() + organization = self.create_organization(owner=user) + acc = access.from_user(user, organization) + + with self.feature("organizations:granular-replay-permissions"): + serializer = DetailedOrganizationSerializer() + result = serialize(organization, user, serializer, access=acc) + + assert result["replayAccessMembers"] == [] + class DetailedOrganizationSerializerWithProjectsAndTeamsTest(TestCase): def test_detailed_org_projs_teams(self) -> None: diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index e23d969b61bff6..df27e21253f6c1 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -31,6 +31,7 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationmapping import OrganizationMapping +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationslugreservation import OrganizationSlugReservation from sentry.signals import project_created from sentry.silo.safety import unguarded_write @@ -1476,6 +1477,202 @@ def test_enable_seer_coding_can_be_enabled(self) -> None: assert self.organization.get_option("sentry:enable_seer_coding") is True + @with_feature("organizations:granular-replay-permissions") + def test_granular_replay_permissions_flag_set(self) -> None: + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"hasGranularReplayPermissions": True} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + option_value = OrganizationOption.objects.get( + organization=self.organization, key="sentry:granular-replay-permissions" + ) + assert option_value.value is True + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + assert "to True" in log.data["hasGranularReplayPermissions"] + + @with_feature("organizations:granular-replay-permissions") + def test_granular_replay_permissions_flag_unset(self) -> None: + self.organization.update_option("sentry:granular-replay-permissions", True) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"hasGranularReplayPermissions": False} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + option_value = OrganizationOption.objects.get( + organization=self.organization, key="sentry:granular-replay-permissions" + ) + assert option_value.value is False + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + + assert "to False" in log.data["hasGranularReplayPermissions"] + + def test_granular_replay_permissions_flag_requires_feature(self) -> None: + data = {"hasGranularReplayPermissions": True} + response = self.get_error_response(self.organization.slug, **data, status_code=400) + assert "hasGranularReplayPermissions" in response.data + + @with_feature("organizations:granular-replay-permissions") + def test_granular_replay_permissions_flag_requires_admin_scope(self) -> None: + member_user = self.create_user() + self.create_member( + organization=self.organization, user=member_user, role="member", teams=[] + ) + self.login_as(member_user) + + data = {"hasGranularReplayPermissions": True} + response = self.get_error_response(self.organization.slug, **data, status_code=403) + assert response.status_code == 403 + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_add(self) -> None: + member1 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + member2 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"replayAccessMembers": [member1.id, member2.id]} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + access_members = list( + OrganizationMemberReplayAccess.objects.filter( + organization=self.organization + ).values_list("organizationmember_id", flat=True) + ) + assert set(access_members) == {member1.id, member2.id} + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + assert "added 2 member(s)" in log.data["replayAccessMembers"] + assert "total: 2 member(s)" in log.data["replayAccessMembers"] + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_remove(self) -> None: + member1 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + member2 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=member1 + ) + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=member2 + ) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"replayAccessMembers": [member1.id]} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + access_members = list( + OrganizationMemberReplayAccess.objects.filter( + organization=self.organization + ).values_list("organizationmember_id", flat=True) + ) + assert access_members == [member1.id] + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + assert "removed 1 member(s)" in log.data["replayAccessMembers"] + assert "total: 1 member(s)" in log.data["replayAccessMembers"] + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_add_and_remove(self) -> None: + member1 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + member2 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + member3 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=member1 + ) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"replayAccessMembers": [member2.id, member3.id]} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + access_members = set( + OrganizationMemberReplayAccess.objects.filter( + organization=self.organization + ).values_list("organizationmember_id", flat=True) + ) + assert access_members == {member2.id, member3.id} + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + assert "added 2 member(s)" in log.data["replayAccessMembers"] + assert "removed 1 member(s)" in log.data["replayAccessMembers"] + assert "total: 2 member(s)" in log.data["replayAccessMembers"] + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_clear_all(self) -> None: + member1 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=member1 + ) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"replayAccessMembers": []} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + access_count = OrganizationMemberReplayAccess.objects.filter( + organization=self.organization + ).count() + assert access_count == 0 + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + assert "removed 1 member(s)" in log.data["replayAccessMembers"] + assert "total: 0 member(s)" in log.data["replayAccessMembers"] + + def test_replay_access_members_requires_feature(self) -> None: + member1 = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + data = {"replayAccessMembers": [member1.id]} + response = self.get_error_response(self.organization.slug, **data, status_code=400) + assert "hasGranularReplayPermissions" in response.data + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_requires_admin_scope(self) -> None: + member_user = self.create_user() + self.create_member( + organization=self.organization, user=member_user, role="member", teams=[] + ) + self.login_as(member_user) + + other_member = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + data = {"replayAccessMembers": [other_member.id]} + self.get_error_response(self.organization.slug, **data, status_code=403) + class OrganizationDeleteTest(OrganizationDetailsTestBase): method = "delete" From e19b9d078ff6f55a642b5a5c05940bc522606961 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 15:21:58 +0100 Subject: [PATCH 03/37] fix typing issues --- src/sentry/api/serializers/models/organization.py | 8 +++----- tests/sentry/core/endpoints/test_organization_details.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index b124cb2e6ed79c..c948acc4c8884a 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -567,6 +567,7 @@ class DetailedOrganizationSerializerResponse(_DetailedOrganizationSerializerResp autoEnableCodeReview: bool autoOpenPrs: bool defaultCodeReviewTriggers: list[str] + hasGranularReplayPermissions: bool replayAccessMembers: list[int] @@ -750,11 +751,11 @@ def serialize( # type: ignore[override] team__organization=obj ).count(), "isDynamicallySampled": is_dynamically_sampled, + "hasGranularReplayPermissions": False, + "replayAccessMembers": [], } if features.has("organizations:granular-replay-permissions", obj): - # The organization option can be missing, False, or null. - # Only set hasGranularReplayPermissions to True if the option exists and is truthy. permissions_enabled = ( OrganizationOption.objects.filter( organization=obj, key="sentry:granular-replay-permissions" @@ -768,9 +769,6 @@ def serialize( # type: ignore[override] "organizationmember_id", flat=True ) ) - else: - context["hasGranularReplayPermissions"] = False - context["replayAccessMembers"] = [] if has_custom_dynamic_sampling(obj, actor=user): context["targetSampleRate"] = float( diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index df27e21253f6c1..ecb301af9ca305 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -1637,7 +1637,7 @@ def test_replay_access_members_clear_all(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() - data = {"replayAccessMembers": []} + data: dict[str, Any] = {"replayAccessMembers": []} with outbox_runner(): self.get_success_response(self.organization.slug, **data) From c17e5625432c25a9764aded690d27ca4b8009f37 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 15:37:09 +0100 Subject: [PATCH 04/37] add to docs test ignore for now --- src/sentry/api/serializers/models/organization.py | 2 ++ src/sentry/core/endpoints/organization_details.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index c948acc4c8884a..38bd70f861881b 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -819,6 +819,8 @@ def serialize( # type: ignore[override] "streamlineOnly", "ingestThroughTrustedRelaysOnly", "enabledConsolePlatforms", + "hasGranularReplayPermissions", + "replayAccessMembers", ] ) class DetailedOrganizationSerializerWithProjectsAndTeamsResponse( diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index e66e35de28d99f..b164db35b13905 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -889,7 +889,7 @@ class OrganizationDetailsPutSerializer(serializers.Serializer): ) replayAccessMembers = serializers.ListField( child=serializers.IntegerField(), - help_text="A list of organization member IDs who have permission to access replay data. When empty, all members have access. Requires the granular-replay-permissions feature flag.", + help_text="A list of organization member IDs who have permission to access replay data. Requires the hasGranularReplayPermissions flag to be true to be enforced.", required=False, allow_null=True, ) From 16476791caa2110da2c85f4730870efcf380ed44 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 16:27:58 +0100 Subject: [PATCH 05/37] update logic for invalid ids and add tests --- .../core/endpoints/organization_details.py | 14 ++++++++ .../endpoints/test_organization_details.py | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index b164db35b13905..8e2d54378aac36 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -94,6 +94,7 @@ from sentry.models.options.organization_option import OrganizationOption from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus +from sentry.models.organizationmember import OrganizationMember from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.project import Project from sentry.organizations.services.organization import organization_service @@ -639,6 +640,19 @@ def save(self, **kwargs): to_remove = current_member_ids - new_member_ids if to_add: + valid_member_ids = set( + OrganizationMember.objects.filter( + organization=org, id__in=to_add + ).values_list("id", flat=True) + ) + invalid_member_ids = to_add - valid_member_ids + if invalid_member_ids: + raise serializers.ValidationError( + { + "replayAccessMembers": f"Invalid organization member IDs: {sorted(invalid_member_ids)}" + } + ) + OrganizationMemberReplayAccess.objects.bulk_create( [ OrganizationMemberReplayAccess( diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index ecb301af9ca305..4d83ddcb32f4e5 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -1673,6 +1673,42 @@ def test_replay_access_members_requires_admin_scope(self) -> None: data = {"replayAccessMembers": [other_member.id]} self.get_error_response(self.organization.slug, **data, status_code=403) + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_invalid_member_ids(self) -> None: + nonexistent_id = 999999999 + data = {"replayAccessMembers": [nonexistent_id]} + response = self.get_error_response(self.organization.slug, **data, status_code=400) + assert "replayAccessMembers" in response.data + assert str(nonexistent_id) in response.data["replayAccessMembers"] + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_from_other_organization(self) -> None: + other_org = self.create_organization(owner=self.create_user()) + other_org_member = self.create_member( + organization=other_org, user=self.create_user(), role="member" + ) + data = {"replayAccessMembers": [other_org_member.id]} + response = self.get_error_response(self.organization.slug, **data, status_code=400) + assert "replayAccessMembers" in response.data + assert str(other_org_member.id) in response.data["replayAccessMembers"] + + @with_feature("organizations:granular-replay-permissions") + def test_replay_access_members_mixed_valid_and_invalid(self) -> None: + valid_member = self.create_member( + organization=self.organization, user=self.create_user(), role="member" + ) + nonexistent_id = 999999999 + data = {"replayAccessMembers": [valid_member.id, nonexistent_id]} + response = self.get_error_response(self.organization.slug, **data, status_code=400) + assert "replayAccessMembers" in response.data + assert str(nonexistent_id) in response.data["replayAccessMembers"] + assert str(valid_member.id) not in response.data["replayAccessMembers"] + + access_count = OrganizationMemberReplayAccess.objects.filter( + organization=self.organization + ).count() + assert access_count == 0 + class OrganizationDeleteTest(OrganizationDetailsTestBase): method = "delete" From 44fb9826ed99bceccb4efcc0e808a7efc9fa31d0 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 16:34:46 +0100 Subject: [PATCH 06/37] ignore conflicts in bulk create --- src/sentry/core/endpoints/organization_details.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 8e2d54378aac36..715182531160d6 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -659,7 +659,8 @@ def save(self, **kwargs): organization=org, organizationmember_id=member_id ) for member_id in to_add - ] + ], + ignore_conflicts=True, ) if to_remove: From b32f30ca85fa37a50259171bbdea529edec65839 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 13:26:51 +0100 Subject: [PATCH 07/37] move validation to validate_* methods & return correct HTTP status codes --- .../core/endpoints/organization_details.py | 144 +++++++++--------- .../endpoints/test_organization_details.py | 6 +- 2 files changed, 75 insertions(+), 75 deletions(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 715182531160d6..85a7a8ded24949 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -5,12 +5,14 @@ from datetime import datetime, timedelta, timezone from typing import TypedDict +from django.core.exceptions import PermissionDenied from django.db import models, router, transaction from django.db.models.query_utils import DeferredAttribute from django.urls import reverse from django.utils import timezone as django_timezone from drf_spectacular.utils import OpenApiResponse, extend_schema, extend_schema_serializer from rest_framework import serializers, status +from rest_framework.exceptions import NotFound from sentry_sdk import capture_exception from bitfield.types import BitHandler @@ -484,6 +486,26 @@ def validate_samplingMode(self, value): return value + def validate_hasGranularReplayPermissions(self, value): + self._validate_granular_replay_permissions() + return value + + def validate_replayAccessMembers(self, value): + self._validate_granular_replay_permissions() + return value + + def _validate_granular_replay_permissions(self): + organization = self.context["organization"] + request = self.context["request"] + + if not features.has("organizations:granular-replay-permissions", organization): + raise NotFound("This feature is not enabled for your organization.") + + if not request.access.has_scope("org:admin"): + raise PermissionDenied( + "You do not have permission to modify granular replay permissions." + ) + def validate(self, attrs): attrs = super().validate(attrs) if attrs.get("avatarType") == "upload": @@ -598,85 +620,65 @@ def save(self, **kwargs): if trusted_relay_info is not None: self.save_trusted_relays(trusted_relay_info, changed_data, org) - if "hasGranularReplayPermissions" in data or "replayAccessMembers" in data: - if not features.has("organizations:granular-replay-permissions", org): - raise serializers.ValidationError( - { - "hasGranularReplayPermissions": "This feature is not enabled for your organization." - } - ) - - if not self.context["request"].access.has_scope("org:admin"): - raise serializers.ValidationError( - { - "hasGranularReplayPermissions": "You do not have permission to modify granular replay permissions." - } - ) - - if "hasGranularReplayPermissions" in data: - option_key = "sentry:granular-replay-permissions" - new_value = data["hasGranularReplayPermissions"] - - option_inst, created = OrganizationOption.objects.update_or_create( - organization=org, key=option_key, defaults={"value": new_value} + if "hasGranularReplayPermissions" in data: + option_key = "sentry:granular-replay-permissions" + new_value = data["hasGranularReplayPermissions"] + option_inst, created = OrganizationOption.objects.update_or_create( + organization=org, key=option_key, defaults={"value": new_value} + ) + changed_data["hasGranularReplayPermissions"] = f"to {new_value}" + + if "replayAccessMembers" in data: + member_ids = data["replayAccessMembers"] + if member_ids is None: + member_ids = [] + current_member_ids = set( + OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( + "organizationmember_id", flat=True ) - - changed_data["hasGranularReplayPermissions"] = f"to {new_value}" - - if "replayAccessMembers" in data: - member_ids = data["replayAccessMembers"] - - if member_ids is None: - member_ids = [] - - current_member_ids = set( - OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( - "organizationmember_id", flat=True + ) + new_member_ids = set(member_ids) + + to_add = new_member_ids - current_member_ids + to_remove = current_member_ids - new_member_ids + if to_add: + valid_member_ids = set( + OrganizationMember.objects.filter(organization=org, id__in=to_add).values_list( + "id", flat=True ) ) - new_member_ids = set(member_ids) - - to_add = new_member_ids - current_member_ids - to_remove = current_member_ids - new_member_ids - - if to_add: - valid_member_ids = set( - OrganizationMember.objects.filter( - organization=org, id__in=to_add - ).values_list("id", flat=True) + invalid_member_ids = to_add - valid_member_ids + if invalid_member_ids: + raise serializers.ValidationError( + { + "replayAccessMembers": f"Invalid organization member IDs: {sorted(invalid_member_ids)}" + } ) - invalid_member_ids = to_add - valid_member_ids - if invalid_member_ids: - raise serializers.ValidationError( - { - "replayAccessMembers": f"Invalid organization member IDs: {sorted(invalid_member_ids)}" - } + + OrganizationMemberReplayAccess.objects.bulk_create( + [ + OrganizationMemberReplayAccess( + organization=org, organizationmember_id=member_id ) + for member_id in to_add + ], + ignore_conflicts=True, + ) - OrganizationMemberReplayAccess.objects.bulk_create( - [ - OrganizationMemberReplayAccess( - organization=org, organizationmember_id=member_id - ) - for member_id in to_add - ], - ignore_conflicts=True, - ) + if to_remove: + OrganizationMemberReplayAccess.objects.filter( + organization=org, organizationmember_id__in=to_remove + ).delete() + if to_add or to_remove: + changes = [] + if to_add: + changes.append(f"added {len(to_add)} member(s)") if to_remove: - OrganizationMemberReplayAccess.objects.filter( - organization=org, organizationmember_id__in=to_remove - ).delete() - - if to_add or to_remove: - changes = [] - if to_add: - changes.append(f"added {len(to_add)} member(s)") - if to_remove: - changes.append(f"removed {len(to_remove)} member(s)") - changed_data["replayAccessMembers"] = ( - f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" - ) + changes.append(f"removed {len(to_remove)} member(s)") + changed_data["replayAccessMembers"] = ( + f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" + ) if "openMembership" in data: org.flags.allow_joinleave = data["openMembership"] diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index 4d83ddcb32f4e5..0082b1c158371b 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -1517,8 +1517,7 @@ def test_granular_replay_permissions_flag_unset(self) -> None: def test_granular_replay_permissions_flag_requires_feature(self) -> None: data = {"hasGranularReplayPermissions": True} - response = self.get_error_response(self.organization.slug, **data, status_code=400) - assert "hasGranularReplayPermissions" in response.data + self.get_error_response(self.organization.slug, **data, status_code=404) @with_feature("organizations:granular-replay-permissions") def test_granular_replay_permissions_flag_requires_admin_scope(self) -> None: @@ -1656,8 +1655,7 @@ def test_replay_access_members_requires_feature(self) -> None: organization=self.organization, user=self.create_user(), role="member" ) data = {"replayAccessMembers": [member1.id]} - response = self.get_error_response(self.organization.slug, **data, status_code=400) - assert "hasGranularReplayPermissions" in response.data + self.get_error_response(self.organization.slug, **data, status_code=404) @with_feature("organizations:granular-replay-permissions") def test_replay_access_members_requires_admin_scope(self) -> None: From 71f39ae2007a6bf50309dd80ddbd12a561a3f5cc Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 13:28:32 +0100 Subject: [PATCH 08/37] use DefaultFieldsModel --- src/sentry/models/organizationmemberreplayaccess.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/sentry/models/organizationmemberreplayaccess.py b/src/sentry/models/organizationmemberreplayaccess.py index bb928aea29e7b6..70f1853c9c50da 100644 --- a/src/sentry/models/organizationmemberreplayaccess.py +++ b/src/sentry/models/organizationmemberreplayaccess.py @@ -1,14 +1,13 @@ from __future__ import annotations from django.db import models -from django.utils import timezone from sentry.backup.scopes import RelocationScope -from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr +from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_model, sane_repr @region_silo_model -class OrganizationMemberReplayAccess(Model): +class OrganizationMemberReplayAccess(DefaultFieldsModel): """ Tracks which organization members have permission to access replay data. @@ -24,7 +23,6 @@ class OrganizationMemberReplayAccess(Model): on_delete=models.CASCADE, related_name="replay_access", ) - date_added = models.DateTimeField(default=timezone.now) class Meta: app_label = "sentry" From eb8f3df2463af7285df8e2fdcf28d18d28115455 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 13:57:54 +0100 Subject: [PATCH 09/37] remove organization, move model to replays --- .../api/serializers/models/organization.py | 8 ++--- .../core/endpoints/organization_details.py | 14 ++++---- src/sentry/models/__init__.py | 1 - .../models/organizationmemberreplayaccess.py | 32 ------------------- .../0007_organizationmember_replay_access.py} | 16 +++------- src/sentry/replays/models.py | 26 +++++++++++++++ src/sentry/testutils/helpers/backups.py | 6 ++-- .../api/serializers/test_organization.py | 10 ++---- .../endpoints/test_organization_details.py | 28 ++++++---------- 9 files changed, 55 insertions(+), 86 deletions(-) delete mode 100644 src/sentry/models/organizationmemberreplayaccess.py rename src/sentry/{migrations/1012_organizationmember_replay_access.py => replays/migrations/0007_organizationmember_replay_access.py} (77%) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 38bd70f861881b..49dbce61a52434 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -76,12 +76,12 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationaccessrequest import OrganizationAccessRequest -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationonboardingtask import OrganizationOnboardingTask from sentry.models.project import Project from sentry.models.team import Team, TeamStatus from sentry.organizations.absolute_url import generate_organization_url from sentry.organizations.services.organization import RpcOrganizationSummary +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.users.models.user import User from sentry.users.services.user.model import RpcUser from sentry.users.services.user.service import user_service @@ -765,9 +765,9 @@ def serialize( # type: ignore[override] ) context["hasGranularReplayPermissions"] = bool(permissions_enabled) context["replayAccessMembers"] = list( - OrganizationMemberReplayAccess.objects.filter(organization=obj).values_list( - "organizationmember_id", flat=True - ) + OrganizationMemberReplayAccess.objects.filter( + organizationmember__organization=obj + ).values_list("organizationmember_id", flat=True) ) if has_custom_dynamic_sampling(obj, actor=user): diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 85a7a8ded24949..580823c34c033a 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -97,7 +97,6 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationmember import OrganizationMember -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.project import Project from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import ( @@ -106,6 +105,7 @@ RpcOrganizationDeleteState, ) from sentry.relay.datascrubbing import validate_pii_config_update, validate_pii_selectors +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.seer.autofix.constants import AutofixAutomationTuningSettings from sentry.services.organization.provisioning import ( OrganizationSlugCollisionException, @@ -633,9 +633,9 @@ def save(self, **kwargs): if member_ids is None: member_ids = [] current_member_ids = set( - OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( - "organizationmember_id", flat=True - ) + OrganizationMemberReplayAccess.objects.filter( + organizationmember__organization=org + ).values_list("organizationmember_id", flat=True) ) new_member_ids = set(member_ids) @@ -657,9 +657,7 @@ def save(self, **kwargs): OrganizationMemberReplayAccess.objects.bulk_create( [ - OrganizationMemberReplayAccess( - organization=org, organizationmember_id=member_id - ) + OrganizationMemberReplayAccess(organizationmember_id=member_id) for member_id in to_add ], ignore_conflicts=True, @@ -667,7 +665,7 @@ def save(self, **kwargs): if to_remove: OrganizationMemberReplayAccess.objects.filter( - organization=org, organizationmember_id__in=to_remove + organizationmember__organization=org, organizationmember_id__in=to_remove ).delete() if to_add or to_remove: diff --git a/src/sentry/models/__init__.py b/src/sentry/models/__init__.py index bccfd75ddb3a82..c34615e484f620 100644 --- a/src/sentry/models/__init__.py +++ b/src/sentry/models/__init__.py @@ -74,7 +74,6 @@ from .organizationmember import * # NOQA from .organizationmemberinvite import * # NOQA from .organizationmembermapping import * # NOQA -from .organizationmemberreplayaccess import * # NOQA from .organizationmemberteam import * # NOQA from .organizationmemberteamreplica import * # NOQA from .organizationonboardingtask import * # NOQA diff --git a/src/sentry/models/organizationmemberreplayaccess.py b/src/sentry/models/organizationmemberreplayaccess.py deleted file mode 100644 index 70f1853c9c50da..00000000000000 --- a/src/sentry/models/organizationmemberreplayaccess.py +++ /dev/null @@ -1,32 +0,0 @@ -from __future__ import annotations - -from django.db import models - -from sentry.backup.scopes import RelocationScope -from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_model, sane_repr - - -@region_silo_model -class OrganizationMemberReplayAccess(DefaultFieldsModel): - """ - Tracks which organization members have permission to access replay data. - - When no records exist for an organization, all members have access (default). - When records exist, only members with a record can access replays. - """ - - __relocation_scope__ = RelocationScope.Organization - - organization = FlexibleForeignKey("sentry.Organization", related_name="replay_access_set") - organizationmember = FlexibleForeignKey( - "sentry.OrganizationMember", - on_delete=models.CASCADE, - related_name="replay_access", - ) - - class Meta: - app_label = "sentry" - db_table = "sentry_organizationmemberreplayaccess" - unique_together = (("organization", "organizationmember"),) - - __repr__ = sane_repr("organization_id", "organizationmember_id") diff --git a/src/sentry/migrations/1012_organizationmember_replay_access.py b/src/sentry/replays/migrations/0007_organizationmember_replay_access.py similarity index 77% rename from src/sentry/migrations/1012_organizationmember_replay_access.py rename to src/sentry/replays/migrations/0007_organizationmember_replay_access.py index 9dba851c1e91ea..464cfb8f69efd4 100644 --- a/src/sentry/migrations/1012_organizationmember_replay_access.py +++ b/src/sentry/replays/migrations/0007_organizationmember_replay_access.py @@ -1,7 +1,6 @@ # Generated by Django 5.2.8 on 2025-12-02 12:31 import django.db.models.deletion -import django.utils.timezone from django.db import migrations, models import sentry.db.models.fields.bounded @@ -25,7 +24,7 @@ class Migration(CheckedMigration): is_post_deployment = False dependencies = [ - ("sentry", "1011_update_oc_integration_cascade_to_null"), + ("replays", "0006_add_bulk_delete_job"), ] operations = [ @@ -38,27 +37,20 @@ class Migration(CheckedMigration): primary_key=True, serialize=False ), ), - ("date_added", models.DateTimeField(default=django.utils.timezone.now)), - ( - "organization", - sentry.db.models.fields.foreignkey.FlexibleForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name="replay_access_set", - to="sentry.organization", - ), - ), + ("date_updated", models.DateTimeField(auto_now=True)), + ("date_added", models.DateTimeField(auto_now_add=True)), ( "organizationmember", sentry.db.models.fields.foreignkey.FlexibleForeignKey( on_delete=django.db.models.deletion.CASCADE, related_name="replay_access", to="sentry.organizationmember", + unique=True, ), ), ], options={ "db_table": "sentry_organizationmemberreplayaccess", - "unique_together": {("organization", "organizationmember")}, }, ), ] diff --git a/src/sentry/replays/models.py b/src/sentry/replays/models.py index b1fb841b71ae79..a9eebe85508cab 100644 --- a/src/sentry/replays/models.py +++ b/src/sentry/replays/models.py @@ -8,6 +8,7 @@ from sentry.db.models import ( BoundedBigIntegerField, DefaultFieldsModel, + FlexibleForeignKey, Model, region_silo_model, sane_repr, @@ -78,3 +79,28 @@ def delete(self, *args, **kwargs): rv = super().delete(*args, **kwargs) return rv + + +@region_silo_model +class OrganizationMemberReplayAccess(DefaultFieldsModel): + """ + Tracks which organization members have permission to access replay data. + + When no records exist for an organization, all members have access (default). + When records exist, only members with a record can access replays. + """ + + __relocation_scope__ = RelocationScope.Organization + + organizationmember = FlexibleForeignKey( + "sentry.OrganizationMember", + on_delete=models.CASCADE, + related_name="replay_access", + unique=True, + ) + + class Meta: + app_label = "replays" + db_table = "sentry_organizationmemberreplayaccess" + + __repr__ = sane_repr("organizationmember_id") diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index 43e98615197943..d6aae933df7eb6 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -473,12 +473,10 @@ def create_exhaustive_organization( ) # OrganizationMemberReplayAccess - add for the owner member - from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess + from sentry.replays.models import OrganizationMemberReplayAccess owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) - OrganizationMemberReplayAccess.objects.create( - organization=org, organizationmember=owner_member - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=owner_member) # Team team = self.create_team(name=f"test_team_in_{slug}", organization=org) diff --git a/tests/sentry/api/serializers/test_organization.py b/tests/sentry/api/serializers/test_organization.py index 60a7c05a311bbf..4a89a38eb99305 100644 --- a/tests/sentry/api/serializers/test_organization.py +++ b/tests/sentry/api/serializers/test_organization.py @@ -17,13 +17,13 @@ from sentry.models.deploy import Deploy from sentry.models.environment import Environment from sentry.models.options.organization_option import OrganizationOption -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationonboardingtask import ( OnboardingTask, OnboardingTaskStatus, OrganizationOnboardingTask, ) from sentry.models.releaseprojectenvironment import ReleaseProjectEnvironment +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.testutils.cases import TestCase from sentry.testutils.skips import requires_snuba @@ -204,12 +204,8 @@ def test_replay_access_members_serialized(self) -> None: member2 = self.create_member( organization=organization, user=self.create_user(), role="member" ) - OrganizationMemberReplayAccess.objects.create( - organization=organization, organizationmember=member1 - ) - OrganizationMemberReplayAccess.objects.create( - organization=organization, organizationmember=member2 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member1) + OrganizationMemberReplayAccess.objects.create(organizationmember=member2) acc = access.from_user(user, organization) with self.feature("organizations:granular-replay-permissions"): diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index 0082b1c158371b..6735a9f12946b4 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -31,8 +31,8 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationmapping import OrganizationMapping -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationslugreservation import OrganizationSlugReservation +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.signals import project_created from sentry.silo.safety import unguarded_write from sentry.snuba.metrics import TransactionMRI @@ -1548,7 +1548,7 @@ def test_replay_access_members_add(self) -> None: access_members = list( OrganizationMemberReplayAccess.objects.filter( - organization=self.organization + organizationmember__organization=self.organization ).values_list("organizationmember_id", flat=True) ) assert set(access_members) == {member1.id, member2.id} @@ -1566,12 +1566,8 @@ def test_replay_access_members_remove(self) -> None: member2 = self.create_member( organization=self.organization, user=self.create_user(), role="member" ) - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=member1 - ) - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=member2 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member1) + OrganizationMemberReplayAccess.objects.create(organizationmember=member2) with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() @@ -1581,7 +1577,7 @@ def test_replay_access_members_remove(self) -> None: access_members = list( OrganizationMemberReplayAccess.objects.filter( - organization=self.organization + organizationmember__organization=self.organization ).values_list("organizationmember_id", flat=True) ) assert access_members == [member1.id] @@ -1602,9 +1598,7 @@ def test_replay_access_members_add_and_remove(self) -> None: member3 = self.create_member( organization=self.organization, user=self.create_user(), role="member" ) - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=member1 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member1) with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() @@ -1614,7 +1608,7 @@ def test_replay_access_members_add_and_remove(self) -> None: access_members = set( OrganizationMemberReplayAccess.objects.filter( - organization=self.organization + organizationmember__organization=self.organization ).values_list("organizationmember_id", flat=True) ) assert access_members == {member2.id, member3.id} @@ -1630,9 +1624,7 @@ def test_replay_access_members_clear_all(self) -> None: member1 = self.create_member( organization=self.organization, user=self.create_user(), role="member" ) - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=member1 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member1) with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() @@ -1641,7 +1633,7 @@ def test_replay_access_members_clear_all(self) -> None: self.get_success_response(self.organization.slug, **data) access_count = OrganizationMemberReplayAccess.objects.filter( - organization=self.organization + organizationmember__organization=self.organization ).count() assert access_count == 0 @@ -1703,7 +1695,7 @@ def test_replay_access_members_mixed_valid_and_invalid(self) -> None: assert str(valid_member.id) not in response.data["replayAccessMembers"] access_count = OrganizationMemberReplayAccess.objects.filter( - organization=self.organization + organizationmember__organization=self.organization ).count() assert access_count == 0 From 1ae68bec6f461d21f4996f3d3341be43ec3516e5 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 14:01:28 +0100 Subject: [PATCH 10/37] migration lockfile --- migrations_lockfile.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 1ad5e7d42e8552..35b0216d35b55e 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -29,7 +29,7 @@ prevent: 0002_alter_integration_id_not_null releases: 0004_cleanup_failed_safe_deletes -replays: 0006_add_bulk_delete_job +replays: 0007_organizationmember_replay_access sentry: 1012_add_event_id_to_open_period From 9353cb094c0aabb564e136f1a6e20422385efec4 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 14:33:34 +0100 Subject: [PATCH 11/37] fix backup tests --- src/sentry/backup/comparators.py | 3 +++ src/sentry/testutils/helpers/backups.py | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry/backup/comparators.py b/src/sentry/backup/comparators.py index 5636fbdf08c46c..323ada5a8136d2 100644 --- a/src/sentry/backup/comparators.py +++ b/src/sentry/backup/comparators.py @@ -982,6 +982,9 @@ def get_default_comparators() -> dict[str, list[JSONScrubbingComparator]]: DateUpdatedComparator("date_updated", "date_added") ], "monitors.monitor": [UUID4Comparator("guid")], + "replays.organizationmemberreplayaccess": [ + DateUpdatedComparator("date_updated", "date_added") + ], }, ) diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index d6aae933df7eb6..113ae113bb779e 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -103,6 +103,7 @@ from sentry.models.savedsearch import SavedSearch, Visibility from sentry.models.search_common import SearchType from sentry.monitors.models import Monitor, ScheduleType +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.sentry_apps.logic import SentryAppUpdater from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.services.nodestore.django.models import Node @@ -472,9 +473,6 @@ def create_exhaustive_organization( organization=org, key="sentry:scrape_javascript", value=True ) - # OrganizationMemberReplayAccess - add for the owner member - from sentry.replays.models import OrganizationMemberReplayAccess - owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) OrganizationMemberReplayAccess.objects.create(organizationmember=owner_member) From 31b4f4e9a71ef8953e4603f2611e4588cace4857 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 14:48:11 +0100 Subject: [PATCH 12/37] import exceptions from drf --- src/sentry/core/endpoints/organization_details.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 580823c34c033a..0dfe5a1fa21ba0 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -5,14 +5,13 @@ from datetime import datetime, timedelta, timezone from typing import TypedDict -from django.core.exceptions import PermissionDenied from django.db import models, router, transaction from django.db.models.query_utils import DeferredAttribute from django.urls import reverse from django.utils import timezone as django_timezone from drf_spectacular.utils import OpenApiResponse, extend_schema, extend_schema_serializer from rest_framework import serializers, status -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import NotFound, PermissionDenied from sentry_sdk import capture_exception from bitfield.types import BitHandler From 3b98ee982804401713c48c5a3b9ea72bb32c5bb9 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 15:16:19 +0100 Subject: [PATCH 13/37] additional dependency in migration --- .../replays/migrations/0007_organizationmember_replay_access.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry/replays/migrations/0007_organizationmember_replay_access.py b/src/sentry/replays/migrations/0007_organizationmember_replay_access.py index 464cfb8f69efd4..95c5813d13bdb5 100644 --- a/src/sentry/replays/migrations/0007_organizationmember_replay_access.py +++ b/src/sentry/replays/migrations/0007_organizationmember_replay_access.py @@ -25,6 +25,7 @@ class Migration(CheckedMigration): dependencies = [ ("replays", "0006_add_bulk_delete_job"), + ("sentry", "1011_update_oc_integration_cascade_to_null"), ] operations = [ From 2d5d0e4fbdbdd42658878dafc14af3ad15d75b88 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 16:53:25 +0100 Subject: [PATCH 14/37] use user id for setting the allowlist instead of memberid --- .../api/serializers/models/organization.py | 2 +- .../core/endpoints/organization_details.py | 50 ++++++++++--------- .../api/serializers/test_organization.py | 2 +- .../endpoints/test_organization_details.py | 38 +++++++------- 4 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 49dbce61a52434..28e0f8445f1cfa 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -767,7 +767,7 @@ def serialize( # type: ignore[override] context["replayAccessMembers"] = list( OrganizationMemberReplayAccess.objects.filter( organizationmember__organization=obj - ).values_list("organizationmember_id", flat=True) + ).values_list("organizationmember__user_id", flat=True) ) if has_custom_dynamic_sampling(obj, actor=user): diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 0dfe5a1fa21ba0..ef7562a751862f 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -377,7 +377,7 @@ class OrganizationSerializer(BaseOrganizationSerializer): child=serializers.IntegerField(), required=False, allow_null=True, - help_text="List of organization member IDs that have access to replay data. Only modifiable by owners and managers.", + help_text="List of user IDs that have access to replay data. Only modifiable by owners and managers.", ) def _has_sso_enabled(self): @@ -628,53 +628,57 @@ def save(self, **kwargs): changed_data["hasGranularReplayPermissions"] = f"to {new_value}" if "replayAccessMembers" in data: - member_ids = data["replayAccessMembers"] - if member_ids is None: - member_ids = [] - current_member_ids = set( + user_ids = data["replayAccessMembers"] + if user_ids is None: + user_ids = [] + + current_user_ids = set( OrganizationMemberReplayAccess.objects.filter( organizationmember__organization=org - ).values_list("organizationmember_id", flat=True) + ).values_list("organizationmember__user_id", flat=True) ) - new_member_ids = set(member_ids) + new_user_ids = set(user_ids) + + to_add = new_user_ids - current_user_ids + to_remove = current_user_ids - new_user_ids - to_add = new_member_ids - current_member_ids - to_remove = current_member_ids - new_member_ids if to_add: - valid_member_ids = set( - OrganizationMember.objects.filter(organization=org, id__in=to_add).values_list( - "id", flat=True - ) + user_to_member = dict( + OrganizationMember.objects.filter( + organization=org, user_id__in=to_add + ).values_list("user_id", "id") ) - invalid_member_ids = to_add - valid_member_ids - if invalid_member_ids: + invalid_user_ids = to_add - set(user_to_member.keys()) + if invalid_user_ids: raise serializers.ValidationError( { - "replayAccessMembers": f"Invalid organization member IDs: {sorted(invalid_member_ids)}" + "replayAccessMembers": f"Invalid user IDs (not members of this organization): {sorted(invalid_user_ids)}" } ) OrganizationMemberReplayAccess.objects.bulk_create( [ - OrganizationMemberReplayAccess(organizationmember_id=member_id) - for member_id in to_add + OrganizationMemberReplayAccess( + organizationmember_id=user_to_member[user_id] + ) + for user_id in to_add ], ignore_conflicts=True, ) if to_remove: OrganizationMemberReplayAccess.objects.filter( - organizationmember__organization=org, organizationmember_id__in=to_remove + organizationmember__organization=org, organizationmember__user_id__in=to_remove ).delete() if to_add or to_remove: changes = [] if to_add: - changes.append(f"added {len(to_add)} member(s)") + changes.append(f"added {len(to_add)} user(s)") if to_remove: - changes.append(f"removed {len(to_remove)} member(s)") + changes.append(f"removed {len(to_remove)} user(s)") changed_data["replayAccessMembers"] = ( - f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" + f"{' and '.join(changes)} (total: {len(new_user_ids)} user(s) with access)" ) if "openMembership" in data: @@ -903,7 +907,7 @@ class OrganizationDetailsPutSerializer(serializers.Serializer): ) replayAccessMembers = serializers.ListField( child=serializers.IntegerField(), - help_text="A list of organization member IDs who have permission to access replay data. Requires the hasGranularReplayPermissions flag to be true to be enforced.", + help_text="A list of user IDs who have permission to access replay data. Requires the hasGranularReplayPermissions flag to be true to be enforced.", required=False, allow_null=True, ) diff --git a/tests/sentry/api/serializers/test_organization.py b/tests/sentry/api/serializers/test_organization.py index 4a89a38eb99305..7f6b099fd914d1 100644 --- a/tests/sentry/api/serializers/test_organization.py +++ b/tests/sentry/api/serializers/test_organization.py @@ -212,7 +212,7 @@ def test_replay_access_members_serialized(self) -> None: serializer = DetailedOrganizationSerializer() result = serialize(organization, user, serializer, access=acc) - assert set(result["replayAccessMembers"]) == {member1.id, member2.id} + assert set(result["replayAccessMembers"]) == {member1.user_id, member2.user_id} def test_replay_access_members_empty_when_none_set(self) -> None: user = self.create_user() diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index 6735a9f12946b4..9c9e22d19f2b91 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -1542,7 +1542,7 @@ def test_replay_access_members_add(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() - data = {"replayAccessMembers": [member1.id, member2.id]} + data = {"replayAccessMembers": [member1.user_id, member2.user_id]} with outbox_runner(): self.get_success_response(self.organization.slug, **data) @@ -1555,8 +1555,8 @@ def test_replay_access_members_add(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): log = AuditLogEntry.objects.get(organization_id=self.organization.id) - assert "added 2 member(s)" in log.data["replayAccessMembers"] - assert "total: 2 member(s)" in log.data["replayAccessMembers"] + assert "added 2 user(s)" in log.data["replayAccessMembers"] + assert "total: 2 user(s)" in log.data["replayAccessMembers"] @with_feature("organizations:granular-replay-permissions") def test_replay_access_members_remove(self) -> None: @@ -1571,7 +1571,7 @@ def test_replay_access_members_remove(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() - data = {"replayAccessMembers": [member1.id]} + data = {"replayAccessMembers": [member1.user_id]} with outbox_runner(): self.get_success_response(self.organization.slug, **data) @@ -1584,8 +1584,8 @@ def test_replay_access_members_remove(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): log = AuditLogEntry.objects.get(organization_id=self.organization.id) - assert "removed 1 member(s)" in log.data["replayAccessMembers"] - assert "total: 1 member(s)" in log.data["replayAccessMembers"] + assert "removed 1 user(s)" in log.data["replayAccessMembers"] + assert "total: 1 user(s)" in log.data["replayAccessMembers"] @with_feature("organizations:granular-replay-permissions") def test_replay_access_members_add_and_remove(self) -> None: @@ -1602,7 +1602,7 @@ def test_replay_access_members_add_and_remove(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() - data = {"replayAccessMembers": [member2.id, member3.id]} + data = {"replayAccessMembers": [member2.user_id, member3.user_id]} with outbox_runner(): self.get_success_response(self.organization.slug, **data) @@ -1615,9 +1615,9 @@ def test_replay_access_members_add_and_remove(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): log = AuditLogEntry.objects.get(organization_id=self.organization.id) - assert "added 2 member(s)" in log.data["replayAccessMembers"] - assert "removed 1 member(s)" in log.data["replayAccessMembers"] - assert "total: 2 member(s)" in log.data["replayAccessMembers"] + assert "added 2 user(s)" in log.data["replayAccessMembers"] + assert "removed 1 user(s)" in log.data["replayAccessMembers"] + assert "total: 2 user(s)" in log.data["replayAccessMembers"] @with_feature("organizations:granular-replay-permissions") def test_replay_access_members_clear_all(self) -> None: @@ -1639,14 +1639,14 @@ def test_replay_access_members_clear_all(self) -> None: with assume_test_silo_mode_of(AuditLogEntry): log = AuditLogEntry.objects.get(organization_id=self.organization.id) - assert "removed 1 member(s)" in log.data["replayAccessMembers"] - assert "total: 0 member(s)" in log.data["replayAccessMembers"] + assert "removed 1 user(s)" in log.data["replayAccessMembers"] + assert "total: 0 user(s)" in log.data["replayAccessMembers"] def test_replay_access_members_requires_feature(self) -> None: member1 = self.create_member( organization=self.organization, user=self.create_user(), role="member" ) - data = {"replayAccessMembers": [member1.id]} + data = {"replayAccessMembers": [member1.user_id]} self.get_error_response(self.organization.slug, **data, status_code=404) @with_feature("organizations:granular-replay-permissions") @@ -1660,11 +1660,11 @@ def test_replay_access_members_requires_admin_scope(self) -> None: other_member = self.create_member( organization=self.organization, user=self.create_user(), role="member" ) - data = {"replayAccessMembers": [other_member.id]} + data = {"replayAccessMembers": [other_member.user_id]} self.get_error_response(self.organization.slug, **data, status_code=403) @with_feature("organizations:granular-replay-permissions") - def test_replay_access_members_invalid_member_ids(self) -> None: + def test_replay_access_members_invalid_user_ids(self) -> None: nonexistent_id = 999999999 data = {"replayAccessMembers": [nonexistent_id]} response = self.get_error_response(self.organization.slug, **data, status_code=400) @@ -1677,10 +1677,10 @@ def test_replay_access_members_from_other_organization(self) -> None: other_org_member = self.create_member( organization=other_org, user=self.create_user(), role="member" ) - data = {"replayAccessMembers": [other_org_member.id]} + data = {"replayAccessMembers": [other_org_member.user_id]} response = self.get_error_response(self.organization.slug, **data, status_code=400) assert "replayAccessMembers" in response.data - assert str(other_org_member.id) in response.data["replayAccessMembers"] + assert str(other_org_member.user_id) in response.data["replayAccessMembers"] @with_feature("organizations:granular-replay-permissions") def test_replay_access_members_mixed_valid_and_invalid(self) -> None: @@ -1688,11 +1688,11 @@ def test_replay_access_members_mixed_valid_and_invalid(self) -> None: organization=self.organization, user=self.create_user(), role="member" ) nonexistent_id = 999999999 - data = {"replayAccessMembers": [valid_member.id, nonexistent_id]} + data = {"replayAccessMembers": [valid_member.user_id, nonexistent_id]} response = self.get_error_response(self.organization.slug, **data, status_code=400) assert "replayAccessMembers" in response.data assert str(nonexistent_id) in response.data["replayAccessMembers"] - assert str(valid_member.id) not in response.data["replayAccessMembers"] + assert str(valid_member.user_id) not in response.data["replayAccessMembers"] access_count = OrganizationMemberReplayAccess.objects.filter( organizationmember__organization=self.organization From ca3af66ee9430a9712d6d91750053adc91b55edb Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 17:17:05 +0100 Subject: [PATCH 15/37] fix type issue and make sure we dont have audit log if unchanged+ --- .../api/serializers/models/organization.py | 8 +++-- .../core/endpoints/organization_details.py | 10 ++++-- .../endpoints/test_organization_details.py | 33 +++++++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 28e0f8445f1cfa..905020c9ef6897 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -764,11 +764,13 @@ def serialize( # type: ignore[override] .first() ) context["hasGranularReplayPermissions"] = bool(permissions_enabled) - context["replayAccessMembers"] = list( - OrganizationMemberReplayAccess.objects.filter( + context["replayAccessMembers"] = [ + user_id + for user_id in OrganizationMemberReplayAccess.objects.filter( organizationmember__organization=obj ).values_list("organizationmember__user_id", flat=True) - ) + if user_id is not None + ] if has_custom_dynamic_sampling(obj, actor=user): context["targetSampleRate"] = float( diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index ef7562a751862f..d3701ea447d03e 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -622,10 +622,16 @@ def save(self, **kwargs): if "hasGranularReplayPermissions" in data: option_key = "sentry:granular-replay-permissions" new_value = data["hasGranularReplayPermissions"] - option_inst, created = OrganizationOption.objects.update_or_create( + option_inst, created = OrganizationOption.objects.get_or_create( organization=org, key=option_key, defaults={"value": new_value} ) - changed_data["hasGranularReplayPermissions"] = f"to {new_value}" + if not created and option_inst.value != new_value: + old_val = option_inst.value + option_inst.value = new_value + option_inst.save() + changed_data["hasGranularReplayPermissions"] = f"from {old_val} to {new_value}" + elif created: + changed_data["hasGranularReplayPermissions"] = f"to {new_value}" if "replayAccessMembers" in data: user_ids = data["replayAccessMembers"] diff --git a/tests/sentry/core/endpoints/test_organization_details.py b/tests/sentry/core/endpoints/test_organization_details.py index 9c9e22d19f2b91..76a3965eb9e28b 100644 --- a/tests/sentry/core/endpoints/test_organization_details.py +++ b/tests/sentry/core/endpoints/test_organization_details.py @@ -1515,6 +1515,39 @@ def test_granular_replay_permissions_flag_unset(self) -> None: assert "to False" in log.data["hasGranularReplayPermissions"] + @with_feature("organizations:granular-replay-permissions") + def test_granular_replay_permissions_no_spurious_audit_log(self) -> None: + self.organization.update_option("sentry:granular-replay-permissions", True) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"hasGranularReplayPermissions": True} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + with assume_test_silo_mode_of(AuditLogEntry): + audit_logs = AuditLogEntry.objects.filter(organization_id=self.organization.id) + assert audit_logs.count() == 0 + + @with_feature("organizations:granular-replay-permissions") + def test_granular_replay_permissions_change_logs_old_value(self) -> None: + self.organization.update_option("sentry:granular-replay-permissions", False) + with assume_test_silo_mode_of(AuditLogEntry): + AuditLogEntry.objects.filter(organization_id=self.organization.id).delete() + + data = {"hasGranularReplayPermissions": True} + with outbox_runner(): + self.get_success_response(self.organization.slug, **data) + + option_value = OrganizationOption.objects.get( + organization=self.organization, key="sentry:granular-replay-permissions" + ) + assert option_value.value is True + + with assume_test_silo_mode_of(AuditLogEntry): + log = AuditLogEntry.objects.get(organization_id=self.organization.id) + assert log.data["hasGranularReplayPermissions"] == "from False to True" + def test_granular_replay_permissions_flag_requires_feature(self) -> None: data = {"hasGranularReplayPermissions": True} self.get_error_response(self.organization.slug, **data, status_code=404) From 4a5f3d75bb467d4f3296b3c55dbb7ca7e129093b Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 11:24:31 +0100 Subject: [PATCH 16/37] move query from serialize to get_attrs --- .../api/serializers/models/organization.py | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 905020c9ef6897..63c2ad6def59cd 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -575,7 +575,27 @@ class DetailedOrganizationSerializer(OrganizationSerializer): def get_attrs( self, item_list: Sequence[Organization], user: User | RpcUser | AnonymousUser, **kwargs: Any ) -> MutableMapping[Organization, MutableMapping[str, Any]]: - return super().get_attrs(item_list, user) + attrs = super().get_attrs(item_list, user) + + replay_permissions = { + opt.organization_id: opt.value + for opt in OrganizationOption.objects.filter( + organization__in=item_list, key="sentry:granular-replay-permissions" + ) + } + + replay_access_by_org: dict[int, list[int]] = {} + for org_id, user_id in OrganizationMemberReplayAccess.objects.filter( + organizationmember__organization__in=item_list + ).values_list("organizationmember__organization_id", "organizationmember__user_id"): + if user_id is not None: + replay_access_by_org.setdefault(org_id, []).append(user_id) + + for item in item_list: + attrs[item]["replay_permissions_enabled"] = replay_permissions.get(item.id, False) + attrs[item]["replay_access_members"] = replay_access_by_org.get(item.id, []) + + return attrs def serialize( # type: ignore[override] self, @@ -756,21 +776,8 @@ def serialize( # type: ignore[override] } if features.has("organizations:granular-replay-permissions", obj): - permissions_enabled = ( - OrganizationOption.objects.filter( - organization=obj, key="sentry:granular-replay-permissions" - ) - .values_list("value", flat=True) - .first() - ) - context["hasGranularReplayPermissions"] = bool(permissions_enabled) - context["replayAccessMembers"] = [ - user_id - for user_id in OrganizationMemberReplayAccess.objects.filter( - organizationmember__organization=obj - ).values_list("organizationmember__user_id", flat=True) - if user_id is not None - ] + context["hasGranularReplayPermissions"] = bool(attrs.get("replay_permissions_enabled")) + context["replayAccessMembers"] = attrs.get("replay_access_members", []) if has_custom_dynamic_sampling(obj, actor=user): context["targetSampleRate"] = float( From d41251303606e0d0df0a33766923f65ee1a64c44 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 12:24:48 +0100 Subject: [PATCH 17/37] add feature check to get_attrs --- .../api/serializers/models/organization.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 63c2ad6def59cd..9df883ffba8423 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -577,23 +577,25 @@ def get_attrs( ) -> MutableMapping[Organization, MutableMapping[str, Any]]: attrs = super().get_attrs(item_list, user) - replay_permissions = { - opt.organization_id: opt.value - for opt in OrganizationOption.objects.filter( - organization__in=item_list, key="sentry:granular-replay-permissions" - ) - } + replay_permissions = {} + if features.has("organizations:granular-replay-permissions", actor=user): + replay_permissions = { + opt.organization_id: opt.value + for opt in OrganizationOption.objects.filter( + organization__in=item_list, key="sentry:granular-replay-permissions" + ) + } - replay_access_by_org: dict[int, list[int]] = {} - for org_id, user_id in OrganizationMemberReplayAccess.objects.filter( - organizationmember__organization__in=item_list - ).values_list("organizationmember__organization_id", "organizationmember__user_id"): - if user_id is not None: - replay_access_by_org.setdefault(org_id, []).append(user_id) + replay_access_by_org: dict[int, list[int]] = {} + for org_id, user_id in OrganizationMemberReplayAccess.objects.filter( + organizationmember__organization__in=item_list + ).values_list("organizationmember__organization_id", "organizationmember__user_id"): + if user_id is not None: + replay_access_by_org.setdefault(org_id, []).append(user_id) - for item in item_list: - attrs[item]["replay_permissions_enabled"] = replay_permissions.get(item.id, False) - attrs[item]["replay_access_members"] = replay_access_by_org.get(item.id, []) + for item in item_list: + attrs[item]["replay_permissions_enabled"] = replay_permissions.get(item.id, False) + attrs[item]["replay_access_members"] = replay_access_by_org.get(item.id, []) return attrs From d913c8001c6a55b256c525d9723b05f4c981fb14 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 12:26:10 +0100 Subject: [PATCH 18/37] add feature check to get_attrs --- .../api/serializers/models/organization.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 9df883ffba8423..5a4c0e5b6c8071 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -586,16 +586,23 @@ def get_attrs( ) } + # Only process replay access data if replay_permissions is enabled for at least one org + enabled_org_ids = [org_id for org_id, enabled in replay_permissions.items() if enabled] replay_access_by_org: dict[int, list[int]] = {} - for org_id, user_id in OrganizationMemberReplayAccess.objects.filter( - organizationmember__organization__in=item_list - ).values_list("organizationmember__organization_id", "organizationmember__user_id"): - if user_id is not None: - replay_access_by_org.setdefault(org_id, []).append(user_id) + if enabled_org_ids: + for org_id, user_id in OrganizationMemberReplayAccess.objects.filter( + organizationmember__organization__in=enabled_org_ids + ).values_list("organizationmember__organization_id", "organizationmember__user_id"): + if user_id is not None: + replay_access_by_org.setdefault(org_id, []).append(user_id) for item in item_list: attrs[item]["replay_permissions_enabled"] = replay_permissions.get(item.id, False) - attrs[item]["replay_access_members"] = replay_access_by_org.get(item.id, []) + attrs[item]["replay_access_members"] = ( + replay_access_by_org.get(item.id, []) + if replay_permissions.get(item.id, False) + else [] + ) return attrs From d899cbd6f8addf04f0680ba4385b7d778eabe458 Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 11:29:07 +0000 Subject: [PATCH 19/37] :hammer_and_wrench: apply pre-commit fixes --- src/sentry/api/serializers/models/organization.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 5a4c0e5b6c8071..48002d22b54922 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -88,10 +88,7 @@ if TYPE_CHECKING: from sentry.api.serializers.models.project import OrganizationProjectResponse - from sentry.users.api.serializers.user import ( - UserSerializerResponse, - UserSerializerResponseSelf, - ) + from sentry.users.api.serializers.user import UserSerializerResponse, UserSerializerResponseSelf # This cut-off date ensures that only new organizations created after this date go # through the logic that checks for the 'onboarding:complete' key in OrganizationOption. From c6acc4cb6e57970c177c7ed22cb029998dbe6159 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 12:49:05 +0100 Subject: [PATCH 20/37] add batch feature check for orgs --- src/sentry/api/serializers/models/organization.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 48002d22b54922..24c7e103c81e92 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -575,7 +575,10 @@ def get_attrs( attrs = super().get_attrs(item_list, user) replay_permissions = {} - if features.has("organizations:granular-replay-permissions", actor=user): + has_feature = features.batch_has_for_organizations( + "organizations:granular-replay-permissions", item_list + ) + if any(has_feature.values()): replay_permissions = { opt.organization_id: opt.value for opt in OrganizationOption.objects.filter( From 562875430a93b95b303783daff02e7fc2d75801c Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 13:01:05 +0100 Subject: [PATCH 21/37] add fallback if the feature is disabled --- src/sentry/api/serializers/models/organization.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 24c7e103c81e92..25ce1b2a292493 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -603,6 +603,10 @@ def get_attrs( if replay_permissions.get(item.id, False) else [] ) + else: + for item in item_list: + attrs[item]["replay_permissions_enabled"] = False + attrs[item]["replay_access_members"] = [] return attrs From 459a5dd6ae949dc98619639238db250df4f7c1f2 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 13:19:22 +0100 Subject: [PATCH 22/37] fix tests & add permission fallback --- .../api/serializers/models/organization.py | 2 +- .../api/serializers/test_organization.py | 27 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 25ce1b2a292493..52b34576bbe9a6 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -578,7 +578,7 @@ def get_attrs( has_feature = features.batch_has_for_organizations( "organizations:granular-replay-permissions", item_list ) - if any(has_feature.values()): + if has_feature and any(has_feature.values()): replay_permissions = { opt.organization_id: opt.value for opt in OrganizationOption.objects.filter( diff --git a/tests/sentry/api/serializers/test_organization.py b/tests/sentry/api/serializers/test_organization.py index 7f6b099fd914d1..0033291e3b4555 100644 --- a/tests/sentry/api/serializers/test_organization.py +++ b/tests/sentry/api/serializers/test_organization.py @@ -211,8 +211,33 @@ def test_replay_access_members_serialized(self) -> None: with self.feature("organizations:granular-replay-permissions"): serializer = DetailedOrganizationSerializer() result = serialize(organization, user, serializer, access=acc) + assert set(result["replayAccessMembers"]) == set() - assert set(result["replayAccessMembers"]) == {member1.user_id, member2.user_id} + def test_replay_access_members_serialized_with_option_enabled(self) -> None: + user = self.create_user() + organization = self.create_organization(owner=user) + member1 = self.create_member( + organization=organization, user=self.create_user(), role="member" + ) + member2 = self.create_member( + organization=organization, user=self.create_user(), role="member" + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member1) + OrganizationMemberReplayAccess.objects.create(organizationmember=member2) + acc = access.from_user(user, organization) + + with self.feature("organizations:granular-replay-permissions"): + # Set the org option to enable granular replay permissions + OrganizationOption.objects.set_value( + organization=organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + serializer = DetailedOrganizationSerializer() + result = serialize(organization, user, serializer, access=acc) + assert result["hasGranularReplayPermissions"] is True + assert set(result["replayAccessMembers"]) == {member1.id, member2.id} def test_replay_access_members_empty_when_none_set(self) -> None: user = self.create_user() From 3bd0e6ae60bfa845597b17a79d28de8c309e2208 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 13:38:32 +0100 Subject: [PATCH 23/37] feat(replay): add model to allow per-user access control for replays --- .../api/serializers/models/organization.py | 6 +- .../core/endpoints/organization_details.py | 127 +++++++++--------- .../1012_organizationmember_replay_access.py | 64 +++++++++ src/sentry/models/__init__.py | 1 + .../models/organizationmemberreplayaccess.py | 34 +++++ src/sentry/testutils/helpers/backups.py | 10 ++ 6 files changed, 180 insertions(+), 62 deletions(-) create mode 100644 src/sentry/migrations/1012_organizationmember_replay_access.py create mode 100644 src/sentry/models/organizationmemberreplayaccess.py diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index 52b34576bbe9a6..c8682b6eebf155 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -76,6 +76,7 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationaccessrequest import OrganizationAccessRequest +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationonboardingtask import OrganizationOnboardingTask from sentry.models.project import Project from sentry.models.team import Team, TeamStatus @@ -88,7 +89,10 @@ if TYPE_CHECKING: from sentry.api.serializers.models.project import OrganizationProjectResponse - from sentry.users.api.serializers.user import UserSerializerResponse, UserSerializerResponseSelf + from sentry.users.api.serializers.user import ( + UserSerializerResponse, + UserSerializerResponseSelf, + ) # This cut-off date ensures that only new organizations created after this date go # through the logic that checks for the 'onboarding:complete' key in OrganizationOption. diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index d3701ea447d03e..ee5bb129103252 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -9,7 +9,11 @@ from django.db.models.query_utils import DeferredAttribute from django.urls import reverse from django.utils import timezone as django_timezone -from drf_spectacular.utils import OpenApiResponse, extend_schema, extend_schema_serializer +from drf_spectacular.utils import ( + OpenApiResponse, + extend_schema, + extend_schema_serializer, +) from rest_framework import serializers, status from rest_framework.exceptions import NotFound, PermissionDenied from sentry_sdk import capture_exception @@ -95,7 +99,6 @@ from sentry.models.options.organization_option import OrganizationOption from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus -from sentry.models.organizationmember import OrganizationMember from sentry.models.project import Project from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import ( @@ -103,7 +106,10 @@ RpcOrganizationDeleteResponse, RpcOrganizationDeleteState, ) -from sentry.relay.datascrubbing import validate_pii_config_update, validate_pii_selectors +from sentry.relay.datascrubbing import ( + validate_pii_config_update, + validate_pii_selectors, +) from sentry.replays.models import OrganizationMemberReplayAccess from sentry.seer.autofix.constants import AutofixAutomationTuningSettings from sentry.services.organization.provisioning import ( @@ -619,73 +625,72 @@ def save(self, **kwargs): if trusted_relay_info is not None: self.save_trusted_relays(trusted_relay_info, changed_data, org) - if "hasGranularReplayPermissions" in data: - option_key = "sentry:granular-replay-permissions" - new_value = data["hasGranularReplayPermissions"] - option_inst, created = OrganizationOption.objects.get_or_create( - organization=org, key=option_key, defaults={"value": new_value} - ) - if not created and option_inst.value != new_value: - old_val = option_inst.value - option_inst.value = new_value - option_inst.save() - changed_data["hasGranularReplayPermissions"] = f"from {old_val} to {new_value}" - elif created: - changed_data["hasGranularReplayPermissions"] = f"to {new_value}" - - if "replayAccessMembers" in data: - user_ids = data["replayAccessMembers"] - if user_ids is None: - user_ids = [] - - current_user_ids = set( - OrganizationMemberReplayAccess.objects.filter( - organizationmember__organization=org - ).values_list("organizationmember__user_id", flat=True) - ) - new_user_ids = set(user_ids) + if "hasGranularReplayPermissions" in data or "replayAccessMembers" in data: + if not features.has("organizations:granular-replay-permissions", org): + raise serializers.ValidationError( + { + "hasGranularReplayPermissions": "This feature is not enabled for your organization." + } + ) + + if not self.context["request"].access.has_scope("org:admin"): + raise serializers.ValidationError( + { + "hasGranularReplayPermissions": "You do not have permission to modify granular replay permissions." + } + ) - to_add = new_user_ids - current_user_ids - to_remove = current_user_ids - new_user_ids + if "hasGranularReplayPermissions" in data: + option_key = "sentry:granular-replay-permissions" + new_value = data["hasGranularReplayPermissions"] - if to_add: - user_to_member = dict( - OrganizationMember.objects.filter( - organization=org, user_id__in=to_add - ).values_list("user_id", "id") + option_inst, created = OrganizationOption.objects.update_or_create( + organization=org, key=option_key, defaults={"value": new_value} ) - invalid_user_ids = to_add - set(user_to_member.keys()) - if invalid_user_ids: - raise serializers.ValidationError( - { - "replayAccessMembers": f"Invalid user IDs (not members of this organization): {sorted(invalid_user_ids)}" - } - ) - OrganizationMemberReplayAccess.objects.bulk_create( - [ - OrganizationMemberReplayAccess( - organizationmember_id=user_to_member[user_id] - ) - for user_id in to_add - ], - ignore_conflicts=True, + if new_value or created: + changed_data["hasGranularReplayPermissions"] = f"to {new_value}" + + if "replayAccessMembers" in data: + member_ids = data["replayAccessMembers"] + + if member_ids is None: + member_ids = [] + + current_member_ids = set( + OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( + "organizationmember_id", flat=True + ) ) + new_member_ids = set(member_ids) - if to_remove: - OrganizationMemberReplayAccess.objects.filter( - organizationmember__organization=org, organizationmember__user_id__in=to_remove - ).delete() + to_add = new_member_ids - current_member_ids + to_remove = current_member_ids - new_member_ids - if to_add or to_remove: - changes = [] if to_add: - changes.append(f"added {len(to_add)} user(s)") + OrganizationMemberReplayAccess.objects.bulk_create( + [ + OrganizationMemberReplayAccess( + organization=org, organizationmember_id=member_id + ) + for member_id in to_add + ] + ) + if to_remove: - changes.append(f"removed {len(to_remove)} user(s)") - changed_data["replayAccessMembers"] = ( - f"{' and '.join(changes)} (total: {len(new_user_ids)} user(s) with access)" - ) + OrganizationMemberReplayAccess.objects.filter( + organization=org, organizationmember_id__in=to_remove + ).delete() + + if to_add or to_remove: + changes = [] + if to_add: + changes.append(f"added {len(to_add)} member(s)") + if to_remove: + changes.append(f"removed {len(to_remove)} member(s)") + changed_data["replayAccessMembers"] = ( + f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" + ) if "openMembership" in data: org.flags.allow_joinleave = data["openMembership"] diff --git a/src/sentry/migrations/1012_organizationmember_replay_access.py b/src/sentry/migrations/1012_organizationmember_replay_access.py new file mode 100644 index 00000000000000..9dba851c1e91ea --- /dev/null +++ b/src/sentry/migrations/1012_organizationmember_replay_access.py @@ -0,0 +1,64 @@ +# Generated by Django 5.2.8 on 2025-12-02 12:31 + +import django.db.models.deletion +import django.utils.timezone +from django.db import migrations, models + +import sentry.db.models.fields.bounded +import sentry.db.models.fields.foreignkey +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "1011_update_oc_integration_cascade_to_null"), + ] + + operations = [ + migrations.CreateModel( + name="OrganizationMemberReplayAccess", + fields=[ + ( + "id", + sentry.db.models.fields.bounded.BoundedBigAutoField( + primary_key=True, serialize=False + ), + ), + ("date_added", models.DateTimeField(default=django.utils.timezone.now)), + ( + "organization", + sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="replay_access_set", + to="sentry.organization", + ), + ), + ( + "organizationmember", + sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="replay_access", + to="sentry.organizationmember", + ), + ), + ], + options={ + "db_table": "sentry_organizationmemberreplayaccess", + "unique_together": {("organization", "organizationmember")}, + }, + ), + ] diff --git a/src/sentry/models/__init__.py b/src/sentry/models/__init__.py index c34615e484f620..bccfd75ddb3a82 100644 --- a/src/sentry/models/__init__.py +++ b/src/sentry/models/__init__.py @@ -74,6 +74,7 @@ from .organizationmember import * # NOQA from .organizationmemberinvite import * # NOQA from .organizationmembermapping import * # NOQA +from .organizationmemberreplayaccess import * # NOQA from .organizationmemberteam import * # NOQA from .organizationmemberteamreplica import * # NOQA from .organizationonboardingtask import * # NOQA diff --git a/src/sentry/models/organizationmemberreplayaccess.py b/src/sentry/models/organizationmemberreplayaccess.py new file mode 100644 index 00000000000000..bb928aea29e7b6 --- /dev/null +++ b/src/sentry/models/organizationmemberreplayaccess.py @@ -0,0 +1,34 @@ +from __future__ import annotations + +from django.db import models +from django.utils import timezone + +from sentry.backup.scopes import RelocationScope +from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr + + +@region_silo_model +class OrganizationMemberReplayAccess(Model): + """ + Tracks which organization members have permission to access replay data. + + When no records exist for an organization, all members have access (default). + When records exist, only members with a record can access replays. + """ + + __relocation_scope__ = RelocationScope.Organization + + organization = FlexibleForeignKey("sentry.Organization", related_name="replay_access_set") + organizationmember = FlexibleForeignKey( + "sentry.OrganizationMember", + on_delete=models.CASCADE, + related_name="replay_access", + ) + date_added = models.DateTimeField(default=timezone.now) + + class Meta: + app_label = "sentry" + db_table = "sentry_organizationmemberreplayaccess" + unique_together = (("organization", "organizationmember"),) + + __repr__ = sane_repr("organization_id", "organizationmember_id") diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index 113ae113bb779e..b113de4ee07c1d 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -473,8 +473,18 @@ def create_exhaustive_organization( organization=org, key="sentry:scrape_javascript", value=True ) +<<<<<<< HEAD owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) OrganizationMemberReplayAccess.objects.create(organizationmember=owner_member) +======= + # OrganizationMemberReplayAccess - add for the owner member + from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess + + owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) + OrganizationMemberReplayAccess.objects.create( + organization=org, organizationmember=owner_member + ) +>>>>>>> 536c8a0c02b (feat(replay): add model to allow per-user access control for replays) # Team team = self.create_team(name=f"test_team_in_{slug}", organization=org) From e6f0cd57cdb5ebfac58353bded05cb691d6f8e79 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 15:03:38 +0100 Subject: [PATCH 24/37] feat(replay): guard endpoints by granular access permissions --- .../endpoints/organization_replay_count.py | 3 + .../endpoints/organization_replay_details.py | 9 +- .../endpoints/organization_replay_endpoint | 32 +++ .../organization_replay_events_meta.py | 4 + .../endpoints/organization_replay_index.py | 11 +- .../organization_replay_selector_index.py | 11 +- .../endpoints/project_replay_clicks_index.py | 11 +- .../endpoints/project_replay_details.py | 18 +- .../endpoints/project_replay_endpoint.py | 32 +++ .../endpoints/project_replay_jobs_delete.py | 13 +- ...roject_replay_recording_segment_details.py | 11 +- .../project_replay_recording_segment_index.py | 11 +- .../endpoints/project_replay_summary.py | 10 +- .../endpoints/project_replay_video_details.py | 11 +- .../endpoints/project_replay_viewed_by.py | 18 +- src/sentry/replays/permissions.py | 60 ++++++ .../test_replay_granular_permissions.py | 198 ++++++++++++++++++ tests/sentry/replays/test_permissions.py | 68 ++++++ 18 files changed, 464 insertions(+), 67 deletions(-) create mode 100644 src/sentry/replays/endpoints/organization_replay_endpoint create mode 100644 src/sentry/replays/endpoints/project_replay_endpoint.py create mode 100644 src/sentry/replays/permissions.py create mode 100644 tests/sentry/replays/endpoints/test_replay_granular_permissions.py create mode 100644 tests/sentry/replays/test_permissions.py diff --git a/src/sentry/replays/endpoints/organization_replay_count.py b/src/sentry/replays/endpoints/organization_replay_count.py index 54c64fdd89aacf..cda70e170226f9 100644 --- a/src/sentry/replays/endpoints/organization_replay_count.py +++ b/src/sentry/replays/endpoints/organization_replay_count.py @@ -21,6 +21,7 @@ from sentry.models.organization import Organization from sentry.models.project import Project from sentry.ratelimits.config import RateLimitConfig +from sentry.replays.permissions import has_replay_permission from sentry.replays.usecases.replay_counts import get_replay_counts from sentry.snuba.dataset import Dataset from sentry.types.ratelimit import RateLimit, RateLimitCategory @@ -84,6 +85,8 @@ def get(self, request: Request, organization: Organization) -> Response: """ if not features.has("organizations:session-replay", organization, actor=request.user): return Response(status=404) + if not has_replay_permission(organization, request.user): + return Response(status=403) try: snuba_params = self.get_snuba_params(request, organization) diff --git a/src/sentry/replays/endpoints/organization_replay_details.py b/src/sentry/replays/endpoints/organization_replay_details.py index f5109d5caf15d5..f78c326f17c656 100644 --- a/src/sentry/replays/endpoints/organization_replay_details.py +++ b/src/sentry/replays/endpoints/organization_replay_details.py @@ -20,13 +20,14 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.organization import NoProjects, OrganizationEndpoint +from sentry.api.bases.organization import NoProjects from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import GlobalParams, ReplayParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.constants import ALL_ACCESS_PROJECTS from sentry.models.organization import Organization +from sentry.replays.endpoints.organization_replay_endpoint import OrganizationReplayEndpoint from sentry.replays.lib.eap import read as eap_read from sentry.replays.lib.eap.snuba_transpiler import RequestMeta, Settings from sentry.replays.post_process import ReplayDetailsResponse, process_raw_response @@ -216,7 +217,7 @@ def query_replay_instance_eap( @region_silo_endpoint @extend_schema(tags=["Replays"]) -class OrganizationReplayDetailsEndpoint(OrganizationEndpoint): +class OrganizationReplayDetailsEndpoint(OrganizationReplayEndpoint): """ The same data as ProjectReplayDetails, except no project is required. This works as we'll query for this replay_id across all projects in the @@ -243,8 +244,8 @@ def get(self, request: Request, organization: Organization, replay_id: str) -> R """ Return details on an individual replay. """ - if not features.has("organizations:session-replay", organization, actor=request.user): - return Response(status=404) + if response := self.check_replay_access(request, organization): + return response try: filter_params = self.get_filter_params( diff --git a/src/sentry/replays/endpoints/organization_replay_endpoint b/src/sentry/replays/endpoints/organization_replay_endpoint new file mode 100644 index 00000000000000..43a62866341707 --- /dev/null +++ b/src/sentry/replays/endpoints/organization_replay_endpoint @@ -0,0 +1,32 @@ +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry import features +from sentry.api.bases.organization import OrganizationEndpoint +from sentry.models.organization import Organization +from sentry.replays.permissions import has_replay_permission + + +class OrganizationReplayEndpoint(OrganizationEndpoint): + """ + Base endpoint for replay-related organizationendpoints. + Provides centralized feature and permission checks for session replay access. + Added to ensure that all replay endpoints are consistent and follow the same pattern + for allowing granular user-based replay access control, in addition to the existing + role-based access control and feature flag-based access control. + """ + + def check_replay_access(self, request: Request, organization: Organization) -> Response | None: + """ + Check if the session replay feature is enabled and user has replay permissions. + Returns a Response object if access should be denied, None if access is granted. + """ + if not features.has( + "organizations:session-replay", organization, actor=request.user + ): + return Response(status=404) + + if not has_replay_permission(organization, request.user): + return Response(status=403) + + return None diff --git a/src/sentry/replays/endpoints/organization_replay_events_meta.py b/src/sentry/replays/endpoints/organization_replay_events_meta.py index 8a7fcc4a223af6..db3a8ef74ce6dd 100644 --- a/src/sentry/replays/endpoints/organization_replay_events_meta.py +++ b/src/sentry/replays/endpoints/organization_replay_events_meta.py @@ -12,6 +12,7 @@ from sentry.api.paginator import GenericOffsetPaginator from sentry.api.utils import reformat_timestamp_ms_to_isoformat from sentry.models.organization import Organization +from sentry.replays.permissions import has_replay_permission @region_silo_endpoint @@ -53,6 +54,9 @@ def get(self, request: Request, organization: Organization) -> Response: if not features.has("organizations:session-replay", organization, actor=request.user): return Response(status=404) + if not has_replay_permission(organization, request.user): + return Response(status=403) + try: snuba_params = self.get_snuba_params(request, organization) except NoProjects: diff --git a/src/sentry/replays/endpoints/organization_replay_index.py b/src/sentry/replays/endpoints/organization_replay_index.py index 5430938f8b7cd3..1a2356c2a326a0 100644 --- a/src/sentry/replays/endpoints/organization_replay_index.py +++ b/src/sentry/replays/endpoints/organization_replay_index.py @@ -6,11 +6,10 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.organization import NoProjects, OrganizationEndpoint +from sentry.api.bases.organization import NoProjects from sentry.api.event_search import parse_search_query from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN from sentry.apidocs.examples.replay_examples import ReplayExamples @@ -18,6 +17,7 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.exceptions import InvalidSearchQuery from sentry.models.organization import Organization +from sentry.replays.endpoints.organization_replay_endpoint import OrganizationReplayEndpoint from sentry.replays.post_process import ReplayDetailsResponse, process_raw_response from sentry.replays.query import query_replays_collection_paginated, replay_url_parser_config from sentry.replays.usecases.errors import handled_snuba_exceptions @@ -28,7 +28,7 @@ @region_silo_endpoint @extend_schema(tags=["Replays"]) -class OrganizationReplayIndexEndpoint(OrganizationEndpoint): +class OrganizationReplayIndexEndpoint(OrganizationReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.PUBLIC, @@ -50,8 +50,9 @@ def get(self, request: Request, organization: Organization) -> Response: Return a list of replays belonging to an organization. """ - if not features.has("organizations:session-replay", organization, actor=request.user): - return Response(status=404) + if response := self.check_replay_access(request, organization): + return response + try: filter_params = self.get_filter_params(request, organization) except NoProjects: diff --git a/src/sentry/replays/endpoints/organization_replay_selector_index.py b/src/sentry/replays/endpoints/organization_replay_selector_index.py index f24bb97eb6921f..92307ce65b08ae 100644 --- a/src/sentry/replays/endpoints/organization_replay_selector_index.py +++ b/src/sentry/replays/endpoints/organization_replay_selector_index.py @@ -23,11 +23,10 @@ ) from snuba_sdk import Request as SnubaRequest -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.organization import NoProjects, OrganizationEndpoint +from sentry.api.bases.organization import NoProjects from sentry.api.event_search import QueryToken, parse_search_query from sentry.api.paginator import GenericOffsetPaginator from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN @@ -36,6 +35,7 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.exceptions import InvalidSearchQuery from sentry.models.organization import Organization +from sentry.replays.endpoints.organization_replay_endpoint import OrganizationReplayEndpoint from sentry.replays.lib.new_query.conditions import IntegerScalar from sentry.replays.lib.new_query.fields import FieldProtocol, IntegerColumnField from sentry.replays.lib.new_query.parsers import parse_int @@ -75,7 +75,7 @@ class ReplaySelectorResponse(TypedDict): @region_silo_endpoint @extend_schema(tags=["Replays"]) -class OrganizationReplaySelectorIndexEndpoint(OrganizationEndpoint): +class OrganizationReplaySelectorIndexEndpoint(OrganizationReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.PUBLIC, @@ -106,8 +106,9 @@ def get_replay_filter_params(self, request, organization): ) def get(self, request: Request, organization: Organization) -> Response: """Return a list of selectors for a given organization.""" - if not features.has("organizations:session-replay", organization, actor=request.user): - return Response(status=404) + if response := self.check_replay_access(request, organization): + return response + try: filter_params = self.get_replay_filter_params(request, organization) except NoProjects: diff --git a/src/sentry/replays/endpoints/project_replay_clicks_index.py b/src/sentry/replays/endpoints/project_replay_clicks_index.py index 78e42a0bdac095..119e009fda4b52 100644 --- a/src/sentry/replays/endpoints/project_replay_clicks_index.py +++ b/src/sentry/replays/endpoints/project_replay_clicks_index.py @@ -24,11 +24,9 @@ ) from snuba_sdk.orderby import Direction -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint from sentry.api.event_search import ParenExpression, QueryToken, SearchFilter, parse_search_query from sentry.api.paginator import GenericOffsetPaginator from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND @@ -37,6 +35,7 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.exceptions import InvalidSearchQuery from sentry.models.project import Project +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.new_query.errors import CouldNotParseValue, OperatorNotSupported from sentry.replays.lib.new_query.fields import FieldProtocol from sentry.replays.lib.query import attempt_compressed_condition @@ -58,7 +57,7 @@ class ReplayClickResponse(TypedDict): @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplayClicksIndexEndpoint(ProjectEndpoint): +class ProjectReplayClicksIndexEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.PUBLIC, @@ -85,10 +84,8 @@ class ProjectReplayClicksIndexEndpoint(ProjectEndpoint): ) def get(self, request: Request, project: Project, replay_id: str) -> Response: """Retrieve a collection of RRWeb DOM node-ids and the timestamp they were clicked.""" - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return Response(status=404) + if response := self.check_replay_access(request, project): + return response filter_params = self.get_filter_params(request, project) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 87ca10c29cf2e3..55d35795a06bfe 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -8,10 +8,11 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint, ProjectPermission +from sentry.api.bases.project import ProjectPermission from sentry.apidocs.constants import RESPONSE_NO_CONTENT, RESPONSE_NOT_FOUND from sentry.apidocs.parameters import GlobalParams, ReplayParams from sentry.models.project import Project +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.post_process import process_raw_response from sentry.replays.query import query_replay_instance from sentry.replays.tasks import delete_replay @@ -29,7 +30,7 @@ class ReplayDetailsPermission(ProjectPermission): @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplayDetailsEndpoint(ProjectEndpoint): +class ProjectReplayDetailsEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "DELETE": ApiPublishStatus.PUBLIC, @@ -39,10 +40,8 @@ class ProjectReplayDetailsEndpoint(ProjectEndpoint): permission_classes = (ReplayDetailsPermission,) def get(self, request: Request, project: Project, replay_id: str) -> Response: - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return Response(status=404) + if response := self.check_replay_access(request, project): + return response filter_params = self.get_filter_params(request, project) @@ -87,11 +86,8 @@ def delete(self, request: Request, project: Project, replay_id: str) -> Response """ Delete a replay. """ - - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return Response(status=404) + if response := self.check_replay_access(request, project): + return response if has_archived_segment(project.id, replay_id): return Response(status=404) diff --git a/src/sentry/replays/endpoints/project_replay_endpoint.py b/src/sentry/replays/endpoints/project_replay_endpoint.py new file mode 100644 index 00000000000000..ef968c2b56cc47 --- /dev/null +++ b/src/sentry/replays/endpoints/project_replay_endpoint.py @@ -0,0 +1,32 @@ +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry import features +from sentry.api.bases.project import ProjectEndpoint +from sentry.models.project import Project +from sentry.replays.permissions import has_replay_permission + + +class ProjectReplayEndpoint(ProjectEndpoint): + """ + Base endpoint for replay-related endpoints. + Provides centralized feature and permission checks for session replay access. + Added to ensure that all replay endpoints are consistent and follow the same pattern + for allowing granular user-based replay access control, in addition to the existing + role-based access control and feature flag-based access control. + """ + + def check_replay_access(self, request: Request, project: Project) -> Response | None: + """ + Check if the session replay feature is enabled and user has replay permissions. + Returns a Response object if access should be denied, None if access is granted. + """ + if not features.has( + "organizations:session-replay", project.organization, actor=request.user + ): + return Response(status=404) + + if not has_replay_permission(project.organization, request.user): + return Response(status=403) + + return None diff --git a/src/sentry/replays/endpoints/project_replay_jobs_delete.py b/src/sentry/replays/endpoints/project_replay_jobs_delete.py index e3d73ec93b066b..52a922e44c6f74 100644 --- a/src/sentry/replays/endpoints/project_replay_jobs_delete.py +++ b/src/sentry/replays/endpoints/project_replay_jobs_delete.py @@ -10,7 +10,9 @@ from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.paginator import OffsetPaginator from sentry.api.serializers import Serializer, serialize +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.models import ReplayDeletionJobModel +from sentry.replays.permissions import has_replay_permission from sentry.replays.tasks import run_bulk_replay_delete_job @@ -67,6 +69,9 @@ def get(self, request: Request, project) -> Response: """ Retrieve a collection of replay delete jobs. """ + if not has_replay_permission(project.organization, request.user): + return Response(status=403) + queryset = ReplayDeletionJobModel.objects.filter( organization_id=project.organization_id, project_id=project.id ) @@ -85,6 +90,9 @@ def post(self, request: Request, project) -> Response: """ Create a new replay deletion job. """ + if not has_replay_permission(project.organization, request.user): + return Response(status=403) + serializer = ReplayDeletionJobCreateSerializer(data=request.data) if not serializer.is_valid(): return Response(serializer.errors, status=400) @@ -124,7 +132,7 @@ def post(self, request: Request, project) -> Response: @region_silo_endpoint -class ProjectReplayDeletionJobDetailEndpoint(ProjectEndpoint): +class ProjectReplayDeletionJobDetailEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.PRIVATE, @@ -135,6 +143,9 @@ def get(self, request: Request, project, job_id: int) -> Response: """ Fetch a replay delete job instance. """ + if response := self.check_replay_access(request, project): + return response + try: job = ReplayDeletionJobModel.objects.get( id=job_id, organization_id=project.organization_id, project_id=project.id diff --git a/src/sentry/replays/endpoints/project_replay_recording_segment_details.py b/src/sentry/replays/endpoints/project_replay_recording_segment_details.py index 95b7bf37bebd28..ee7c62067c3940 100644 --- a/src/sentry/replays/endpoints/project_replay_recording_segment_details.py +++ b/src/sentry/replays/endpoints/project_replay_recording_segment_details.py @@ -7,22 +7,21 @@ from drf_spectacular.utils import extend_schema from rest_framework.request import Request -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import GlobalParams, ReplayParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.storage import RecordingSegmentStorageMeta, make_recording_filename from sentry.replays.usecases.reader import download_segment, fetch_segment_metadata @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplayRecordingSegmentDetailsEndpoint(ProjectEndpoint): +class ProjectReplayRecordingSegmentDetailsEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.PUBLIC, @@ -48,10 +47,8 @@ class ProjectReplayRecordingSegmentDetailsEndpoint(ProjectEndpoint): ) def get(self, request: Request, project, replay_id, segment_id) -> HttpResponseBase: """Return a replay recording segment.""" - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return self.respond(status=404) + if response := self.check_replay_access(request, project): + return response segment = fetch_segment_metadata(project.id, replay_id, int(segment_id)) if not segment: diff --git a/src/sentry/replays/endpoints/project_replay_recording_segment_index.py b/src/sentry/replays/endpoints/project_replay_recording_segment_index.py index 0f53dcd75ecbd8..d14b0e43ffbec2 100644 --- a/src/sentry/replays/endpoints/project_replay_recording_segment_index.py +++ b/src/sentry/replays/endpoints/project_replay_recording_segment_index.py @@ -6,23 +6,22 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint from sentry.api.paginator import GenericOffsetPaginator from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import CursorQueryParam, GlobalParams, ReplayParams, VisibilityParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.storage import storage from sentry.replays.usecases.reader import download_segments, fetch_segments_metadata @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplayRecordingSegmentIndexEndpoint(ProjectEndpoint): +class ProjectReplayRecordingSegmentIndexEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.PUBLIC, @@ -53,10 +52,8 @@ def __init__(self, **options) -> None: ) def get(self, request: Request, project, replay_id: str) -> Response: """Return a collection of replay recording segments.""" - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return self.respond(status=404) + if response := self.check_replay_access(request, project): + return response return self.paginate( request=request, diff --git a/src/sentry/replays/endpoints/project_replay_summary.py b/src/sentry/replays/endpoints/project_replay_summary.py index 542da6aed0784f..d074ca6b5f28e5 100644 --- a/src/sentry/replays/endpoints/project_replay_summary.py +++ b/src/sentry/replays/endpoints/project_replay_summary.py @@ -12,9 +12,10 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint, ProjectPermission +from sentry.api.bases.project import ProjectPermission from sentry.api.utils import default_start_end_dates from sentry.models.project import Project +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.seer_api import seer_summarization_connection_pool from sentry.replays.lib.storage import storage from sentry.replays.post_process import process_raw_response @@ -44,7 +45,7 @@ class ReplaySummaryPermission(ProjectPermission): @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplaySummaryEndpoint(ProjectEndpoint): +class ProjectReplaySummaryEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, @@ -127,6 +128,8 @@ def get(self, request: Request, project: Project, replay_id: str) -> Response: {"sample_rate": self.sample_rate_get} if self.sample_rate_get else None ), ): + if response := self.check_replay_access(request, project): + return response if not self.has_replay_summary_access(project, request): return self.respond( @@ -154,6 +157,9 @@ def post(self, request: Request, project: Project, replay_id: str) -> Response: {"sample_rate": self.sample_rate_post} if self.sample_rate_post else None ), ): + if response := self.check_replay_access(request, project): + return response + if not self.has_replay_summary_access(project, request): return self.respond( {"detail": "Replay summaries are not available for this organization."}, diff --git a/src/sentry/replays/endpoints/project_replay_video_details.py b/src/sentry/replays/endpoints/project_replay_video_details.py index 3dcf4d6e66e01e..7f5439811001aa 100644 --- a/src/sentry/replays/endpoints/project_replay_video_details.py +++ b/src/sentry/replays/endpoints/project_replay_video_details.py @@ -8,15 +8,14 @@ from drf_spectacular.utils import extend_schema from rest_framework.request import Request -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import GlobalParams, ReplayParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.http import ( MalformedRangeHeader, UnsatisfiableRange, @@ -32,7 +31,7 @@ @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplayVideoDetailsEndpoint(ProjectEndpoint): +class ProjectReplayVideoDetailsEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, @@ -56,10 +55,8 @@ class ProjectReplayVideoDetailsEndpoint(ProjectEndpoint): ) def get(self, request: Request, project, replay_id, segment_id) -> HttpResponseBase: """Return a replay video.""" - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return self.respond(status=404) + if response := self.check_replay_access(request, project): + return response segment = fetch_segment_metadata(project.id, replay_id, int(segment_id)) if not segment: diff --git a/src/sentry/replays/endpoints/project_replay_viewed_by.py b/src/sentry/replays/endpoints/project_replay_viewed_by.py index 84180d2c454f73..d9a9c371bfd7cd 100644 --- a/src/sentry/replays/endpoints/project_replay_viewed_by.py +++ b/src/sentry/replays/endpoints/project_replay_viewed_by.py @@ -6,16 +6,16 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint, ProjectEventPermission +from sentry.api.bases.project import ProjectEventPermission from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import GlobalParams, ReplayParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.models.project import Project +from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.query import query_replay_viewed_by_ids from sentry.replays.usecases.events import publish_replay_event, viewed_event from sentry.replays.usecases.query import execute_query, make_full_aggregation_query @@ -33,7 +33,7 @@ class ReplayViewedByResponse(TypedDict): @region_silo_endpoint @extend_schema(tags=["Replays"]) -class ProjectReplayViewedByEndpoint(ProjectEndpoint): +class ProjectReplayViewedByEndpoint(ProjectReplayEndpoint): owner = ApiOwner.REPLAY publish_status = {"GET": ApiPublishStatus.PUBLIC, "POST": ApiPublishStatus.PRIVATE} permission_classes = (ProjectEventPermission,) @@ -55,10 +55,8 @@ class ProjectReplayViewedByEndpoint(ProjectEndpoint): ) def get(self, request: Request, project: Project, replay_id: str) -> Response: """Return a list of users who have viewed a replay.""" - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return Response(status=404) + if response := self.check_replay_access(request, project): + return response try: uuid.UUID(replay_id) @@ -98,10 +96,8 @@ def post(self, request: Request, project: Project, replay_id: str) -> Response: if not request.user.is_authenticated: return Response(status=400) - if not features.has( - "organizations:session-replay", project.organization, actor=request.user - ): - return Response(status=404) + if response := self.check_replay_access(request, project): + return response try: replay_id = str(uuid.UUID(replay_id)) diff --git a/src/sentry/replays/permissions.py b/src/sentry/replays/permissions.py new file mode 100644 index 00000000000000..506faefaf012ea --- /dev/null +++ b/src/sentry/replays/permissions.py @@ -0,0 +1,60 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from django.contrib.auth.models import AnonymousUser + +from sentry import features +from sentry.models.organizationmember import OrganizationMember +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess +from sentry.models.organizationoption import OrganizationOption + +if TYPE_CHECKING: + from sentry.models.organization import Organization + from sentry.users.models.user import User + + +def has_replay_permission(organization: Organization, user: User | AnonymousUser | None) -> bool: + """ + Check if a user has permission to access replay data for an organization. This + change is backwards compatible with the existing behavior and introduces the + ability to granularly control replay access for organization members, irrespective + of their role. + + Logic: + - If feature flag is disabled, return True (existing behavior, everyone has access) + - User must be authenticated and a member of the org + - If no allowlist records exist for org, return True for all members + - If allowlist records exist, check if user's org membership is in the allowlist + - Return True if user is in allowlist, False otherwise + """ + if not features.has("organizations:granular-replay-permissions", organization): + return True + + if user is None or not user.is_authenticated: + return False + + try: + member = OrganizationMember.objects.get(organization=organization, user_id=user.id) + except OrganizationMember.DoesNotExist: + return False + + # if the feature to gate replays by organization option is disabled, return True to allow access to all members + org_option = OrganizationOption.objects.filter( + organization=organization, key="sentry:granular-replay-permissions" + ).first() + if not org_option or not org_option.value: + return True + + allowlist_exists = OrganizationMemberReplayAccess.objects.filter( + organization=organization + ).exists() + + if not allowlist_exists: + return True + + has_access = OrganizationMemberReplayAccess.objects.filter( + organization=organization, organizationmember=member + ).exists() + + return has_access diff --git a/tests/sentry/replays/endpoints/test_replay_granular_permissions.py b/tests/sentry/replays/endpoints/test_replay_granular_permissions.py new file mode 100644 index 00000000000000..f5e69255790a25 --- /dev/null +++ b/tests/sentry/replays/endpoints/test_replay_granular_permissions.py @@ -0,0 +1,198 @@ +from sentry.models.options.organization_option import OrganizationOption +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess +from sentry.testutils.cases import APITestCase +from sentry.testutils.silo import region_silo_test + + +@region_silo_test +class TestReplayGranularPermissions(APITestCase): + def setUp(self) -> None: + super().setUp() + self.organization = self.create_organization() + self.project = self.create_project(organization=self.organization) + self.user_with_access = self.create_user() + self.user_without_access = self.create_user() + + self.member_with_access = self.create_member( + organization=self.organization, user=self.user_with_access + ) + self.member_without_access = self.create_member( + organization=self.organization, user=self.user_without_access + ) + + def _enable_granular_permissions(self) -> None: + """Enable the organization option for granular replay permissions""" + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + def test_organization_replay_index_with_permission(self) -> None: + """User with replay permission can access org replay index""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_with_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 200 + + def test_organization_replay_index_without_permission(self) -> None: + """User without replay permission cannot access org replay index""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 403 + + def test_organization_replay_details_with_permission(self) -> None: + """User with replay permission can access org replay details (gets 404 for non-existent replay, not 403)""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_with_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/123e4567-e89b-12d3-a456-426614174000/" + response = self.client.get(url) + # Should get 404 for non-existent replay, NOT 403 Forbidden (which would indicate permission denial) + assert response.status_code == 404 + + def test_organization_replay_details_without_permission(self) -> None: + """User without replay permission cannot access org replay details""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/123e4567-e89b-12d3-a456-426614174000/" + response = self.client.get(url) + assert response.status_code == 403 + + def test_organization_replay_count_without_permission(self) -> None: + """User without replay permission cannot access org replay count""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replay-count/" + response = self.client.get(url, {"query": "issue.id:1"}) + assert response.status_code == 403 + + def test_project_replay_details_without_permission(self) -> None: + """User without replay permission cannot access project replay details""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_without_access) + url = f"/api/0/projects/{self.organization.slug}/{self.project.slug}/replays/123e4567-e89b-12d3-a456-426614174000/" + response = self.client.get(url) + assert response.status_code == 403 + + def test_empty_allowlist_allows_all_users(self) -> None: + """When allowlist is empty, all org members have access (even with org option enabled)""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 200 + + def test_org_option_disabled_allows_all_users(self) -> None: + """When org option is disabled, all org members have access even with allowlist""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 200 + + def test_feature_flag_disabled_allows_all_users(self) -> None: + """When feature flag is disabled, all org members have access""" + with self.feature("organizations:session-replay"): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 200 + + def test_removing_last_user_from_allowlist_reopens_access(self) -> None: + """When the last user is removed from allowlist, all org members regain access""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + access_record = OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 403 + + access_record.delete() + + assert not OrganizationMemberReplayAccess.objects.filter( + organization=self.organization + ).exists() + + response = self.client.get(url) + assert response.status_code == 200 + + def test_disabling_org_option_reopens_access(self) -> None: + """When org option is disabled, all org members regain access""" + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member_with_access + ) + + self.login_as(self.user_without_access) + url = f"/api/0/organizations/{self.organization.slug}/replays/" + response = self.client.get(url) + assert response.status_code == 403 + + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=False, + ) + + response = self.client.get(url) + assert response.status_code == 200 diff --git a/tests/sentry/replays/test_permissions.py b/tests/sentry/replays/test_permissions.py new file mode 100644 index 00000000000000..b0bbb34abd3605 --- /dev/null +++ b/tests/sentry/replays/test_permissions.py @@ -0,0 +1,68 @@ +from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess +from sentry.replays.permissions import has_replay_permission +from sentry.testutils.cases import TestCase +from sentry.testutils.silo import region_silo_test + + +@region_silo_test +class TestReplayPermissions(TestCase): + def setUp(self) -> None: + super().setUp() + self.organization = self.create_organization() + self.user1 = self.create_user() + self.user2 = self.create_user() + self.user3 = self.create_user() + self.member1 = self.create_member(organization=self.organization, user=self.user1) + self.member2 = self.create_member(organization=self.organization, user=self.user2) + self.member3 = self.create_member(organization=self.organization, user=self.user3) + + def test_feature_flag_disabled_returns_true(self) -> None: + """When feature flag is disabled, all members should have access""" + assert has_replay_permission(self.organization, self.user1) is True + + def test_empty_allowlist_returns_true(self) -> None: + """When allowlist is empty, all members should have access""" + with self.feature("organizations:granular-replay-permissions"): + assert has_replay_permission(self.organization, self.user1) is True + assert has_replay_permission(self.organization, self.user2) is True + + def test_member_in_allowlist_returns_true(self) -> None: + """When member is in allowlist, they should have access""" + with self.feature("organizations:granular-replay-permissions"): + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member1 + ) + assert has_replay_permission(self.organization, self.user1) is True + + def test_member_not_in_allowlist_returns_false(self) -> None: + """When member is not in allowlist and allowlist exists, they should not have access""" + with self.feature("organizations:granular-replay-permissions"): + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member1 + ) + assert has_replay_permission(self.organization, self.user2) is False + + def test_multiple_members_in_allowlist(self) -> None: + """Test multiple members in allowlist""" + with self.feature("organizations:granular-replay-permissions"): + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member1 + ) + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member2 + ) + + assert has_replay_permission(self.organization, self.user1) is True + assert has_replay_permission(self.organization, self.user2) is True + assert has_replay_permission(self.organization, self.user3) is False + + def test_non_member_returns_false(self) -> None: + """Non-members should not have access""" + non_member_user = self.create_user() + with self.feature("organizations:granular-replay-permissions"): + assert has_replay_permission(self.organization, non_member_user) is False + + def test_unauthenticated_user_returns_false(self) -> None: + """Unauthenticated users should not have access""" + with self.feature("organizations:granular-replay-permissions"): + assert has_replay_permission(self.organization, None) is False From 8aec8661f34f6ee4736f77021967f041fa9db970 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 15:38:40 +0100 Subject: [PATCH 25/37] rename org replay endpoint file --- ...zation_replay_endpoint => organization_replay_endpoint.py} | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) rename src/sentry/replays/endpoints/{organization_replay_endpoint => organization_replay_endpoint.py} (91%) diff --git a/src/sentry/replays/endpoints/organization_replay_endpoint b/src/sentry/replays/endpoints/organization_replay_endpoint.py similarity index 91% rename from src/sentry/replays/endpoints/organization_replay_endpoint rename to src/sentry/replays/endpoints/organization_replay_endpoint.py index 43a62866341707..498c516d977a8e 100644 --- a/src/sentry/replays/endpoints/organization_replay_endpoint +++ b/src/sentry/replays/endpoints/organization_replay_endpoint.py @@ -21,9 +21,7 @@ def check_replay_access(self, request: Request, organization: Organization) -> R Check if the session replay feature is enabled and user has replay permissions. Returns a Response object if access should be denied, None if access is granted. """ - if not features.has( - "organizations:session-replay", organization, actor=request.user - ): + if not features.has("organizations:session-replay", organization, actor=request.user): return Response(status=404) if not has_replay_permission(organization, request.user): From 25b82660f9192fd77e2d754f7f7a54e1e0bcf445 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 15:41:21 +0100 Subject: [PATCH 26/37] wrong import --- src/sentry/replays/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/permissions.py b/src/sentry/replays/permissions.py index 506faefaf012ea..1e378a7416c058 100644 --- a/src/sentry/replays/permissions.py +++ b/src/sentry/replays/permissions.py @@ -5,9 +5,9 @@ from django.contrib.auth.models import AnonymousUser from sentry import features +from sentry.models.options.organization_option import OrganizationOption from sentry.models.organizationmember import OrganizationMember from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess -from sentry.models.organizationoption import OrganizationOption if TYPE_CHECKING: from sentry.models.organization import Organization From 7faaeb8318d8d3617127c82cfc322da1e24c321b Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 15:56:46 +0100 Subject: [PATCH 27/37] use separate variable names to prevent typing issues --- .../endpoints/organization_replay_details.py | 6 +++--- .../endpoints/project_replay_details.py | 6 +++--- .../endpoints/project_replay_video_details.py | 18 +++++++++++------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/sentry/replays/endpoints/organization_replay_details.py b/src/sentry/replays/endpoints/organization_replay_details.py index f78c326f17c656..ec615987d0acbf 100644 --- a/src/sentry/replays/endpoints/organization_replay_details.py +++ b/src/sentry/replays/endpoints/organization_replay_details.py @@ -295,12 +295,12 @@ def get(self, request: Request, organization: Organization, replay_id: str) -> R request_user_id=request.user.id, ) - response = process_raw_response( + replay_data = process_raw_response( snuba_response, fields=request.query_params.getlist("field"), ) - if len(response) == 0: + if len(replay_data) == 0: return Response(status=404) else: - return Response({"data": response[0]}, status=200) + return Response({"data": replay_data[0]}, status=200) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 55d35795a06bfe..dd9576bafaad2b 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -59,15 +59,15 @@ def get(self, request: Request, project: Project, replay_id: str) -> Response: request_user_id=request.user.id, ) - response = process_raw_response( + replay_data = process_raw_response( snuba_response, fields=request.query_params.getlist("field"), ) - if len(response) == 0: + if len(replay_data) == 0: return Response(status=404) else: - return Response({"data": response[0]}, status=200) + return Response({"data": replay_data[0]}, status=200) @extend_schema( operation_id="Delete a Replay Instance", diff --git a/src/sentry/replays/endpoints/project_replay_video_details.py b/src/sentry/replays/endpoints/project_replay_video_details.py index 7f5439811001aa..3743b9e51c3c11 100644 --- a/src/sentry/replays/endpoints/project_replay_video_details.py +++ b/src/sentry/replays/endpoints/project_replay_video_details.py @@ -67,16 +67,20 @@ def get(self, request: Request, project, replay_id, segment_id) -> HttpResponseB return self.respond({"detail": "Replay recording segment not found."}, status=404) if range_header := request.headers.get("Range"): - response = handle_range_response(range_header, video) + video_response = handle_range_response(range_header, video) else: video_io = BytesIO(video) iterator = iter(lambda: video_io.read(4096), b"") - response = StreamingHttpResponse(iterator, content_type="application/octet-stream") - response["Content-Length"] = len(video) - - response["Accept-Ranges"] = "bytes" - response["Content-Disposition"] = f'attachment; filename="{make_video_filename(segment)}"' - return response + video_response = StreamingHttpResponse( + iterator, content_type="application/octet-stream" + ) + video_response["Content-Length"] = len(video) + + video_response["Accept-Ranges"] = "bytes" + video_response["Content-Disposition"] = ( + f'attachment; filename="{make_video_filename(segment)}"' + ) + return video_response def handle_range_response(range_header: str, video: bytes) -> HttpResponseBase: From 5dea0f6739a6d1dcb9ab57a2b9d804ed1321e8e8 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 16:08:33 +0100 Subject: [PATCH 28/37] use organizationoption for permission tests --- tests/sentry/replays/test_permissions.py | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/sentry/replays/test_permissions.py b/tests/sentry/replays/test_permissions.py index b0bbb34abd3605..9a04ea2add6615 100644 --- a/tests/sentry/replays/test_permissions.py +++ b/tests/sentry/replays/test_permissions.py @@ -1,3 +1,4 @@ +from sentry.models.options.organization_option import OrganizationOption from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.replays.permissions import has_replay_permission from sentry.testutils.cases import TestCase @@ -16,19 +17,38 @@ def setUp(self) -> None: self.member2 = self.create_member(organization=self.organization, user=self.user2) self.member3 = self.create_member(organization=self.organization, user=self.user3) + def _enable_granular_permissions(self) -> None: + """Enable the organization option for granular replay permissions""" + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + def test_feature_flag_disabled_returns_true(self) -> None: """When feature flag is disabled, all members should have access""" + self._enable_granular_permissions() assert has_replay_permission(self.organization, self.user1) is True + def test_org_option_disabled_returns_true(self) -> None: + """When org option is disabled, all members should have access even with allowlist""" + with self.feature("organizations:granular-replay-permissions"): + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member1 + ) + assert has_replay_permission(self.organization, self.user2) is True + def test_empty_allowlist_returns_true(self) -> None: """When allowlist is empty, all members should have access""" with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() assert has_replay_permission(self.organization, self.user1) is True assert has_replay_permission(self.organization, self.user2) is True def test_member_in_allowlist_returns_true(self) -> None: """When member is in allowlist, they should have access""" with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( organization=self.organization, organizationmember=self.member1 ) @@ -37,6 +57,7 @@ def test_member_in_allowlist_returns_true(self) -> None: def test_member_not_in_allowlist_returns_false(self) -> None: """When member is not in allowlist and allowlist exists, they should not have access""" with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( organization=self.organization, organizationmember=self.member1 ) @@ -45,6 +66,7 @@ def test_member_not_in_allowlist_returns_false(self) -> None: def test_multiple_members_in_allowlist(self) -> None: """Test multiple members in allowlist""" with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( organization=self.organization, organizationmember=self.member1 ) @@ -60,9 +82,27 @@ def test_non_member_returns_false(self) -> None: """Non-members should not have access""" non_member_user = self.create_user() with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() assert has_replay_permission(self.organization, non_member_user) is False def test_unauthenticated_user_returns_false(self) -> None: """Unauthenticated users should not have access""" with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() assert has_replay_permission(self.organization, None) is False + + def test_disabling_org_option_reopens_access(self) -> None: + """When org option is disabled after being enabled, access is restored""" + with self.feature("organizations:granular-replay-permissions"): + self._enable_granular_permissions() + OrganizationMemberReplayAccess.objects.create( + organization=self.organization, organizationmember=self.member1 + ) + assert has_replay_permission(self.organization, self.user2) is False + + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=False, + ) + assert has_replay_permission(self.organization, self.user2) is True From 2a1ac988207118520e764e1f9620ef63845d209b Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 16:17:39 +0100 Subject: [PATCH 29/37] update function description & default with empty list --- src/sentry/replays/permissions.py | 22 ++++++++++------------ tests/sentry/replays/test_permissions.py | 8 ++++---- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/sentry/replays/permissions.py b/src/sentry/replays/permissions.py index 1e378a7416c058..33e885269823e4 100644 --- a/src/sentry/replays/permissions.py +++ b/src/sentry/replays/permissions.py @@ -16,17 +16,15 @@ def has_replay_permission(organization: Organization, user: User | AnonymousUser | None) -> bool: """ - Check if a user has permission to access replay data for an organization. This - change is backwards compatible with the existing behavior and introduces the - ability to granularly control replay access for organization members, irrespective - of their role. - - Logic: - - If feature flag is disabled, return True (existing behavior, everyone has access) - - User must be authenticated and a member of the org - - If no allowlist records exist for org, return True for all members - - If allowlist records exist, check if user's org membership is in the allowlist - - Return True if user is in allowlist, False otherwise + Determine whether a user has permission to access replay data for a given organization. + + Rules: + - User must be authenticated and an active org member. + - If the 'organizations:granular-replay-permissions' feature flag is OFF, all users have access. + - If the 'sentry:granular-replay-permissions' org option is not set or falsy, all org members have access. + - If no allowlist records exist for the organization but the feature flag is on, no one has access. + - If allowlist records exist, only users explicitly present in the OrganizationMemberReplayAccess allowlist have access. + - Returns True if allowed, False otherwise. """ if not features.has("organizations:granular-replay-permissions", organization): return True @@ -51,7 +49,7 @@ def has_replay_permission(organization: Organization, user: User | AnonymousUser ).exists() if not allowlist_exists: - return True + return False has_access = OrganizationMemberReplayAccess.objects.filter( organization=organization, organizationmember=member diff --git a/tests/sentry/replays/test_permissions.py b/tests/sentry/replays/test_permissions.py index 9a04ea2add6615..ccea7802cedd7a 100644 --- a/tests/sentry/replays/test_permissions.py +++ b/tests/sentry/replays/test_permissions.py @@ -38,12 +38,12 @@ def test_org_option_disabled_returns_true(self) -> None: ) assert has_replay_permission(self.organization, self.user2) is True - def test_empty_allowlist_returns_true(self) -> None: - """When allowlist is empty, all members should have access""" + def test_empty_allowlist_returns_false(self) -> None: + """When allowlist is empty access control is active, no one should have access""" with self.feature("organizations:granular-replay-permissions"): self._enable_granular_permissions() - assert has_replay_permission(self.organization, self.user1) is True - assert has_replay_permission(self.organization, self.user2) is True + assert has_replay_permission(self.organization, self.user1) is False + assert has_replay_permission(self.organization, self.user2) is False def test_member_in_allowlist_returns_true(self) -> None: """When member is in allowlist, they should have access""" From 04e4b546276110872b86fed8f4e58a73b909a0ce Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 14:05:09 +0100 Subject: [PATCH 30/37] update model imports --- .../replays/endpoints/test_replay_granular_permissions.py | 2 +- tests/sentry/replays/test_permissions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/sentry/replays/endpoints/test_replay_granular_permissions.py b/tests/sentry/replays/endpoints/test_replay_granular_permissions.py index f5e69255790a25..1ec9b233bc4141 100644 --- a/tests/sentry/replays/endpoints/test_replay_granular_permissions.py +++ b/tests/sentry/replays/endpoints/test_replay_granular_permissions.py @@ -1,5 +1,5 @@ from sentry.models.options.organization_option import OrganizationOption -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.testutils.cases import APITestCase from sentry.testutils.silo import region_silo_test diff --git a/tests/sentry/replays/test_permissions.py b/tests/sentry/replays/test_permissions.py index ccea7802cedd7a..49b60fec73e84c 100644 --- a/tests/sentry/replays/test_permissions.py +++ b/tests/sentry/replays/test_permissions.py @@ -1,5 +1,5 @@ from sentry.models.options.organization_option import OrganizationOption -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess +from sentry.replays.models import OrganizationMemberReplayAccess from sentry.replays.permissions import has_replay_permission from sentry.testutils.cases import TestCase from sentry.testutils.silo import region_silo_test From 17c3531e742dfe476cbdcf9e5f5f97b0df5d758d Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 15:17:56 +0100 Subject: [PATCH 31/37] fix import --- src/sentry/replays/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/permissions.py b/src/sentry/replays/permissions.py index 33e885269823e4..e0748ddbeaa464 100644 --- a/src/sentry/replays/permissions.py +++ b/src/sentry/replays/permissions.py @@ -7,7 +7,7 @@ from sentry import features from sentry.models.options.organization_option import OrganizationOption from sentry.models.organizationmember import OrganizationMember -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess +from sentry.replays.models import OrganizationMemberReplayAccess if TYPE_CHECKING: from sentry.models.organization import Organization From 757f9364127a644746c2e2ea30284195751a422c Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 9 Dec 2025 16:43:10 +0100 Subject: [PATCH 32/37] fix existing tests --- src/sentry/replays/permissions.py | 6 +- .../test_project_replay_jobs_delete.py | 366 ++++++++++++++++-- .../endpoints/test_project_replay_summary.py | 5 +- .../test_replay_granular_permissions.py | 34 +- tests/sentry/replays/test_permissions.py | 24 +- 5 files changed, 362 insertions(+), 73 deletions(-) diff --git a/src/sentry/replays/permissions.py b/src/sentry/replays/permissions.py index e0748ddbeaa464..58c30c4e2ae2f9 100644 --- a/src/sentry/replays/permissions.py +++ b/src/sentry/replays/permissions.py @@ -45,14 +45,12 @@ def has_replay_permission(organization: Organization, user: User | AnonymousUser return True allowlist_exists = OrganizationMemberReplayAccess.objects.filter( - organization=organization + organizationmember__organization=organization ).exists() if not allowlist_exists: return False - has_access = OrganizationMemberReplayAccess.objects.filter( - organization=organization, organizationmember=member - ).exists() + has_access = OrganizationMemberReplayAccess.objects.filter(organizationmember=member).exists() return has_access diff --git a/tests/sentry/replays/endpoints/test_project_replay_jobs_delete.py b/tests/sentry/replays/endpoints/test_project_replay_jobs_delete.py index 9d1aab93d603a2..84ef71b826f8c2 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_jobs_delete.py +++ b/tests/sentry/replays/endpoints/test_project_replay_jobs_delete.py @@ -5,7 +5,8 @@ from sentry.hybridcloud.outbox.category import OutboxScope from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry -from sentry.replays.models import ReplayDeletionJobModel +from sentry.models.options.organization_option import OrganizationOption +from sentry.replays.models import OrganizationMemberReplayAccess, ReplayDeletionJobModel from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode, region_silo_test @@ -342,6 +343,133 @@ def test_permission_granted_with_project_admin(self) -> None: ) assert response.status_code == 201 + def test_granular_permissions_without_replay_access(self) -> None: + """Test that users without replay access cannot access endpoints even with project:write""" + with self.feature("organizations:granular-replay-permissions"): + # Enable granular permissions org option + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + # Create another user with replay access + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, organization=self.organization, role="admin" + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + + # Login as user without replay access (but has project:write via admin role) + user_without_access = self.create_user() + self.create_member( + user=user_without_access, organization=self.organization, role="admin" + ) + self.login_as(user=user_without_access) + + # GET should return 403 + self.get_error_response(self.organization.slug, self.project.slug, status_code=403) + + # POST should return 403 + data = { + "data": { + "rangeStart": "2023-01-01T00:00:00Z", + "rangeEnd": "2023-01-02T00:00:00Z", + "environments": ["production"], + "query": "test query", + } + } + self.get_error_response( + self.organization.slug, self.project.slug, method="post", **data, status_code=403 + ) + + def test_granular_permissions_with_replay_access(self) -> None: + """Test that users with replay access can access endpoints with project:write""" + with self.feature("organizations:granular-replay-permissions"): + # Enable granular permissions org option + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + # Create user with replay access (admin role gives project:write) + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, organization=self.organization, role="admin" + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + self.login_as(user=user_with_access) + + # GET should succeed + response = self.get_success_response(self.organization.slug, self.project.slug) + assert response.data == {"data": []} + + # POST should succeed + data = { + "data": { + "rangeStart": "2023-01-01T00:00:00Z", + "rangeEnd": "2023-01-02T00:00:00Z", + "environments": ["production"], + "query": "test query", + } + } + with patch("sentry.replays.tasks.run_bulk_replay_delete_job.delay"): + response = self.get_success_response( + self.organization.slug, + self.project.slug, + method="post", + **data, + status_code=201, + ) + assert "data" in response.data + + def test_granular_permissions_feature_disabled_allows_all(self) -> None: + """Test that when feature flag is disabled, all users with project:write can access""" + # Enable org option but disable feature flag + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + # Create user with replay access + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, organization=self.organization, role="admin" + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + + # Login as user without replay access (admin role gives project:write) + user_without_access = self.create_user() + self.create_member(user=user_without_access, organization=self.organization, role="admin") + self.login_as(user=user_without_access) + + # GET should succeed (feature flag is OFF) + response = self.get_success_response(self.organization.slug, self.project.slug) + assert response.data == {"data": []} + + def test_granular_permissions_org_option_disabled_allows_all(self) -> None: + """Test that when org option is disabled, all users with project:write can access""" + with self.feature("organizations:granular-replay-permissions"): + # Create user with replay access + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, organization=self.organization, role="admin" + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + + # Login as user without replay access (org option is NOT enabled, admin role gives project:write) + user_without_access = self.create_user() + self.create_member( + user=user_without_access, organization=self.organization, role="admin" + ) + self.login_as(user=user_without_access) + + # GET should succeed (org option is OFF) + response = self.get_success_response(self.organization.slug, self.project.slug) + assert response.data == {"data": []} + @patch("sentry.replays.tasks.run_bulk_replay_delete_job.delay") def test_post_has_seer_data(self, mock_task: MagicMock) -> None: """Test POST with summaries enabled schedules task with has_seer_data=True.""" @@ -375,7 +503,8 @@ def setUp(self) -> None: super().setUp() self.login_as(self.user) self.organization = self.create_organization(owner=self.user) - self.project = self.create_project(organization=self.organization) + self.team = self.create_team(organization=self.organization) + self.project = self.create_project(organization=self.organization, teams=[self.team]) self.other_project = self.create_project() # Different organization def test_get_success(self) -> None: @@ -390,19 +519,20 @@ def test_get_success(self) -> None: status="in-progress", ) - response = self.get_success_response(self.organization.slug, self.project.slug, job.id) - - assert "data" in response.data - job_data = response.data["data"] - assert job_data["id"] == job.id - assert job_data["status"] == "in-progress" - assert job_data["environments"] == ["prod", "staging"] - assert job_data["query"] == "test query" - assert job_data["countDeleted"] == 0 # Default offset value - assert "dateCreated" in job_data - assert "dateUpdated" in job_data - assert "rangeStart" in job_data - assert "rangeEnd" in job_data + with self.feature("organizations:session-replay"): + response = self.get_success_response(self.organization.slug, self.project.slug, job.id) + + assert "data" in response.data + job_data = response.data["data"] + assert job_data["id"] == job.id + assert job_data["status"] == "in-progress" + assert job_data["environments"] == ["prod", "staging"] + assert job_data["query"] == "test query" + assert job_data["countDeleted"] == 0 # Default offset value + assert "dateCreated" in job_data + assert "dateUpdated" in job_data + assert "rangeStart" in job_data + assert "rangeEnd" in job_data def test_get_count_deleted_reflects_offset(self) -> None: """Test that countDeleted field correctly reflects the offset value""" @@ -417,12 +547,13 @@ def test_get_count_deleted_reflects_offset(self) -> None: offset=123, # Set specific offset value ) - response = self.get_success_response(self.organization.slug, self.project.slug, job.id) + with self.feature("organizations:session-replay"): + response = self.get_success_response(self.organization.slug, self.project.slug, job.id) - assert "data" in response.data - job_data = response.data["data"] - assert job_data["id"] == job.id - assert job_data["countDeleted"] == 123 + assert "data" in response.data + job_data = response.data["data"] + assert job_data["id"] == job.id + assert job_data["countDeleted"] == 123 def test_get_nonexistent_job(self) -> None: """Test GET for non-existent job returns 404""" @@ -523,13 +654,14 @@ def test_permission_granted_with_project_write(self) -> None: token = ApiToken.objects.create(user=self.user, scope_list=["project:write"]) # GET should succeed - response = self.client.get( - f"/api/0/projects/{self.organization.slug}/{self.project.slug}/replays/jobs/delete/{job.id}/", - HTTP_AUTHORIZATION=f"Bearer {token.token}", - format="json", - ) - assert response.status_code == 200 - assert response.data["data"]["id"] == job.id + with self.feature("organizations:session-replay"): + response = self.client.get( + f"/api/0/projects/{self.organization.slug}/{self.project.slug}/replays/jobs/delete/{job.id}/", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + format="json", + ) + assert response.status_code == 200 + assert response.data["data"]["id"] == job.id def test_permission_granted_with_project_admin(self) -> None: """Test that users with project:admin permissions can access endpoint""" @@ -548,10 +680,178 @@ def test_permission_granted_with_project_admin(self) -> None: token = ApiToken.objects.create(user=self.user, scope_list=["project:admin"]) # GET should succeed - response = self.client.get( - f"/api/0/projects/{self.organization.slug}/{self.project.slug}/replays/jobs/delete/{job.id}/", - HTTP_AUTHORIZATION=f"Bearer {token.token}", - format="json", + with self.feature("organizations:session-replay"): + response = self.client.get( + f"/api/0/projects/{self.organization.slug}/{self.project.slug}/replays/jobs/delete/{job.id}/", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + format="json", + ) + assert response.status_code == 200 + assert response.data["data"]["id"] == job.id + + def test_granular_permissions_without_replay_access(self) -> None: + """Test that users without replay access cannot access endpoint even with project:write""" + job = ReplayDeletionJobModel.objects.create( + project_id=self.project.id, + organization_id=self.organization.id, + range_start=datetime.datetime(2023, 1, 1, tzinfo=datetime.UTC), + range_end=datetime.datetime(2023, 1, 2, tzinfo=datetime.UTC), + query="test query", + environments=[], + status="pending", ) - assert response.status_code == 200 - assert response.data["data"]["id"] == job.id + + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + # Enable granular permissions org option + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + # Create another user with replay access + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + + # Login as user without replay access (but has project:write via admin role) + user_without_access = self.create_user() + self.create_member( + user=user_without_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + self.login_as(user=user_without_access) + + # GET should return 403 + self.get_error_response( + self.organization.slug, self.project.slug, job.id, status_code=403 + ) + + def test_granular_permissions_with_replay_access(self) -> None: + """Test that users with replay access can access endpoint with project:write""" + job = ReplayDeletionJobModel.objects.create( + project_id=self.project.id, + organization_id=self.organization.id, + range_start=datetime.datetime(2023, 1, 1, tzinfo=datetime.UTC), + range_end=datetime.datetime(2023, 1, 2, tzinfo=datetime.UTC), + query="test query", + environments=[], + status="pending", + ) + + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + # Enable granular permissions org option + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + # Create user with replay access (admin role gives project:write) + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + self.login_as(user=user_with_access) + + # GET should succeed + response = self.get_success_response(self.organization.slug, self.project.slug, job.id) + assert response.data["data"]["id"] == job.id + + def test_granular_permissions_feature_disabled_allows_all(self) -> None: + """Test that when feature flag is disabled, all users with project:write can access""" + job = ReplayDeletionJobModel.objects.create( + project_id=self.project.id, + organization_id=self.organization.id, + range_start=datetime.datetime(2023, 1, 1, tzinfo=datetime.UTC), + range_end=datetime.datetime(2023, 1, 2, tzinfo=datetime.UTC), + query="test query", + environments=[], + status="pending", + ) + + # Enable session-replay and org option but disable granular-replay-permissions feature flag + with self.feature("organizations:session-replay"): + OrganizationOption.objects.set_value( + organization=self.organization, + key="sentry:granular-replay-permissions", + value=True, + ) + + # Create user with replay access + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + + # Login as user without replay access (admin role gives project:write) + user_without_access = self.create_user() + self.create_member( + user=user_without_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + self.login_as(user=user_without_access) + + # GET should succeed (feature flag is OFF) + response = self.get_success_response(self.organization.slug, self.project.slug, job.id) + assert response.data["data"]["id"] == job.id + + def test_granular_permissions_org_option_disabled_allows_all(self) -> None: + """Test that when org option is disabled, all users with project:write can access""" + job = ReplayDeletionJobModel.objects.create( + project_id=self.project.id, + organization_id=self.organization.id, + range_start=datetime.datetime(2023, 1, 1, tzinfo=datetime.UTC), + range_end=datetime.datetime(2023, 1, 2, tzinfo=datetime.UTC), + query="test query", + environments=[], + status="pending", + ) + + with self.feature( + ["organizations:session-replay", "organizations:granular-replay-permissions"] + ): + # Create user with replay access + user_with_access = self.create_user() + member_with_access = self.create_member( + user=user_with_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + OrganizationMemberReplayAccess.objects.create(organizationmember=member_with_access) + + # Login as user without replay access (org option is NOT enabled, admin role gives project:write) + user_without_access = self.create_user() + self.create_member( + user=user_without_access, + organization=self.organization, + role="admin", + teams=[self.team], + ) + self.login_as(user=user_without_access) + + # GET should succeed (org option is OFF) + response = self.get_success_response(self.organization.slug, self.project.slug, job.id) + assert response.data["data"]["id"] == job.id diff --git a/tests/sentry/replays/endpoints/test_project_replay_summary.py b/tests/sentry/replays/endpoints/test_project_replay_summary.py index b2e1eadaf0f931..1a58013cfdd75b 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_summary.py +++ b/tests/sentry/replays/endpoints/test_project_replay_summary.py @@ -84,7 +84,10 @@ def test_feature_flag_disabled(self) -> None: response = ( self.client.get(self.url) if method == "GET" else self.client.post(self.url) ) - assert response.status_code == 403, (replay, replay_ai, method) + # When session-replay is disabled, endpoint returns 404 + # When session-replay is enabled but replay-ai-summaries is disabled, returns 403 + expected_status = 404 if not replay else 403 + assert response.status_code == expected_status, (replay, replay_ai, method) def test_no_seer_access(self) -> None: self.mock_has_seer_access.return_value = False diff --git a/tests/sentry/replays/endpoints/test_replay_granular_permissions.py b/tests/sentry/replays/endpoints/test_replay_granular_permissions.py index 1ec9b233bc4141..ea4ca5f4347c24 100644 --- a/tests/sentry/replays/endpoints/test_replay_granular_permissions.py +++ b/tests/sentry/replays/endpoints/test_replay_granular_permissions.py @@ -35,7 +35,7 @@ def test_organization_replay_index_with_permission(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_with_access) url = f"/api/0/organizations/{self.organization.slug}/replays/" @@ -49,7 +49,7 @@ def test_organization_replay_index_without_permission(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) url = f"/api/0/organizations/{self.organization.slug}/replays/" @@ -63,7 +63,7 @@ def test_organization_replay_details_with_permission(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_with_access) url = f"/api/0/organizations/{self.organization.slug}/replays/123e4567-e89b-12d3-a456-426614174000/" @@ -78,7 +78,7 @@ def test_organization_replay_details_without_permission(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) url = f"/api/0/organizations/{self.organization.slug}/replays/123e4567-e89b-12d3-a456-426614174000/" @@ -92,7 +92,7 @@ def test_organization_replay_count_without_permission(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) url = f"/api/0/organizations/{self.organization.slug}/replay-count/" @@ -106,15 +106,15 @@ def test_project_replay_details_without_permission(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) url = f"/api/0/projects/{self.organization.slug}/{self.project.slug}/replays/123e4567-e89b-12d3-a456-426614174000/" response = self.client.get(url) assert response.status_code == 403 - def test_empty_allowlist_allows_all_users(self) -> None: - """When allowlist is empty, all org members have access (even with org option enabled)""" + def test_empty_allowlist_denies_all_users(self) -> None: + """When allowlist is empty and org option is enabled, no org members have access""" with self.feature( ["organizations:session-replay", "organizations:granular-replay-permissions"] ): @@ -122,7 +122,7 @@ def test_empty_allowlist_allows_all_users(self) -> None: self.login_as(self.user_without_access) url = f"/api/0/organizations/{self.organization.slug}/replays/" response = self.client.get(url) - assert response.status_code == 200 + assert response.status_code == 403 def test_org_option_disabled_allows_all_users(self) -> None: """When org option is disabled, all org members have access even with allowlist""" @@ -130,7 +130,7 @@ def test_org_option_disabled_allows_all_users(self) -> None: ["organizations:session-replay", "organizations:granular-replay-permissions"] ): OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) url = f"/api/0/organizations/{self.organization.slug}/replays/" @@ -142,21 +142,21 @@ def test_feature_flag_disabled_allows_all_users(self) -> None: with self.feature("organizations:session-replay"): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) url = f"/api/0/organizations/{self.organization.slug}/replays/" response = self.client.get(url) assert response.status_code == 200 - def test_removing_last_user_from_allowlist_reopens_access(self) -> None: - """When the last user is removed from allowlist, all org members regain access""" + def test_removing_last_user_from_allowlist_keeps_access_denied(self) -> None: + """When the last user is removed from allowlist, access remains denied (empty allowlist = no access)""" with self.feature( ["organizations:session-replay", "organizations:granular-replay-permissions"] ): self._enable_granular_permissions() access_record = OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) @@ -167,11 +167,11 @@ def test_removing_last_user_from_allowlist_reopens_access(self) -> None: access_record.delete() assert not OrganizationMemberReplayAccess.objects.filter( - organization=self.organization + organizationmember__organization=self.organization ).exists() response = self.client.get(url) - assert response.status_code == 200 + assert response.status_code == 403 def test_disabling_org_option_reopens_access(self) -> None: """When org option is disabled, all org members regain access""" @@ -180,7 +180,7 @@ def test_disabling_org_option_reopens_access(self) -> None: ): self._enable_granular_permissions() OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member_with_access + organizationmember=self.member_with_access ) self.login_as(self.user_without_access) diff --git a/tests/sentry/replays/test_permissions.py b/tests/sentry/replays/test_permissions.py index 49b60fec73e84c..f65a834213e200 100644 --- a/tests/sentry/replays/test_permissions.py +++ b/tests/sentry/replays/test_permissions.py @@ -33,9 +33,7 @@ def test_feature_flag_disabled_returns_true(self) -> None: def test_org_option_disabled_returns_true(self) -> None: """When org option is disabled, all members should have access even with allowlist""" with self.feature("organizations:granular-replay-permissions"): - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member1 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=self.member1) assert has_replay_permission(self.organization, self.user2) is True def test_empty_allowlist_returns_false(self) -> None: @@ -49,30 +47,22 @@ def test_member_in_allowlist_returns_true(self) -> None: """When member is in allowlist, they should have access""" with self.feature("organizations:granular-replay-permissions"): self._enable_granular_permissions() - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member1 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=self.member1) assert has_replay_permission(self.organization, self.user1) is True def test_member_not_in_allowlist_returns_false(self) -> None: """When member is not in allowlist and allowlist exists, they should not have access""" with self.feature("organizations:granular-replay-permissions"): self._enable_granular_permissions() - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member1 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=self.member1) assert has_replay_permission(self.organization, self.user2) is False def test_multiple_members_in_allowlist(self) -> None: """Test multiple members in allowlist""" with self.feature("organizations:granular-replay-permissions"): self._enable_granular_permissions() - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member1 - ) - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member2 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=self.member1) + OrganizationMemberReplayAccess.objects.create(organizationmember=self.member2) assert has_replay_permission(self.organization, self.user1) is True assert has_replay_permission(self.organization, self.user2) is True @@ -95,9 +85,7 @@ def test_disabling_org_option_reopens_access(self) -> None: """When org option is disabled after being enabled, access is restored""" with self.feature("organizations:granular-replay-permissions"): self._enable_granular_permissions() - OrganizationMemberReplayAccess.objects.create( - organization=self.organization, organizationmember=self.member1 - ) + OrganizationMemberReplayAccess.objects.create(organizationmember=self.member1) assert has_replay_permission(self.organization, self.user2) is False OrganizationOption.objects.set_value( From 30632cee9e49e5e5580c9d0e61df35054def9b6c Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 13:20:52 +0000 Subject: [PATCH 33/37] :hammer_and_wrench: apply pre-commit fixes --- src/sentry/api/serializers/models/organization.py | 5 +---- src/sentry/core/endpoints/organization_details.py | 11 ++--------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index c8682b6eebf155..cdee156c75fc60 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -89,10 +89,7 @@ if TYPE_CHECKING: from sentry.api.serializers.models.project import OrganizationProjectResponse - from sentry.users.api.serializers.user import ( - UserSerializerResponse, - UserSerializerResponseSelf, - ) + from sentry.users.api.serializers.user import UserSerializerResponse, UserSerializerResponseSelf # This cut-off date ensures that only new organizations created after this date go # through the logic that checks for the 'onboarding:complete' key in OrganizationOption. diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index ee5bb129103252..c814fd154e31db 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -9,11 +9,7 @@ from django.db.models.query_utils import DeferredAttribute from django.urls import reverse from django.utils import timezone as django_timezone -from drf_spectacular.utils import ( - OpenApiResponse, - extend_schema, - extend_schema_serializer, -) +from drf_spectacular.utils import OpenApiResponse, extend_schema, extend_schema_serializer from rest_framework import serializers, status from rest_framework.exceptions import NotFound, PermissionDenied from sentry_sdk import capture_exception @@ -106,10 +102,7 @@ RpcOrganizationDeleteResponse, RpcOrganizationDeleteState, ) -from sentry.relay.datascrubbing import ( - validate_pii_config_update, - validate_pii_selectors, -) +from sentry.relay.datascrubbing import validate_pii_config_update, validate_pii_selectors from sentry.replays.models import OrganizationMemberReplayAccess from sentry.seer.autofix.constants import AutofixAutomationTuningSettings from sentry.services.organization.provisioning import ( From f3f281a64be233e9aa29db3d3817b3486e5b679a Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 14:22:53 +0100 Subject: [PATCH 34/37] cleanup bad rebase --- .../api/serializers/models/organization.py | 1 - .../1012_organizationmember_replay_access.py | 64 ------------------- src/sentry/models/__init__.py | 1 - .../models/organizationmemberreplayaccess.py | 34 ---------- src/sentry/testutils/helpers/backups.py | 10 --- 5 files changed, 110 deletions(-) delete mode 100644 src/sentry/migrations/1012_organizationmember_replay_access.py delete mode 100644 src/sentry/models/organizationmemberreplayaccess.py diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index cdee156c75fc60..52b34576bbe9a6 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -76,7 +76,6 @@ from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus from sentry.models.organizationaccessrequest import OrganizationAccessRequest -from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess from sentry.models.organizationonboardingtask import OrganizationOnboardingTask from sentry.models.project import Project from sentry.models.team import Team, TeamStatus diff --git a/src/sentry/migrations/1012_organizationmember_replay_access.py b/src/sentry/migrations/1012_organizationmember_replay_access.py deleted file mode 100644 index 9dba851c1e91ea..00000000000000 --- a/src/sentry/migrations/1012_organizationmember_replay_access.py +++ /dev/null @@ -1,64 +0,0 @@ -# Generated by Django 5.2.8 on 2025-12-02 12:31 - -import django.db.models.deletion -import django.utils.timezone -from django.db import migrations, models - -import sentry.db.models.fields.bounded -import sentry.db.models.fields.foreignkey -from sentry.new_migrations.migrations import CheckedMigration - - -class Migration(CheckedMigration): - # This flag is used to mark that a migration shouldn't be automatically run in production. - # This should only be used for operations where it's safe to run the migration after your - # code has deployed. So this should not be used for most operations that alter the schema - # of a table. - # Here are some things that make sense to mark as post deployment: - # - Large data migrations. Typically we want these to be run manually so that they can be - # monitored and not block the deploy for a long period of time while they run. - # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to - # run this outside deployments so that we don't block them. Note that while adding an index - # is a schema change, it's completely safe to run the operation after the code has deployed. - # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment - - is_post_deployment = False - - dependencies = [ - ("sentry", "1011_update_oc_integration_cascade_to_null"), - ] - - operations = [ - migrations.CreateModel( - name="OrganizationMemberReplayAccess", - fields=[ - ( - "id", - sentry.db.models.fields.bounded.BoundedBigAutoField( - primary_key=True, serialize=False - ), - ), - ("date_added", models.DateTimeField(default=django.utils.timezone.now)), - ( - "organization", - sentry.db.models.fields.foreignkey.FlexibleForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name="replay_access_set", - to="sentry.organization", - ), - ), - ( - "organizationmember", - sentry.db.models.fields.foreignkey.FlexibleForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name="replay_access", - to="sentry.organizationmember", - ), - ), - ], - options={ - "db_table": "sentry_organizationmemberreplayaccess", - "unique_together": {("organization", "organizationmember")}, - }, - ), - ] diff --git a/src/sentry/models/__init__.py b/src/sentry/models/__init__.py index bccfd75ddb3a82..c34615e484f620 100644 --- a/src/sentry/models/__init__.py +++ b/src/sentry/models/__init__.py @@ -74,7 +74,6 @@ from .organizationmember import * # NOQA from .organizationmemberinvite import * # NOQA from .organizationmembermapping import * # NOQA -from .organizationmemberreplayaccess import * # NOQA from .organizationmemberteam import * # NOQA from .organizationmemberteamreplica import * # NOQA from .organizationonboardingtask import * # NOQA diff --git a/src/sentry/models/organizationmemberreplayaccess.py b/src/sentry/models/organizationmemberreplayaccess.py deleted file mode 100644 index bb928aea29e7b6..00000000000000 --- a/src/sentry/models/organizationmemberreplayaccess.py +++ /dev/null @@ -1,34 +0,0 @@ -from __future__ import annotations - -from django.db import models -from django.utils import timezone - -from sentry.backup.scopes import RelocationScope -from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr - - -@region_silo_model -class OrganizationMemberReplayAccess(Model): - """ - Tracks which organization members have permission to access replay data. - - When no records exist for an organization, all members have access (default). - When records exist, only members with a record can access replays. - """ - - __relocation_scope__ = RelocationScope.Organization - - organization = FlexibleForeignKey("sentry.Organization", related_name="replay_access_set") - organizationmember = FlexibleForeignKey( - "sentry.OrganizationMember", - on_delete=models.CASCADE, - related_name="replay_access", - ) - date_added = models.DateTimeField(default=timezone.now) - - class Meta: - app_label = "sentry" - db_table = "sentry_organizationmemberreplayaccess" - unique_together = (("organization", "organizationmember"),) - - __repr__ = sane_repr("organization_id", "organizationmember_id") diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index b113de4ee07c1d..113ae113bb779e 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -473,18 +473,8 @@ def create_exhaustive_organization( organization=org, key="sentry:scrape_javascript", value=True ) -<<<<<<< HEAD owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) OrganizationMemberReplayAccess.objects.create(organizationmember=owner_member) -======= - # OrganizationMemberReplayAccess - add for the owner member - from sentry.models.organizationmemberreplayaccess import OrganizationMemberReplayAccess - - owner_member = OrganizationMember.objects.get(organization=org, user_id=owner_id) - OrganizationMemberReplayAccess.objects.create( - organization=org, organizationmember=owner_member - ) ->>>>>>> 536c8a0c02b (feat(replay): add model to allow per-user access control for replays) # Team team = self.create_team(name=f"test_team_in_{slug}", organization=org) From 81c9089bba81eedd1ffb89d38d17abe02488707a Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 14:25:17 +0100 Subject: [PATCH 35/37] cleanup bad rebase some more --- .../core/endpoints/organization_details.py | 97 ++++++++----------- 1 file changed, 41 insertions(+), 56 deletions(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index c814fd154e31db..7fd293c3013f2d 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -618,72 +618,57 @@ def save(self, **kwargs): if trusted_relay_info is not None: self.save_trusted_relays(trusted_relay_info, changed_data, org) - if "hasGranularReplayPermissions" in data or "replayAccessMembers" in data: - if not features.has("organizations:granular-replay-permissions", org): - raise serializers.ValidationError( - { - "hasGranularReplayPermissions": "This feature is not enabled for your organization." - } - ) - - if not self.context["request"].access.has_scope("org:admin"): - raise serializers.ValidationError( - { - "hasGranularReplayPermissions": "You do not have permission to modify granular replay permissions." - } - ) - - if "hasGranularReplayPermissions" in data: - option_key = "sentry:granular-replay-permissions" - new_value = data["hasGranularReplayPermissions"] + if "hasGranularReplayPermissions" in data: + option_key = "sentry:granular-replay-permissions" + new_value = data["hasGranularReplayPermissions"] - option_inst, created = OrganizationOption.objects.update_or_create( - organization=org, key=option_key, defaults={"value": new_value} - ) + option_inst, created = OrganizationOption.objects.update_or_create( + organization=org, key=option_key, defaults={"value": new_value} + ) - if new_value or created: - changed_data["hasGranularReplayPermissions"] = f"to {new_value}" + if new_value or created: + changed_data["hasGranularReplayPermissions"] = f"to {new_value}" - if "replayAccessMembers" in data: - member_ids = data["replayAccessMembers"] + if "replayAccessMembers" in data: + member_ids = data["replayAccessMembers"] - if member_ids is None: - member_ids = [] + if member_ids is None: + member_ids = [] - current_member_ids = set( - OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( - "organizationmember_id", flat=True - ) + current_member_ids = set( + OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( + "organizationmember_id", flat=True + ) + ) + new_member_ids = set(member_ids) + + to_add = new_member_ids - current_member_ids + to_remove = current_member_ids - new_member_ids + + if to_add: + OrganizationMemberReplayAccess.objects.bulk_create( + [ + OrganizationMemberReplayAccess( + organization=org, organizationmember_id=member_id + ) + for member_id in to_add + ] ) - new_member_ids = set(member_ids) - to_add = new_member_ids - current_member_ids - to_remove = current_member_ids - new_member_ids + if to_remove: + OrganizationMemberReplayAccess.objects.filter( + organization=org, organizationmember_id__in=to_remove + ).delete() + if to_add or to_remove: + changes = [] if to_add: - OrganizationMemberReplayAccess.objects.bulk_create( - [ - OrganizationMemberReplayAccess( - organization=org, organizationmember_id=member_id - ) - for member_id in to_add - ] - ) - + changes.append(f"added {len(to_add)} member(s)") if to_remove: - OrganizationMemberReplayAccess.objects.filter( - organization=org, organizationmember_id__in=to_remove - ).delete() - - if to_add or to_remove: - changes = [] - if to_add: - changes.append(f"added {len(to_add)} member(s)") - if to_remove: - changes.append(f"removed {len(to_remove)} member(s)") - changed_data["replayAccessMembers"] = ( - f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" - ) + changes.append(f"removed {len(to_remove)} member(s)") + changed_data["replayAccessMembers"] = ( + f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" + ) if "openMembership" in data: org.flags.allow_joinleave = data["openMembership"] From e08910229ee083b4b17f7107f5ebcea9a40ace2f Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 14:26:22 +0100 Subject: [PATCH 36/37] cleanup bad rebase some more --- .../core/endpoints/organization_details.py | 61 ++++++++++++------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 7fd293c3013f2d..d3701ea447d03e 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -95,6 +95,7 @@ from sentry.models.options.organization_option import OrganizationOption from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization, OrganizationStatus +from sentry.models.organizationmember import OrganizationMember from sentry.models.project import Project from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import ( @@ -621,53 +622,69 @@ def save(self, **kwargs): if "hasGranularReplayPermissions" in data: option_key = "sentry:granular-replay-permissions" new_value = data["hasGranularReplayPermissions"] - - option_inst, created = OrganizationOption.objects.update_or_create( + option_inst, created = OrganizationOption.objects.get_or_create( organization=org, key=option_key, defaults={"value": new_value} ) - - if new_value or created: + if not created and option_inst.value != new_value: + old_val = option_inst.value + option_inst.value = new_value + option_inst.save() + changed_data["hasGranularReplayPermissions"] = f"from {old_val} to {new_value}" + elif created: changed_data["hasGranularReplayPermissions"] = f"to {new_value}" if "replayAccessMembers" in data: - member_ids = data["replayAccessMembers"] + user_ids = data["replayAccessMembers"] + if user_ids is None: + user_ids = [] - if member_ids is None: - member_ids = [] - - current_member_ids = set( - OrganizationMemberReplayAccess.objects.filter(organization=org).values_list( - "organizationmember_id", flat=True - ) + current_user_ids = set( + OrganizationMemberReplayAccess.objects.filter( + organizationmember__organization=org + ).values_list("organizationmember__user_id", flat=True) ) - new_member_ids = set(member_ids) + new_user_ids = set(user_ids) - to_add = new_member_ids - current_member_ids - to_remove = current_member_ids - new_member_ids + to_add = new_user_ids - current_user_ids + to_remove = current_user_ids - new_user_ids if to_add: + user_to_member = dict( + OrganizationMember.objects.filter( + organization=org, user_id__in=to_add + ).values_list("user_id", "id") + ) + invalid_user_ids = to_add - set(user_to_member.keys()) + if invalid_user_ids: + raise serializers.ValidationError( + { + "replayAccessMembers": f"Invalid user IDs (not members of this organization): {sorted(invalid_user_ids)}" + } + ) + OrganizationMemberReplayAccess.objects.bulk_create( [ OrganizationMemberReplayAccess( - organization=org, organizationmember_id=member_id + organizationmember_id=user_to_member[user_id] ) - for member_id in to_add - ] + for user_id in to_add + ], + ignore_conflicts=True, ) if to_remove: OrganizationMemberReplayAccess.objects.filter( - organization=org, organizationmember_id__in=to_remove + organizationmember__organization=org, organizationmember__user_id__in=to_remove ).delete() if to_add or to_remove: changes = [] if to_add: - changes.append(f"added {len(to_add)} member(s)") + changes.append(f"added {len(to_add)} user(s)") if to_remove: - changes.append(f"removed {len(to_remove)} member(s)") + changes.append(f"removed {len(to_remove)} user(s)") changed_data["replayAccessMembers"] = ( - f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)" + f"{' and '.join(changes)} (total: {len(new_user_ids)} user(s) with access)" ) if "openMembership" in data: From 928ead98a4a0b8266541685650cc93d4186a22b3 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 10 Dec 2025 15:27:49 +0100 Subject: [PATCH 37/37] fix bad test logic --- tests/sentry/api/serializers/test_organization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/api/serializers/test_organization.py b/tests/sentry/api/serializers/test_organization.py index 0033291e3b4555..437c9cd03b8fa1 100644 --- a/tests/sentry/api/serializers/test_organization.py +++ b/tests/sentry/api/serializers/test_organization.py @@ -237,7 +237,7 @@ def test_replay_access_members_serialized_with_option_enabled(self) -> None: serializer = DetailedOrganizationSerializer() result = serialize(organization, user, serializer, access=acc) assert result["hasGranularReplayPermissions"] is True - assert set(result["replayAccessMembers"]) == {member1.id, member2.id} + assert set(result["replayAccessMembers"]) == {member1.user_id, member2.user_id} def test_replay_access_members_empty_when_none_set(self) -> None: user = self.create_user()