Skip to content

Commit 6fb25c7

Browse files
committed
AAP-72278 Fix token visibility
1 parent d4ea85e commit 6fb25c7

2 files changed

Lines changed: 121 additions & 12 deletions

File tree

ansible_base/oauth2_provider/views/token.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
from django.utils.timezone import now
55
from oauth2_provider import views as oauth_views
66
from oauthlib import oauth2
7+
from django.conf import settings
8+
from django.db.models import Q
9+
from rest_framework.permissions import SAFE_METHODS
710
from rest_framework.viewsets import ModelViewSet
811

912
from ansible_base.lib.utils.hashing import hash_string
@@ -75,3 +78,19 @@ class OAuth2TokenViewSet(ModelViewSet, AnsibleBaseDjangoAppApiView):
7578
queryset = OAuth2AccessToken.objects.all()
7679
serializer_class = OAuth2TokenSerializer
7780
permission_classes = [OAuth2ScopePermission, OAuth2TokenPermission]
81+
82+
def get_queryset(self):
83+
qs = super().get_queryset()
84+
user = self.request.user
85+
if user.is_superuser:
86+
return qs
87+
if self.request.method in SAFE_METHODS and getattr(user, 'is_platform_auditor', False):
88+
return qs
89+
q = Q(user=user)
90+
if 'ansible_base.rbac' in settings.INSTALLED_APPS:
91+
from ansible_base.lib.utils.auth import get_organization_model
92+
93+
org_cls = get_organization_model()
94+
admin_org_ids = org_cls.access_ids_qs(user, 'change')
95+
q |= Q(application__organization_id__in=admin_org_ids)
96+
return qs.filter(q)

test_app/tests/oauth2_provider/test_rbac.py

