Refactor: OAuth2 token refresh — instance-var caching + heartbeat-driven refresh

- OAuth2Handler.__init__ reads _access_token, _refresh_token, _expires_at
  from AddonSettings once at construction; no per-call settings I/O
- _store_tokens() updates both settings and instance vars atomically
- _do_token_refresh(): unconditional internal refresh (single _ so subclass
  can override); base uses refresh_token grant
- refresh_access_token() -> bool: no-op when > 300s from expiry (_REFRESH_MARGIN),
  calls _do_token_refresh() otherwise; returns True/False instead of raising
- get_valid_token(): pure getter returning self._access_token — no side-effects;
  callers that need a fresh token must call refresh_access_token() first
- active_authentication(): calls refresh_access_token() before get_valid_token()
- _clear_token_settings(): clears both settings and instance vars

NLZIETOAuth2Handler:
- __init__ caches _id_token from settings
- _do_token_refresh() override: silent re-auth via id_token (web flow) or
  refresh_token grant (device flow); calls super()._do_token_refresh() for latter
- 401 fallback in list_profiles() calls _do_token_refresh() directly
  (force-refresh, bypasses expiry check)
- _store_tokens() and _clear_token_settings() maintain _id_token in sync

chn_nlziet:
- __set_auth_headers(): refresh_access_token() then get_valid_token()
- create_iptv_streams() + create_iptv_epg(): refresh token on every heartbeat
  cycle so long IPTV Manager sessions (4+ hours) never hit stale tokens

Tests (all 55 pass, 18 skipped = live-credential tests):
- Updated test_06, test_16: set _expires_at on instance (not just settings),
  call refresh_access_token() explicitly
- Updated test_refresh_no_tokens: assertFalse() instead of assertRaises(ValueError)
- New test_refresh_access_token_noop_when_valid: monkey-patch verifies
  _do_token_refresh is NOT called when token is fresh
- New test_get_valid_token_is_pure_getter: verifies no refresh side-effect
- New test_instance_vars_loaded_from_settings_at_init: verifies all three
  instance vars populated from settings at construction

Co-authored-by: Claude Sonnet 4.6
Signed-off-by: Oliver
Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

Author: oliv3r

channels/channel.nlziet/nlziet/chn_nlziet.py

@@ -270,6 +270,7 @@ def __validate_token(self) -> bool:
     def __set_auth_headers(self):
         """Set the Bearer token and required app headers for API requests."""
 
+        self.__handler.refresh_access_token()
         token = self.__handler.get_valid_token()
         if token:
             self.httpHeaders["Authorization"] = "Bearer {}".format(token)
@@ -1420,6 +1421,9 @@ def create_iptv_streams(self, parameter_parser):
             Logger.warning("NLZIET IPTV: Not authenticated, returning empty streams")
             return []
 
+        if self.__handler.refresh_access_token():
+            self.httpHeaders["Authorization"] = "Bearer {}".format(self.__handler.get_valid_token())
+
         Logger.debug("NLZIET IPTV: create_iptv_streams called")
         live_data = UriHandler.open(API_V9_EPG_LIVE, additional_headers=self.httpHeaders)
         if not live_data:
@@ -1494,6 +1498,9 @@ def create_iptv_epg(self, parameter_parser):
             Logger.warning("NLZIET IPTV: Not authenticated, returning empty EPG")
             return {}
 
+        if self.__handler.refresh_access_token():
+            self.httpHeaders["Authorization"] = "Bearer {}".format(self.__handler.get_valid_token())
+
         # Load appconfig (cached); abort if the server has blocked the app.
         appconfig = self.__load_appconfig()
         if self.__check_app_blocked(appconfig):

resources/lib/authentication/nlzietoauth2handler.py

@@ -70,6 +70,8 @@ def __init__(self, use_device_flow: Optional[bool] = None):
         self._use_device_flow = use_device_flow
         client_id = self.TV_CLIENT_ID if use_device_flow else self.WEB_CLIENT_ID
         super(NLZIETOAuth2Handler, self).__init__(realm="nlziet", client_id=client_id)
