Merge pull request #315 from AzureAD/keep-rt-for-others

Keep RT even during invalid_grant

Author: rayluo

msal/application.py

@@ -918,11 +918,17 @@ def _acquire_token_silent_by_finding_specific_refresh_token(
         client = self._build_client(self.client_credential, authority)
 
         response = None  # A distinguishable value to mean cache is empty
-        for entry in matches:
+        for entry in sorted(  # Since unfit RTs would not be aggressively removed,
+                              # we start from newer RTs which are more likely fit.
+                matches,
+                key=lambda e: int(e.get("last_modification_time", "0")),
+                reverse=True):
             logger.debug("Cache attempts an RT")
             response = client.obtain_token_by_refresh_token(
                 entry, rt_getter=lambda token_item: token_item["secret"],
-                on_removing_rt=rt_remover or self.token_cache.remove_rt,
+                on_removing_rt=lambda rt_item: None,  # Disable RT removal,
+                    # because an invalid_grant could be caused by new MFA policy,
+                    # the RT could still be useful for other MFA-less scope or tenant
                 on_obtaining_tokens=lambda event: self.token_cache.add(dict(
                     event,
                     environment=authority.instance,

msal/token_cache.py

@@ -148,9 +148,9 @@ def __add(self, event, now=None):
         target = ' '.join(event.get("scope", []))  # Per schema, we don't sort it
 
         with self._lock:
+            now = int(time.time() if now is None else now)
 
             if access_token:
-                now = int(time.time() if now is None else now)
                 expires_in = int(  # AADv1-like endpoint returns a string
 			response.get("expires_in", 3599))
                 ext_expires_in = int(  # AADv1-like endpoint returns a string
@@ -212,6 +212,7 @@ def __add(self, event, now=None):
                     "environment": environment,
                     "client_id": event.get("client_id"),
                     "target": target,  # Optional per schema though
+                    "last_modification_time": str(now),  # Optional. Schema defines it as a string.
                     }
                 if "foci" in response:
                     rt["family_id"] = response["foci"]
@@ -249,8 +250,10 @@ def remove_rt(self, rt_item):
 
     def update_rt(self, rt_item, new_rt):
         assert rt_item.get("credential_type") == self.CredentialType.REFRESH_TOKEN
-        return self.modify(
-            self.CredentialType.REFRESH_TOKEN, rt_item, {"secret": new_rt})
+        return self.modify(self.CredentialType.REFRESH_TOKEN, rt_item, {
+            "secret": new_rt,
+            "last_modification_time": str(int(time.time())),  # Optional. Schema defines it as a string.
+            })
 
     def remove_at(self, at_item):
         assert at_item.get("credential_type") == self.CredentialType.ACCESS_TOKEN

tests/test_application.py

@@ -73,8 +73,7 @@ def setUp(self):
             self.client_id, authority=self.authority_url, token_cache=self.cache)
 
     def test_cache_empty_will_be_returned_as_None(self):
-        self.assertEqual(
-            None, self.app.acquire_token_silent(['cache_miss'], self.account))
+        self.app.token_cache = msal.SerializableTokenCache()  # Reset it to empty
         self.assertEqual(
             None, self.app.acquire_token_silent_with_error(['cache_miss'], self.account))
 

tests/test_token_cache.py

@@ -84,6 +84,7 @@ def testAddByAad(self):
                 'credential_type': 'RefreshToken',
                 'environment': 'login.example.com',
                 'home_account_id': "uid.utid",
+                'last_modification_time': '1000',
                 'secret': 'a refresh token',
                 'target': 's2 s1 s3',
             },
@@ -157,6 +158,7 @@ def testAddByAdfs(self):
                 'credential_type': 'RefreshToken',
                 'environment': 'fs.msidlab8.com',
                 'home_account_id': "subject",
+                'last_modification_time': "1000",
                 'secret': 'a refresh token',
                 'target': 's2 s1 s3',
             },