AzureAD/azure-activedirectory-library-for-dotnet

ADAL 5.4.2 Client package is dropping /adfs/ literal from the passed authority URI

sumantshiv opened this issue · 6 comments

MSAL is the recommended auth library for use with the Microsoft identity platform

No new features will be implemented on ADAL. The team's efforts are on improving MSAL, the next-gen auth library. MSAL's wiki contains a migration guide from ADAL.

Only regressions, high severity issues and security issues will be fixed on ADAL. Other issues are likely to have already been fixed in MSAL.

If you think that your issue falls into the above categories, please fill in the form below.

Which Version of ADAL are you using ?
Note that to get help, you need to run the latest preview or non-preview version
For MSAL, please log issues to https://github.com/AzureAD/microsoft-authentication-library-for-dotnet

ADAL 5.2.4

Which platform has the issue?
net452

What authentication flow has the issue?

  • Desktop / Mobile
    • Interactive
    • Integrated Windows Auth
    • Username Password
    • Device code flow (browserless)
  • Web App
    • Authorization code
    • OBO
  • Web API
    • OBO

Other? - please describe;

Is this a new or existing app?

Azure Key vault -
The app is in production, and I have upgraded to a new version of ADAL from 4.19.X/4.5.1 to 5.4.2

Azure Key vault VSTS Repo

var your = (code) => here;

Expected behavior
ADAL Client library should continue to goto the specified Authority URL rather than dropping literals from it.

Actual behavior
Azure Key Vault is looking to update the ADAL client library nuget primarily from 3.19.X/4.5.1 to 5.4.2. We need this updated version to support our upcoming features. However, while testing we hit an issue with respect to the Get Token API. In case the callback Authority has the literal /adfs/, the ADAL client library for some reason is dropping it.
Hence, in case the Authority URL is: https://AuthEndpointBase/adfs/TenantId, the ADAL client is dropping this literal and actually sending the request to https://AuthEndpointBase/TenantId.

Possible Solution

Additional context/ Logs / Screenshots
Add any other context about the problem here, such as logs and screebshots. Logging is described at https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/wiki/Logging-in-ADAL.Net. Don't post logs containing PII on GitHub!

From investigation from @sumantshiv
commit c0f5b4e has updated the logic around parsing the tenantId from the AuthorityURL. The ADAL library always formats the endpoint url as https://host/tenantID format.

If the authority URL is like: https://localhost/adfs/tenantID.

  • As per the previous parsing logic, the tenant ID was coming as “adfs”. Picking that up, the ADAL client was reaching out to the endpoint https://localhost/adfs. This was working fine.
  • In the new parsing logic (call it a fix), the tenant ID because “tenantID” and the ADAL client started to reach out for a server endpoint https://localhost/tenantID. This started to break.

@sumantshiv has cases where they need the server endpoint format as: https://localhost/adfs or https://localhost/graph.

The changed file is: https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/blob/c0f5b4edf166861db1f56d319877a212921ae055/adal/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Instance/Authenticator.cs

Tenant ID parsing is at Line number 108

cc: @trwalke @henrik-me @jennyf19 @bgavrilMS

Updated the typo in Version. It should be 5.2.4.

I'll take a look at this.

@jmprieur / @bgavrilMS this is the same thing discussed in #1653, not sure if you want to consolidate. The preference from keyvault team is that all the identity clients are updated to accommodate the spurious authority endpoint.

@sumantshiv @keystroke I have a private build you can try out if you're interested. just send me an email -> jeferrie@microsoft.com

Included in 5.2.7 release