Skip to content

Commit

Permalink
fixed github oauth app behaviour (#168)
Browse files Browse the repository at this point in the history
  • Loading branch information
opaduchak authored Dec 2, 2024
1 parent ac17ceb commit b11c779
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 5 deletions.
7 changes: 6 additions & 1 deletion addon_service/authorized_account/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion addon_service/configured_addon/storage/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions addon_service/migrations/0007_oauth2clientconfig_quirks.py
Original file line number Diff line number Diff line change
@@ -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,
),
),
]
22 changes: 20 additions & 2 deletions addon_service/oauth2/models.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion addon_service/oauth2/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit b11c779

Please sign in to comment.