Add comprehensive authority configuration and precedence documentation#3617
Add comprehensive authority configuration and precedence documentation#3617
Conversation
… CIAM, and B2C (#3614) * Initial plan * Implement authority-only configuration support for AAD, CIAM, and B2C - Move ParseAuthorityIfNecessary to start of UpdateConfidentialClientApplicationOptionsFromMergedOptions - Add defensive fallback parse in PrepareAuthorityInstanceForMsal - Add TODO comment for future warning logging (issue #3611) - Create new unit test file MergedOptionsAuthorityParsingTests.cs with 7 tests - Modify E2E test to use Authority-only configuration - Add new B2C authority-only E2E test All unit tests pass successfully Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> * Address code review feedback - Add explicit using statement for Microsoft.Identity.Web in test file - Improve defensive fallback to also check TenantId to avoid redundant parsing All tests still pass Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> * Restore original test and create new Authority-only test - Restore AcquireToken_WithMicrosoftIdentityApplicationOptions_ClientCredentialsAsync with Instance/TenantId - Create new AcquireToken_WithMicrosoftIdentityApplicationOptions_Authority_ClientCredentialsAsync for Authority-only scenario Both tests now exist to demonstrate both recommended (Instance/TenantId) and Authority-only approaches Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com>
…d conflicts (#3615) * Initial plan * Add authority conflict warning logging infrastructure Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> * Add comprehensive unit tests for authority conflict warnings Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> * Wire up logger factory to MergedOptionsStore for runtime logging Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> * Fix MergedOptionsStore constructor signature for DI compatibility Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> * Rename MsalLogger alias to IdWebLogger for clarity Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com> Co-authored-by: Jean-Marc Prieur <jmprieur@microsoft.com>
- Created docs/authority-configuration.md with decision tree, precedence table, and configuration patterns - Created docs/b2c-authority-examples.md with B2C-specific patterns and /tfp/ normalization details - Created docs/ciam-authority-examples.md with CIAM scenarios and PreserveAuthority guidance - Created docs/migration-authority-vs-instance.md with upgrade paths and migration examples - Created docs/faq-authority-precedence.md with Q&A addressing common misconfigurations - Updated README.md with Authority Configuration section linking to new guides - Updated changelog.md with documentation entry for issue #3613 Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com>
docs/authority-configuration.md
Outdated
| | ❌ | ✅ | ❌ | any | Instance used, tenant resolved at runtime | No* | | ||
| | ❌ | ❌ | ✅ | any | Invalid configuration | Error | | ||
|
|
||
| \* For single-tenant apps, this may cause issues. Always specify TenantId when using Instance. |
There was a problem hiding this comment.
| \* For single-tenant apps, this may cause issues. Always specify TenantId when using Instance. | |
| For single-tenant apps, this may cause issues. Always specify TenantId when using Instance. |
There was a problem hiding this comment.
Updated the footnote text as suggested in commit 4491701.
- Removed all references to PreserveAuthority as it's an internal property not accessible to developers - Added link to sovereign cloud instances documentation in Instance definition - Improved footnote text for single-tenant apps per review feedback - Updated Mermaid diagram to remove PreserveAuthority decision point - Updated all CIAM examples to show Authority-only configuration without PreserveAuthority - Clarified that the library automatically handles CIAM authorities correctly Co-authored-by: jmprieur <13203188+jmprieur@users.noreply.github.com>
Removed all references to PreserveAuthority in commit 4491701. You're correct - it's an internal MergedOptions property not accessible to developers. Updated all documentation to clarify that the library automatically handles CIAM authorities correctly without requiring any special configuration from developers. |
bgavrilMS
left a comment
There was a problem hiding this comment.
Let's discuss about deprecating Authority altoghter instead?
bgavrilMS
left a comment
There was a problem hiding this comment.
I think we should not advise ppl to use Authority for AAD authorities, because they risk disabling defenses like authority validation and other AAD specific logic.
|
@trwalke - you worked on CIAM before. Can you please take over this PR from Jean-Marc and get it merged? |
|
@jmprieur - is this ready to be merged? It is marked as draft. |
yes, you had blocked it ... |
|
@copilot - update this PR by rebasing it to master |
I can't rebase this PR because the environment doesn't support force-push. The merge also reveals conflicts in source files ( |
|
@copilot - resolve conflicts |
- Updated all source code files to match master (commits d4c1360 and 64139f1 were already merged to master in improved form) - Synced LoggingEventId.cs: AuthorityConflict (EventId 408) -> AuthorityIgnored (EventId 500) - Synced MergedOptionsLogging.cs: use LoggingEventId.AuthorityIgnored - Synced MergedOptionsStore.cs: ILoggerFactory -> IServiceProvider constructor - Synced MergedOptions.cs: fix trailing whitespace; pass logger to ParseAuthorityIfNecessary - Synced InternalAPI.Shipped.txt for all TFMs: APIs now in Shipped.txt - Synced InternalAPI.Unshipped.txt for all TFMs: empty (APIs are shipped) - Synced MergedOptionsAuthorityParsingTests.cs: use id4slab1 test tenant name - Added all new files from master (MtlsPop, new test files, skills, docs) - Merged changelog.md: preserved our authority doc entry, added master's 4.4.0 entries - Preserved README.md with our authority configuration section - Preserved all documentation additions (docs/*.md files) Co-authored-by: bgavrilMS <12273384+bgavrilMS@users.noreply.github.com>
Resolved conflicts and synced with master in commit 0927e20. The branch now includes all master changes (4.4.0 release, new files) while preserving the documentation additions from this PR. Key conflict resolutions:
|
Authority Precedence Documentation Implementation
This PR implements comprehensive documentation for authority configuration and precedence in Microsoft.Identity.Web (Issue #3613), now synced with master.
Documentation Changes (this PR's unique contribution)
Sync with master
Original prompt
Implement Issue #3613: Add comprehensive documentation clarifying precedence between Authority, Instance, TenantId, Domain, PreserveAuthority, and policy settings (B2C, CIAM), following fixes (#3612) and diagnostics (#3611).
Scope
Create new documentation assets and update existing docs to help developers configure authentication correctly and interpret the new warning log message. Branch: jmprieur/docs-authority-precedence off jmprieur/fix-authority-parsing.
Goals
Planned Files / Changes
Content Outline (authority-configuration.md)
Title: Authority Configuration & Precedence in Microsoft.Identity.Web
Sections:
Mermaid Diagram (planned in authority-configuration.md)
flowchart TD A[Config Provided] --> B{Instance & TenantId set?} B -- Yes --> C[Use Instance & TenantId; Ignore Authority] B -- No --> D{Authority Provided?} D -- No --> E[Configuration invalid / requires Instance+TenantId] D -- Yes --> F{PreserveAuthority True?} F -- Yes --> G[Use full Authority as Instance; TenantId=null] F -- No --> H[Parse Authority -> Instance + TenantId] H --> I{Is B2C?} I -- Yes --> J[Normalize /tfp/; Derive PreparedInstance] I -- No --> K[Derive PreparedInstance] C --> K G --> K K --> L[Pass PreparedInstance to MSAL]FAQ (faq-authority-precedence.md) sample entries:
Q: Why do I see a warning "Both Authority and Instance/TenantId are set"?
A: You configured Authority along with Instance or TenantId. Instance/TenantId take precedence; remove Authority to suppress the warning.
Q: Should I use Authority or Instance/TenantId for B2C?
A: Prefer Authority including the policy path (e.g., https://///v2.0). Avoid mixing Instance/TenantId.
Q: How do I configure CIAM?
A: Use the CIAM Authority domain, set PreserveAuthority=true implicitly (by library defaults), do not specify TenantId.
Migration Doc (migration-authority-vs-instance.md)
CHANGELOG.md
Add under Unreleased:
Acceptance Criteria
Non-Goals
Risks / Mitigation
This pull request was created as a result of the following prompt from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.