Deprecate decode_id_token and stop validating ID tokens (#911)

msal/oauth2cli/oidc.py

@@ -74,14 +74,47 @@ class IdTokenAudienceError(IdTokenError):
 class IdTokenNonceError(IdTokenError):
     pass
 
+def _decode_id_token_claims(id_token):
+    """Decode an id_token and return its claims as a dictionary, WITHOUT validation.
+
+    MSAL does not validate the ID token. Per OpenID Connect, an ID token that is
+    obtained via direct communication between the client and the token endpoint
+    (which is how MSAL retrieves tokens) does not need to be validated by the
+    client. Validation, if needed, is the responsibility of the application,
+    which can perform it at the appropriate time (see the issue below).
+    https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
+
+    ID token claims would at least contain: "iss", "sub", "aud", "exp", "iat",
+    per `specs <https://openid.net/specs/openid-connect-core-1_0.html#IDToken>`_
+    and it may contain other optional content such as "preferred_username",
+    `maybe more <https://openid.net/specs/openid-connect-core-1_0.html#Claims>`_
+    """
+    return json.loads(decode_part(id_token.split('.')[1]))
+
+
 def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None):
     """Decodes and validates an id_token and returns its claims as a dictionary.
 
+    .. deprecated:: 1.38.0
+        MSAL's token acquisition no longer validates the ID token, because the
+        SDK should not perform any ID token validation
+        (`issue #911 <https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911>`_).
+        This standalone helper is kept only for backward compatibility and it
+        still performs the legacy validations described below. Prefer the
+        ``id_token_claims`` that MSAL already returns alongside each token, or
+        decode the token yourself, and perform any validation your app requires.
+
     ID token claims would at least contain: "iss", "sub", "aud", "exp", "iat",
     per `specs <https://openid.net/specs/openid-connect-core-1_0.html#IDToken>`_
     and it may contain other optional content such as "preferred_username",
     `maybe more <https://openid.net/specs/openid-connect-core-1_0.html#Claims>`_
     """
+    warnings.warn(
+        "decode_id_token() is deprecated. MSAL's token acquisition no longer "
+        "validates the ID token; validation is the application's "
+        "responsibility. This legacy helper still performs some validation. "
+        "Prefer the id_token_claims returned alongside the token.",
+        DeprecationWarning, stacklevel=2)
     decoded = json.loads(decode_part(id_token.split('.')[1]))
     # Based on https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
     _now = int(now or time.time())
@@ -157,18 +190,27 @@ class Client(oauth2.Client):
     """
 
     def decode_id_token(self, id_token, nonce=None):
-        """See :func:`~decode_id_token`."""
+        """See :func:`~decode_id_token`.
+
+        .. deprecated:: 1.38.0
+            MSAL's token acquisition no longer validates the ID token. This
+            method is kept for backward compatibility and still performs the
+            legacy validations. See :func:`~decode_id_token`.
+        """
         return decode_id_token(
             id_token, nonce=nonce,
             client_id=self.client_id, issuer=self.configuration.get("issuer"))
 
     def _obtain_token(self, grant_type, *args, **kwargs):
         """The result will also contain one more key "id_token_claims",
-        whose value will be a dictionary returned by :func:`~decode_id_token`.
+        whose value is a dictionary of the (non-validated) ID token claims.
         """
         ret = super(Client, self)._obtain_token(grant_type, *args, **kwargs)
         if "id_token" in ret:
-            ret["id_token_claims"] = self.decode_id_token(ret["id_token"])
+            # MSAL does not validate the ID token. It only decodes the claims,
+            # so that downstream components can build accounts, etc.
+            # https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
+            ret["id_token_claims"] = _decode_id_token_claims(ret["id_token"])
         return ret
 
     def build_auth_request_uri(self, response_type, nonce=None, **kwargs):

msal/token_cache.py

@@ -7,7 +7,7 @@
 import warnings
 
 from .authority import canonicalize
-from .oauth2cli.oidc import decode_part, decode_id_token
+from .oauth2cli.oidc import decode_part, _decode_id_token_claims
 from .oauth2cli.oauth2 import Client
 
 
@@ -349,8 +349,9 @@ def __add(self, event, now=None):
         refresh_token = response.get("refresh_token")
         id_token = response.get("id_token")
         id_token_claims = response.get("id_token_claims") or (  # Prefer the claims from broker
-            # Only use decode_id_token() when necessary, it contains time-sensitive validation
-            decode_id_token(id_token, client_id=event["client_id"]) if id_token else {})
+            # MSAL does not validate the ID token; it only decodes the claims.
+            # https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
+            _decode_id_token_claims(id_token) if id_token else {})
         client_info, home_account_id = self.__parse_account(response, id_token_claims)
 
         target = ' '.join(sorted(event.get("scope") or []))  # Schema should have required sorting

tests/test_oidc.py

@@ -80,16 +80,30 @@ def test_oidc_nonce_is_url_safe_and_unpredictable(self):
 class TestIdToken(unittest.TestCase):
     EXPIRED_ID_TOKEN = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJpc3N1ZXIiLCJpYXQiOjE3MDY1NzA3MzIsImV4cCI6MTY3NDk0ODMzMiwiYXVkIjoiZm9vIiwic3ViIjoic3ViamVjdCJ9.wyWNFxnE35SMP6FpxnWZmWQAy4KD0No_Q1rUy5bNnLs"
 
-    def test_id_token_should_tolerate_time_error(self):
-        self.assertEqual(oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN), {
-            "iss": "issuer",
-            "iat": 1706570732,
-            "exp": 1674948332,  # 2023-1-28
-            "aud": "foo",
-            "sub": "subject",
-            }, "id_token is decoded correctly, without raising exception")
-
-    def test_id_token_should_error_out_on_client_id_error(self):
-        with self.assertRaises(msal.IdTokenError):
-            oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN, client_id="not foo")
+    _EXPECTED_CLAIMS = {
+        "iss": "issuer",
+        "iat": 1706570732,
+        "exp": 1674948332,  # 2023-1-28
+        "aud": "foo",
+        "sub": "subject",
+        }
+
+    def test_decode_id_token_is_deprecated_but_still_tolerates_time_error(self):
+        with self.assertWarns(DeprecationWarning):
+            claims = oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN)
+        self.assertEqual(claims, self._EXPECTED_CLAIMS,
+            "id_token is decoded correctly, without raising exception")
+
+    def test_deprecated_decode_id_token_should_error_out_on_client_id_error(self):
+        with self.assertWarns(DeprecationWarning):
+            with self.assertRaises(msal.IdTokenError):
+                oauth2cli.oidc.decode_id_token(
+                    self.EXPIRED_ID_TOKEN, client_id="not foo")
+
+    def test_internal_decoder_does_not_validate_the_id_token(self):
+        # https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
+        # The SDK must not perform any ID token validation on retrieval.
+        # This expired token, decoded via the internal helper, must not raise.
+        claims = oauth2cli.oidc._decode_id_token_claims(self.EXPIRED_ID_TOKEN)
+        self.assertEqual(claims, self._EXPECTED_CLAIMS)