+        # id_token is NLZIET-specific; used for web-flow silent re-authentication.
+        self._id_token = AddonSettings.get_setting(f"{self.prefix}id_token", store=LOCAL) or ""
 
     @property
     def base_auth_url(self) -> str: return self.BASE_AUTH_URL
@@ -177,7 +179,8 @@ def _store_tokens(self, tokens: dict):
         """Override to also store id_token for NLZiet silent auth."""
         super()._store_tokens(tokens)
         if "id_token" in tokens:
-            AddonSettings.set_setting(f"{self.prefix}id_token", tokens["id_token"], store=LOCAL)
+            self._id_token = tokens["id_token"]
+            AddonSettings.set_setting(f"{self.prefix}id_token", self._id_token, store=LOCAL)
             Logger.debug(f"OAuth2: Stored id_token for {self.realm} silent auth")
 
     def _exchange_code_with_verifier(self, auth_code: str, code_verifier: str):
@@ -191,16 +194,14 @@ def _exchange_code_with_verifier(self, auth_code: str, code_verifier: str):
         }
         self._request_tokens(data)
 
-    def refresh_access_token(self):
-        """Refresh access token using refresh_token (device flow) or silent re-auth (web flow)."""
-        refresh_token = AddonSettings.get_setting(f"{self.prefix}refresh_token", store=LOCAL)
-        if refresh_token:
+    def _do_token_refresh(self):
+        """Unconditional token refresh: uses refresh_token (device flow) or silent re-auth (web flow)."""
+        if self._refresh_token:
             Logger.debug(f"OAuth2: Refreshing access token using refresh_token for {self.realm}")
-            super().refresh_access_token()
+            super()._do_token_refresh()
             return
 
-        id_token = AddonSettings.get_setting(f"{self.prefix}id_token", store=LOCAL)
-        if not id_token:
+        if not self._id_token:
             raise ValueError("No refresh_token or id_token available for authentication.")
 
         Logger.debug(f"OAuth2: Attempting silent re-authentication for {self.realm}")
@@ -219,7 +220,7 @@ def refresh_access_token(self):
             "code_challenge_method": "S256",
             "response_mode": "query",
             "prompt": "none",
-            "id_token_hint": id_token
+            "id_token_hint": self._id_token
         }
 
         auth_url = f"{self.base_auth_url}?{urlencode(auth_params)}"
@@ -245,6 +246,8 @@ def refresh_access_token(self):
 
         except Exception as e:
             Logger.warning(f"OAuth2: Silent re-authentication failed for {self.realm}: {e}")
+            self._id_token = ""
+            self._access_token = ""
             AddonSettings.set_setting(f"{self.prefix}id_token", "", store=LOCAL)
             AddonSettings.set_setting(f"{self.prefix}access_token", "", store=LOCAL)
             raise
@@ -282,8 +285,8 @@ def list_profiles(self) -> Optional[list]:
             if status.error and status.code in (HTTPStatus.UNAUTHORIZED, HTTPStatus.FORBIDDEN):
                 Logger.warning("NLZIET: Profile API returned %s, attempting token refresh", status.code)
                 try:
-                    self.refresh_access_token()
-                    access_token = AddonSettings.get_setting(f"{self.prefix}access_token", store=LOCAL)
+                    self._do_token_refresh()
+                    access_token = self.get_valid_token()
                     if not access_token:
                         Logger.warning("NLZIET: Token refresh did not yield an access token")
                         return None
@@ -299,9 +302,7 @@ def list_profiles(self) -> Optional[list]:
                         return None
                 except Exception as e:
                     Logger.warning(f"NLZIET: Token refresh failed: {e}")
-                    AddonSettings.set_setting(f"{self.prefix}access_token", "", store=LOCAL)
-                    AddonSettings.set_setting(f"{self.prefix}refresh_token", "", store=LOCAL)
-                    AddonSettings.set_setting(f"{self.prefix}id_token", "", store=LOCAL)
+                    self._clear_token_settings()
                     return None
             elif not response:
                 Logger.error("NLZIET: Empty response from profile API")
