Fix Entra ID tenant parsing for multi-segment STSURL authorities

[cheenamalhotra]

Description

Connecting to the Dataverse / Dynamics 365 TDS endpoint with Authentication=Active Directory Service Principal fails on 7.0.0+ with ClientSecretCredential authentication failed.

Dataverse returns an OAuth v1 style STSURL in the FEDAUTHINFO TDS token:

https://login.microsoftonline.com/{tenantId}/oauth2/authorize

ActiveDirectoryAuthenticationProvider.AcquireTokenAsync split the authority at the last /, so ClientSecretCredential received the literal string "authorize" as its tenant id, and AuthorityHost became https://login.microsoftonline.com/{tenantId}/oauth2/. Azure SQL and Fabric return the bare https://login.microsoftonline.com/{tenantId} form, so only multi-segment STSURLs were affected. The same split existed in 6.x, where the older Azure.Identity / MSAL combination happened to tolerate the malformed authority.

Approach

The tenant is now taken from the first path segment of the authority URL rather than the last, so trailing endpoint suffixes (/oauth2/authorize, /oauth2/v2.0/token, ...) are ignored:

• New TryParseAuthority helper splits the STSURL into an authority host (https://login.microsoftonline.com/), a tenant, and a normalized MSAL authority (host + tenant).
• The normalized authority is now what gets passed to MSAL's PublicClientAppKey / WithAuthority, so the interactive, password, integrated, and device-code flows are fixed too, not just the Azure.Identity based ones.
• The old last-separator split has been removed rather than kept as a fallback. Entra ID authorities are always absolute HTTPS URLs, and both MSAL's WithAuthority and Azure.Identity's AuthorityHost (new Uri(...)) require an absolute URI, so the fallback could never have produced a working credential. An unparseable authority now raises a clear AuthenticationException naming the offending value and the expected shape instead of failing obscurely deeper in the stack.
• Added a catch (AuthenticationException) { throw; } guard in AcquireTokenAsync so provider-raised authentication errors are no longer re-wrapped by the generic catch-all as "Unexpected error". This also improves the pre-existing "authentication method not supported" path.

Backwards compatibility

No public API changes. Behavior is unchanged for the bare https://login.microsoftonline.com/{tenantId} authority that Azure SQL, Fabric, and Synapse send. The only behavior difference for existing working scenarios is the improved error message when an authority is malformed.

Issues

Fixes #4496

Testing

Added AuthorityParsingTests in src/Microsoft.Data.SqlClient.Extensions/Azure/test/, covering only authority shapes that Entra ID actually documents:

• Bare tenant authority (Azure SQL / Fabric), with and without a trailing slash
• v1.0 /oauth2/authorize endpoint (the Dataverse repro)
• v2.0 /oauth2/v2.0/token endpoint
• National clouds: login.microsoftonline.us and login.partner.microsoftonline.cn
• Domain-name tenant and the common / organizations placeholders
• Rejection cases: an authority with no tenant segment, and an empty STSURL

Results: 12/12 new tests pass; 34 passed / 1 skipped across the non-integration Azure test suite on net9.0. The remaining AADConnectionTest failure in the full run is a pre-existing integration test that requires a live Azure SQL server and network access.

Manual verification against a real Dataverse TDS endpoint has not been performed and would be valuable before merge, since that environment is not available in this workspace.

Guidelines

Please review the contribution guidelines before submitting a pull request:

• Contributing
• Code of Conduct
• Best Practices
• Coding Style
• Review Process

[Copilot]

Pull request overview

This PR fixes Entra ID authority parsing when SQL Server (notably Dataverse/Dynamics 365 TDS) returns multi-segment STSURL values (e.g., /oauth2/authorize) in the FEDAUTHINFO token, ensuring the tenant is parsed correctly and the MSAL authority is normalized.

Changes:

• Add TryParseAuthority to split an absolute HTTPS STSURL into authorityHost, tenant, and a normalized msalAuthority (host + tenant).
• Use normalized msalAuthority for MSAL-based flows (via PublicClientAppKey / WithAuthority) and fail fast with a clearer AuthenticationException when the STSURL is malformed.
• Add unit tests covering documented authority URL shapes, including v1/v2 endpoints and national cloud hosts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs	Normalizes STSURL parsing (first path segment tenant) and ensures MSAL/Azure.Identity receive correct authority/tenant; improves exception pass-through.
src/Microsoft.Data.SqlClient.Extensions/Azure/test/AuthorityParsingTests.cs	Adds test coverage for supported STSURL shapes and rejection cases for missing tenant/empty authority.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[edwardneal]

The test below connects to a Power Apps Developer Dataverse instance; this test fails on main and passes with this PR.

[Fact]
public void ActiveDirectoryDefaultAuthenticationToDataverse_Succeeds()
{
    const string ConnectionString = "Data Source=<org name>.crm11.dynamics.com;User ID=<user id>;Encrypt=True;TrustServerCertificate=True;Authentication=ActiveDirectoryDefault;";

    using SqlConnection conn = new(ConnectionString);
    conn.Open();

    using SqlCommand cmd = new("select * from sys.databases", conn);
    using SqlDataReader rd = cmd.ExecuteReader();

    while (rd.Read())
    { }
}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs:268

• Typo in comment: "DefaultAzureCredenial" should be "DefaultAzureCredential" to match the type name and avoid confusion during future maintenance/searching.

                // Cache DefaultAzureCredenial based on scope, authority host, tenant, and clientId

[benrr101]

Overall, looks good - I'd just prefer it if we use the Uri class to handle Uri manipulation rather than direct string manipulation.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/Microsoft.Data.SqlClient.Extensions/Azure/test/AuthorityParsingTests.cs:114

• This new test method doesn’t include an XML
  comment. Adding a brief summary (consistent with the rest of the Azure test suite) helps communicate why these rejection cases matter and makes failures easier to interpret.

    [Theory]
    // A tenant is required; an authority without one cannot yield a usable credential.
    [InlineData("https://login.microsoftonline.com")]
    [InlineData("https://login.microsoftonline.com/")]
    // An empty leading path segment leaves no tenant to authenticate against.

src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs:244

• The comment claims “We always have a tenant here, because the server supplies one in the STSURL.” but the provider explicitly supports (and the new tests cover) cases where the server may omit STSURL or provide an authority without a tenant segment. This comment is therefore misleading and should be updated to reflect the actual behavior (fail fast with an AuthenticationException when no tenant is present).

            // If no tenant is specified, the app targets Entra ID and personal
            // Microsoft accounts as an audience.  (That is, it behaves as though
            // `common` were specified.)  We always have a tenant here, because the
            // server supplies one in the STSURL.

src/Microsoft.Data.SqlClient.Extensions/Azure/test/AuthorityParsingTests.cs:93

• This new test method doesn’t include an XML
  comment. Other tests in this project consistently document test intent with XML docs, and adding a summary here keeps the test suite consistent and easier to maintain.

This issue also appears on line 110 of the same file.

    [Theory]
    [MemberData(nameof(AuthorityData))]
    public void TryParseAuthority_SplitsHostAndTenant(

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.24%. Comparing base (9b20e5e) to head (f0759f2).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4521      +/-   ##
==========================================
- Coverage   64.73%   63.24%   -1.49%     
==========================================
  Files         288      283       -5     
  Lines       44088    67609   +23521     
==========================================
+ Hits        28542    42762   +14220     
- Misses      15546    24847    +9301     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	63.24% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[priyankatiwari08]

Fix looks correct and the parsing is well covered. A few points before merge.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs:581

• TryParseAuthority’s remarks currently say an “unparseable authority cannot produce a working credential” because MSAL/Azure.Identity require an absolute URI. However, the failure case here is often “missing tenant segment” (even if the URI is absolute). Consider rewording the remarks to state that the provider requires an absolute HTTPS URL and a tenant segment so it can normalize the authority for downstream libraries.

    /// Entra ID authorities are always absolute HTTPS URLs, so anything else is rejected rather
    /// than guessed at. Both MSAL (<c>WithAuthority</c>) and Azure.Identity
    /// (<c>TokenCredentialOptions.AuthorityHost</c>) require an absolute URI as well, so an
    /// unparseable authority cannot produce a working credential.

src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs:244

• The comment says we “always have a tenant here”, but the code now explicitly throws when the STSURL has no tenant segment (TryParseAuthority returns false). Please update this comment to match the actual behavior (tenant is required; missing tenant becomes a clear AuthenticationException).

This issue also appears on line 578 of the same file.

            // If no tenant is specified, the app targets Entra ID and personal
            // Microsoft accounts as an audience.  (That is, it behaves as though
            // `common` were specified.)  We always have a tenant here, because the
            // server supplies one in the STSURL.

[priyankatiwari08]

LGTM. Approving