diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index d9ccdbaaa738ea..7a90bc0f60f3b8 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -194,7 +194,11 @@ def validate_name(self, value: str) -> str: return value - def validate_slug(self, value: str) -> str: + def _validate_slug_shape(self, value: str) -> str: + """ + Validate slug values without any DB queries. + This method is re-used across cell + control silos. + """ # Historically, the only check just made sure there was more than 1 # character for the slug, but since then, there are many slugs that # fit within this new imposed limit. We're not fixing existing, but @@ -209,12 +213,6 @@ def validate_slug(self, value: str) -> str: ) if value in RESERVED_ORGANIZATION_SLUGS: raise serializers.ValidationError(f'This slug "{value}" is reserved and not allowed.') - qs = Organization.objects.filter(slug=value) - if "organization" in self.context: - qs = qs.exclude(id=self.context["organization"].id) - if qs.exists(): - raise serializers.ValidationError(f'The slug "{value}" is already in use.') - contains_whitespace = any(c.isspace() for c in self.initial_data["slug"]) if contains_whitespace: raise serializers.ValidationError( @@ -222,6 +220,15 @@ def validate_slug(self, value: str) -> str: ) return value + def validate_slug(self, value: str) -> str: + value = self._validate_slug_shape(value) + qs = Organization.objects.filter(slug=value) + if "organization" in self.context: + qs = qs.exclude(id=self.context["organization"].id) + if qs.exists(): + raise serializers.ValidationError(f'The slug "{value}" is already in use.') + return value + class TrustedRelaySerializerResponse(TypedDict, total=False): name: str diff --git a/src/sentry/core/endpoints/organization_index.py b/src/sentry/core/endpoints/organization_index.py index 94f557acd8b1de..8b55fcb1232775 100644 --- a/src/sentry/core/endpoints/organization_index.py +++ b/src/sentry/core/endpoints/organization_index.py @@ -33,6 +33,7 @@ from sentry.models.organizationmapping import OrganizationMapping from sentry.models.organizationmember import OrganizationMember from sentry.models.organizationmembermapping import OrganizationMemberMapping +from sentry.organizations.services.organization import organization_service from sentry.search.utils import tokenize_query from sentry.services.organization import ( OrganizationOptions, @@ -41,6 +42,12 @@ ) from sentry.services.organization.provisioning import organization_provisioning_service from sentry.silo.base import SiloMode +from sentry.types.cell import ( + CellResolutionError, + RegionCategory, + get_locality_by_name, + get_new_org_cell_for_locality, +) from sentry.users.services.user.serial import serialize_generic_user from sentry.users.services.user.service import user_service from sentry.utils.pagination_factory import PaginatorLike @@ -53,6 +60,7 @@ class OrganizationPostSerializer(BaseOrganizationSerializer): agreeTerms = serializers.BooleanField(required=True) aggregatedDataConsent = serializers.BooleanField(required=False) idempotencyKey = serializers.CharField(max_length=IDEMPOTENCY_KEY_LENGTH, required=False) + dataStorageLocation = serializers.CharField(required=False) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -66,6 +74,38 @@ def validate_agreeTerms(self, value): raise serializers.ValidationError("This attribute is required.") return value + def validate_slug(self, value: str) -> str: + if SiloMode.get_current_mode() == SiloMode.CONTROL: + value = self._validate_slug_shape(value) + if OrganizationMapping.objects.filter(slug=value).exists(): + raise serializers.ValidationError(f'The slug "{value}" is already in use.') + + return value + # TODO(cells) remove this path when cell scoped provisioning is removed. + return super().validate_slug(value) + + def validate_dataStorageLocation(self, value: str) -> str: + try: + locality = get_locality_by_name(value) + except CellResolutionError: + raise serializers.ValidationError(f"Unknown data storage location {value!r}.") + if locality.category != RegionCategory.MULTI_TENANT or not locality.visible: + raise serializers.ValidationError(f"Unknown data storage location {value!r}.") + return value + + def validate(self, attrs): + attrs = super().validate(attrs) + + locality_name = attrs.get("dataStorageLocation") + + if locality_name: + attrs["cell_name"] = get_new_org_cell_for_locality(locality_name).name + else: + # TODO(cells) Remove this when cell silo compatibility is removed. + attrs["cell_name"] = settings.SENTRY_LOCAL_CELL or settings.SENTRY_MONOLITH_REGION + + return attrs + @extend_schema(tags=["Users"]) @all_silo_endpoint @@ -312,7 +352,6 @@ def _get_from_control(self, request: Request) -> Response: paginator_cls=paginator_cls, ) - # XXX: endpoint useless for end-users as it needs user context. def post(self, request: Request) -> Response: """ Create a New Organization @@ -329,13 +368,6 @@ def post(self, request: Request) -> Response: terms of service and privacy policy. :auth: required, user-context-needed """ - # TODO(cells): Move org creation to control as part of the broader - # org-listing/org-provisioning cutover. Since POST is private, the - # legacy cell-side path can be removed once the control implementation - # is ready. - if SiloMode.get_current_mode() == SiloMode.CONTROL: - return Response(status=404) - if not request.user.is_authenticated: return Response({"detail": "This endpoint requires user info"}, status=401) @@ -403,16 +435,16 @@ def post(self, request: Request) -> Response: ) rpc_org = organization_provisioning_service.provision_organization_in_cell( - cell_name=settings.SENTRY_LOCAL_CELL or settings.SENTRY_MONOLITH_REGION, + cell_name=result["cell_name"], provisioning_options=provision_args, ) - org = Organization.objects.get(id=rpc_org.id) - - # TODO(hybrid-cloud): We'll need to catch a more generic error - # when the internal RPC is implemented. except IntegrityError: + # TODO(cells) Remove this once all provisioning goes through control + # Instead we'll need to handle error messages from the RPC call. return Response( {"detail": "An organization with this slug already exists."}, status=409 ) - return Response(serialize(org, request.user), status=201) + org_data = organization_service.serialize_organization(id=rpc_org.id, as_user=rpc_user) + + return Response(org_data, status=201) diff --git a/tests/sentry/core/endpoints/test_organization_index.py b/tests/sentry/core/endpoints/test_organization_index.py index f4f55d44e7f238..ce8ae123d301aa 100644 --- a/tests/sentry/core/endpoints/test_organization_index.py +++ b/tests/sentry/core/endpoints/test_organization_index.py @@ -25,16 +25,19 @@ from sentry.silo.base import SiloMode from sentry.testutils.asserts import assert_org_audit_log_exists from sentry.testutils.cases import APITestCase, TwoFactorAPITestCase +from sentry.testutils.cell import get_test_env_directory from sentry.testutils.helpers.analytics import assert_any_analytics_event from sentry.testutils.helpers.options import override_options from sentry.testutils.hybrid_cloud import HybridCloudTestMixin from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import ( assume_test_silo_mode, + assume_test_silo_mode_of, cell_silo_test, control_silo_test, create_test_cells, ) +from sentry.types.cell import Cell, Locality, RegionCategory from sentry.users.models.authenticator import Authenticator from sentry.utils.slug import ORG_SLUG_PATTERN @@ -234,7 +237,212 @@ def test_response_compatible_with_cell(self) -> None: } -class OrganizationsCreateTest(OrganizationIndexTest, HybridCloudTestMixin): +@control_silo_test(cells=create_test_cells("us", "de")) +class OrganizationsCreateControlTest(OrganizationIndexTest, HybridCloudTestMixin): + method = "post" + + def test_implicit_data_storage_location(self) -> None: + response = self.get_success_response(name="implicit org", slug="implicit-org") + + organization_id = response.data["id"] + with assume_test_silo_mode(SiloMode.CONTROL): + mapping = OrganizationMapping.objects.get(organization_id=organization_id) + assert mapping.cell_name == "us" + + def test_invalid_data_storage_location(self) -> None: + response = self.get_error_response( + name="bad locality", + slug="bad-locality", + dataStorageLocation="atlantis", + status_code=400, + ) + assert "dataStorageLocation" in response.data + + def test_locality_to_cell_resolution(self) -> None: + cells = [ + Cell( + name="us", + snowflake_id=1, + category=RegionCategory.MULTI_TENANT, + address="10.0.0.1", + visible=True, + ), + Cell( + name="us2", + snowflake_id=3, + category=RegionCategory.MULTI_TENANT, + address="10.0.0.2", + visible=True, + ), + Cell( + name="de", + snowflake_id=4, + category=RegionCategory.MULTI_TENANT, + address="10.0.0.4", + visible=True, + ), + ] + + localities = [ + Locality( + name="us", + cells=frozenset(["us", "us2"]), + category=RegionCategory.MULTI_TENANT, + new_org_cell="us2", + visible=True, + ), + Locality( + name="de", + cells=frozenset(["de"]), + category=RegionCategory.MULTI_TENANT, + new_org_cell="de", + visible=True, + ), + ] + with get_test_env_directory().swap_state(cells, localities): + data = {"name": "Acme co", "slug": "acme-co", "dataStorageLocation": "us"} + resp = self.get_success_response(**data) + assert "acme-co" == resp.data["slug"] + org_id = resp.data["id"] + + with assume_test_silo_mode_of(OrganizationMapping): + org_mapping = OrganizationMapping.objects.get(slug="acme-co", organization_id=org_id) + assert org_mapping.cell_name == "us2", "Should be created in us2 cell, not us" + + with assume_test_silo_mode_of(Organization): + org = Organization.objects.get(id=org_id) + assert org.slug == org_mapping.slug + + # Validate ownership of the new org + owners = [owner.id for owner in org.get_owners()] + assert [self.user.id] == owners + + def test_with_default_team_true(self) -> None: + data = {"name": "hello world", "slug": "foobar", "defaultTeam": True} + response = self.get_success_response(**data) + + organization_id = response.data["id"] + + with assume_test_silo_mode_of(Organization): + Organization.objects.get(id=organization_id) + team = Team.objects.get(organization_id=organization_id) + assert team.name == "hello world" + + org_member = OrganizationMember.objects.get( + organization_id=organization_id, user_id=self.user.id + ) + OrganizationMemberTeam.objects.get(organizationmember_id=org_member.id, team_id=team.id) + + def test_invalid_slug_values(self) -> None: + with self.options({"api.rate-limit.org-create": 9001}): + self.get_error_response(name="name", slug=" i have whitespace ", status_code=400) + self.get_error_response(name="name", slug="bird-company!", status_code=400) + + def test_conflicting_slug(self) -> None: + self.create_organization(slug="acme-co") + + resp = self.get_error_response(name="name", slug="acme-co", status_code=400) + assert 'The slug "acme-co" is already in use' in str(resp.data) + + def test_name_with_url_scheme_rejected(self) -> None: + with self.options({"api.rate-limit.org-create": 9001}): + self.get_error_response( + name="https://evil.com Click Here", slug="legit-slug", status_code=400 + ) + self.get_error_response(name="http://evil.com", slug="legit-slug-2", status_code=400) + + def test_name_with_spam_signals_rejected(self) -> None: + response = self.get_error_response( + name="Win $50 ETH bit.ly/offer Claim Now", + slug="spam-org", + status_code=400, + ) + assert "disallowed content" in str(response.data) + + def test_name_with_single_signal_allowed(self) -> None: + response = self.get_success_response(name="BTC Analytics", slug="btc-analytics") + with assume_test_silo_mode_of(Organization): + org = Organization.objects.get(id=response.data["id"]) + assert org.name == "BTC Analytics" + + def test_required_terms_with_terms_url(self) -> None: + data: dict[str, Any] = {"name": "hello world"} + with self.settings(PRIVACY_URL=None, TERMS_URL="https://example.com/terms"): + self.get_success_response(**data) + + with self.settings(TERMS_URL=None, PRIVACY_URL="https://example.com/privacy"): + self.get_success_response(**data) + + with self.settings( + TERMS_URL="https://example.com/terms", PRIVACY_URL="https://example.com/privacy" + ): + data = {"name": "hello world", "agreeTerms": False} + self.get_error_response(status_code=400, **data) + + data = {"name": "hello world", "agreeTerms": True} + self.get_success_response(**data) + + @mock.patch("sentry.analytics.record") + def test_success_analytics_in_rpc_call(self, mock_record: mock.MagicMock) -> None: + self.login_as(user=self.user) + + with outbox_runner(): + data = { + "name": "org name", + "aggregatedDataConsent": True, + "agreeTerms": True, + "defaultTeam": True, + } + response = self.get_success_response(**data) + assert response.status_code == 201 + + with assume_test_silo_mode_of(Organization): + org = Organization.objects.get(slug="org-name") + + assert_any_analytics_event( + mock_record, + OrganizationCreatedEvent( + id=org.id, + actor_id=self.user.id, + name=org.name, + slug=org.slug, + ), + ) + assert_any_analytics_event( + mock_record, AggregatedDataConsentOrganizationCreatedEvent(organization_id=org.id) + ) + assert_org_audit_log_exists( + organization=org, + event=audit_log.get_event_id("ORG_ADD"), + ) + with assume_test_silo_mode_of(Organization): + assert org.get_option("sentry:aggregated_data_consent") is True + assert org.get_option("sentry:streamline_ui_only") is True + assert OrganizationMember.objects.filter( + organization_id=org.id, user_id=self.user.id + ).exists() + assert Team.objects.filter(organization_id=org.id).exists() + + def test_demo_user_cannot_create_organization(self) -> None: + demo_user = self.create_user("demo@example.com") + self.login_as(demo_user) + with override_options({"demo-mode.enabled": True, "demo-mode.users": [demo_user.id]}): + self.get_error_response(name="demo org", slug="demo-org", status_code=403) + + with assume_test_silo_mode_of(Organization): + assert not Organization.objects.filter(slug="demo-org").exists() + + def test_demo_user_cannot_create_organization_when_demo_mode_disabled(self) -> None: + demo_user = self.create_user("demo@example.com") + self.login_as(demo_user) + with override_options({"demo-mode.enabled": False, "demo-mode.users": [demo_user.id]}): + self.get_error_response(name="demo org", slug="demo-org", status_code=403) + + with assume_test_silo_mode_of(Organization): + assert not Organization.objects.filter(slug="demo-org").exists() + + +class OrganizationsCreateInCellTest(OrganizationIndexTest, HybridCloudTestMixin): method = "post" def test_missing_params(self) -> None: