fix: harden Credly OAuth client and admin, add tests

credentials/apps/badges/admin.py

@@ -159,13 +159,39 @@ class CredlyOrganizationAdmin(admin.ModelAdmin):
     list_display = (
         "name",
         "uuid",
+        "oauth_client_id",
         "api_key_hidden",
     )
-    fields = [
-        "name",
-        "uuid",
-        "api_key_hidden",
-    ]
+    fieldsets = (
+        (
+            _("General Information"),
+            {
+                "fields": (
+                    "name",
+                    "uuid",
+                ),
+            },
+        ),
+        (
+            _("OAuth 2.0 Credentials"),
+            {
+                "description": _("Authentication credentials for OAuth 2.0 Client Credentials flow."),
+                "fields": (
+                    "oauth_client_id",
+                    "oauth_client_secret",
+                ),
+            },
+        ),
+        (
+            _("Legacy API Key Authentication"),
+            {
+                "description": _(
+                    "Legacy Basic Auth API key. Use this only if OAuth 2.0 credentials are not available."
+                ),
+                "fields": ("api_key",),
+            },
+        ),
+    )
     readonly_fields = [
         "name",
     ]
@@ -194,22 +220,34 @@ def api_key_hidden(self, obj):
 
         return _("Pre-configured from the environment.") if obj.is_preconfigured else obj.api_key
 
-    def get_fields(self, request, obj=None):
-        fields = super().get_fields(request, obj)
+    def get_fieldsets(self, request, obj=None):
+        """
+        Show the masked API key instead of the editable one for pre-configured organizations.
+        """
+        fieldsets = super().get_fieldsets(request, obj)
 
         if not (obj and obj.is_preconfigured):
-            fields = [field for field in fields if field != "api_key_hidden"]
-            fields.append("api_key")
-        return fields
+            return fieldsets
+
+        return tuple(
+            (
+                name,
+                {
+                    **options,
+                    "fields": tuple("api_key_hidden" if field == "api_key" else field for field in options["fields"]),
+                },
+            )
+            for name, options in fieldsets
+        )
 
     def get_readonly_fields(self, request, obj=None):
+        """
+        Credentials of pre-configured organizations come from the environment and can't be edited.
+        """
         readonly_fields = list(super().get_readonly_fields(request, obj))
 
-        if not obj:
-            return readonly_fields
-
-        if obj.is_preconfigured:
-            readonly_fields.append("api_key_hidden")
+        if obj and obj.is_preconfigured:
+            readonly_fields.extend(["api_key_hidden", "oauth_client_id", "oauth_client_secret"])
         return readonly_fields
 
 

credentials/apps/badges/admin_forms.py

@@ -8,7 +8,8 @@
 from model_utils import Choices
 
 from credentials.apps.badges.credly.api_client import CredlyAPIClient
