feat(cells): Implement owner=1 on control silo org listing by lynnagara · Pull Request #116439 · getsentry/sentry

lynnagara · 2026-05-28T21:28:59Z

Replaces the owner=1 → 400 stub in OrganizationIndexEndpoint._get_from_control with a real implementation that queries OrganizationMemberMapping for the user's owner-role orgs and computes singleOwner from an aggregated count of active co-owners. Returns the same [{organization, singleOwner}] shape as the cell-side path, so existing frontend callers (account close + security enrolment) can migrate to this endpoint without major frontend changes.

Also fixes an N+1 query in the existing cell-side path. The cell version runs has_single_owner() and a full feature-flag evaluation (features.batch_has

per-feature features.has + onboarding/option queries) per org. Control issues the same queries regardless of org count and skips feature evaluation entirely since org features are no longer returned from control.

Replaces the `owner=1` → 400 stub in `OrganizationIndexEndpoint._get_from_control` with a real implementation that queries `OrganizationMemberMapping` for the user's owner-role orgs and computes `singleOwner` from an aggregated count of active co-owners. Returns the same `[{organization, singleOwner}]` shape as the cell-side path, so existing frontend callers (account close + security enrolment) can migrate to this endpoint without major frontend changes. Also fixes an N+1 query in the existing cell-side path. The cell version runs `has_single_owner()` and a full feature-flag evaluation (`features.batch_has` + per-feature `features.has` + onboarding/option queries) per org. Control issues the same queries regardless of org count and skips feature evaluation entirely since org features are no longer returned from control.

strongs · 2026-05-28T21:55:09Z

-                status=status.HTTP_400_BAD_REQUEST,
-            )
+        # This is used when closing an account.
+        if owner_only and request.user.is_authenticated:


If we do owner_only for an unauth'd user, it seems a little weird we would 200 (empty response) instead of a 401 (or permission_denied, etc)

Agreed. a 200 to an unauthenticated request isn't right. The endpoint's permission_classes and authentication_classes should be keeping unauthenticated users out.

The auth is checked in two steps

first the endpoint level OrganizationPermission runs. This enforces the right permissions for users, but it also supports token access (in which case request.user would be empty)

the request.user.is_authenticated ensures that it's a user and not a token

I added a comment in code describing this since it's not particularly clear. I ported this logic 1:1 from the cell endpoint list. We could add another check + raise PermissionDenied here but in practice with the two above checks that code path would never run, so I would probably leave it as-is.

I changed the PR slightly so now token requests with owner=1 do raise PermissionDenied

In the cell implementation they ignore the owner param and fall through to the regular implementation.

It seems better to be a bit more strict here.

strongs · 2026-05-28T21:56:14Z

            paginator_cls=paginator_cls,
        )

+    def _get_owned_from_control(self, request: Request) -> Response:


cell path uses OffsetPaginator to paginate orgs, maybe it's not a big deal because no single owner would have that many orgs? 🤷

👍 to having pagination here too. We have pagination in both the cell and other control path.

The cell path doesn't have pagination either for the owner path, i copied it exactly.

I do agree it's more correct to have pagination, but I also think adding it breaks the contract with the frontend which doesn't handle pagination, Link header or anything like that, so it's a bit harder to cut across.

I would prefer leave it as-is as it's a larger change and changes the data expected by the frontend. If we only added it on the backend, the resultset would be limited to 100 which would probably be worse for users who own more than 100 orgs as those over that limit would just get hidden by the frontend.

Fair enough, I looked in production db, and there aren't any users who are owner of >100 orgs yet.

GabeVillalobos · 2026-05-28T22:46:27Z

+            OrganizationMemberMapping.objects.filter(
+                organization_id__in=[m.organization_id for m in org_mappings],
+                role=owner_role,
+                user_id__isnull=False,
+                user__is_active=True,
+            )
+            .values("organization_id")
+            .annotate(count=Count("id"))


I'm probably missing some context, but is there a guarantee this or the org mapping table are up to date when we run this?

