Remove identity ID consistency validation from ClientApp (#1111)

* Recreated failing test case in unit test

* Removed identity id validation from ClientApps

* PR suggestions

changelog.d/20241121_133309_derek_client_app_expired_token_bug.rst

@@ -0,0 +1,10 @@
+
+Changed
+~~~~~~~
+
+-   Removed identity ID consistency validation from ``ClientApp``. (:pr:`NUMBER`)
+
+Fixed
+~~~~~
+
+-   Fixed a bug that would cause ``ClientApp`` token refreshes to fail. (:pr:`NUMBER`)

src/globus_sdk/globus_app/app.py

@@ -14,7 +14,6 @@
     ScopeRequirementsValidator,
     TokenStorage,
     TokenValidationError,
-    UnchangingIdentityIDValidator,
     ValidatingTokenStorage,
 )
 
@@ -92,7 +91,7 @@ def __init__(
 
         # create the requisite token storage for the app, with validation based on
         # the provided parameters
-        self.token_storage = _build_default_validating_token_storage(
+        self.token_storage = self._initialize_validating_token_storage(
             token_storage=self._token_storage,
             consent_client=consent_client,
             scope_requirements=self._scope_requirements,
@@ -178,6 +177,26 @@ def _initialize_login_client(
         Initializes and returns an AuthLoginClient to be used in authorization requests.
         """
 
+    def _initialize_validating_token_storage(
+        self,
+        token_storage: TokenStorage,
+        consent_client: AuthClient,
+        scope_requirements: t.Mapping[str, t.Sequence[Scope]],
+    ) -> ValidatingTokenStorage:
+        """
+        Initializes the validating token storage for the app.
+        """
+        validating_token_storage = ValidatingTokenStorage(token_storage)
+
+        # construct ValidatingTokenStorage around the TokenStorage and
+        # our initial scope requirements
+        scope_validator = ScopeRequirementsValidator(scope_requirements, consent_client)
+
+        # use validators to enforce invariants about scopes
+        validating_token_storage.validators.append(scope_validator)
+
+        return validating_token_storage
+
     def _resolve_token_storage(
         self, app_name: str, client_id: UUIDLike, config: GlobusAppConfig
     ) -> TokenStorage:
@@ -391,36 +410,3 @@ def scope_requirements(self) -> dict[str, list[Scope]]:
         """
         # Scopes are mutable objects so we return a deepcopy
         return copy.deepcopy(self._scope_requirements)
-
-
-def _build_default_validating_token_storage(
-    *,
-    token_storage: TokenStorage,
-    consent_client: AuthClient,
-    scope_requirements: t.Mapping[str, t.Sequence[Scope]],
-) -> ValidatingTokenStorage:
-    """
-    Given the appropriate configuration data, build the default
-    ValidatingTokenStorage for use within GlobusApp.
-
-    :param token_storage: The token storage to wrap in a ValidatingTokenStorage.
-    :param consent_client: The app's internal AuthClient instance which is used
-        to fetch consent information.
-    :param scope_requirements: The scope requirements for the app.
-    """
-    validating_token_storage = ValidatingTokenStorage(token_storage)
-
-    # construct ValidatingTokenStorage around the TokenStorage and
-    # our initial scope requirements
-    scope_validator = ScopeRequirementsValidator(scope_requirements, consent_client)
-    identity_id_validator = UnchangingIdentityIDValidator()
-
-    # use validators to enforce invariants about scopes and identity ID
-    validating_token_storage.validators.extend(
-        (
-            scope_validator,
-            identity_id_validator,
-        )
-    )
-
-    return validating_token_storage

src/globus_sdk/globus_app/user_app.py

@@ -3,15 +3,23 @@
 import typing as t
 
 from globus_sdk import (
+    AuthClient,
     AuthLoginClient,
     ConfidentialAppAuthClient,
     GlobusSDKUsageError,
     NativeAppAuthClient,
+    Scope,
 )
 from globus_sdk._types import ScopeCollectionType, UUIDLike
 from globus_sdk.gare import GlobusAuthorizationParameters
 from globus_sdk.login_flows import CommandLineLoginFlowManager, LoginFlowManager
-from globus_sdk.tokenstorage import HasRefreshTokensValidator, NotExpiredValidator
+from globus_sdk.tokenstorage import (
+    HasRefreshTokensValidator,
+    NotExpiredValidator,
+    TokenStorage,
+    UnchangingIdentityIDValidator,
+    ValidatingTokenStorage,
+)
 
 from .app import GlobusApp
 from .authorizer_factory import (
@@ -136,6 +144,18 @@ def _initialize_login_client(
                 environment=config.environment,
             )
 
+    def _initialize_validating_token_storage(
+        self,
+        token_storage: TokenStorage,
+        consent_client: AuthClient,
+        scope_requirements: t.Mapping[str, t.Sequence[Scope]],
+    ) -> ValidatingTokenStorage:
+        validating_token_storage = super()._initialize_validating_token_storage(
+            token_storage, consent_client, scope_requirements
+        )
+        validating_token_storage.validators.append(UnchangingIdentityIDValidator())
+        return validating_token_storage
+
     def _initialize_authorizer_factory(self) -> None:
         if self.config.request_refresh_tokens:
             self._authorizer_factory = RefreshTokenAuthorizerFactory(

tests/unit/globus_app/test_globus_app.py

@@ -15,6 +15,7 @@
     GlobusAppConfig,
     NativeAppAuthClient,
     RefreshTokenAuthorizer,
+    TransferClient,
     UserApp,
 )
 from globus_sdk._testing import load_response
@@ -42,15 +43,20 @@
 )
 
 
-def _mock_token_data_by_rs():
+def _mock_token_data_by_rs(
+    resource_server: str = "auth.globus.org",
+    scope: str = "openid",
+    refresh_token: str | None = "mock_refresh_token",
+    expiration_delta: int = 300,
+):
     return {
-        "auth.globus.org": TokenStorageData(
-            resource_server="auth.globus.org",
+        resource_server: TokenStorageData(
+            resource_server=resource_server,
             identity_id="mock_identity_id",
-            scope="openid",
+            scope=scope,
             access_token="mock_access_token",
-            refresh_token="mock_refresh_token",
-            expires_at_seconds=int(time.time() + 300),
+            refresh_token=refresh_token,
+            expires_at_seconds=int(time.time() + expiration_delta),
             token_type="Bearer",
         )
     }
@@ -434,13 +440,14 @@ def run_login_flow(
         raise CustomExitException("mock login attempt")
 
 
-def test_user_app_expired_authorizer_triggers_login():
+def test_user_app_expired_token_triggers_login():
     # Set up token data with an expired access token and no refresh token
     client_id = "mock_client_id"
     memory_storage = MemoryTokenStorage()
-    token_data = _mock_token_data_by_rs()
-    token_data["auth.globus.org"].expires_at_seconds = int(time.time() - 3600)
-    token_data["auth.globus.org"].refresh_token = None
+    token_data = _mock_token_data_by_rs(
+        refresh_token=None,
+        expiration_delta=-3600,  # Expired by 1 hour
+    )
     memory_storage.store_token_data_by_resource_server(token_data)
 
     login_flow_manager = RaisingLoginFlowManagerCounter()
@@ -455,6 +462,42 @@ def test_user_app_expired_authorizer_triggers_login():
     assert login_flow_manager.counter == 1
 
 
+def test_client_app_expired_token_is_auto_resolved():
+    """
+    This test exercises ClientApp token grant behavior.
+    ClientApps may request updated tokens outside the normal token authorization flow.
+    """
+    client_creds = {
+        "client_id": "mock_client_id",
+        "client_secret": "mock_client_secret",
+    }
+    meta = load_response("auth.oauth2_client_credentials_tokens").metadata
+
+    memory_storage = MemoryTokenStorage()
+    token_data = _mock_token_data_by_rs(
+        resource_server=meta["resource_server"],
+        scope=meta["scope"],
+        refresh_token=None,
+        expiration_delta=-3600,  # Expired by 1 hour
+    )
+    memory_storage.store_token_data_by_resource_server(token_data)
+
+    config = GlobusAppConfig(token_storage=memory_storage)
+    client_app = ClientApp("test-app", **client_creds, config=config)
+
+    transfer = TransferClient(app=client_app, app_scopes=[Scope(meta["scope"])])
+    load_response(transfer.task_list)
+
+    starting_token = memory_storage.get_token_data(meta["resource_server"]).access_token
+    assert starting_token == token_data[meta["resource_server"]].access_token
+
+    transfer.task_list()
+
+    ending_token = memory_storage.get_token_data(meta["resource_server"]).access_token
+    assert starting_token != ending_token
+    assert ending_token == meta["access_token"]
+
+
 def test_client_app_get_authorizer():
     client_id = "mock_client_id"
     client_secret = "mock_client_secret"