-from credentials.apps.badges.credly.exceptions import CredlyAPIError
+from credentials.apps.badges.credly.exceptions import CredlyError
+from credentials.apps.badges.exceptions import BadgeProviderError
 from credentials.apps.badges.models import (
     AbstractDataRule,
     BadgePenalty,
@@ -42,21 +43,45 @@ def clean(self):
 
         uuid = cleaned_data.get("uuid")
         api_key = cleaned_data.get("api_key")
+        oauth_client_id = cleaned_data.get("oauth_client_id")
+        oauth_client_secret = cleaned_data.get("oauth_client_secret")
 
-        if str(uuid) in CredlyOrganization.get_preconfigured_organizations().keys():
-            if api_key:
-                raise forms.ValidationError(_("You can't provide an API key for a configured organization."))
+        is_preconfigured = str(uuid) in CredlyOrganization.get_preconfigured_organizations().keys()
 
+        if is_preconfigured:
+            if api_key or oauth_client_id or oauth_client_secret:
+                raise forms.ValidationError(
+                    _("You can't provide API keys or OAuth credentials for a pre-configured organization.")
+                )
             api_key = settings.BADGES_CONFIG["credly"]["ORGANIZATIONS"][str(uuid)]
 
-        credly_api_client = CredlyAPIClient(uuid, api_key)
+        else:
+            has_oauth = bool(oauth_client_id and oauth_client_secret)
+            has_api_key = bool(api_key)
+
+            if not (has_oauth or has_api_key):
+                raise forms.ValidationError(
+                    _("You must provide either OAuth credentials (Client ID & Client Secret) or a legacy API Key.")
+                )
+
+            if bool(oauth_client_id) != bool(oauth_client_secret):
+                raise forms.ValidationError(
+                    _("Both Client ID and Client Secret are required for OAuth authentication.")
+                )
+
+        credly_api_client = CredlyAPIClient(
+            organization_id=uuid,
+            api_key=api_key,
+            oauth_client_id=oauth_client_id,
+            oauth_client_secret=oauth_client_secret,
+        )
         self.ensure_organization_exists(credly_api_client)
 
         return cleaned_data
 
     def save(self, commit=True):
         """
-        Auto-fill addition properties.
+        Auto-fill additional properties.
         """
         instance = super().save(commit=False)
         instance.name = self.api_data.get("name")
@@ -69,11 +94,35 @@ def ensure_organization_exists(self, api_client):
         Try to fetch organization data by the configured Credly Organization ID.
         """
         try:
-            response_json = api_client.fetch_organization()
-            if org_data := response_json.get("data"):
-                self.api_data = org_data
-        except CredlyAPIError as err:
-            raise forms.ValidationError(message=str(err))
+            if api_client.oauth_client_id and api_client.oauth_client_secret:
+                response_json = api_client.fetch_badge_templates()
+
+                templates = response_json.get("data", [])
+                org_name = None
+
+                if templates and isinstance(templates, list):
+                    first_template = templates[0]
+                    owner = first_template.get("owner", {})
+                    org_name = owner.get("name")
+
+                if not org_name:
+                    org_name = f"Credly Organization ({api_client.organization_id})"
+
+                self.api_data = {"name": org_name}
+            else:
+                response_json = api_client.fetch_organization()
+                if org_data := response_json.get("data"):
+                    self.api_data = org_data
+
+        except (CredlyError, BadgeProviderError) as exc:
+            if "Status(401)" in str(exc) or "Unauthorized" in str(exc):
+                raise forms.ValidationError(
+                    _("Invalid OAuth credentials or API key. Credly rejected the authentication request.")
+                ) from exc
+
+            raise forms.ValidationError(
+                _("Error communicating with Credly API: %(error)s") % {"error": str(exc)}
+            ) from exc
 
 
 class BadgePenaltyForm(forms.ModelForm):

credentials/apps/badges/base_api_client.py

@@ -64,7 +64,9 @@ def _raise_for_error(self, response):
             logger.error(
                 f"Error while processing {self.PROVIDER_NAME} request: {response.status_code} - {response.text}"
             )
-            raise BadgeProviderError(f"{response.text} Status({response.status_code})")
+            raise BadgeProviderError(
+                f"{response.text} Status({response.status_code})", status_code=response.status_code
+            )
 
     @property
     def base_api_url(self):

credentials/apps/badges/credly/api_client.py

@@ -8,10 +8,12 @@
 from attrs import asdict
 from django.conf import settings
 from django.contrib.sites.models import Site
+from django.core.cache import cache
 
 from credentials.apps.badges.base_api_client import BaseBadgeProviderClient
 from credentials.apps.badges.credly.exceptions import CredlyError
 from credentials.apps.badges.credly.utils import get_credly_api_base_url
+from credentials.apps.badges.exceptions import BadgeProviderError
 from credentials.apps.badges.models import CredlyBadgeTemplate, CredlyOrganization
 
 logger = logging.getLogger(__name__)
@@ -28,20 +30,34 @@ class CredlyAPIClient(BaseBadgeProviderClient):
 
     PROVIDER_NAME = "Credly"
 
-    def __init__(self, organization_id, api_key=None):  # pylint: disable=super-init-not-called
+    def __init__(
+        self,
+        organization_id,
+        api_key=None,
+        oauth_client_id=None,
+        oauth_client_secret=None,
+    ):  # pylint: disable=super-init-not-called
         """
         Initializes a CredlyRestAPI object.
 
         Args:
             organization_id (str, uuid): ID of the organization.
-            api_key (str): optional ID of the organization.
+            api_key (str): Optional legacy API key of the organization.
+            oauth_client_id (str): Optional OAuth Client ID.
+            oauth_client_secret (str): Optional OAuth Client Secret.
         """
-        if api_key is None:
+        self.organization_id = organization_id
+        self.organization = None
+
+        if not (api_key or (oauth_client_id and oauth_client_secret)):
             self.organization = self._get_organization(organization_id)
             api_key = self.organization.api_key
+            oauth_client_id = getattr(self.organization, "oauth_client_id", None)
+            oauth_client_secret = getattr(self.organization, "oauth_client_secret", None)
 
         self.api_key = api_key
-        self.organization_id = organization_id
+        self.oauth_client_id = oauth_client_id
+        self.oauth_client_secret = oauth_client_secret
 
     def _get_base_api_url(self):
         return urljoin(get_credly_api_base_url(settings), f"organizations/{self.organization_id}/")
@@ -56,24 +72,116 @@ def _get_organization(self, organization_id):
         except CredlyOrganization.DoesNotExist:
             raise CredlyError(f"CredlyOrganization with the uuid {organization_id} does not exist!")
 
+    @property
+    def uses_oauth(self):
+        """
+        OAuth takes precedence over the legacy API key when both are configured.
+        """
+        return bool(self.oauth_client_id and self.oauth_client_secret)
+
+    @property
+    def _oauth_token_cache_key(self):
+        """
+        Keyed by client ID, so organizations sharing an OAuth application share the token.
+        """
+        return f"credly_oauth_access_token_{self.oauth_client_id}"
+
+    def _get_oauth_token(self, force_refresh=False):
+        """
+        Obtains a Bearer Access Token from Credly OAuth endpoint using Client Credentials grant.
+
+        Tokens are cached for their lifetime (minus a safety margin).
+
+        Args:
+            force_refresh (bool): skip the cached token and request a new one.
+        """
+        if not self.uses_oauth:
+            return None
+
+        token = None if force_refresh else cache.get(self._oauth_token_cache_key)
+        if token:
+            return token
+
+        token_url = urljoin(get_credly_api_base_url(settings), "/oauth/token")
+        payload = {
+            "grant_type": "client_credentials",
+            "scope": "badge_templates issued_badges",
+        }
+
+        try:
+            response = requests.post(
+                token_url,
+                data=payload,
+                auth=(self.oauth_client_id, self.oauth_client_secret),
+                headers={"Accept": "application/json"},
+                timeout=self.REQUESTS_TIMEOUT,
+            )
+            response.raise_for_status()
+        except requests.RequestException as exc:
+            raise CredlyError(f"Failed to fetch OAuth token from Credly: {str(exc)}") from exc
+
+        try:
+            data = response.json()
+        except ValueError as exc:
+            raise CredlyError(f"Credly returned a non-JSON OAuth token response: {response.text[:200]}") from exc
+
+        token = data.get("access_token")
+        if not token:
+            raise CredlyError(f"Credly response did not contain access_token: {data}")
+
+        expires_in = data.get("expires_in", 7200)
+        cache.set(self._oauth_token_cache_key, token, timeout=max(expires_in - 60, 60))
+
+        return token
+
+    def perform_request(self, method, url_suffix, data=None):
+        """
+        Perform an HTTP request to the Credly API.
+
+        When authenticating with OAuth, a 401 response most likely means the cached
+        access token was revoked or expired early: refresh the token and retry once.
+        """
+        try:
+            return super().perform_request(method, url_suffix, data)
+        except BadgeProviderError as exc:
+            if not (self.uses_oauth and exc.status_code == 401):
+                raise
+
+        logger.warning("Credly rejected the OAuth access token, refreshing the token and retrying the request.")
+        self._get_oauth_token(force_refresh=True)
+        return super().perform_request(method, url_suffix, data)
+
     def _get_headers(self):
         """
         Returns the headers for making API requests to Credly.
+        Supports both OAuth Bearer Tokens and legacy Basic Auth API Keys.
         """
-        return {
+        headers = {
             "Accept": "application/json",
             "Content-Type": "application/json",
-            "Authorization": f"Basic {self._build_authorization_token()}",
         }
 
+        if self.uses_oauth:
+            headers["Authorization"] = f"Bearer {self._get_oauth_token()}"
+
+        elif self.api_key:
+            headers["Authorization"] = f"Basic {self._build_authorization_token()}"
+
+        else:
+            raise CredlyError("No valid authentication credentials (OAuth or API Key) available for Credly.")
+
+        return headers
+
     @lru_cache
     def _build_authorization_token(self):
         """
-        Build the authorization token for the Credly API.
+        Build the authorization token for the Credly API (Legacy).
 
         Returns:
             str: Authorization token.
         """
+        if not self.api_key:
+            return ""
         return base64.b64encode(self.api_key.encode("ascii")).decode("ascii")
 
     def fetch_organization(self):
@@ -165,6 +273,9 @@ def sync_organization_badge_templates(self, site_id):
             logger.error(f"Site with the id {site_id} does not exist!")
             raise
 
+        if not self.organization:
+            self.organization = self._get_organization(self.organization_id)
+
         badge_templates_data = self.fetch_badge_templates()
         raw_badge_templates = badge_templates_data.get("data", [])
 

credentials/apps/badges/exceptions.py

@@ -18,4 +18,12 @@ class BadgesProcessingError(BadgesError):
 class BadgeProviderError(BadgesError):
     """
     Exception raised for errors that occur during badge API client processing.
+
+    Args:
+        message (str): Error description.
+        status_code (int, optional): HTTP status code of the failed provider response, if any.
     """
+
+    def __init__(self, message, status_code=None):
+        super().__init__(message)
+        self.status_code = status_code