Skip to content

Commit 09e8fb5

Browse files
Merge resync'd #938 (origin/dev) into FIC branch #939
Bring the FIC branch current with base dev by merging the resync'd SNI branch (rginsburg/sni-mtls-pop @ 4ec592f, which merged origin/dev), so #939 also carries PRs #911, #937, #940, #942. Single conflict in msal/application.py acquire_token_for_client(), resolved by integrating BOTH sides: keep the FIC-aware mTLS condition (mtls_proof_of_possession or self._mtls_is_fic_leg2) and its comment from this branch, AND add dev's forwarded_client_claims request-setup block above it. The private _acquire_token_for_client() auto-merged correctly, combining the FIC/mtls client selection (is_mtls_pop or self._mtls_is_fic_leg2) with dev's client_claims -> OAuth claims merge. The FIC e2e test test_fic_two_leg_over_mtls_pop and its leg-1 skip-hatch are left UNTOUCHED (de-hatch + resource-call port + Credential_Fic_Output_* naming remain Task 2). Region split, x509 pop/bearer matrix, and downgrade fail-closed are all preserved. Verified: py_compile clean; e2e matrix (pop/bearer) + FIC test collect; 22 mtls_pop + forwarded_client_claims unit tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2 parents 3ce5d96 + 4ec592f commit 09e8fb5

11 files changed

Lines changed: 943 additions & 107 deletions

File tree

.Pipelines/CI-AND-RELEASE-PIPELINES.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ including what each pipeline does, when it runs, and how to trigger a release.
2626
| Push / merge | `dev` |
2727
| Scheduled | Daily at 11:45 PM Pacific, `dev` branch (only when there are new changes) |
2828

29-
Fast unit-test feedback for PRs targeting **other** branches (e.g. `release-x.y.z`)
30-
is provided separately by the GitHub Actions workflow
29+
Package build verification for PRs targeting any branch is provided separately
30+
by the GitHub Actions workflow
3131
[`.github/workflows/python-package.yml`](../.github/workflows/python-package.yml),
32-
which runs the package build and unit tests on every PR.
32+
which builds the package and runs `twine check` on every PR.
3333

3434
### Stages
3535

Lines changed: 7 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
# Build verification + unit tests for the msal Python package.
1+
# Build verification for the msal Python package.
22
#
33
# This workflow runs on every PR (against any target branch) to give contributors
4-
# fast feedback that the package still builds and that the unit tests pass across
5-
# all supported Python versions.
4+
# fast feedback that the package still builds and that the distribution metadata
5+
# passes `twine check`.
66
#
7-
# Post-merge validation on dev, E2E tests, benchmarks, SDL scans, and PyPI
8-
# publishing are NOT run here. Those run in the ADO pipelines:
9-
# - azure-pipelines.yml (PRs + pushes to dev: unit + E2E + SDL)
7+
# Unit tests, E2E tests, benchmarks, SDL scans, and PyPI publishing are NOT run
8+
# here. Those run in the IDDP/ADO pipelines:
9+
# - azure-pipelines.yml (PRs + pushes: unit + E2E + benchmarks + SDL)
1010
# - .Pipelines/pipeline-publish.yml (manual release to TestPyPI / PyPI)
1111

12-
name: Build and Unit Tests
12+
name: Build
1313

1414
on:
1515
pull_request:
@@ -43,39 +43,3 @@ jobs:
4343
name: dist
4444
path: dist/
4545
retention-days: 7
46-
47-
ci:
48-
name: Unit tests Python ${{ matrix.python-version }}
49-
permissions:
50-
contents: read
51-
52-
runs-on: ubuntu-22.04
53-
strategy:
54-
fail-fast: false
55-
matrix:
56-
python-version: ['3.9', '3.10', '3.11', '3.12', '3.13', '3.14']
57-
58-
steps:
59-
- uses: actions/checkout@v4
60-
- name: Set up Python ${{ matrix.python-version }}
61-
uses: actions/setup-python@v5
62-
# It automatically takes care of pip cache, according to
63-
# https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#about-caching-workflow-dependencies
64-
with:
65-
python-version: ${{ matrix.python-version }}
66-
cache: 'pip'
67-
68-
- name: Install dependencies
69-
run: |
70-
python -m pip install --upgrade pip
71-
python -m pip install pytest
72-
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
73-
74-
- name: Run unit tests
75-
# Skip benchmarks and E2E tests — those require lab credentials and run in ADO.
76-
run: |
77-
pytest tests/ \
78-
--benchmark-skip \
79-
--ignore=tests/test_e2e.py \
80-
--ignore=tests/test_e2e_manual.py \
81-
--ignore=tests/test_fmi_e2e.py

