diff --git a/msal/application.py b/msal/application.py index 084f9bf3..1e911915 100644 --- a/msal/application.py +++ b/msal/application.py @@ -960,8 +960,6 @@ def initiate_auth_code_flow( If the elapsed time is greater than this value, Microsoft identity platform will actively re-authenticate the End-User. - MSAL Python will also automatically validate the auth_time in ID token. - New in version 1.15. :param str response_mode: @@ -1204,8 +1202,9 @@ def acquire_token_by_authorization_code( :param nonce: If you provided a nonce when calling :func:`get_authorization_request_url`, - same nonce should also be provided here, so that we'll validate it. - An exception will be raised if the nonce in id token mismatches. + this parameter is ignored and only kept for backward compatibility. + Applications that require nonce validation need to validate the ID + token nonce themselves. :param claims_challenge: The claims_challenge parameter requests specific claims requested by the resource provider @@ -2162,8 +2161,6 @@ def acquire_token_interactive( If the elapsed time is greater than this value, Microsoft identity platform will actively re-authenticate the End-User. - MSAL Python will also automatically validate the auth_time in ID token. - New in version 1.15. :param int parent_window_handle: diff --git a/msal/oauth2cli/oidc.py b/msal/oauth2cli/oidc.py index faa2f966..1439f52c 100644 --- a/msal/oauth2cli/oidc.py +++ b/msal/oauth2cli/oidc.py @@ -75,61 +75,19 @@ class IdTokenNonceError(IdTokenError): pass 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. + """Decodes an id_token and returns its claims as a dictionary. ID token claims would at least contain: "iss", "sub", "aud", "exp", "iat", per `specs `_ and it may contain other optional content such as "preferred_username", `maybe more `_ + + The optional parameters ``client_id``, ``issuer``, ``nonce``, and ``now`` + are ignored and only kept for backward compatibility. + Callers that require claim validation are responsible for performing it + themselves. """ - 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()) - skew = 120 # 2 minutes - - if _now + skew < decoded.get("nbf", _now - 1): # nbf is optional per JWT specs - # This is not an ID token validation, but a JWT validation - # https://tools.ietf.org/html/rfc7519#section-4.1.5 - _IdTokenTimeError("0. The ID token is not yet valid.", _now, decoded).log() - - if issuer and issuer != decoded["iss"]: - # https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse - raise IdTokenIssuerError( - '2. The Issuer Identifier for the OpenID Provider, "%s", ' - "(which is typically obtained during Discovery), " - "MUST exactly match the value of the iss (issuer) Claim." % issuer, - _now, - decoded) - - if client_id: - valid_aud = client_id in decoded["aud"] if isinstance( - decoded["aud"], list) else client_id == decoded["aud"] - if not valid_aud: - raise IdTokenAudienceError( - "3. The aud (audience) claim must contain this client's client_id " - '"%s", case-sensitively. Was your client_id in wrong casing?' - # Some IdP accepts wrong casing request but issues right casing IDT - % client_id, - _now, - decoded) - - # Per specs: - # 6. If the ID Token is received via direct communication between - # the Client and the Token Endpoint (which it is during _obtain_token()), - # the TLS server validation MAY be used to validate the issuer - # in place of checking the token signature. - - if _now - skew > decoded["exp"]: - _IdTokenTimeError("9. The ID token already expires.", _now, decoded).log() - - if nonce and nonce != decoded.get("nonce"): - raise IdTokenNonceError( - "11. Nonce must be the same value " - "as the one that was sent in the Authentication Request.", - _now, - decoded) - - return decoded + return json.loads(decode_part(id_token.split('.')[1])) def _nonce_hash(nonce): @@ -158,9 +116,7 @@ class Client(oauth2.Client): def decode_id_token(self, id_token, nonce=None): """See :func:`~decode_id_token`.""" - return decode_id_token( - id_token, nonce=nonce, - client_id=self.client_id, issuer=self.configuration.get("issuer")) + return decode_id_token(id_token) def _obtain_token(self, grant_type, *args, **kwargs): """The result will also contain one more key "id_token_claims", @@ -193,20 +149,12 @@ def obtain_token_by_authorization_code(self, code, nonce=None, **kwargs): plus new parameter(s): :param nonce: - If you provided a nonce when calling :func:`build_auth_request_uri`, - same nonce should also be provided here, so that we'll validate it. - An exception will be raised if the nonce in id token mismatches. + Optional. Ignored and only kept for backward compatibility. """ warnings.warn( "Use obtain_token_by_auth_code_flow() instead", DeprecationWarning) - result = super(Client, self).obtain_token_by_authorization_code( + return super(Client, self).obtain_token_by_authorization_code( code, **kwargs) - nonce_in_id_token = result.get("id_token_claims", {}).get("nonce") - if "id_token_claims" in result and nonce and nonce != nonce_in_id_token: - raise ValueError( - 'The nonce in id token ("%s") should match your nonce ("%s")' % - (nonce_in_id_token, nonce)) - return result def initiate_auth_code_flow( self, @@ -249,42 +197,17 @@ def obtain_token_by_auth_code_flow(self, auth_code_flow, auth_response, **kwargs """Validate the auth_response being redirected back, and then obtain tokens, including ID token which can be used for user sign in. - Internally, it implements nonce to mitigate replay attack. - It also implements PKCE to mitigate the auth code interception attack. + This method still sends a nonce during flow initiation for protocol + compatibility, but the SDK no longer validates that nonce against the + ID token. + + It implements PKCE to mitigate the auth code interception attack. See :func:`oauth2.Client.obtain_token_by_auth_code_flow` in parent class for descriptions on other parameters and return value. """ - result = super(Client, self).obtain_token_by_auth_code_flow( + return super(Client, self).obtain_token_by_auth_code_flow( auth_code_flow, auth_response, **kwargs) - if "id_token_claims" in result: - nonce_in_id_token = result.get("id_token_claims", {}).get("nonce") - expected_hash = _nonce_hash(auth_code_flow["nonce"]) - if nonce_in_id_token != expected_hash: - raise RuntimeError( - 'The nonce in id token ("%s") should match our nonce ("%s")' % - (nonce_in_id_token, expected_hash)) - - if auth_code_flow.get("max_age") is not None: - auth_time = result.get("id_token_claims", {}).get("auth_time") - if not auth_time: - raise RuntimeError( - "13. max_age was requested, ID token should contain auth_time") - now = int(time.time()) - skew = 120 # 2 minutes. Hardcoded, for now - if now - skew > auth_time + auth_code_flow["max_age"]: - raise RuntimeError( - "13. auth_time ({auth_time}) was requested, " - "by using max_age ({max_age}) parameter, " - "and now ({now}) too much time has elasped " - "since last end-user authentication. " - "The ID token was: {id_token}".format( - auth_time=auth_time, - max_age=auth_code_flow["max_age"], - now=now, - id_token=json.dumps(result["id_token_claims"], indent=2), - )) - return result def obtain_token_by_browser( self, @@ -299,8 +222,11 @@ def obtain_token_by_browser( **kwargs): """A native app can use this method to obtain token via a local browser. - Internally, it implements nonce to mitigate replay attack. - It also implements PKCE to mitigate the auth code interception attack. + This flow still sends a nonce during flow initiation for protocol + compatibility, but the SDK no longer validates that nonce against the + ID token. + + It implements PKCE to mitigate the auth code interception attack. :param string display: Defined in `OIDC `_. @@ -334,4 +260,3 @@ def obtain_token_by_browser( return super(Client, self).obtain_token_by_browser( auth_params=dict(kwargs.pop("auth_params", {}), **filtered_params), **kwargs) - diff --git a/msal/token_cache.py b/msal/token_cache.py index d6e2a2b1..595d2fae 100644 --- a/msal/token_cache.py +++ b/msal/token_cache.py @@ -343,8 +343,7 @@ 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 {}) + decode_id_token(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 @@ -548,4 +547,3 @@ def serialize(self): with self._lock: self.has_state_changed = False return json.dumps(self._cache, indent=4) - diff --git a/tests/test_oidc.py b/tests/test_oidc.py index 44e4658b..00bb0cd3 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -1,8 +1,8 @@ import string +from unittest.mock import patch from tests import unittest -import msal from msal import oauth2cli from msal.oauth2cli.oauth2 import _generate_pkce_code_verifier @@ -89,7 +89,51 @@ def test_id_token_should_tolerate_time_error(self): "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") - + def test_id_token_should_ignore_validation_parameters(self): + self.assertEqual(oauth2cli.oidc.decode_id_token( + self.EXPIRED_ID_TOKEN, + client_id="not foo", + issuer="not issuer", + nonce="not nonce", + now=0, + ), { + "iss": "issuer", + "iat": 1706570732, + "exp": 1674948332, # 2023-01-28 + "aud": "foo", + "sub": "subject", + }) + + def test_obtain_token_by_authorization_code_should_not_validate_nonce(self): + client = oauth2cli.oidc.Client( + {"authorization_endpoint": "https://example.com/auth", + "token_endpoint": "https://example.com/token", + "issuer": "issuer"}, + client_id="foo") + result = {"id_token_claims": {"nonce": "unexpected"}} + with patch.object( + oauth2cli.oauth2.Client, "obtain_token_by_authorization_code", + return_value=result) as mocked: + self.assertEqual( + result, + client.obtain_token_by_authorization_code("code", nonce="expected")) + mocked.assert_called_once_with("code") + + def test_obtain_token_by_auth_code_flow_should_not_validate_id_token_claims(self): + client = oauth2cli.oidc.Client( + {"authorization_endpoint": "https://example.com/auth", + "token_endpoint": "https://example.com/token", + "issuer": "issuer"}, + client_id="foo") + result = {"id_token_claims": {"nonce": "unexpected"}} + with patch.object( + oauth2cli.oauth2.Client, "obtain_token_by_auth_code_flow", + return_value=result) as mocked: + self.assertEqual( + result, + client.obtain_token_by_auth_code_flow( + {"state": "s", "nonce": "expected", "max_age": 1}, + {"state": "s", "code": "code"})) + mocked.assert_called_once_with( + {"state": "s", "nonce": "expected", "max_age": 1}, + {"state": "s", "code": "code"})