Skip to content

Commit b299e3b

Browse files
4gustCopilot
4gust
and
Copilot
authored
Deprecate decode_id_token and stop validating ID tokens (#911) (#943)
* Deprecate decode_id_token and stop validating ID tokens (#911) MSAL should not perform any ID token validation. Per OpenID Connect, an ID token obtained via direct communication with the token endpoint (how MSAL retrieves tokens) does not need client-side validation, and MSAL does not manage sessions, so it should not check exp/iss/aud. - Add non-validating _decode_id_token_claims() and use it on the retrieval path (Client._obtain_token and TokenCache) so no iss/aud/exp/nbf checks run - Deprecate the public decode_id_token() function and Client.decode_id_token() method with a DeprecationWarning - Keep nonce and max_age/auth_time auth-code-flow replay protections - Update docstrings that claimed the SDK validates the ID token - Update tests accordingly Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Clarify deprecation wording and add stacklevel to warning (PR #943 review) - Add stacklevel=2 so the DeprecationWarning points at the caller's site - Reword warning and docstrings to avoid implying this legacy helper is non-validating; clarify that only token acquisition stopped validating while decode_id_token() still performs legacy validation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent d81a29a commit b299e3b

3 files changed

Lines changed: 75 additions & 18 deletions

File tree

‎msal/oauth2cli/oidc.py‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,47 @@ class IdTokenAudienceError(IdTokenError):
7474
class IdTokenNonceError(IdTokenError):
7575
pass
7676

77+
def _decode_id_token_claims(id_token):
78+
"""Decode an id_token and return its claims as a dictionary, WITHOUT validation.
79+
80+
MSAL does not validate the ID token. Per OpenID Connect, an ID token that is
81+
obtained via direct communication between the client and the token endpoint
82+
(which is how MSAL retrieves tokens) does not need to be validated by the
83+
client. Validation, if needed, is the responsibility of the application,
84+
which can perform it at the appropriate time (see the issue below).
85+
https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
86+
87+
ID token claims would at least contain: "iss", "sub", "aud", "exp", "iat",
88+
per `specs <https://openid.net/specs/openid-connect-core-1_0.html#IDToken>`_
89+
and it may contain other optional content such as "preferred_username",
90+
`maybe more <https://openid.net/specs/openid-connect-core-1_0.html#Claims>`_
91+
"""
92+
return json.loads(decode_part(id_token.split('.')[1]))
93+
94+
7795
def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None):
7896
"""Decodes and validates an id_token and returns its claims as a dictionary.
7997
98+
.. deprecated:: 1.38.0
99+
MSAL's token acquisition no longer validates the ID token, because the
100+
SDK should not perform any ID token validation
101+
(`issue #911 <https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911>`_).
102+
This standalone helper is kept only for backward compatibility and it
103+
still performs the legacy validations described below. Prefer the
104+
``id_token_claims`` that MSAL already returns alongside each token, or
105+
decode the token yourself, and perform any validation your app requires.
106+
80107
ID token claims would at least contain: "iss", "sub", "aud", "exp", "iat",
81108
per `specs <https://openid.net/specs/openid-connect-core-1_0.html#IDToken>`_
82109
and it may contain other optional content such as "preferred_username",
83110
`maybe more <https://openid.net/specs/openid-connect-core-1_0.html#Claims>`_
84111
"""
112+
warnings.warn(
113+
"decode_id_token() is deprecated. MSAL's token acquisition no longer "
114+
"validates the ID token; validation is the application's "
115+
"responsibility. This legacy helper still performs some validation. "
116+
"Prefer the id_token_claims returned alongside the token.",
117+
DeprecationWarning, stacklevel=2)
85118
decoded = json.loads(decode_part(id_token.split('.')[1]))
86119
# Based on https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
87120
_now = int(now or time.time())
@@ -157,18 +190,27 @@ class Client(oauth2.Client):
157190
"""
158191

159192
def decode_id_token(self, id_token, nonce=None):
160-
"""See :func:`~decode_id_token`."""
193+
"""See :func:`~decode_id_token`.
194+
195+
.. deprecated:: 1.38.0
196+
MSAL's token acquisition no longer validates the ID token. This
197+
method is kept for backward compatibility and still performs the
198+
legacy validations. See :func:`~decode_id_token`.
199+
"""
161200
return decode_id_token(
162201
id_token, nonce=nonce,
163202
client_id=self.client_id, issuer=self.configuration.get("issuer"))
164203

165204
def _obtain_token(self, grant_type, *args, **kwargs):
166205
"""The result will also contain one more key "id_token_claims",
167-
whose value will be a dictionary returned by :func:`~decode_id_token`.
206+
whose value is a dictionary of the (non-validated) ID token claims.
168207
"""
169208
ret = super(Client, self)._obtain_token(grant_type, *args, **kwargs)
170209
if "id_token" in ret:
171-
ret["id_token_claims"] = self.decode_id_token(ret["id_token"])
210+
# MSAL does not validate the ID token. It only decodes the claims,
211+
# so that downstream components can build accounts, etc.
212+
# https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
213+
ret["id_token_claims"] = _decode_id_token_claims(ret["id_token"])
172214
return ret
173215

174216
def build_auth_request_uri(self, response_type, nonce=None, **kwargs):

‎msal/token_cache.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import warnings
88

99
from .authority import canonicalize
10-
from .oauth2cli.oidc import decode_part, decode_id_token
10+
from .oauth2cli.oidc import decode_part, _decode_id_token_claims
1111
from .oauth2cli.oauth2 import Client
1212

1313

@@ -423,8 +423,9 @@ def __add(self, event, now=None):
423423
refresh_token = response.get("refresh_token")
424424
id_token = response.get("id_token")
425425
id_token_claims = response.get("id_token_claims") or ( # Prefer the claims from broker
426-
# Only use decode_id_token() when necessary, it contains time-sensitive validation
427-
decode_id_token(id_token, client_id=event["client_id"]) if id_token else {})
426+
# MSAL does not validate the ID token; it only decodes the claims.
427+
# https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
428+
_decode_id_token_claims(id_token) if id_token else {})
428429
client_info, home_account_id = self.__parse_account(response, id_token_claims)
429430

430431
target = ' '.join(sorted(event.get("scope") or [])) # Schema should have required sorting

‎tests/test_oidc.py‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,30 @@ def test_oidc_nonce_is_url_safe_and_unpredictable(self):
8080
class TestIdToken(unittest.TestCase):
8181
EXPIRED_ID_TOKEN = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJpc3N1ZXIiLCJpYXQiOjE3MDY1NzA3MzIsImV4cCI6MTY3NDk0ODMzMiwiYXVkIjoiZm9vIiwic3ViIjoic3ViamVjdCJ9.wyWNFxnE35SMP6FpxnWZmWQAy4KD0No_Q1rUy5bNnLs"
8282

83-
def test_id_token_should_tolerate_time_error(self):
84-
self.assertEqual(oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN), {
85-
"iss": "issuer",
86-
"iat": 1706570732,
87-
"exp": 1674948332, # 2023-1-28
88-
"aud": "foo",
89-
"sub": "subject",
90-
}, "id_token is decoded correctly, without raising exception")
91-
92-
def test_id_token_should_error_out_on_client_id_error(self):
93-
with self.assertRaises(msal.IdTokenError):
94-
oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN, client_id="not foo")
83+
_EXPECTED_CLAIMS = {
84+
"iss": "issuer",
85+
"iat": 1706570732,
86+
"exp": 1674948332, # 2023-1-28
87+
"aud": "foo",
88+
"sub": "subject",
89+
}
90+
91+
def test_decode_id_token_is_deprecated_but_still_tolerates_time_error(self):
92+
with self.assertWarns(DeprecationWarning):
93+
claims = oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN)
94+
self.assertEqual(claims, self._EXPECTED_CLAIMS,
95+
"id_token is decoded correctly, without raising exception")
96+
97+
def test_deprecated_decode_id_token_should_error_out_on_client_id_error(self):
98+
with self.assertWarns(DeprecationWarning):
99+
with self.assertRaises(msal.IdTokenError):
100+
oauth2cli.oidc.decode_id_token(
101+
self.EXPIRED_ID_TOKEN, client_id="not foo")
102+
103+
def test_internal_decoder_does_not_validate_the_id_token(self):
104+
# https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
105+
# The SDK must not perform any ID token validation on retrieval.
106+
# This expired token, decoded via the internal helper, must not raise.
107+
claims = oauth2cli.oidc._decode_id_token_claims(self.EXPIRED_ID_TOKEN)
108+
self.assertEqual(claims, self._EXPECTED_CLAIMS)
95109

0 commit comments

Comments
 (0)