Skip to content

Commit c1bbd75

Browse files
authored
fix(service-accounts): Don't look up a member limit with no user (#129906)
`is_member_disabled_from_limit` now returns False for a caller with no user, before any membership lookup. Seat limits apply to members, and a service account isn't one. The function takes an `HttpRequest`, so the `request.user` mypy check didn't cover its `user.id` read. Sentry's own permission classes pass an `RpcUserOrganizationContext` and never reach that read. getsentry's customer details endpoint passes a DRF request with an `Organization`, which goes to `check_membership_by_id(user_id=user.id)`. A service account with `org:billing` gets 403 `user-required` there today, from the `ServiceAccountId` guard. Once #129763 removes the guard, it gets a 500. Passing `acting_user_id` instead would send `None` into a lookup that matches pending invites, so the function returns early. The new test on master: ``` E sentry.api.exceptions.UserRequired: {'detail': {'code': 'user-required', 'message': 'This action requires a user.', 'extra': {}}} FAILED tests/sentry/api/test_utils.py::IsMemberDisabledFromLimitTest::test_service_account_is_not_a_limited_member ``` With the fix, the API utils, organization permission and permissions suites pass (214 tests).
1 parent 0ed1f4f commit c1bbd75

2 files changed

Lines changed: 18 additions & 1 deletion

File tree

‎src/sentry/api/utils.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
from sentry import options, quotas
2525
from sentry.api.exceptions import ResourceDoesNotExist
26+
from sentry.auth.principal import has_user
2627
from sentry.auth.staff import is_active_staff
2728
from sentry.auth.superuser import is_active_superuser
2829
from sentry.db.models.fields.bounded import BoundedBigAutoField
@@ -307,7 +308,9 @@ def is_member_disabled_from_limit(
307308
if is_active_superuser(request) or is_active_staff(request):
308309
return False
309310

310-
# must be a simple user at this point
311+
# Only a member can be limited; with no user, a membership lookup would match pending invites.
312+
if not has_user(user):
313+
return False
311314

312315
member: RpcOrganizationMember | None
313316
if isinstance(organization, RpcUserOrganizationContext):

‎tests/sentry/api/test_utils.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import datetime
22
import unittest
3+
from typing import Any
34
from unittest.mock import MagicMock, patch
45

56
import pytest
@@ -16,12 +17,14 @@
1617
clamp_date_range,
1718
get_date_range_from_params,
1819
handle_query_errors,
20+
is_member_disabled_from_limit,
1921
print_and_capture_handler_exception,
2022
to_valid_int_id,
2123
to_valid_int_id_list,
2224
)
2325
from sentry.db.models.fields.bounded import BoundedBigAutoField
2426
from sentry.exceptions import IncompatibleMetricsQuery, InvalidParams, InvalidSearchQuery
27+
from sentry.service_accounts.principal import ServiceAccountPrincipal
2528
from sentry.testutils.cases import APITestCase
2629
from sentry.testutils.helpers.datetime import freeze_time
2730
from sentry.utils.snuba import (
@@ -336,6 +339,17 @@ def test_handle_snuba_rpc_bad_request(self, mock_capture_exception: MagicMock) -
336339
mock_capture_exception.assert_called_once_with(error)
337340

338341

342+
class IsMemberDisabledFromLimitTest(APITestCase):
343+
def test_service_account_is_not_a_limited_member(self) -> None:
344+
request = self.make_request()
345+
principal: Any = ServiceAccountPrincipal(
346+
service_account_id=1, organization_id=self.organization.id, name="sa"
347+
)
348+
request.user = principal
349+
350+
assert is_member_disabled_from_limit(request, self.organization) is False
351+
352+
339353
class ClampDateRangeTest(unittest.TestCase):
340354
def test_no_clamp_if_range_under_max(self) -> None:
341355
start = datetime.datetime(2024, 1, 1)

0 commit comments

Comments
 (0)