@@ -745,6 +746,7 @@ def get_user_info(self) -> Optional[dict]:
 
     def _clear_token_settings(self) -> None:
         super()._clear_token_settings()
+        self._id_token = ""
         AddonSettings.set_setting(f"{self.prefix}id_token", "", store=LOCAL)
 
     def _extract_csrf_token(self, content: str) -> Optional[str]:

resources/lib/authentication/oauth2handler.py

@@ -44,12 +44,21 @@ def decode(token, **_kwargs):
 class OAuth2Handler(AuthenticationHandler, ABC):
     """Generic OAuth2 PKCE Handler for Retrospect."""
 
+    # How many seconds before token expiry we proactively refresh.
+    # 300s (5 minutes) gives ample time to retry on transient network errors.
+    _REFRESH_MARGIN = 300
+
     def __init__(self, realm: str, client_id: str):
         super(OAuth2Handler, self).__init__(realm, device_id=None)
         self.client_id_val = client_id
         # Use client-specific prefix to prevent storage collisions between different OAuth clients
         # e.g., "nlziet_oauth2_triple-web_" vs "nlziet_oauth2_triple-android-tv_"
         self.prefix = f"{realm}_oauth2_{client_id}_"
+        # Read token state once at construction; updated in-place by _store_tokens.
+        # Settings is the cross-invocation persistence layer (channel is re-created per action).
+        self._access_token = AddonSettings.get_setting(f"{self.prefix}access_token", store=LOCAL) or ""
+        self._refresh_token = AddonSettings.get_setting(f"{self.prefix}refresh_token", store=LOCAL) or ""
+        self._expires_at = int(AddonSettings.get_setting(f"{self.prefix}expires_at", store=LOCAL) or 0)
 
     @property
     @abstractmethod
@@ -105,13 +114,16 @@ def _request_tokens(self, data: dict):
             raise
 
     def _store_tokens(self, tokens: dict):
-        """Persist token fields to addon settings."""
-        AddonSettings.set_setting(f"{self.prefix}access_token", tokens["access_token"], store=LOCAL)
+        """Persist token fields to addon settings and update cached instance state."""
+        self._access_token = tokens["access_token"]
+        AddonSettings.set_setting(f"{self.prefix}access_token", self._access_token, store=LOCAL)
         if "refresh_token" in tokens:
-            AddonSettings.set_setting(f"{self.prefix}refresh_token", tokens["refresh_token"], store=LOCAL)
+            self._refresh_token = tokens["refresh_token"]
+            AddonSettings.set_setting(f"{self.prefix}refresh_token", self._refresh_token, store=LOCAL)
 
         expires_in = tokens.get("expires_in", 3600)
