Skip to content

Commit e624125

Browse files
mdtroameliahsu
authored andcommitted
fix(api): Clean up API grants when revoking authorizations (#85571)
Pulled from #82052 to break out these changes into smaller PRs. - Delete associated grant when revoking API authorizations - Use proper transaction management with outbox_context
1 parent 1b085e7 commit e624125

File tree

2 files changed

+119
-4
lines changed

2 files changed

+119
-4
lines changed

src/sentry/api/endpoints/api_authorizations.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from sentry.hybridcloud.models.outbox import outbox_context
1313
from sentry.models.apiapplication import ApiApplicationStatus
1414
from sentry.models.apiauthorization import ApiAuthorization
15+
from sentry.models.apigrant import ApiGrant
1516
from sentry.models.apitoken import ApiToken
1617

1718

@@ -44,18 +45,33 @@ def delete(self, request: Request) -> Response:
4445
return Response({"authorization": ""}, status=400)
4546

4647
try:
47-
auth = ApiAuthorization.objects.get(user_id=request.user.id, id=authorization)
48+
api_authorization = ApiAuthorization.objects.get(
49+
user_id=request.user.id, id=authorization
50+
)
4851
except ApiAuthorization.DoesNotExist:
4952
return Response(status=404)
5053

5154
with outbox_context(transaction.atomic(using=router.db_for_write(ApiToken)), flush=False):
5255
for token in ApiToken.objects.filter(
5356
user_id=request.user.id,
54-
application=auth.application_id,
55-
scoping_organization_id=auth.organization_id,
57+
application=api_authorization.application_id,
58+
scoping_organization_id=api_authorization.organization_id,
5659
):
5760
token.delete()
5861

59-
auth.delete()
62+
# remove any grants that were created from this authorization
63+
# that may not have been exchanged for a token yet
64+
with outbox_context(transaction.atomic(using=router.db_for_write(ApiGrant)), flush=False):
65+
for grant in ApiGrant.objects.filter(
66+
user_id=request.user.id,
67+
application=api_authorization.application_id,
68+
organization_id=api_authorization.organization_id,
69+
):
70+
grant.delete()
71+
72+
with outbox_context(
73+
transaction.atomic(using=router.db_for_write(ApiAuthorization)), flush=False
74+
):
75+
api_authorization.delete()
6076

6177
return Response(status=204)

tests/sentry/api/endpoints/test_api_authorizations.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
1+
from datetime import timedelta
2+
3+
from django.utils import timezone
4+
15
from sentry.models.apiapplication import ApiApplication
26
from sentry.models.apiauthorization import ApiAuthorization
7+
from sentry.models.apigrant import ApiGrant
38
from sentry.models.apitoken import ApiToken
49
from sentry.testutils.cases import APITestCase
510
from sentry.testutils.silo import control_silo_test
@@ -43,6 +48,17 @@ def test_org_level_auth(self):
4348
class ApiAuthorizationsDeleteTest(ApiAuthorizationsTest):
4449
method = "delete"
4550

51+
def setUp(self):
52+
super().setUp()
53+
self.user = self.create_user(email="test@example.com")
54+
self.login_as(user=self.user)
55+
56+
self.application = ApiApplication.objects.create(owner=self.create_user(), name="test")
57+
self.authorization = ApiAuthorization.objects.create(
58+
user=self.user,
59+
application=self.application,
60+
)
61+
4662
def test_simple(self):
4763
app = ApiApplication.objects.create(name="test", owner=self.user)
4864
auth = ApiAuthorization.objects.create(application=app, user=self.user)
@@ -77,3 +93,86 @@ def test_with_org(self):
7793

7894
assert ApiAuthorization.objects.filter(id=org2_auth.id).exists()
7995
assert ApiToken.objects.filter(id=org2_token.id).exists()
96+
97+
def test_delete_authorization_cleans_up_grants(self):
98+
# Create API grants associated with this authorization
99+
grant_1 = ApiGrant.objects.create(
100+
user=self.user,
101+
application=self.application,
102+
redirect_uri="https://example.com",
103+
expires_at=timezone.now() + timedelta(minutes=10),
104+
)
105+
106+
grant_2 = ApiGrant.objects.create(
107+
user=self.user,
108+
application=self.application,
109+
redirect_uri="https://example.com",
110+
expires_at=timezone.now() + timedelta(minutes=10),
111+
)
112+
113+
# Only exchange one of the grants
114+
token = ApiToken.from_grant(grant_1)
115+
token.save()
116+
117+
# grant_2 should still exist at this point
118+
assert ApiGrant.objects.filter(id=grant_2.id).exists()
119+
120+
# Make the delete request
121+
self.get_success_response(
122+
authorization=self.authorization.id,
123+
status_code=204,
124+
)
125+
126+
# Verify the authorization is deleted
127+
assert not ApiAuthorization.objects.filter(id=self.authorization.id).exists()
128+
129+
# Verify associated grants are deleted
130+
assert not ApiGrant.objects.filter(id=grant_1.id).exists()
131+
assert not ApiGrant.objects.filter(id=grant_2.id).exists()
132+
133+
# Verify associated tokens are deleted
134+
assert not ApiToken.objects.filter(id=token.id).exists()
135+
136+
def test_delete_authorization_with_no_grants(self):
137+
"""Test that deletion works when there are no associated grants"""
138+
self.get_success_response(
139+
authorization=self.authorization.id,
140+
status_code=204,
141+
)
142+
assert not ApiAuthorization.objects.filter(id=self.authorization.id).exists()
143+
144+
def test_delete_authorization_with_organization_scoped_grants(self):
145+
"""Test that only grants for the specific org are deleted"""
146+
org1 = self.create_organization()
147+
org2 = self.create_organization()
148+
149+
# Create grants for different orgs
150+
grant1 = ApiGrant.objects.create(
151+
user=self.user,
152+
application=self.application,
153+
organization_id=org1.id,
154+
redirect_uri="https://example.com",
155+
)
156+
grant2 = ApiGrant.objects.create(
157+
user=self.user,
158+
application=self.application,
159+
organization_id=org2.id,
160+
redirect_uri="https://example.com",
161+
)
162+
163+
# Create authorization for org1
164+
auth1 = ApiAuthorization.objects.create(
165+
user=self.user,
166+
application=self.application,
167+
organization_id=org1.id,
168+
)
169+
170+
# Delete authorization for org1
171+
self.get_success_response(
172+
authorization=auth1.id,
173+
status_code=204,
174+
)
175+
176+
# Verify only org1's grant is deleted
177+
assert not ApiGrant.objects.filter(id=grant1.id).exists()
178+
assert ApiGrant.objects.filter(id=grant2.id).exists()

0 commit comments

Comments
 (0)