There aren't any guarantees that membership replication has caught up, I don't think we can get that either. We'll have to use last known information.

markstory · 2026-05-29T15:59:37Z

-                status=status.HTTP_400_BAD_REQUEST,
-            )
+        # This is used when closing an account.
+        if owner_only and request.user.is_authenticated:


Agreed. a 200 to an unauthenticated request isn't right. The endpoint's permission_classes and authentication_classes should be keeping unauthenticated users out.

markstory · 2026-05-29T16:02:19Z

            paginator_cls=paginator_cls,
        )

+    def _get_owned_from_control(self, request: Request) -> Response:


👍 to having pagination here too. We have pagination in both the cell and other control path.

markstory · 2026-05-29T16:03:44Z

+            OrganizationMemberMapping.objects.filter(
+                organization_id__in=[m.organization_id for m in org_mappings],
+                role=owner_role,
+                user_id__isnull=False,
+                user__is_active=True,
+            )
+            .values("organization_id")
+            .annotate(count=Count("id"))


There aren't any guarantees that membership replication has caught up, I don't think we can get that either. We'll have to use last known information.

markstory · 2026-05-29T16:04:45Z

+        org_mappings = list(
+            OrganizationMapping.objects.filter(
+                organization_id__in=owner_org_ids,
+                status=OrganizationStatus.ACTIVE,
+            ).order_by("name")


Should we add a relationship between OrganizationMapping and OrganizationMemberMapping to make these queries easier to build?

While this might make constructing this query a bit easier, I don't think it's right to add this relationship for other reasons:

these tables are not the source of truth, they are simply replicated from the cell, so the constraints shouldn't be enforced here

the two underlying tables replicate completely independently so this would raise all kinds of consistency issues depending on which tables get populated first

this one query isn't that bad, it might be slightly nicer to write the other way but this isn't really an issue in terms of performance

I agree that we don't want foreign key constraints, but django's relations don't require foreign keys. They can also work with unique indexes and organization_mapping.organization_id has a unique index on it. With the relation we'd be able to have django generate joins which would save us some manual query building work.

Lets see how many more queries like this we end up having to make and re-assess later.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit e0fb494. Configure here.}

markstory · 2026-05-29T21:49:28Z

            paginator_cls=paginator_cls,
        )

+    def _get_owned_from_control(self, request: Request) -> Response:


Fair enough, I looked in production db, and there aren't any users who are owner of >100 orgs yet.

markstory · 2026-05-29T21:51:55Z

+        org_mappings = list(
+            OrganizationMapping.objects.filter(
+                organization_id__in=owner_org_ids,
+                status=OrganizationStatus.ACTIVE,
+            ).order_by("name")


I agree that we don't want foreign key constraints, but django's relations don't require foreign keys. They can also work with unique indexes and organization_mapping.organization_id has a unique index on it. With the relation we'd be able to have django generate joins which would save us some manual query building work.

Lets see how many more queries like this we end up having to make and re-assess later.

lynnagara requested a review from a team as a code owner May 28, 2026 21:28

github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 28, 2026

strongs reviewed May 28, 2026

View reviewed changes

GabeVillalobos reviewed May 28, 2026

View reviewed changes

markstory reviewed May 29, 2026

View reviewed changes

add comment about ownership persmission

e0fb494

sentry Bot reviewed May 29, 2026

View reviewed changes

Comment thread src/sentry/core/endpoints/organization_index.py Outdated

cursor Bot reviewed May 29, 2026

View reviewed changes

Comment thread src/sentry/core/endpoints/organization_index.py

lynnagara added 2 commits May 29, 2026 13:02

.

054be01

raise permission denied if token requests owner

893b4e5

markstory approved these changes May 29, 2026

View reviewed changes

lynnagara merged commit 53c5dd6 into master May 29, 2026
63 checks passed

lynnagara deleted the org-list-ownership branch May 29, 2026 22:16

Uh oh!

Conversation

lynnagara commented May 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lynnagara May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lynnagara May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lynnagara May 29, 2026 •

edited

Loading

lynnagara May 29, 2026 •

edited

Loading