diff --git a/src/sentry/management/commands/generate_controlsilo_urls.py b/src/sentry/management/commands/generate_controlsilo_urls.py index 67bbe3f1a15f3c..2d7536477dd274 100644 --- a/src/sentry/management/commands/generate_controlsilo_urls.py +++ b/src/sentry/management/commands/generate_controlsilo_urls.py @@ -13,6 +13,7 @@ from django.urls import URLPattern, URLResolver from sentry.silo.base import SiloMode +from sentry.web.frontend.react_page import GenericReactPageView, ReactPageView # There are a handful of catchall routes # that we don't want to handle in controlsilo @@ -95,7 +96,7 @@ def handle(self, **options): func = info.callable if isinstance(func, functools.partial): func = func.func - if hasattr(func, "view_class"): + elif hasattr(func, "view_class"): func = func.view_class if hasattr(func, "__name__"): @@ -114,7 +115,21 @@ def handle(self, **options): except ImportError as err: raise CommandError(f"Could not load view in {module}: {err}") - if not hasattr(view_func, "silo_limit"): + # If a view/endpoint doesn't have a silo_limit it is likely a basic django view. + # We have tests in tests/sentry/silo/test_base.py that ensure all views/endpoints + # have silo annotations on them. We skip including URLs for react views + # as the UI doesn't make requests to them, but the UI code *does* make + # requests to other HTML views (like sentry-wizard) + if not hasattr(view_func, "silo_limit") or ( + isinstance(view_func, type) + and issubclass( + view_func, + ( + ReactPageView, + GenericReactPageView, + ), + ) + ): continue silo_limit = view_func.silo_limit diff --git a/src/sentry/testutils/cases.py b/src/sentry/testutils/cases.py index bdb4cb8b556cfc..8ed5a78d6eb2cd 100644 --- a/src/sentry/testutils/cases.py +++ b/src/sentry/testutils/cases.py @@ -719,8 +719,9 @@ def path_2fa(self): return reverse("sentry-account-settings-security") def enable_org_2fa(self, organization): - organization.flags.require_2fa = True - organization.save() + with assume_test_silo_mode(SiloMode.REGION): + organization.flags.require_2fa = True + organization.save() def api_enable_org_2fa(self, organization, user): self.login_as(user) diff --git a/src/sentry/web/frontend/react_page.py b/src/sentry/web/frontend/react_page.py index 61ec348dc46ff5..9ca7ec57e11aa5 100644 --- a/src/sentry/web/frontend/react_page.py +++ b/src/sentry/web/frontend/react_page.py @@ -23,12 +23,7 @@ ) from sentry.utils.http import is_using_customer_domain, query_string from sentry.web.client_config import get_client_config -from sentry.web.frontend.base import ( - BaseView, - ControlSiloOrganizationView, - all_silo_view, - control_silo_view, -) +from sentry.web.frontend.base import BaseView, ControlSiloOrganizationView, control_silo_view from sentry.web.helpers import render_to_response logger = logging.getLogger(__name__) @@ -194,7 +189,7 @@ def handle_react( # While most of our HTML views are rendered in control silo, # our tests frequently redirect to HTML views -@all_silo_view +@control_silo_view class ReactPageView(ControlSiloOrganizationView, ReactMixin): def handle_auth_required(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: # If user is a superuser (but not active, because otherwise this method would never be called) @@ -221,7 +216,7 @@ def handle(self, request: HttpRequest, organization, **kwargs) -> HttpResponse: return self.handle_react(request, organization=organization) -@all_silo_view +@control_silo_view class GenericReactPageView(BaseView, ReactMixin): def handle(self, request: HttpRequest, **kwargs) -> HttpResponse: return self.handle_react(request, **kwargs) diff --git a/src/sentry/web/urls.py b/src/sentry/web/urls.py index 7a6bc062c83190..8dc19362f7e46a 100644 --- a/src/sentry/web/urls.py +++ b/src/sentry/web/urls.py @@ -1089,7 +1089,7 @@ re_path( r"^(?P[^/]+)/projects/(?P[^/]+)/events/(?P[^/]+)/$", ProjectEventRedirect.as_view(), - name="sentry-project-event-redirect", + name="sentry-organization-project-event-redirect", ), re_path( r"^(?P[^/]+)/api-keys/$", diff --git a/static/app/data/controlsiloUrlPatterns.ts b/static/app/data/controlsiloUrlPatterns.ts index bea6d6d640dd4a..bcafc44a816382 100644 --- a/static/app/data/controlsiloUrlPatterns.ts +++ b/static/app/data/controlsiloUrlPatterns.ts @@ -160,7 +160,6 @@ const patterns: RegExp[] = [ new RegExp('^api/0/tempest-ips/$'), new RegExp('^api/0/secret-scanning/github/$'), new RegExp('^api/hooks/mailgun/inbound/'), - new RegExp('^auth-v2/'), new RegExp('^oauth/authorize/$'), new RegExp('^oauth/token/$'), new RegExp('^oauth/userinfo/$'), @@ -194,10 +193,8 @@ const patterns: RegExp[] = [ new RegExp('^auth/sso/account/settings/social/associate/complete/[^/]+/$'), new RegExp('^out/$'), new RegExp('^settings/organization/auth/configure/$'), - new RegExp('^disabled-member/'), new RegExp('^organizations/[^/]+/auth/configure/$'), new RegExp('^organizations/[^/]+/integrations/[^/]+/setup/$'), - new RegExp('^organizations/[^/]+/disabled-member/$'), new RegExp('^avatar/[^/]+/$'), new RegExp('^sentry-app-avatar/[^/]+/$'), new RegExp('^doc-integration-avatar/[^/]+/$'), @@ -229,7 +226,6 @@ const patterns: RegExp[] = [ new RegExp('^extensions/discord/configure/$'), new RegExp('^extensions/discord/link-identity/[^/]+/$'), new RegExp('^extensions/discord/unlink-identity/[^/]+/$'), - new RegExp('^share/(?:group|issue)/[^/]+/$'), ]; export default patterns; diff --git a/tests/sentry/core/endpoints/test_organization_index.py b/tests/sentry/core/endpoints/test_organization_index.py index 26c8d52321b488..b115c38d23660a 100644 --- a/tests/sentry/core/endpoints/test_organization_index.py +++ b/tests/sentry/core/endpoints/test_organization_index.py @@ -18,7 +18,12 @@ from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, TwoFactorAPITestCase from sentry.testutils.hybrid_cloud import HybridCloudTestMixin -from sentry.testutils.silo import assume_test_silo_mode, create_test_regions, region_silo_test +from sentry.testutils.silo import ( + assume_test_silo_mode, + control_silo_test, + create_test_regions, + region_silo_test, +) from sentry.users.models.authenticator import Authenticator from sentry.utils.slug import ORG_SLUG_PATTERN @@ -328,7 +333,7 @@ def test_streamline_only_is_true(self) -> None: @region_silo_test(regions=create_test_regions("de", "us")) -class OrganizationsCreateInRegionTest(OrganizationIndexTest, HybridCloudTestMixin): +class OrganizationsCreateInRegionTest(OrganizationIndexTest): method = "post" @override_settings(SENTRY_MONOLITH_REGION="us", SENTRY_REGION="de") @@ -348,7 +353,9 @@ def test_success(self) -> None: assert mapping.region_name == "de" +@control_silo_test class OrganizationIndex2faTest(TwoFactorAPITestCase): + # This is the HTML view, not an API endpoint endpoint = "sentry-organization-home" def setUp(self) -> None: @@ -365,8 +372,7 @@ def test_preexisting_members_must_enable_2fa(self) -> None: self.login_as(self.no_2fa_user) self.assert_redirected_to_2fa() - with assume_test_silo_mode(SiloMode.CONTROL): - TotpInterface().enroll(self.no_2fa_user) + TotpInterface().enroll(self.no_2fa_user) self.get_success_response(self.org_2fa.slug) def test_new_member_must_enable_2fa(self) -> None: @@ -396,7 +402,9 @@ def test_superuser_can_access_org_home(self) -> None: self.get_success_response(self.org_2fa.slug) +@control_silo_test class OrganizationIndexMemberLimitTest(APITestCase): + # This is a react view, not an API endpoint endpoint = "sentry-organization-index" def setup_user(self, is_superuser=False): @@ -417,4 +425,5 @@ def test_member_limit_redirect(self) -> None: def test_member_limit_superuser_no_redirect(self) -> None: self.setup_user(is_superuser=True) - self.get_success_response(self.organization.slug, status_code=200) + response = self.get_success_response(self.organization.slug, status_code=200) + assert response.headers["Content-Type"] == "text/html" diff --git a/tests/sentry/management/commands/test_generate_controlsilo_urls.py b/tests/sentry/management/commands/test_generate_controlsilo_urls.py index 06283d11222953..eae8a7b1720892 100644 --- a/tests/sentry/management/commands/test_generate_controlsilo_urls.py +++ b/tests/sentry/management/commands/test_generate_controlsilo_urls.py @@ -37,9 +37,14 @@ def test_write_file(self) -> None: tf.seek(0) result = tf.read().decode("utf8") assert "This is generated code" in result - assert "new RegExp('^api/0/users/$')," in result assert "const patterns" in result assert "export default patterns;" in result + assert "new RegExp('^api/0/users/$')," in result + # Wizard is an HTML view that gets POST from UI code. + assert "new RegExp('^account/settings/wizard/[^/]+/$')," in result + # Views that render react application (like /dashboards) + # are not be used by UI code, and shouldn't be in the URL list + assert "new RegExp('^dashboards" not in result def test_no_missing_urls(self) -> None: pattern_file = "static/app/data/controlsiloUrlPatterns.ts" diff --git a/tests/sentry/middleware/test_customer_domain.py b/tests/sentry/middleware/test_customer_domain.py index 5e9af1016cd4dd..7c5c712f70db83 100644 --- a/tests/sentry/middleware/test_customer_domain.py +++ b/tests/sentry/middleware/test_customer_domain.py @@ -12,9 +12,15 @@ from sentry.api.base import Endpoint from sentry.middleware.customer_domain import CustomerDomainMiddleware +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, TestCase from sentry.testutils.helpers import with_feature -from sentry.testutils.silo import all_silo_test, create_test_regions, no_silo_test +from sentry.testutils.silo import ( + all_silo_test, + assume_test_silo_mode, + create_test_regions, + no_silo_test, +) from sentry.web.frontend.auth_logout import AuthLogoutView @@ -162,11 +168,12 @@ def test_billing_route(self) -> None: self.login_as(user=non_staff_user) self.create_organization(name="albertos-apples", owner=non_staff_user) - response = self.client.get( - "/settings/billing/overview/", - data={"querystring": "value"}, - follow=True, - ) + with assume_test_silo_mode(SiloMode.CONTROL): + response = self.client.get( + "/settings/billing/overview/", + data={"querystring": "value"}, + follow=True, + ) assert response.status_code == 200 assert response.redirect_chain == [ ("http://albertos-apples.testserver/settings/billing/overview/?querystring=value", 302) diff --git a/tests/sentry/web/frontend/test_project_event.py b/tests/sentry/web/frontend/test_project_event.py index 8a845c94cdfd4e..3acee561a61df2 100644 --- a/tests/sentry/web/frontend/test_project_event.py +++ b/tests/sentry/web/frontend/test_project_event.py @@ -29,6 +29,7 @@ def test_redirect_to_event(self) -> None: self.assertRedirects( resp, f"http://testserver/organizations/{self.org.slug}/issues/{self.event.group_id}/events/{self.event.event_id}/", + fetch_redirect_response=False, ) def test_event_not_found(self) -> None: @@ -79,7 +80,7 @@ def test_redirect_to_event_customer_domain(self) -> None: self.org.refresh_from_db() resp = self.client.get( reverse( - "sentry-project-event-redirect", + "sentry-organization-project-event-redirect", args=[self.org.slug, self.project.slug, self.event.event_id], ) )