-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(symbols): Add platform-restricted builtin symbol sources with org access control #102013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
70fc053
584cd00
cc15866
6f916cc
54a82d5
b73221a
64eab65
fd46a7d
ffccdec
a36d188
921d8dc
f7b555d
b77b919
868544d
6a4b9f9
efcb564
6076341
c94b5df
68a2c28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,9 @@ | ||
| from sentry.models.organization import Organization | ||
| from sentry.utils.console_platforms import organization_has_console_platform_access | ||
|
|
||
|
|
||
| def has_tempest_access(organization: Organization | None) -> bool: | ||
|
|
||
| if not organization: | ||
| return False | ||
|
|
||
| enabled_platforms = organization.get_option("sentry:enabled_console_platforms", []) | ||
| has_playstation_access = "playstation" in enabled_platforms | ||
|
|
||
| return has_playstation_access | ||
| return organization_has_console_platform_access(organization, "playstation") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thank you 💜 |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| from sentry.constants import ENABLED_CONSOLE_PLATFORMS_DEFAULT | ||
| from sentry.models.organization import Organization | ||
|
|
||
|
|
||
| def organization_has_console_platform_access(organization: Organization, platform: str) -> bool: | ||
| """ | ||
| Check if an organization has access to a specific console platform. | ||
|
|
||
| Args: | ||
| organization: The organization to check | ||
| platform: The console platform (e.g., 'nintendo-switch', 'playstation', 'xbox') | ||
|
|
||
| Returns: | ||
| True if the organization has access to the console platform, False otherwise | ||
| """ | ||
| enabled_console_platforms = organization.get_option( | ||
| "sentry:enabled_console_platforms", ENABLED_CONSOLE_PLATFORMS_DEFAULT | ||
| ) | ||
| return platform in enabled_console_platforms |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,33 @@ | ||
| from django.test import override_settings | ||
|
|
||
| from sentry.testutils.cases import APITestCase | ||
|
|
||
| SENTRY_BUILTIN_SOURCES_PLATFORM_TEST = { | ||
| "public-source-1": { | ||
| "id": "sentry:public-1", | ||
| "name": "Public Source 1", | ||
| "type": "http", | ||
| "url": "https://example.com/symbols/", | ||
| }, | ||
| "public-source-2": { | ||
| "id": "sentry:public-2", | ||
| "name": "Public Source 2", | ||
| "type": "http", | ||
| "url": "https://example.com/symbols2/", | ||
| }, | ||
| "nintendo": { | ||
| "id": "sentry:nintendo", | ||
| "name": "Nintendo SDK", | ||
| "type": "s3", | ||
| "bucket": "nintendo-symbols", | ||
| "region": "us-east-1", | ||
| "access_key": "test-key", | ||
| "secret_key": "test-secret", | ||
| "layout": {"type": "native"}, | ||
| "platforms": ["nintendo-switch"], | ||
| }, | ||
| } | ||
|
|
||
|
Comment on lines
+5
to
+30
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of defining this object, can't we just import the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need it because
|
||
|
|
||
| class BuiltinSymbolSourcesNoSlugTest(APITestCase): | ||
| endpoint = "sentry-api-0-builtin-symbol-sources" | ||
|
|
@@ -39,3 +67,77 @@ def test_with_slug(self) -> None: | |
| assert "id" in body[0] | ||
| assert "name" in body[0] | ||
| assert "hidden" in body[0] | ||
|
|
||
|
|
||
| class BuiltinSymbolSourcesPlatformFilteringTest(APITestCase): | ||
| endpoint = "sentry-api-0-organization-builtin-symbol-sources" | ||
|
|
||
| def setUp(self) -> None: | ||
| super().setUp() | ||
| self.organization = self.create_organization(owner=self.user) | ||
| self.login_as(user=self.user) | ||
|
|
||
| @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_PLATFORM_TEST) | ||
| def test_platform_filtering_nintendo_switch_with_access(self) -> None: | ||
| """Nintendo Switch platform should see nintendo source only if org has access""" | ||
| # Enable nintendo-switch for this organization | ||
| self.organization.update_option("sentry:enabled_console_platforms", ["nintendo-switch"]) | ||
|
|
||
| resp = self.get_response(self.organization.slug, qs_params={"platform": "nintendo-switch"}) | ||
| assert resp.status_code == 200 | ||
|
|
||
| body = resp.data | ||
| source_keys = [source["sentry_key"] for source in body] | ||
|
|
||
| # Nintendo Switch with access should see nintendo | ||
| assert "nintendo" in source_keys | ||
| # Should also see public sources (no platform restriction) | ||
| assert "public-source-1" in source_keys | ||
| assert "public-source-2" in source_keys | ||
|
|
||
| @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_PLATFORM_TEST) | ||
| def test_platform_filtering_nintendo_switch_without_access(self) -> None: | ||
| """Nintendo Switch platform should NOT see nintendo if org lacks access""" | ||
| # Organization does not have nintendo-switch enabled (default is empty list) | ||
|
|
||
| resp = self.get_response(self.organization.slug, qs_params={"platform": "nintendo-switch"}) | ||
| assert resp.status_code == 200 | ||
|
|
||
| body = resp.data | ||
| source_keys = [source["sentry_key"] for source in body] | ||
|
|
||
| # Should NOT see nintendo without console platform access | ||
| assert "nintendo" not in source_keys | ||
| # Should still see public sources | ||
| assert "public-source-1" in source_keys | ||
| assert "public-source-2" in source_keys | ||
|
Comment on lines
+100
to
+113
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we adding an additional security check here? Users should only be able to create a Nintendo Switch project if they have access. Or is the idea that access could later be revoked, in which case they would already have an existing Nintendo Switch project?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes we are. the platform is an API input so if we didn't check for access, anyone could add it just by overriding the request. |
||
|
|
||
| @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_PLATFORM_TEST) | ||
| def test_platform_filtering_unity(self) -> None: | ||
| """Unity platform should NOT see nintendo source""" | ||
| resp = self.get_response(self.organization.slug, qs_params={"platform": "unity"}) | ||
| assert resp.status_code == 200 | ||
|
|
||
| body = resp.data | ||
| source_keys = [source["sentry_key"] for source in body] | ||
|
|
||
| # Unity should see public sources (no platform restriction) | ||
| assert "public-source-1" in source_keys | ||
| assert "public-source-2" in source_keys | ||
| # Unity should NOT see nintendo (restricted to nintendo-switch) | ||
| assert "nintendo" not in source_keys | ||
|
|
||
| @override_settings(SENTRY_BUILTIN_SOURCES=SENTRY_BUILTIN_SOURCES_PLATFORM_TEST) | ||
| def test_no_platform_parameter(self) -> None: | ||
| """Without platform parameter, should see public sources but not platform-restricted ones""" | ||
| resp = self.get_response(self.organization.slug) | ||
| assert resp.status_code == 200 | ||
|
|
||
| body = resp.data | ||
| source_keys = [source["sentry_key"] for source in body] | ||
|
|
||
| # Should see public sources (no platform restriction) | ||
| assert "public-source-1" in source_keys | ||
| assert "public-source-2" in source_keys | ||
| # Should NOT see platform-restricted source when no platform is provided | ||
| assert "nintendo" not in source_keys | ||
Uh oh!
There was an error while loading. Please reload this page.