Fixed the Cross Cloud OIDC fix #916
Open
4gust wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens msal/authority.py around OIDC authority validation to prevent cross-sovereign-cloud misconfigurations by introducing a centralized cloud/host mapping, new host-to-cloud resolution helpers, stricter issuer-validation ordering, and an explicit same-cloud enforcement for OIDC discovery endpoints. It also adds extensive test coverage to validate the new issuer/cloud/endpoint behaviors and key regressions (including the referenced #5927 scenario).
Changes:
- Introduces cloud sentinel constants and host→cloud mappings, plus
_resolve_known_cloud(),_are_in_same_cloud(), and_ensure_endpoint_same_cloud_as_authority()to centralize and enforce cloud logic. - Refactors
Authority.has_valid_issuer()into ordered validation steps (exact match, same host, CIAM tenant pattern, same-cloud rules, regional-prefix rules, B2C subdomain rules). - Adds a large suite of tests covering same-cloud alias acceptance, cross-cloud rejection, regional host handling, CIAM tenant-pattern validation, and endpoint same-cloud enforcement.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
msal/authority.py |
Adds cloud mapping + helpers and tightens issuer/endpoint validation to block cross-cloud OIDC discovery data. |
tests/test_authority.py |
Adds comprehensive regression/unit tests for issuer validation and endpoint same-cloud enforcement across clouds/aliases/regions/CIAM. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+401
to
+405
| if issuer_host.endswith(_CIAM_DOMAIN_SUFFIX): | ||
| issuer_tenant = issuer_host[:-len(_CIAM_DOMAIN_SUFFIX)] | ||
| auth_path_parts = [p for p in authority_parsed.path.split("/") if p] | ||
| if auth_path_parts: | ||
| authority_tenant = auth_path_parts[0].lower() |
| authority_host = authority_parsed.hostname.lower() if authority_parsed.hostname else "" | ||
| if potential_base == authority_host: | ||
| return True | ||
| if (_REGION_PREFIX_PATTERN.match(prefix) |
Comment on lines
+440
to
+443
| if any( | ||
| issuer_host.endswith("." + h) | ||
| for h in WELL_KNOWN_B2C_HOSTS | ||
| if h != "ciamlogin.com"): |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request significantly refactors and strengthens the handling of Microsoft cloud authorities in
msal/authority.py. The main improvements are the introduction of a robust mechanism to determine and validate whether authority and endpoint hosts belong to the same sovereign cloud, stricter issuer validation logic, and better prevention of cross-cloud misconfigurations. These changes enhance security, maintainability, and clarity of authority host management.Cloud authority management and validation:
_HOSTS_BY_CLOUD,_KNOWN_HOST_TO_CLOUD, etc.) to centrally define and manage all known Microsoft cloud authority hosts, their aliases, and their mapping to logical cloud identifiers, with runtime checks for duplicates._resolve_known_cloudfunction to determine the logical cloud for a given host, including support for regional subdomains, and_are_in_same_cloudto check if two hosts belong to the same cloud.Endpoint and issuer security enhancements:
_ensure_endpoint_same_cloud_as_authorityto enforce that OIDC discovery endpoints (token, authorization, device endpoints) must reside in the same cloud as the authority, raising a clear error if not. This is now called during authority initialization for OIDC authorities.has_valid_issuerwith a clear, ordered set of validation steps, including exact match, same host/scheme, CIAM tenant pattern, same cloud checks, region-shaped prefix, and B2C subdomain handling, closely following MSAL.NET's security model.Miscellaneous:
These changes collectively harden the library against misconfiguration and cross-cloud authority/endpoint mismatches, and make future updates to cloud authority logic easier and safer.