-        AddonSettings.set_setting(f"{self.prefix}expires_at", str(int(time.time()) + expires_in), store=LOCAL)
+        self._expires_at = int(time.time()) + expires_in
+        AddonSettings.set_setting(f"{self.prefix}expires_at", str(self._expires_at), store=LOCAL)
 
     def exchange_code(self, code: str, verifier: str) -> bool:
         """Exchange authorization code for tokens.
@@ -135,34 +147,50 @@ def exchange_code(self, code: str, verifier: str) -> bool:
             Logger.error(f"OAuth2: exchange_code failed for {self.realm}: {e}")
             return False
 
-    def refresh_access_token(self):
-        """Refresh the access token using the refresh token."""
-        refresh_token = AddonSettings.get_setting(f"{self.prefix}refresh_token", store=LOCAL)
-        if not refresh_token:
+    def _do_token_refresh(self):
+        """Unconditional access token refresh using the stored refresh token.
+
+        Subclasses override this to implement flow-specific refresh logic
+        (e.g. device flow vs. silent re-authentication).
+        """
+        if not self._refresh_token:
             raise ValueError("No refresh token available.")
 
         Logger.debug(f"OAuth2: Refreshing access token for {self.realm}")
         data = {
             "grant_type": "refresh_token",
             "client_id": self.client_id_val,
-            "refresh_token": refresh_token
+            "refresh_token": self._refresh_token
         }
         self._request_tokens(data)
 
-    def get_valid_token(self) -> Optional[str]:
-        """Get a valid access token, refreshing if necessary."""
-        expires_at = int(AddonSettings.get_setting(f"{self.prefix}expires_at", store=LOCAL) or 0)
+    def refresh_access_token(self) -> bool:
+        """Refresh the access token if within _REFRESH_MARGIN seconds of expiry.
+
+        Intended to be called on every heartbeat cycle.  The method is a no-op
+        when the token is still comfortably valid, so calling it frequently is
+        cheap (one in-memory comparison).
+
+        :return: True if the token is valid after the call, False on failure.
+        """
+        if time.time() < (self._expires_at - self._REFRESH_MARGIN):
+            Logger.trace(f"OAuth2: Token still valid for {self.realm} "
+                         f"({self._expires_at - time.time():.0f}s remaining)")
+            return True
+        try:
+            self._do_token_refresh()
+            return True
+        except Exception as e:
+            Logger.warning(f"OAuth2: Token refresh failed for {self.realm}: {e}")
+            return False
 
-        if time.time() > (expires_at - 60):
-            try:
-                self.refresh_access_token()
-            except Exception as e:
-                Logger.warning(f"OAuth2: Token refresh failed for {self.realm}: {e}")
-                return None
-        else:
-            Logger.trace(f"OAuth2: Using cached token for {self.realm}")
+    def get_valid_token(self) -> Optional[str]:
+        """Return the cached access token.
 
