Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions msal/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
117 changes: 21 additions & 96 deletions msal/oauth2cli/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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>`_

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):
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 <https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest>`_.
Expand Down Expand Up @@ -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)

4 changes: 1 addition & 3 deletions msal/token_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -548,4 +547,3 @@ def serialize(self):
with self._lock:
self.has_state_changed = False
return json.dumps(self._cache, indent=4)

54 changes: 49 additions & 5 deletions tests/test_oidc.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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"})
Loading