From b11c77967973961d4d6d8b54a8fcb77e2f40cd37 Mon Sep 17 00:00:00 2001 From: Oleh Paduchak <158075011+opaduchak@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:49:52 +0200 Subject: [PATCH] fixed github oauth app behaviour (#168) --- addon_service/authorized_account/models.py | 7 ++++- .../configured_addon/storage/views.py | 2 +- .../0007_oauth2clientconfig_quirks.py | 28 +++++++++++++++++++ addon_service/oauth2/models.py | 22 +++++++++++++-- addon_service/oauth2/utils.py | 3 +- 5 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 addon_service/migrations/0007_oauth2clientconfig_quirks.py diff --git a/addon_service/authorized_account/models.py b/addon_service/authorized_account/models.py index 0c944f68..20baca05 100644 --- a/addon_service/authorized_account/models.py +++ b/addon_service/authorized_account/models.py @@ -291,7 +291,10 @@ def validate_oauth_state(self) -> None: "credentials": "OAuth2 accounts must assign exactly one of state_nonce and access_token" } ) - if self.credentials and not self.oauth2_token_metadata.refresh_token: + if self.credentials and not ( + self.oauth2_token_metadata.refresh_token + or self.oauth2_token_metadata.access_token_only + ): raise ValidationError( { "credentials": "OAuth2 accounts with an access token must have a refresh token" @@ -313,6 +316,8 @@ async def refresh_oauth2_access_token(self) -> None: _oauth_client_config, _oauth_token_metadata, ) = await self._load_oauth2_client_config_and_token_metadata() + if sync_to_async(lambda: _oauth_token_metadata.access_token_only)(): + return _fresh_token_result = await oauth2_utils.get_refreshed_access_token( token_endpoint_url=_oauth_client_config.token_endpoint_url, refresh_token=_oauth_token_metadata.refresh_token, diff --git a/addon_service/configured_addon/storage/views.py b/addon_service/configured_addon/storage/views.py index 6adf0878..10d4a521 100644 --- a/addon_service/configured_addon/storage/views.py +++ b/addon_service/configured_addon/storage/views.py @@ -22,7 +22,7 @@ class ConfiguredStorageAddonViewSet(ConfiguredAddonViewSet): url_path="waterbutler-credentials", ) def get_wb_credentials(self, request, pk=None): - addon = self.get_object() + addon: ConfiguredStorageAddon = self.get_object() if addon.external_service.credentials_format is CredentialsFormats.OAUTH2: addon.base_account.refresh_oauth_access_token__blocking() self.resource_name = "waterbutler-credentials" # for the jsonapi resource type diff --git a/addon_service/migrations/0007_oauth2clientconfig_quirks.py b/addon_service/migrations/0007_oauth2clientconfig_quirks.py new file mode 100644 index 00000000..392d99de --- /dev/null +++ b/addon_service/migrations/0007_oauth2clientconfig_quirks.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.7 on 2024-11-27 12:05 + +from django.db import ( + migrations, + models, +) + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "addon_service", + "0006_externalcitationservice_int_supported_features_and_more", + ), + ] + + operations = [ + migrations.AddField( + model_name="oauth2clientconfig", + name="quirks", + field=models.IntegerField( + blank=True, + choices=[(1, "Only Access Token"), (2, "Non Expirable Refresh Token")], + null=True, + ), + ), + ] diff --git a/addon_service/oauth2/models.py b/addon_service/oauth2/models.py index d08f6698..6153a9ad 100644 --- a/addon_service/oauth2/models.py +++ b/addon_service/oauth2/models.py @@ -1,6 +1,7 @@ from __future__ import annotations from datetime import timedelta +from enum import auto from typing import TYPE_CHECKING from asgiref.sync import sync_to_async @@ -22,6 +23,11 @@ from addon_service.authorized_account.models import AuthorizedAccount +class OAuth2ServiceQuirks(models.IntegerChoices): + ONLY_ACCESS_TOKEN = auto() + NON_EXPIRABLE_REFRESH_TOKEN = auto() + + class OAuth2ClientConfig(AddonsServiceBaseModel): """ Model for storing attributes that are required for managing @@ -36,6 +42,9 @@ class OAuth2ClientConfig(AddonsServiceBaseModel): # The registered ID of the OAuth client client_id = models.CharField(null=True) client_secret = models.CharField(null=True) + quirks = models.IntegerField( + choices=OAuth2ServiceQuirks.choices, null=True, blank=True + ) class Meta: verbose_name = "OAuth2 Client Config" @@ -81,7 +90,11 @@ def state_token(self) -> str | None: @property def client_details(self): - return self.authorized_accounts.first().external_service.oauth2_client_config + return self.authorized_account.external_service.oauth2_client_config + + @property + def authorized_account(self): + return self.authorized_accounts.first() def clean_fields(self, *args, **kwargs): super().clean_fields(*args, **kwargs) @@ -94,11 +107,16 @@ def validate_nonce_and_token(self): raise ValidationError( "Error on OAuth2 Flow: state nonce and refresh token both present." ) - if not (self.state_nonce or self.refresh_token): + if not (self.state_nonce or self.access_token_only or self.refresh_token): raise ValidationError( "Error in OAuth2 Flow: Neither state nonce nor refresh token present." ) + @property + def access_token_only(self): + if self.authorized_account: + return self.client_details.quirks == OAuth2ServiceQuirks.ONLY_ACCESS_TOKEN + def validate_shared_client(self): client_configs = set( account.external_service.oauth2_client_config diff --git a/addon_service/oauth2/utils.py b/addon_service/oauth2/utils.py index 875a2189..aa605544 100644 --- a/addon_service/oauth2/utils.py +++ b/addon_service/oauth2/utils.py @@ -125,7 +125,8 @@ async def _token_request( if _token_response.content_type == "application/x-www-form-urlencoded": response_text = await _token_response.text() response_data = dict(urllib.parse.parse_qsl(response_text)) - response_data["expires_in"] = int(response_data["expires_in"]) + if expires := response_data.get("expires_in"): + response_data["expires_in"] = int(expires) return FreshTokenResult.from_token_response_json(response_data) if not HTTPStatus(_token_response.status).is_success: