From 3bd0e6ae60bfa845597b17a79d28de8c309e2208 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 5 Dec 2025 13:38:32 +0100 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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 04/15] 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 05/15] 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 06/15] 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 07/15] 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 08/15] 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 09/15] 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 10/15] 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 11/15] :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 12/15] 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 13/15] 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 14/15] 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 15/15] 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()