-
Notifications
You must be signed in to change notification settings - Fork 214
5.1.0 Authority forced to port 443 #1627
Comments
@keystroke |
@jmprieur yes we have our own STS used in many tests, my team and many partner teams are failing to update to 5.1.0 because they can't all host their mock servers on port 443. We also build our own STS instance for some private cloud scenarios that are bit much to get into right now. Also for ADFS we disable authority validation as is required, but less concern in that scenario as most ADFS instances are hosted on port 443. In general, the authority URI should be used directly. Can you confirm the behavior is bugged? |
Thanks for the clarification, @keystroke. This makes sense now. Also is it an option for you to migrate to MSAL.NET ? (which should handle all what ADAL handles, except versions of ADFS prior to ADFS 2019) |
@keystroke Which version of ADAL are you trying to upgrade from? |
@keystroke has this ever worked with older versions of ADAL? From my research I see that we are not specifying any port in our code. |
@trwalke Just try it out: $ErrorActionPreference='Stop'
try
{
[System.Reflection.Assembly]::LoadFrom('D:\One\AzureUX\Katal\packages\Microsoft.IdentityModel.Clients.ActiveDirectory.5.1.0\lib\net45\Microsoft.IdentityModel.Clients.ActiveDirectory.dll')
$context = [Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContext]::new(
($authority = 'https://localhost:5185/adfs/'),
($validateAuthority = $false),
($tokenCache = [Microsoft.IdentityModel.Clients.ActiveDirectory.TokenCache]::new()))
$result = [Microsoft.IdentityModel.Clients.ActiveDirectory.AuthenticationContextIntegratedAuthExtensions]::AcquireTokenAsync(
$context,
($resource='test'),
($clientId='test'),
($userCredential=[Microsoft.IdentityModel.Clients.ActiveDirectory.UserPasswordCredential]::new('test', 'test')))
$result.GetAwaiter().GetResult()
}
catch
{
throw "$_`r`n$($_.Exception)"
} Make sure you have a fresh PowerShell process and the right version is loaded. You'll see below exception details:
As you can see, we specify the authority on port 5185, checking the $context object shows everything as expected: C:\WINDOWS\system32> $context
ExtendedLifeTimeEnabled : False
Authority : https://localhost:5185/adfs/
ValidateAuthority : False
TokenCache : Microsoft.IdentityModel.Clients.ActiveDirectory.TokenCache
CorrelationId : 00000000-0000-0000-0000-000000000000 But it seems port 443 is required somehow and you will get a socket exception unless something else is listening on port 443 then you'll get some other error. I checked with one of my partner teams and they indicated that updating beyond version 3.17.3 would induce the issue.
The behavior seems to persist through to latest version (5.1.0). I tested using version 2.28.3.860 which I had on my box, and that also correctly honors the port used in the authority value. Seems something changed after version 3.17.3 that modifies the behavior. |
Could this be the offending commit? ea8d369#diff-b8caab14db87a8ac7e70708851e39df7R109 It shows the code parsing the host from the provided authority value, and constructing a token endpoint that does not preserve the original port... this.TokenUri = "https://{host}/{tenant}/oauth2/token".Replace("{host}", entry.PreferredNetwork).Replace("{tenant}", tenant); |
Thanks for that @keystroke Is it possible for you to move to MSAL? We are not planning on making any more feature updates to ADAL and we would like our customers to use our new MSAL library to stay up to date with all of the latest security features/fixes. It has all of the same features as ADAL plus more. The one thing to be aware of is MSAL only supports ADFS 2019 and beyond. What version of ADFS are you using and is it possible to move to 2019? |
@trwalke It isn't "me" that needs to move to MSAL, it is all the clients that would call my STS. Azure PowerShell is one such client that uses ADAL, not MSAL. I myself am not using ADAL clients, but I have to support them, and this bug means my service cannot be hosted on a different port than 443. However, even if you were to release a fix right now, we would need that fix to get absorbed by at least Azure PowerShell and they would have to ship a fix as well, then we would have to require our customers to update to the latest versions of everything, all to enable our service hosting model. In the short-term, we are forced to share port 443 with some other services that have assumed ownership of port 443 and we will have some kind of resolution framework in place to route the request correctly. Please absorb this learning throughout the team, honor authority URIs and URIs everywhere for the ports they declare ;) |
I see, thanks for detailing the scenario @keystroke. I made a fix and verified that it solves the issue. Should be a hotfix out soon |
@trwalke thanks, appreciate the hard work and porting the fix to MSAL as well! |
@keystroke Included in 5.1.1 release |
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
Latest Version (5.1.0)
Which platform has the issue?
.NET 4.6.2
What authentication flow has the issue?
Other? - please describe;
Is this a new or existing app?
Existing test code.
Repro
Expected behavior
Calls should be made on specified port.
Actual behavior
Calls are always made on port 443.
Possible Solution
Don't reset to port 443.
Additional context/ Logs / Screenshots
Unnecessary.
The text was updated successfully, but these errors were encountered: