Skip to content

Commit bf9592d

Browse files
feat(auth): Return auth error if application is requesting a wrong org (#81193)
If an application is organization scope application, their tokens will only have access to one organization of a user. So we should return auth error if: 1. They're calling an API on an organization that is not the same as the org in the token 2. They're calling an API that is not limited to one organization, for example list all user's project [In a previous PR](#80012) I added some logging to make sure this doesn't break other integrations. It actually does, so I have to limit this to token.scoping_organization_id vs token.organization_id
1 parent cbc4a8c commit bf9592d

File tree

2 files changed

+75
-24
lines changed

2 files changed

+75
-24
lines changed

src/sentry/api/authentication.py

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,11 @@ def authenticate_token(self, request: Request, token_str: str) -> tuple[Any, Any
422422
if application_is_inactive:
423423
raise AuthenticationFailed("UserApplication inactive or deleted")
424424

425-
if token.organization_id:
425+
if token.scoping_organization_id:
426426
# We need to make sure the organization to which the token has access is the same as the one in the URL
427427
organization = None
428428
organization_context = organization_service.get_organization_by_id(
429-
id=token.organization_id
429+
id=token.organization_id, include_projects=False, include_teams=False
430430
)
431431
if organization_context:
432432
organization = organization_context.organization
@@ -439,30 +439,15 @@ def authenticate_token(self, request: Request, token_str: str) -> tuple[Any, Any
439439
organization.slug != target_org_id_or_slug
440440
and organization.id != target_org_id_or_slug
441441
):
442-
# TODO (@athena): We want to raise auth excecption here but to be sure
443-
# I soft launch this by only logging the error for now
444-
# raise AuthenticationFailed("Unauthorized organization access")
445-
logger.info(
446-
"Token has access to organization %s but wants to get access to organization %s: %s",
447-
organization.slug,
448-
target_org_id_or_slug,
449-
request.path_info,
450-
)
451-
else:
452-
# TODO (@athena): We want to limit org level token's access to org level endpoints only
453-
# so in the future this will be an auth exception but for now we soft launch by logging an error
454-
logger.info(
455-
"Token has only access to organization %s but is calling an endpoint for multiple organizations: %s",
456-
organization.slug,
457-
request.path_info,
442+
raise AuthenticationFailed("Unauthorized organization access.")
443+
# We want to limit org scoped tokens access to org level endpoints only
444+
# Except some none-org level endpoints that we added special treatments for
445+
elif resolved_url.url_name not in ["sentry-api-0-organizations"]:
446+
raise AuthenticationFailed(
447+
"This token access is limited to organization endpoints."
458448
)
459449
else:
460-
# TODO (@athena): If there is an organization token we should be able to fetch organization context
461-
# Otherwise we should raise an exception
462-
# For now adding logging to investigate if this is a valid case we need to address
463-
logger.info(
464-
"Token has access to an unknown organization: %s", token.organization_id
465-
)
450+
raise AuthenticationFailed("Cannot resolve organization from token.")
466451

467452
return self.transform_auth(
468453
user,

tests/sentry/api/test_authentication.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,72 @@ def test_no_match(self):
210210
self.auth.authenticate(request)
211211

212212

213+
@control_silo_test
214+
class TestOrgScopedAppTokenAuthentication(TestCase):
215+
def setUp(self):
216+
super().setUp()
217+
218+
self.auth = UserAuthTokenAuthentication()
219+
self.org = self.create_organization(owner=self.user)
220+
self.another_org = self.create_organization(owner=self.user)
221+
self.api_token = ApiToken.objects.create(
222+
token_type=AuthTokenType.USER,
223+
user=self.user,
224+
scoping_organization_id=self.org.id,
225+
)
226+
self.token = self.api_token.plaintext_token
227+
228+
def test_authenticate_correct_org(self):
229+
request = HttpRequest()
230+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {self.token}"
231+
request.path_info = f"/api/0/organizations/{self.org.slug}/projects/"
232+
233+
result = self.auth.authenticate(request)
234+
assert result is not None
235+
236+
user, auth = result
237+
assert user.is_anonymous is False
238+
assert user.id == self.user.id
239+
assert AuthenticatedToken.from_token(auth) == AuthenticatedToken.from_token(self.api_token)
240+
241+
def test_authenticate_incorrect_org(self):
242+
request = HttpRequest()
243+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {self.token}"
244+
request.path_info = f"/api/0/organizations/{self.another_org}/projects/"
245+
246+
with pytest.raises(AuthenticationFailed):
247+
self.auth.authenticate(request)
248+
249+
def test_authenticate_user_level_endpoints(self):
250+
request = HttpRequest()
251+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {self.token}"
252+
request.path_info = "/api/0/projects/"
253+
254+
with pytest.raises(AuthenticationFailed):
255+
self.auth.authenticate(request)
256+
257+
def test_authenticate_allowlist_endpoint(self):
258+
request = HttpRequest()
259+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {self.token}"
260+
request.path_info = "/api/0/organizations/"
261+
262+
result = self.auth.authenticate(request)
263+
assert result is not None
264+
265+
user, auth = result
266+
assert user.is_anonymous is False
267+
assert user.id == self.user.id
268+
assert AuthenticatedToken.from_token(auth) == AuthenticatedToken.from_token(self.api_token)
269+
270+
def test_no_match(self):
271+
request = HttpRequest()
272+
request.META["HTTP_AUTHORIZATION"] = "Bearer abc"
273+
request.path_info = f"/api/0/organizations/{self.another_org}/projects/"
274+
275+
with pytest.raises(AuthenticationFailed):
276+
self.auth.authenticate(request)
277+
278+
213279
@django_db_all
214280
@pytest.mark.parametrize("internal", [True, False])
215281
def test_registered_relay(internal):

0 commit comments

Comments
 (0)