doc/python_version_support_policy.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ kept in sync with this policy:
7474
1. **`setup.cfg`**`python_requires`, the `Programming Language ::
7575
Python :: 3.x` trove classifiers, and any `python_version`
7676
environment markers on optional dependencies (e.g. `pymsalruntime`).
77-
2. **`.github/workflows/python-package.yml`** — the `python-version`
78-
matrix used by the `pytest` test job.
77+
2. **`azure-pipelines.yml`** and **`.Pipelines/template-pipeline-stages.yml`**
78+
— the `python-version` matrix used by the PR-gate unit and E2E test stages.
7979
3. **`tests/test_cryptography.py`** — the N+3 ceiling test that enforces
8080
tracking the latest `cryptography` release. Newer `cryptography`
8181
versions routinely drop EOL Python versions, which is the most common

msal/application.py

Lines changed: 154 additions & 20 deletions
Large diffs are not rendered by default.

msal/oauth2cli/oauth2.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,14 @@ def _obtain_token( # The verb "obtain" is influenced by OAUTH2 RFC 6749
254254
_data.update(data or {}) # So the content in data param prevails
255255
_data = {k: v for k, v in _data.items() if v} # Clean up None values
256256

257+
# "client_claims" is a cache-key-only pseudo-parameter: callers merge its
258+
# value into the standard "claims" body parameter upstream, and it is kept
259+
# in the request data solely so it contributes to the extended cache key.
260+
# It must not be sent on the wire. Popping it here (from this method's own
261+
# local copy) keeps the wire body clean while the caller's data dict — used
262+
# for the cache-add event — still carries it.
263+
_data.pop("client_claims", None)
264+
257265
if _data.get('scope'):
258266
_data['scope'] = self._stringify(_data['scope'])
259267

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: 81 additions & 6 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

@@ -92,10 +92,22 @@ def _compute_ext_cache_key(data):
9292
This ensures tokens acquired with different parameter values
9393
(e.g., different FMI paths) are cached separately.
9494
95+
The hash may also intentionally include cache-key-only pseudo-parameters
96+
such as ``client_claims`` -- these are stripped from the wire body by the
97+
oauth2 layer but are retained in *data* precisely so that different
98+
client-originated claims route to separate cache entries.
99+
95100
Returns an empty string when *data* has no hashable fields.
96101
97-
The algorithm matches the Go MSAL implementation (CacheExtKeyGenerator):
98-
sorted key+value pairs are concatenated and SHA256 hashed, then base64url encoded.
102+
The algorithm matches MSAL .NET's ``ComputeAccessTokenExtCacheKey``: sorted
103+
key+value pairs are concatenated (no separators) and SHA256 hashed, then
104+
base64url encoded. This keeps the hash byte-identical to MSAL .NET.
105+
106+
MSAL Go's ``CacheExtKeyGenerator`` has since switched to a length-prefixed
107+
encoding (AzureAD/microsoft-authentication-library-for-go#629) to make it
108+
injective; Python deliberately tracks .NET instead, so these hashes are not
109+
byte-identical to current Go. Caches are not shared across languages, so the
110+
difference does not affect runtime correctness.
99111
"""
100112
if not data:
101113
return ""
@@ -105,14 +117,71 @@ def _compute_ext_cache_key(data):
105117
}
106118
if not cache_components:
107119
return ""
108-
# Sort keys for consistent hashing (matches Go implementation)
120+
# Sort keys, then concatenate key+value pairs with no separators. This
121+
# matches MSAL .NET's ComputeAccessTokenExtCacheKey byte-for-byte. (See the
122+
# docstring re: the Go #629 length-prefixed divergence.)
109123
key_str = "".join(
110124
k + cache_components[k] for k in sorted(cache_components.keys())
111125
)
112126
hash_bytes = hashlib.sha256(key_str.encode("utf-8")).digest()
113127
return base64.urlsafe_b64encode(hash_bytes).rstrip(b"=").decode("ascii").lower()
114128

115129

130+
def _parse_claims_or_raise(claims):
131+
"""Parse a claims JSON string into a dict, or raise a friendly ``ValueError``.
132+
133+
The raw claims value is never included in the error message because it may
134+
contain sensitive data. Mirrors MSAL .NET's ``ClaimsHelper.ParseClaimsOrThrow``.
135+
"""
136+
try:
137+
parsed = json.loads(claims)
138+
except (ValueError, TypeError) as ex:
139+
# json.JSONDecodeError (malformed JSON) is a subclass of ValueError;
140+
# TypeError is raised when *claims* is not a str/bytes/bytearray. Both
141+
# are surfaced as the same friendly ValueError so every caller behaves
142+
# consistently regardless of the bad input's type.
143+
raise ValueError(
144+
"The claims value is not valid JSON. "
145+
"See https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter."
146+
) from ex
147+
if not isinstance(parsed, dict):
148+
# A valid JSON array, scalar, or the literal "null" is not a claims object.
149+
raise ValueError(
150+
"The claims value is not a valid JSON object. "
151+
"See https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter.")
152+
return parsed
153+
154+
155+
def _deep_merge_dict(base, overlay):
156+
"""Recursively merge ``overlay`` into ``base``, returning a new dict.
157+
158+
Nested dicts are merged; for any other value type, ``overlay`` wins.
159+
"""
160+
result = dict(base)
161+
for key, value in overlay.items():
162+
if (key in result
163+
and isinstance(result[key], dict) and isinstance(value, dict)):
164+
result[key] = _deep_merge_dict(result[key], value)
165+
else:
166+
result[key] = value
167+
return result
168+
169+
170+
def _merge_claims(claims_a, claims_b):
171+
"""Merge two claims JSON strings into a single JSON string.
172+
173+
If either side is empty/None, the other is returned as-is. Mirrors MSAL
174+
.NET's ``ClaimsHelper.MergeClaimsObjects``.
175+
"""
176+
if not claims_a:
177+
return claims_b
178+
if not claims_b:
179+
return claims_a
180+
merged = _deep_merge_dict(
181+
_parse_claims_or_raise(claims_a), _parse_claims_or_raise(claims_b))
182+
return json.dumps(merged)
183+
184+
116185
def is_subdict_of(small, big):
117186
return dict(big, **small) == big
118187

@@ -350,6 +419,11 @@ def make_clean_copy(dictionary, sensitive_fields): # Masks sensitive info
350419
data=make_clean_copy(event.get("data", {}), (
351420
"password", "client_secret", "refresh_token", "assertion",
352421
"user_federated_identity_credential",
422+
# Client-originated claims may carry sensitive values; they are
423+
# kept in data only for ext_cache_key computation, so redact them
424+
# from the debug log (both the cache-key pseudo-param and the
425+
# merged wire parameter).
426+
"client_claims", "claims",
353427
)),
354428
response=make_clean_copy(event.get("response", {}), (
355429
"id_token_claims", # Provided by broker
@@ -392,8 +466,9 @@ def __add(self, event, now=None):
392466
refresh_token = response.get("refresh_token")
393467
id_token = response.get("id_token")
394468
id_token_claims = response.get("id_token_claims") or ( # Prefer the claims from broker
395-
# Only use decode_id_token() when necessary, it contains time-sensitive validation
396-
decode_id_token(id_token, client_id=event["client_id"]) if id_token else {})
469+
# MSAL does not validate the ID token; it only decodes the claims.
470+
# https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/911
471+
_decode_id_token_claims(id_token) if id_token else {})
397472
client_info, home_account_id = self.__parse_account(response, id_token_claims)
398473

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

0 commit comments

Comments
 (0)