Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPublicClientApplication.AcquireTokenInteractive API throws NullReferenceException #1214

Closed
1 task done
saketkataruka opened this issue Jun 12, 2019 · 5 comments
Closed
1 task done
Assignees
Labels
Milestone

Comments

@saketkataruka
Copy link

Which Version of MSAL are you using ?

v4 of Microsoft.Identity.Client

Platform
.net 4.7

What authentication flow has the issue?

  • Desktop / Mobile
    • Interactive

Other? - please describe;

Is this a new or existing app?
New app

Repro

// create public client using WithAdfsAuthority
IPublicClientApplication _app = PublicClientApplicationBuilder.Create(ClientId)
.WithAdfsAuthority("<>", false)
.Build();

// Authenticate the user interactive
AuthenticationResult result =await _app.AcquireTokenInteractive(Scopes).ExecuteAsync().ConfigureAwait(false);

// clear the cache
var accounts = (await _app.GetAccountsAsync()).ToList();
while (accounts.Any())
{
await _app.RemoveAsync(accounts.First());
accounts = (await _app.GetAccountsAsync()).ToList();
}

// try to authenticate the user again:
AuthenticationResult result =await _app.AcquireTokenInteractive(Scopes).ExecuteAsync().ConfigureAwait(false);

Expected behavior
User is asked for creds and authenticated

Actual behavior
That API throws NullReferenceException with the following callstack.

at Microsoft.Identity.Client.Instance.AdfsUpnHelper.GetDomainFromUpn(String upn)
at Microsoft.Identity.Client.Instance.AuthorityEndpointResolutionManager.TryGetCacheValue(AuthorityInfo authorityInfo, String userPrincipalName, AuthorityEndpoints& endpoints)
at Microsoft.Identity.Client.Instance.AuthorityEndpointResolutionManager.d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.d__20.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.d__14.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Identity.Client.ApiConfig.Executors.PublicClientExecutor.d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
at TodoListClient.MainWindow.d__18.MoveNext() in H:\oauthExamples\active-directory-dotnet-native-desktop\TodoListClient\MainWindow.xaml.cs:line 332

Possible Solution
Handle UPN being not present

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/microsoft-authentication-library-for-dotnet/wiki/logging

@jennyf19
Copy link
Collaborator

@saketkataruka confirms the fix.

@henrik-me henrik-me added the bug label Jun 21, 2019
@jennyf19 jennyf19 self-assigned this Jun 27, 2019
@fdbeirao
Copy link

fdbeirao commented Jul 1, 2019

After more than 2 days struggling with this issue, I want to give you my most heartfelt Thank you! for the resolution of this issue. 🙏 ❤

@jennyf19
Copy link
Collaborator

jennyf19 commented Jul 1, 2019

@fdbeirao thanks for the feedback and ❤️ ...this fix will be out shortly in the 4.1 release.

Sorry it took you two days of struggling...any suggestions on what we could have done to make this issue more discoverable? ex. a short write-up in the wiki (until the fix is released), a better title for the issue, releasing sooner, etc..? 🐈

@fdbeirao
Copy link

fdbeirao commented Jul 2, 2019

Hey @jennyf19 . I don't think there was something you could have done. ADFS is a quite complex beast, and when something goes wrong it's always going to be difficult. Together with a "random" NRE, it becomes slightly harder even.

I was providing consulting for a friend who is using this library. Even after downloading the debug symbols, the NRE was still in a somewhat unrelated place (null.Contains("@")). Most of the struggle was with Visual Studio, for losing bits and pieces of the call stack due to the usage of async/awaits. It was quite a stroke of luck that we found this Issue, after searching for "TryGetCacheValue" here on GitHub.

Also, the "bug" was also only triggered after the second login to the system, because the second execution of TryGetCacheValue would have a value in the cache, but the first didn't (so it returned false immediately). 🤷🏻‍♂️

Thank you for the fix and the follow up 🙏🏻

@jmprieur
Copy link
Contributor

jmprieur commented Jul 2, 2019

@saketkataruka @fdbeirao : the fixe is now available in MSAL.NET 4.1
cc: @jennyf19

@jmprieur jmprieur closed this as completed Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants