Revert ext-cache-key to MSAL .NET byte-parity (undo hardening fix #1)

Restore _compute_ext_cache_key to MSAL .NET's ComputeAccessTokenExtCacheKey
encoding: sorted, separator-less key+value concatenation -> SHA-256 -> base64url.
This makes the ext cache key byte-identical to MSAL .NET again.

The earlier hardening commit (4fc3639) had switched to Go's post-#629
length-prefixed encoding to make the key injective. Per maintainer decision,
msal-python should match MSAL .NET, not current Go, so that change is reverted:

- token_cache.py: restore plain key+value concatenation; docstring now notes the
  .NET match and the deliberate divergence from Go's #629 length-prefixed form.
- test_token_cache.py: restore the .NET parity hashes (bns2ytmx..., 3-rg6_wy...,
  rn_gkpxx...) and rename the parity tests to *_matches_dotnet; remove the two
  length-prefix boundary-collision regression tests (they asserted the injective
  property that .NET's encoding does not provide).

Hardening fixes #2-#6 (MI allow-list removal, MI source pre-validation, merge
conflict-precedence tests, generic docs, send-on-every-request docs) are
unchanged. 202 tests pass across test_token_cache.py, test_mi.py,
test_application.py.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Robbie-Microsoft

msal/token_cache.py

@@ -83,15 +83,15 @@ def _compute_ext_cache_key(data):
 
     Returns an empty string when *data* has no hashable fields.
 
-    The algorithm matches the Go MSAL implementation (CacheExtKeyGenerator,
-    post-collision-fix): length-prefixed key/value pairs (sorted by key) are
-    concatenated and SHA256 hashed, then base64url encoded. The length prefixes
-    make the encoding injective, so two distinct component sets can never collide
-    onto the same cache key. (MSAL .NET's ``ComputeAccessTokenExtCacheKey`` still
-    uses an unprefixed concatenation, so the hash is intentionally not
-    byte-identical to current .NET; the cache *key format* still matches both.
-    Caches are not shared across languages, so this only affects within-process
-    isolation, where injectivity is what matters.)
+    The algorithm matches MSAL .NET's ``ComputeAccessTokenExtCacheKey``: sorted
+    key+value pairs are concatenated (no separators) and SHA256 hashed, then
+    base64url encoded. This keeps the hash byte-identical to MSAL .NET.
+
+    MSAL Go's ``CacheExtKeyGenerator`` has since switched to a length-prefixed
+    encoding (AzureAD/microsoft-authentication-library-for-go#629) to make it
+    injective; Python deliberately tracks .NET instead, so these hashes are not
+    byte-identical to current Go. Caches are not shared across languages, so the
+    difference does not affect runtime correctness.
     """
     if not data:
         return ""
@@ -101,16 +101,11 @@ def _compute_ext_cache_key(data):
     }
     if not cache_components:
         return ""
-    # Concatenate length-prefixed key/value pairs so component boundaries are
-    # unambiguous (matches Go's CacheExtKeyGenerator). A plain key+value
-    # concatenation with no separators can collide when one value happens to
-    # contain another component's key or value -- and client_claims is arbitrary
-    # caller-supplied JSON that may embed e.g. "fmi_path" at a boundary -- mapping
-    # two distinct component sets onto the same hash and returning the wrong
-    # cached token. Length prefixes make the encoding injective.
+    # Sort keys, then concatenate key+value pairs with no separators. This
+    # matches MSAL .NET's ComputeAccessTokenExtCacheKey byte-for-byte. (See the
+    # docstring re: the Go #629 length-prefixed divergence.)
     key_str = "".join(
-        "{}:{}{}:{}".format(len(k), k, len(v), v)
-        for k, v in sorted(cache_components.items())
+        k + cache_components[k] for k in sorted(cache_components.keys())
     )
     hash_bytes = hashlib.sha256(key_str.encode("utf-8")).digest()
     return base64.urlsafe_b64encode(hash_bytes).rstrip(b"=").decode("ascii").lower()

tests/test_token_cache.py

@@ -392,29 +392,6 @@ def test_different_client_claims_produce_different_hashes(self):
     def test_empty_client_claims_value_is_ignored(self):
         self.assertEqual("", _compute_ext_cache_key({"client_claims": ""}))
 
-    def test_length_prefixed_encoding_avoids_boundary_collision(self):
-        # Mirrors Go's TestCacheKeyComponentHashNoBoundaryCollision. With a plain
-        # key+value concatenation (no separators) these two distinct component
-        # sets both render to "axbYbZ" (sorted keys "a","b") and would collide,
-        # returning the wrong cached token. The length-prefixed encoding must keep
-        # them distinct. This matters because client_claims is arbitrary caller
-        # JSON that can embed another component's key (e.g. "fmi_path").
-        h1 = _compute_ext_cache_key({"a": "xbY", "b": "Z"})
-        h2 = _compute_ext_cache_key({"a": "x", "b": "YbZ"})
-        self.assertNotEqual(
-            h1, h2,
-            "distinct cache key components must not produce the same hash")
-
-    def test_client_claims_and_fmi_path_do_not_collide_at_boundary(self):
-        # Realistic surface: client_claims and fmi_path co-occur in
-        # acquire_token_for_client. A claims value that happens to contain the
-        # other component's key+value at a boundary must not collide.
-        h1 = _compute_ext_cache_key(
-            {"client_claims": "Xfmi_pathY", "fmi_path": "Z"})
-        h2 = _compute_ext_cache_key(
-            {"client_claims": "X", "fmi_path": "Yfmi_pathZ"})
-        self.assertNotEqual(h1, h2)
-
 
 class TestClaimsHelpers(unittest.TestCase):
     """Tests for the shared _parse_claims_or_raise / _merge_claims helpers."""
@@ -565,48 +542,45 @@ def test_non_fmi_tokens_not_affected_by_fmi_cache(self):
 
 
 class TestCrossMsalCacheKeyCompatibility(unittest.TestCase):
-    """Verify that _compute_ext_cache_key matches MSAL Go's CacheExtKeyGenerator
-    (post collision-fix, AzureAD/microsoft-authentication-library-for-go#629).
+    """Verify that _compute_ext_cache_key produces hashes identical to MSAL .NET
+    (CoreHelpers.ComputeAccessTokenExtCacheKey).
 
     The algorithm:
       1. Sort key-value pairs alphabetically by key (ordinal / case-sensitive)
-      2. Concatenate length-prefixed pairs ("{len(k)}:{k}{len(v)}:{v}" per pair;
-         e.g. {"key1": "value1"} -> "4:key16:value1"). The length prefixes make
-         the encoding injective -- see TestClientClaimsCacheKey for the collision
-         guard.
+      2. Concatenate them with no separators: "key1value1key2value2…"
       3. SHA-256 hash
       4. Base64url encode (no padding), lowercased
 
-    The expected hashes below are copied from MSAL Go's
-    authority_ext_cachekey_test.go (TestAppKeyWithCacheKeyComponent).
+    The expected hashes below are copied from MSAL .NET's CacheKeyExtensionTests.cs
+    (RunHappyPathTest, CacheExtEnsurePopKeysFunctionAsync).
 
-    NOTE: MSAL .NET's ComputeAccessTokenExtCacheKey still uses an *unprefixed*
-    concatenation, so these hashes are intentionally NOT byte-identical to current
-    .NET. The cache *key format* (the 'atext' segment layout, asserted below) still
-    matches both Go and .NET; only the trailing hash bytes differ. Caches are not
-    shared across languages, so within-process injectivity -- not cross-language
-    byte-parity -- is what matters for correctness.
+    NOTE: MSAL Go's CacheExtKeyGenerator has since switched to a *length-prefixed*
+    encoding (AzureAD/microsoft-authentication-library-for-go#629), so these hashes
+    are intentionally NOT byte-identical to current Go; Python deliberately tracks
+    .NET here. The cache *key format* (the 'atext' segment layout, asserted below)
+    still matches both Go and .NET. Caches are not shared across languages, so this
+    cross-language hash difference does not affect runtime correctness.
     """
 
-    def test_two_params_hash_matches_go(self):
-        """Go expected: latlwkpewb_a0rcsmjvkecqt0_huumkw4sflzociike"""
+    def test_two_params_hash_matches_dotnet(self):
+        """.NET expected: bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi"""
         result = _compute_ext_cache_key({"key1": "value1", "key2": "value2"})
-        self.assertEqual("latlwkpewb_a0rcsmjvkecqt0_huumkw4sflzociike", result)
+        self.assertEqual("bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi", result)
 
-    def test_two_different_params_hash_matches_go(self):
-        """Go expected: jjoe9jgfmdtnj0rzuetsqy7kzs2m1xfnjjxwsfxsrxq"""
+    def test_two_different_params_hash_matches_dotnet(self):
+        """.NET expected: 3-rg6_wyjx5bcy0c3cqq7gajtzgsqy3oxqpwj4y8k4u"""
         result = _compute_ext_cache_key({"key3": "value3", "key4": "value4"})
-        self.assertEqual("jjoe9jgfmdtnj0rzuetsqy7kzs2m1xfnjjxwsfxsrxq", result)
+        self.assertEqual("3-rg6_wyjx5bcy0c3cqq7gajtzgsqy3oxqpwj4y8k4u", result)
 
-    def test_five_params_hash_matches_go(self):
-        """Go expected: prrdp31y37ufw3lo7hly0oimjjvg_34m9ji30ocu4tw"""
+    def test_five_params_hash_matches_dotnet(self):
+        """.NET expected (full hash): rn_gkpxxkkqjxcqnvnmr2duvxg66xanvkz6qfqpwp2e"""
         result = _compute_ext_cache_key({
             "key3": "value3", "key4": "value4",
             "key5": "value5", "key6": "value6", "key7": "value7",
         })
-        self.assertEqual("prrdp31y37ufw3lo7hly0oimjjvg_34m9ji30ocu4tw", result)
+        self.assertEqual("rn_gkpxxkkqjxcqnvnmr2duvxg66xanvkz6qfqpwp2e", result)
 
-    def test_order_independence_matches_go(self):
+    def test_order_independence_matches_dotnet(self):
         """Same keys in different insertion order must produce the same hash
         (mirrors TestCacheKeyComponentHashConsistency in Go)."""
         h1 = _compute_ext_cache_key({"key3": "value3", "key4": "value4",
@@ -627,9 +601,9 @@ def test_at_cache_key_uses_atext_credential_type(self):
         key = key_maker(
             home_account_id="hid", environment="env", client_id="cid",
             realm="realm", target="scope",
-            ext_cache_key="latlwkpewb_a0rcsmjvkecqt0_huumkw4sflzociike")
+            ext_cache_key="bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi")
         self.assertEqual(
-            "hid-env-atext-cid-realm-scope-latlwkpewb_a0rcsmjvkecqt0_huumkw4sflzociike",
+            "hid-env-atext-cid-realm-scope-bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi",
             key)
 
     def test_at_cache_key_without_ext_uses_accesstoken(self):
@@ -641,10 +615,9 @@ def test_at_cache_key_without_ext_uses_accesstoken(self):
             realm="realm", target="scope")
         self.assertEqual("hid-env-accesstoken-cid-realm-scope", key)
 
-    def test_atext_full_at_cache_key_format(self):
-        """The AT cache key *format* matches MSAL .NET's CacheKeyExtensionTests
-        layout ('-{env}-atext-{clientId}-{tenant}-{scopes}-{hash}'); only the
-        trailing hash now follows Go's length-prefixed encoding (see class note).
+    def test_dotnet_style_full_at_cache_key(self):
+        """Reproduce the exact cache key from MSAL .NET CacheKeyExtensionTests:
+        expectedCacheKey1 = '-login.windows.net-atext-d3adb33f-c0de-ed0c-c0de-deadb33fc0d3-common-r1/scope1 r1/scope2-bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi'
         """
         cache = TokenCache()
         key_maker = cache.key_makers[TokenCache.CredentialType.ACCESS_TOKEN]
@@ -656,11 +629,11 @@ def test_atext_full_at_cache_key_format(self):
             realm="common",
             target="r1/scope1 r1/scope2",
             ext_cache_key=ext_hash)
-        expected = "-login.windows.net-atext-d3adb33f-c0de-ed0c-c0de-deadb33fc0d3-common-r1/scope1 r1/scope2-latlwkpewb_a0rcsmjvkecqt0_huumkw4sflzociike"
+        expected = "-login.windows.net-atext-d3adb33f-c0de-ed0c-c0de-deadb33fc0d3-common-r1/scope1 r1/scope2-bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi"
         self.assertEqual(expected, key)
 
-    def test_atext_second_full_at_cache_key_format(self):
-        """Second key-format vector (mirrors CacheKeyExtensionTests expectedCacheKey2)."""
+    def test_dotnet_style_second_cache_key(self):
+        """Reproduce CacheKeyExtensionTests expectedCacheKey2."""
         cache = TokenCache()
         key_maker = cache.key_makers[TokenCache.CredentialType.ACCESS_TOKEN]
         ext_hash = _compute_ext_cache_key({"key3": "value3", "key4": "value4"})
@@ -671,12 +644,13 @@ def test_atext_second_full_at_cache_key_format(self):
             realm="common",
             target="r1/scope1 r1/scope2",
             ext_cache_key=ext_hash)
-        expected = "-login.windows.net-atext-d3adb33f-c0de-ed0c-c0de-deadb33fc0d3-common-r1/scope1 r1/scope2-jjoe9jgfmdtnj0rzuetsqy7kzs2m1xfnjjxwsfxsrxq"
+        expected = "-login.windows.net-atext-d3adb33f-c0de-ed0c-c0de-deadb33fc0d3-common-r1/scope1 r1/scope2-3-rg6_wyjx5bcy0c3cqq7gajtzgsqy3oxqpwj4y8k4u"
         self.assertEqual(expected, key)
 
     def test_go_style_at_cache_key(self):
-        """Reproduce the Go AccessToken.Key() format with Go's post-#629 hash:
-        'testhid-env-atext-clientid-realm-user.read-{hash}'.
+        """Reproduce the Go AccessToken.Key() *format* (segment layout):
+        'testhid-env-atext-clientid-realm-user.read-{hash}'. The hash follows our
+        .NET-matching encoding (see class note on the Go #629 divergence).
         """
         cache = TokenCache()
         key_maker = cache.key_makers[TokenCache.CredentialType.ACCESS_TOKEN]
@@ -688,5 +662,5 @@ def test_go_style_at_cache_key(self):
             realm="realm",
             target="user.read",
             ext_cache_key=ext_hash)
-        expected = "testhid-env-atext-clientid-realm-user.read-latlwkpewb_a0rcsmjvkecqt0_huumkw4sflzociike"
+        expected = "testhid-env-atext-clientid-realm-user.read-bns2ytmx5hxkh4fnfixridmezpbbayhnmuh6t4bbghi"
         self.assertEqual(expected, key)