Restructure OIDC patron auth for reuse by future OIDC-based providers (PP-3110)

src/palace/manager/api/authenticator.py

@@ -3,9 +3,9 @@
 import enum
 import json
 import logging
-from abc import ABC
+from abc import ABC, abstractmethod
 from collections.abc import Iterable
-from typing import Any, Self
+from typing import TYPE_CHECKING, Any, Self
 
 import flask
 import jwt
@@ -64,6 +64,16 @@
 from palace.manager.util.opds_writer import OPDSFeed
 from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException
 
+if TYPE_CHECKING:
+    from palace.manager.api.authentication.base import PatronData
+    from palace.manager.integration.patron_auth.oidc.auth import (
+        OIDCAuthenticationManager,
+    )
+    from palace.manager.integration.patron_auth.oidc.credential import (
+        OIDCCredentialManager,
+    )
+    from palace.manager.sqlalchemy.model.credential import Credential
+
 
 class CirculationPatronProfileStorage(PatronProfileStorage):
     """A patron profile storage that can also provide short client tokens"""
@@ -970,8 +980,49 @@ class BaseOIDCAuthenticationProvider[
     OPDSAuthenticationFlow,
     ABC,
 ):
-    """Base class for OIDC authentication providers."""
+    """Base class for OIDC authentication providers.
+
+    The abstract members below are the contract OIDCController relies on.
+    Every provider registered through the OIDC dispatch path must supply
+    them. Keeping the contract here lets the controller work with any
+    OIDC-family provider without reaching into implementation details.
+    """
 
     @property
     def flow_type(self) -> str:
         return "http://palaceproject.io/authtype/OpenIDConnect"
+
+    @abstractmethod
+    def extract_patron_identifier(self, id_token_claims: dict[str, Any]) -> str | None:
+        """Extract the patron identifier from validated token claims.
+
+        Must apply the same extraction used at login, so that logout
+        lookups match the stored authorization identifier.
+
+        :param id_token_claims: Validated token claims
+        :return: Patron identifier, or None if it cannot be determined
+        """
+
+    @property
+    @abstractmethod
+    def credential_manager(self) -> OIDCCredentialManager:
+        """Credential manager storing this provider's tokens."""
+
+    @abstractmethod
+    def get_authentication_manager(self) -> OIDCAuthenticationManager:
+        """Return the authentication manager driving the OIDC protocol flow."""
+
+    @abstractmethod
+    def oidc_callback(
+        self,
+        db: Session,
+        id_token_claims: dict[str, Any],
+        access_token: str,
+        refresh_token: str | None = None,
+        expires_in: int | None = None,
+        id_token: str | None = None,
+    ) -> tuple[Credential, Patron, PatronData]:
+        """Authorize and persist a patron session after ID token validation.
+
+        :return: 3-tuple (Credential, Patron, PatronData)
+        """

src/palace/manager/integration/patron_auth/oidc/auth.py

@@ -12,7 +12,8 @@
 from __future__ import annotations
 
 import json
-from typing import Any, cast
+from collections.abc import Sequence
+from typing import Any, Protocol, cast
 from urllib.parse import urlencode
 
 from pydantic import HttpUrl
@@ -21,9 +22,6 @@
 from palace.util.exceptions import BasePalaceException
 from palace.util.log import LoggerMixin
 
-from palace.manager.integration.patron_auth.oidc.configuration.model import (
-    OIDCAuthSettings,
-)
 from palace.manager.integration.patron_auth.oidc.util import (
     OIDCDiscoveryError,
     OIDCUtility,
@@ -49,6 +47,62 @@ class OIDCRefreshTokenError(OIDCAuthenticationError):
     """Raised when token refresh fails."""
 
 
+class OIDCConnectionSettings(Protocol):
+    """The connection attributes OIDCAuthenticationManager reads from its settings.
+
+    Read-only properties keep the contract structural. It is satisfied both by
+    the admin-configured OIDCAuthSettings model and by any plain object with
+    these attributes (for example, a provider-specific configuration with
+    fixed endpoints).
+
+    Either ``issuer_url`` (discovery mode) or the individual endpoint fields
+    (manual mode) must be populated, matching the validation performed by
+    OIDCAuthSettings.
+    """
+
+    @property
+    def issuer_url(self) -> HttpUrl | None: ...
+
+    @property
+    def issuer(self) -> str | None: ...
+
+    @property
+    def authorization_endpoint(self) -> HttpUrl | None: ...
+
+    @property
+    def token_endpoint(self) -> HttpUrl | None: ...
+
+    @property
+    def jwks_uri(self) -> HttpUrl | None: ...
+
+    @property
+    def userinfo_endpoint(self) -> HttpUrl | None: ...
+
+    @property
+    def end_session_endpoint(self) -> HttpUrl | None: ...
+
+    @property
+    def revocation_endpoint(self) -> HttpUrl | None: ...
+
+    @property
+    def client_id(self) -> str: ...
+
+    @property
+    def client_secret(self) -> str: ...
+
+    @property
+    def scopes(self) -> Sequence[str]: ...
+
+    @property
+    def use_pkce(self) -> bool: ...
+
+    @property
+    def token_endpoint_auth_method(self) -> str: ...
+
+    @property
+    def access_type(self) -> str: ...
+
+
 class OIDCAuthenticationManager(LoggerMixin):
     """Manages OIDC authentication flow.
 
@@ -62,13 +116,13 @@ class OIDCAuthenticationManager(LoggerMixin):
 
     def __init__(
         self,
-        settings: OIDCAuthSettings,
+        settings: OIDCConnectionSettings,
         redis_client: Redis | None = None,
         secret_key: str | None = None,
     ):
         """Initialize OIDC authentication manager.
 
-        :param settings: OIDC authentication settings
+        :param settings: Connection settings for the OIDC provider
         :param redis_client: Optional Redis client for caching
         :param secret_key: Secret key for state generation (required for auth flow)
         """
@@ -79,6 +133,11 @@ def __init__(
         self._validator = OIDCTokenValidator()
         self._metadata: dict[str, Any] | None = None
 
+    @property
+    def use_pkce(self) -> bool:
+        """Whether the authorization flow should use PKCE."""
+        return self._settings.use_pkce
+
     def _extract_error_detail_from_response(
         self, exception: RequestNetworkException
     ) -> str: