Offer access to refresh credentals outside the request authenticator (#26)

* Updating request authenticator to provide an option telling it
to refresh the credential when pulling it from the authenticator
object.

This provides a way to obtain a refreshed token without depending
on using the authenticator to make an application call.

Drive-by improvements to type hinting.

Author: carl-adams-planet

src/planet_auth/oidc/request_authenticator.py

@@ -14,7 +14,7 @@
 
 import jwt
 import time
-from typing import Dict
+from typing import Dict, Optional
 
 import planet_auth.logging.auth_logger
 from planet_auth.credential import Credential
@@ -79,45 +79,55 @@ def _load(self):
 
     def _refresh(self):
         if self._auth_client:
+            # TODO: cleanup?  We did this before we wrote "update_credential_data()"
+            #    We maybe should just be using that now.  Taking that clean-up slow
+            #    since it may have interactions with derived code.
+            #    It seems to me we should be able to collapse the credential.save()
+            #    calls in this file with the superclass.
             new_credentials = self._auth_client.refresh(self._credential.refresh_token())
             new_credentials.set_path(self._credential.path())
             new_credentials.set_storage_provider(self._credential.storage_provider())
             new_credentials.save()
 
+            # self.update_credential(new_credential=new_credentials)
             self._credential = new_credentials
             self._load()
 
-    def pre_request_hook(self):
+    def _refresh_if_needed(self):
         # Reload the file before refreshing. Another process might
         # have done it for us, and save us the network call.
         #
         # Also, if refresh tokens are configured to be one time use,
-        # we want a fresh refresh token. Stale refresh tokens are
-        # invalid in this case.
+        # we want a fresh refresh token. Stale refresh tokens may be
+        # invalid depending on server configuration.
         #
         # Also, it's possible that we have a valid refresh token,
         # but not an access token.  When that's true, we should
         # try to cash in the refresh token.
         #
         # If everything fails, continue with what we have. Let the API
         # we are calling decide if it's good enough.
-        if int(time.time()) > self._refresh_at:
+        now = int(time.time())
+        if now > self._refresh_at:
             try:
                 self._load()
             except Exception as e:  # pylint: disable=broad-exception-caught
                 auth_logger.warning(
                     msg="Error loading auth token. Continuing with old auth token. Load error: " + str(e)
                 )
-        if int(time.time()) > self._refresh_at:
+        if now > self._refresh_at:
             try:
                 self._refresh()
             except Exception as e:  # pylint: disable=broad-exception-caught
                 auth_logger.warning(
                     msg="Error refreshing auth token. Continuing with old auth token. Refresh error: " + str(e)
                 )
+
+    def pre_request_hook(self):
+        self._refresh_if_needed()
         super().pre_request_hook()
 
-    def update_credential(self, new_credential: Credential):
+    def update_credential(self, new_credential: Credential) -> None:
         if not isinstance(new_credential, FileBackedOidcCredential):
             raise TypeError(
                 f"{type(self).__name__} does not support {type(new_credential)} credentials.  Use FileBackedOidcCredential."
@@ -126,7 +136,7 @@ def update_credential(self, new_credential: Credential):
         self._refresh_at = 0
         # self._load()  # Mimic __init__.  Don't load, let that happen JIT.
 
-    def update_credential_data(self, new_credential_data: Dict):
+    def update_credential_data(self, new_credential_data: Dict) -> None:
         # This is more different than update_credential() than it may
         # appear. Inherent in being passed a Credential in update_credential()
         # is that it may not yet be loaded from disk, and so deferring
@@ -137,6 +147,11 @@ def update_credential_data(self, new_credential_data: Dict):
         super().update_credential_data(new_credential_data=new_credential_data)
         self._load()
 
+    def credential(self, refresh_if_needed: bool = False) -> Optional[Credential]:
+        if refresh_if_needed:
+            self._refresh_if_needed()
+        return super().credential()
+
 
 class RefreshOrReloginOidcTokenRequestAuthenticator(RefreshingOidcTokenRequestAuthenticator):
     """
@@ -171,5 +186,6 @@ def _refresh(self):
             new_credentials.set_storage_provider(self._credential.storage_provider())
             new_credentials.save()
 
+            # self.update_credential(new_credential=new_credentials)
             self._credential = new_credentials
             self._load()

src/planet_auth/planet_legacy/request_authenticator.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from planet_auth.credential import Credential
 from planet_auth.planet_legacy.legacy_api_key import FileBackedPlanetLegacyApiKey
 from planet_auth.request_authenticator import CredentialRequestAuthenticator
 
@@ -32,7 +33,7 @@ def __init__(self, planet_legacy_credential: FileBackedPlanetLegacyApiKey, **kwa
     def pre_request_hook(self):
         self._token_body = self._api_key_file.legacy_api_key()
 
-    def update_credential(self, new_credential):
+    def update_credential(self, new_credential: Credential) -> None:
         if not isinstance(new_credential, FileBackedPlanetLegacyApiKey):
             raise TypeError(
                 f"{type(self).__name__} does not support {type(new_credential)} credentials.  Use FileBackedPlanetLegacyApiKey."

src/planet_auth/request_authenticator.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 from abc import abstractmethod, ABC
-from typing import Dict
+from typing import Dict, Optional
 
 import httpx
 import requests.auth
@@ -94,7 +94,7 @@ def __init__(self, credential: Credential = None, **kwargs):
         super().__init__(**kwargs)
         self._credential = credential
 
-    def update_credential(self, new_credential: Credential):
+    def update_credential(self, new_credential: Credential) -> None:
         """
         Update the request authenticator with a new credential.
         It is the job of a derived class to know how to map the contents
@@ -108,7 +108,7 @@ def update_credential(self, new_credential: Credential):
         # requests.
         self._token_body = None
 
-    def update_credential_data(self, new_credential_data: Dict):
+    def update_credential_data(self, new_credential_data: Dict) -> None:
         """
         Provide raw data that should be used to update the Credential
         object used to authenticate requests.  This information will
@@ -130,13 +130,14 @@ def update_credential_data(self, new_credential_data: Dict):
         # requests.
         self._token_body = None
 
-    def credential(self):
+    def credential(self, refresh_if_needed: bool = False) -> Optional[Credential]:
         """
         Return the current credential.
 
         This may not be the credential the authenticator was constructed with.
         Request Authenticators are free to refresh credentials depending in the
-        needs of the implementation.
+        needs of the implementation.  This may happen upon this request,
+        or may happen as a side effect of RequestAuthenticator operations.
         """
         return self._credential
 
@@ -162,7 +163,7 @@ class SimpleInMemoryRequestAuthenticator(CredentialRequestAuthenticator):
     def pre_request_hook(self):
         pass
 
-    def update_credential(self, new_credential: Credential):
+    def update_credential(self, new_credential: Credential) -> None:
         auth_logger.warning(msg="Generic SimpleInMemoryRequestAuthenticator ignores update_credential()")