From 5dbf5b23b069ef0e67468603eabc383a3d6da3eb Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Sun, 21 Apr 2024 18:15:59 +0200 Subject: [PATCH] Get "related" working for application tokens Signed-off-by: Rick Elrod --- ...003_alter_oauth2accesstoken_application.py | 20 +++++++++++ .../oauth2_provider/models/access_token.py | 15 +++++--- .../oauth2_provider/models/application.py | 5 +-- .../oauth2_provider/serializers/token.py | 1 - ansible_base/oauth2_provider/urls.py | 33 ++++++++---------- .../oauth2_provider/views/token_root.py | 29 ---------------- .../oauth2_provider/views/test_application.py | 34 +++++++++++++++++++ 7 files changed, 81 insertions(+), 56 deletions(-) create mode 100644 ansible_base/oauth2_provider/migrations/0003_alter_oauth2accesstoken_application.py delete mode 100644 ansible_base/oauth2_provider/views/token_root.py diff --git a/ansible_base/oauth2_provider/migrations/0003_alter_oauth2accesstoken_application.py b/ansible_base/oauth2_provider/migrations/0003_alter_oauth2accesstoken_application.py new file mode 100644 index 000000000..0d0f0874f --- /dev/null +++ b/ansible_base/oauth2_provider/migrations/0003_alter_oauth2accesstoken_application.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.11 on 2024-04-21 15:48 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('dab_oauth2_provider', '0002_alter_oauth2accesstoken_created_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='oauth2accesstoken', + name='application', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='access_tokens', to=settings.OAUTH2_PROVIDER_APPLICATION_MODEL), + ), + ] diff --git a/ansible_base/oauth2_provider/models/access_token.py b/ansible_base/oauth2_provider/models/access_token.py index ed0516985..a3e950c58 100644 --- a/ansible_base/oauth2_provider/models/access_token.py +++ b/ansible_base/oauth2_provider/models/access_token.py @@ -11,11 +11,8 @@ class OAuth2AccessToken(oauth2_models.AbstractAccessToken, CommonModel): - reverse_name_override = 'token' - # There is a special condition where, as the user is logging in we want to update the last_used field. - # However, this happens before the user is set for the request. - # If this is the only field attempting to be saved, don't update the modified on/by fields - not_user_modified_fields = ['last_used'] + router_basename = 'token' + ignore_relations = ['refresh_token'] class Meta(oauth2_models.AbstractAccessToken.Meta): verbose_name = _('access token') @@ -30,6 +27,14 @@ class Meta(oauth2_models.AbstractAccessToken.Meta): related_name="%(app_label)s_%(class)s", help_text=_('The user representing the token owner'), ) + # Overriding to set related_name + application = models.ForeignKey( + settings.OAUTH2_PROVIDER_APPLICATION_MODEL, + on_delete=models.CASCADE, + blank=True, + null=True, + related_name='access_tokens', + ) description = models.TextField( default='', blank=True, diff --git a/ansible_base/oauth2_provider/models/application.py b/ansible_base/oauth2_provider/models/application.py index 0b6eaf4de..ce75c19d8 100644 --- a/ansible_base/oauth2_provider/models/application.py +++ b/ansible_base/oauth2_provider/models/application.py @@ -14,7 +14,8 @@ class OAuth2Application(oauth2_models.AbstractApplication, NamedCommonModel): - reverse_name_override = 'application' + router_basename = 'application' + ignore_relations = ['oauth2idtoken', 'grant', 'oauth2refreshtoken'] encrypted_fields = ['client_secret'] class Meta(oauth2_models.AbstractAccessToken.Meta): @@ -67,4 +68,4 @@ class Meta(oauth2_models.AbstractAccessToken.Meta): def get_absolute_url(self): # This is kind of annoying. This method lives on the superclass and we check for it in CommonModel. # But better would be to not have this method and let the CommonModel logic fall back to the "right" way of finding this. - return reverse('application-detail', kwargs={'pk': self.pk}) + return reverse(f'{self.router_basename}-detail', kwargs={'pk': self.pk}) diff --git a/ansible_base/oauth2_provider/serializers/token.py b/ansible_base/oauth2_provider/serializers/token.py index 034e152c6..5bace509b 100644 --- a/ansible_base/oauth2_provider/serializers/token.py +++ b/ansible_base/oauth2_provider/serializers/token.py @@ -18,7 +18,6 @@ class BaseOAuth2TokenSerializer(CommonModelSerializer): - reverse_url_name = 'token-detail' refresh_token = SerializerMethodField() token = SerializerMethodField() ALLOWED_SCOPES = ['read', 'write'] diff --git a/ansible_base/oauth2_provider/urls.py b/ansible_base/oauth2_provider/urls.py index baa815ed9..24e673610 100644 --- a/ansible_base/oauth2_provider/urls.py +++ b/ansible_base/oauth2_provider/urls.py @@ -9,30 +9,25 @@ router = AssociationResourceRouter() -router.register(r'applications', oauth2_provider_views.OAuth2ApplicationViewSet, basename='application') - -router.register(r'tokens', oauth2_provider_views.OAuth2TokenViewSet, basename='token') +router.register( + r'applications', + oauth2_provider_views.OAuth2ApplicationViewSet, + basename='application', + related_views={ + 'tokens': (oauth2_provider_views.OAuth2TokenViewSet, 'access_tokens'), + }, +) + +router.register( + r'tokens', + oauth2_provider_views.OAuth2TokenViewSet, + basename='token', +) api_version_urls = [ path('', include(router.urls)), ] -# re_path( -# r'^applications/(?P[0-9]+)/tokens/$', -# oauth2_provider_views.ApplicationOAuth2TokenList.as_view(), -# name='o_auth2_application_token_list' -# ), -# re_path( -# r'^applications/(?P[0-9]+)/activity_stream/$', -# oauth2_provider_views.OAuth2ApplicationActivityStreamList.as_view(), -# name='o_auth2_application_activity_stream_list' -# ), -# re_path( -# r'^tokens/(?P[0-9]+)/activity_stream/$', -# oauth2_provider_views.OAuth2TokenActivityStreamList.as_view(), -# name='o_auth2_token_activity_stream_list' -# ), - root_urls = [ re_path(r'^o/$', oauth2_provider_views.ApiOAuthAuthorizationRootView.as_view(), name='oauth_authorization_root_view'), re_path(r"^o/authorize/$", oauth_views.AuthorizationView.as_view(), name="authorize"), diff --git a/ansible_base/oauth2_provider/views/token_root.py b/ansible_base/oauth2_provider/views/token_root.py deleted file mode 100644 index b40b70ed1..000000000 --- a/ansible_base/oauth2_provider/views/token_root.py +++ /dev/null @@ -1,29 +0,0 @@ -from datetime import timedelta - -from django.conf import settings -from django.utils.timezone import now -from oauth2_provider.views import TokenView -from oauthlib.oauth2 import AccessDeniedError - -from ansible_base.oauth2_provider.models import OAuth2RefreshToken - - -class TokenView(TokenView): - def create_token_response(self, request): - # Django OAuth2 Toolkit has a bug whereby refresh tokens are *never* - # properly expired (ugh): - # - # https://github.com/jazzband/django-oauth-toolkit/issues/746 - # - # This code detects and auto-expires them on refresh grant - # requests. - if request.POST.get('grant_type') == 'refresh_token' and 'refresh_token' in request.POST: - refresh_token = OAuth2RefreshToken.objects.filter(token=request.POST['refresh_token']).first() - if refresh_token: - expire_seconds = settings.OAUTH2_PROVIDER.get('REFRESH_TOKEN_EXPIRE_SECONDS', 0) - if refresh_token.created + timedelta(seconds=expire_seconds) < now(): - return request.build_absolute_uri(), {}, 'The refresh token has expired.', '403' - try: - return super(TokenView, self).create_token_response(request) - except AccessDeniedError as e: - return request.build_absolute_uri(), {}, str(e), '403' diff --git a/test_app/tests/oauth2_provider/views/test_application.py b/test_app/tests/oauth2_provider/views/test_application.py index feccd676f..248939ee3 100644 --- a/test_app/tests/oauth2_provider/views/test_application.py +++ b/test_app/tests/oauth2_provider/views/test_application.py @@ -28,6 +28,40 @@ def test_oauth2_provider_application_list(request, client_fixture, expected_stat assert response.data['results'][0]['name'] == oauth2_application.name +@pytest.mark.parametrize( + "view, path", + [ + ("application-list", lambda data: data['results'][0]), + ("application-detail", lambda data: data), + ], +) +def test_oauth2_provider_application_related(admin_api_client, oauth2_application, organization, view, path): + """ + Test that the related fields are correct. + + Organization should only be shown if the application is associated with an organization. + Associating an application with an organization should not affect other related fields. + """ + if view == "application-list": + url = reverse(view) + else: + url = reverse(view, args=[oauth2_application.pk]) + + oauth2_application.organization = None + oauth2_application.save() + response = admin_api_client.get(url) + assert response.status_code == 200 + assert path(response.data)['related']['access_tokens'] == reverse("application-access_tokens-list", args=[oauth2_application.pk]) + assert 'organization' not in path(response.data)['related'] + + oauth2_application.organization = organization + oauth2_application.save() + response = admin_api_client.get(url) + assert response.status_code == 200 + assert path(response.data)['related']['access_tokens'] == reverse("application-access_tokens-list", args=[oauth2_application.pk]) + assert path(response.data)['related']['organization'] == reverse("organization-detail", args=[organization.pk]) + + @pytest.mark.parametrize( "client_fixture,expected_status", [