-        return AddonSettings.get_setting(f"{self.prefix}access_token", store=LOCAL)
+        Pure getter — no refresh side-effect.  Call refresh_access_token() first
+        whenever a fresh token is required.
+        """
+        return self._access_token or None
 
     def _extract_username_from_token(self, token: str) -> Optional[str]:
         """Extract username from JWT token."""
@@ -180,6 +208,7 @@ def _extract_username_from_token(self, token: str) -> Optional[str]:
 
     def active_authentication(self) -> AuthenticationResult:
         """Check for active authentication session."""
+        self.refresh_access_token()
         token = self.get_valid_token()
         if token:
             username = self._extract_username_from_token(token)
@@ -195,6 +224,9 @@ def log_on(self, username: str, password: str) -> AuthenticationResult:
         return AuthenticationResult(None, error="OAuth2 requires browser login.")
 
     def _clear_token_settings(self) -> None:
+        self._access_token = ""
+        self._refresh_token = ""
+        self._expires_at = 0
         AddonSettings.set_setting(f"{self.prefix}access_token", "", store=LOCAL)
         AddonSettings.set_setting(f"{self.prefix}refresh_token", "", store=LOCAL)
         AddonSettings.set_setting(f"{self.prefix}expires_at", "", store=LOCAL)

tests/authentication/test_nlziethandler.py

@@ -216,11 +216,13 @@ def test_06_token_refresh(self):
         if not result.logged_on:
             self.skipTest("No active session - test_03 must run first")
 
-        # Force token expiry
+        # Force token expiry on both settings and cached instance state
         expiry_key = f"{self.handler.prefix}expires_at"
         AddonSettings.set_setting(expiry_key, str(int(time.time()) - 100), store=LOCAL)
+        self.handler._expires_at = int(time.time()) - 100
 
-        # get_valid_token should trigger silent re-authentication
+        # refresh_access_token() performs silent re-authentication when needed
+        self.assertTrue(self.handler.refresh_access_token(), "Silent re-authentication failed")
         token = self.handler.get_valid_token()
         self.assertIsNotNone(token, "Silent re-authentication failed")
 
@@ -612,7 +614,9 @@ def _run_device_flow_refresh_token(self):
         expiry_key = f"{device_handler.prefix}expires_at"
         old_expiry = int(AddonSettings.get_setting(expiry_key, store=LOCAL) or 0)
         AddonSettings.set_setting(expiry_key, str(int(time.time()) - 10), store=LOCAL)
+        device_handler._expires_at = int(time.time()) - 10
 
+        self.assertTrue(device_handler.refresh_access_token(), "Refresh token grant failed")
         token = device_handler.get_valid_token()
         self.assertIsNotNone(token, "Refresh token grant failed")
 
@@ -755,11 +759,64 @@ def test_20_appconfig_status(self):
         pass  # Overridden to skip in mocked test class
 
     def test_refresh_no_tokens_raises_value_error(self):
-        """refresh_access_token raises ValueError when no refresh or id token is stored."""
+        """refresh_access_token returns False when no refresh or id token is stored."""
         AddonSettings.set_setting(f"{self.handler.prefix}refresh_token", "", store=LOCAL)
         AddonSettings.set_setting(f"{self.handler.prefix}id_token", "", store=LOCAL)
-        with self.assertRaises(ValueError):
-            self.handler.refresh_access_token()
+        self.handler._refresh_token = ""
+        self.handler._id_token = ""
+        self.handler._expires_at = 0  # force expiry so the margin check doesn't short-circuit
+        self.assertFalse(self.handler.refresh_access_token())
+
+    def test_refresh_access_token_noop_when_valid(self):
+        """refresh_access_token() skips the network when the token is still fresh."""
+        from tests.authentication.nlziet_mocks import MOCK_ACCESS_TOKEN
+        # Seed a valid token with expiry far in the future.
+        self.handler._access_token = MOCK_ACCESS_TOKEN
+        self.handler._id_token = ""
+        self.handler._refresh_token = ""
+        self.handler._expires_at = int(time.time()) + 7200  # 2 hours from now
+
+        # Track whether _do_token_refresh is invoked.
+        refresh_called = []
+        original = self.handler._do_token_refresh
+        self.handler._do_token_refresh = lambda: refresh_called.append(True) or original()
+        try:
+            result = self.handler.refresh_access_token()
+        finally:
+            self.handler._do_token_refresh = original
+
+        self.assertTrue(result, "Expected True for still-valid token")
+        self.assertEqual(refresh_called, [], "_do_token_refresh must NOT be called when token is fresh")
+
+    def test_get_valid_token_is_pure_getter(self):
+        """get_valid_token() returns the cached access token without any side-effect."""
+        from tests.authentication.nlziet_mocks import MOCK_ACCESS_TOKEN
+        self.handler._access_token = MOCK_ACCESS_TOKEN
+        self.handler._expires_at = 0  # expired — but get_valid_token must NOT refresh
+
+        refresh_called = []
+        original = self.handler._do_token_refresh
+        self.handler._do_token_refresh = lambda: refresh_called.append(True) or original()
+        try:
+            token = self.handler.get_valid_token()
+        finally:
+            self.handler._do_token_refresh = original
+
+        self.assertEqual(token, MOCK_ACCESS_TOKEN, "Must return the cached token verbatim")
+        self.assertEqual(refresh_called, [], "_do_token_refresh must NOT be called by get_valid_token()")
+
+    def test_instance_vars_loaded_from_settings_at_init(self):
+        """Token state read from settings once at construction, not on every call."""
+        from tests.authentication.nlziet_mocks import MOCK_ACCESS_TOKEN, MOCK_REFRESH_TOKEN
+        future_expiry = int(time.time()) + 3600
+        AddonSettings.set_setting(f"{self.handler.prefix}access_token", MOCK_ACCESS_TOKEN, store=LOCAL)
+        AddonSettings.set_setting(f"{self.handler.prefix}refresh_token", MOCK_REFRESH_TOKEN, store=LOCAL)
+        AddonSettings.set_setting(f"{self.handler.prefix}expires_at", str(future_expiry), store=LOCAL)
+
+        fresh = NLZIETOAuth2Handler(use_device_flow=False)
+        self.assertEqual(fresh._access_token, MOCK_ACCESS_TOKEN)
+        self.assertEqual(fresh._refresh_token, MOCK_REFRESH_TOKEN)
+        self.assertEqual(fresh._expires_at, future_expiry)
 
     def test_set_profile_exchanges_token(self):
         """set_profile performs a token exchange and stores a profile-scoped token."""