NullRefException if getting a 504 from Gateway

[henrik-me]

Which Version of ADAL are you using ?
Happens on v3 as well as v5.0.5

Which platform has the issue?
net45/netcore

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?
Existing apps.

Repro

  tbd (something)

Expected behavior
ADAL should return an AdalServiceException containing the details of the http error code.

Actual behavior
NullReferenceException

Possible Solution
Troubleshoot why after the retry that we end up throwing a null ref exception

Additional context/ Logs / Screenshots

[12:28:46.951] [ 17] [INFO ] Authenticate-ADAL [Acquiring token]: STS endpoint (HTTPS://LOGIN.WINDOWS.NET/[tenant_pii]), resource (https://graph.windows.net), userName ([email_pii]).
[12:28:46.965] [ 17] [INFO ] ADAL: 2019-06-05T19:28:46.9645831Z: 00000000-0000-0000-0000-000000000000 - AdalLoggerBase.cs: Clearing Cache :- 0 items to be removed
[12:28:46.965] [ 17] [INFO ] ADAL: 2019-06-05T19:28:46.9655856Z: 00000000-0000-0000-0000-000000000000 - AdalLoggerBase.cs: Successfully Cleared Cache
[12:28:46.976] [ 17] [INFO ] ADAL: 2019-06-05T19:28:46.9766192Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: ADAL PCL.Desktop with assembly version '5.0.5.0', file version '5.0.5.0' and informational version '5.0.5' is running...
[12:28:46.978] [ 17] [INFO ] ADAL: 2019-06-05T19:28:46.9786258Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: === Token Acquisition started: 
                CacheType: Microsoft.IdentityModel.Clients.ActiveDirectory.TokenCache (0 items)
                Authentication Target: User
                , Authority Host: login.windows.net
[12:29:03.141] [  5] [INFO ] ADAL: 2019-06-05T19:29:03.1412590Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: Response status code does not indicate success: 504 (GatewayTimeout).
[12:29:03.141] [  5] [INFO ] ADAL: 2019-06-05T19:29:03.1412590Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: Retrying one more time..
[12:29:20.200] [  7] [INFO ] ADAL: 2019-06-05T19:29:20.2008868Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: Response status code does not indicate success: 504 (GatewayTimeout).
[12:29:20.200] [  7] [INFO ] ADAL: 2019-06-05T19:29:20.2008868Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: Request retry failed.
[12:29:20.510] [  7] [ERROR] ADAL: 2019-06-05T19:29:20.5106200Z: 99f773d1-1ae5-4ef8-b294-e12071e72fe0 - AdalLoggerBase.cs: Exception type: System.NullReferenceException

   at Microsoft.Identity.Core.OAuth2.OAuthClient.<GetResponseAsync>d__18`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Core.OAuth2.OAuthClient.<GetResponseAsync>d__17`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.InstanceDiscovery.<DiscoverAsync>d__14.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.InstanceDiscovery.<GetMetadataEntryAsync>d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Instance.Authenticator.<UpdateFromTemplateAsync>d__51.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Flows.AcquireTokenHandlerBase.<PreRunAsync>d__67.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Flows.AcquireTokenUsernamePasswordHandler.<PreRunAsync>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Flows.AcquireTokenHandlerBase.<RunAsync>d__60.MoveNext() 

[bgavrilMS]

I can take a look at this. The underlying issue is the 504 though.

[bgavrilMS]

Normally for errors in the 500-600 range we retry once. Which seems to happen. Then instead of throwing an MsalServiceException we have a null ref, definitely a bug on our side.

Workaround - treat the NullReferenceException as an MsalServiceException. I will have a look at this in more detail, it should not be too dificult to come up with a fix, but it will take us a day or 2 to release.

[bgavrilMS]

I can't repro this. I even tried injecting a 504 using Fiddler. The only potential bad path I see is if ValidateAuthority is set to false. Is it?

[henrik-me]

@bgavrilMS the underlying issue doesn't really matter (there can be many reasons for a proxy to time-out). If the library returns a null reference exception instead of a meaningful http error then there is no good way to write code to handle the scenario. Really this should be an msal service exception to ensure callers can have the simplest possible error handling code.

[bgavrilMS]

Sure, we should never throw a NullRef, it's a bug in our code, but I just can't repro it. I added more guards against null checks and more logging (based on the callstack), so if this repros we should have a better idea what's happening.

[jennyf19]

fixed in adal 5.1.0