Lines changed: 102 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
conditions are not met).
1111
"""
1212

13+
from datetime import datetime, timezone
14+
1315
import pytest
16+
from oauthlib.common import generate_token
1417

1518
from ansible_base.lib.utils.response import get_relative_url
1619
from ansible_base.oauth2_provider.models import OAuth2AccessToken
@@ -191,9 +194,18 @@ def test_oauth2_application_token_read_change_delete(
191194
app.user = random_user
192195
app.save()
193196

194-
expected_read_status = 200 if has_access else 403
195-
expected_change_status = 200 if has_access else 403
196-
expected_delete_status = 204 if has_access else 403
197+
if has_access:
198+
expected_read_status = 200
199+
expected_change_status = 200
200+
expected_delete_status = 204
201+
elif user_case == 'anon':
202+
expected_read_status = 401
203+
expected_change_status = 401
204+
expected_delete_status = 401
205+
else:
206+
expected_read_status = 404
207+
expected_change_status = 404
208+
expected_delete_status = 404
197209

198210
# Determine the user based on the test case (adding them to organizations, etc. as necessary).
199211
if user_case == 'superuser':
@@ -215,9 +227,6 @@ def test_oauth2_application_token_read_change_delete(
215227
pass
216228
elif user_case == 'anon':
217229
user = None
218-
expected_read_status = 401
219-
expected_change_status = 401
220-
expected_delete_status = 401
221230
else:
222231
raise ValueError(f"Invalid user_case: {user_case}")
223232

@@ -346,6 +355,81 @@ def test_oauth2_pat_create(request, org_member_rd, org_admin_rd, user, random_us
346355
assert token.user == user # not random_user
347356

348357

358+
@pytest.mark.parametrize(
359+
'user_case, sees_all_tokens',
360+
[
361+
('superuser', True),
362+
('org_admin', False),
363+
('token_owner', False),
364+
('other_user', False),
365+
],
366+
)
367+
@pytest.mark.django_db
368+
def test_oauth2_token_list_filtered_by_user(
369+
request, user_case, sees_all_tokens, admin_user, user, random_user,
370+
unauthenticated_api_client, oauth2_user_pat, oauth2_application, organization, org_admin_rd,
371+
):
372+
"""
373+
Non-superusers should only see their own tokens when listing.
374+
Superusers should see all tokens.
375+
Org admins should see tokens for applications in their org.
376+
(AAP-72278)
377+
"""
378+
other_token = OAuth2AccessToken.objects.create(
379+
user=random_user,
380+
description="Token for random_user",
381+
expires=datetime(2088, 1, 1, tzinfo=timezone.utc),
382+
token=generate_token(),
383+
)
384+
385+
app = oauth2_application[0]
386+
app.organization = organization
387+
app.save()
388+
org_app_token = OAuth2AccessToken.objects.create(
389+
user=random_user,
390+
application=app,
391+
description="App token in org",
392+
expires=datetime(2088, 1, 1, tzinfo=timezone.utc),
393+
token=generate_token(),
394+
)
395+
396+
if user_case == 'superuser':
397+
acting_user = admin_user
398+
elif user_case == 'org_admin':
399+
acting_user = request.getfixturevalue('random_user_1')
400+
RoleDefinition.objects.managed.org_admin.give_permission(acting_user, organization)
401+
elif user_case == 'token_owner':
402+
acting_user = user
403+
elif user_case == 'other_user':
404+
acting_user = random_user
405+
else:
406+
raise ValueError(f"Invalid user_case: {user_case}")
407+
408+
unauthenticated_api_client.force_login(acting_user)
409+
url = get_relative_url("token-list")
410+
response = unauthenticated_api_client.get(url)
411+
assert response.status_code == 200
412+
413+
token_ids = {t['id'] for t in response.data['results']}
414+
415+
if sees_all_tokens:
416+
assert oauth2_user_pat.id in token_ids
417+
assert other_token.id in token_ids
418+
assert org_app_token.id in token_ids
419+
elif user_case == 'org_admin':
420+
assert org_app_token.id in token_ids, "Org admin should see app tokens in their org"
421+
assert oauth2_user_pat.id not in token_ids, "Org admin should not see unrelated PATs"
422+
assert other_token.id not in token_ids, "Org admin should not see unrelated PATs"
423+
elif user_case == 'token_owner':
424+
assert oauth2_user_pat.id in token_ids
425+
assert other_token.id not in token_ids
426+
assert org_app_token.id not in token_ids
427+
elif user_case == 'other_user':
428+
assert other_token.id in token_ids
429+
assert org_app_token.id in token_ids, "Token owner should see their own app tokens"
430+
assert oauth2_user_pat.id not in token_ids
431+
432+
349433
@pytest.mark.parametrize(
350434
'user_case, has_access',
351435
[
@@ -364,9 +448,18 @@ def test_oauth2_pat_read_change_delete(request, user_case, has_access, org_membe
364448
- I am the user of the token
365449
- I am the superuser
366450
"""
367-
expected_read_status = 200 if has_access else 403
368-
expected_change_status = 200 if has_access else 403
369-
expected_delete_status = 204 if has_access else 403
451+
if has_access:
452+
expected_read_status = 200
453+
expected_change_status = 200
454+
expected_delete_status = 204
455+
elif user_case == 'anon':
456+
expected_read_status = 401
457+
expected_change_status = 401
458+
expected_delete_status = 401
459+
else:
460+
expected_read_status = 404
461+
expected_change_status = 404
462+
expected_delete_status = 404
370463

371464
# Determine the user based on the test case (adding them to organizations, etc. as necessary).
372465
if user_case == 'superuser':
@@ -380,9 +473,6 @@ def test_oauth2_pat_read_change_delete(request, user_case, has_access, org_membe
380473
user = request.getfixturevalue('random_user')
381474
elif user_case == 'anon':
382475
user = None
383-
expected_read_status = 401
384-
expected_change_status = 401
385-
expected_delete_status = 401
386476
else:
387477
raise ValueError(f"Invalid user_case: {user_case}")
388478

0 commit comments

Comments
 (0)