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

Duplicate key in dictionary when accessing UserAgent during AcquireTokenSilentAsync #1014

Closed
1 of 7 tasks
rickykaare opened this issue Mar 26, 2019 · 6 comments
Closed
1 of 7 tasks
Assignees
Milestone

Comments

@rickykaare
Copy link

Which Version of MSAL are you using ?
MSAL 3.0.1-preview

Platform
Xamarin Android

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?
a. The app is in production, and I have upgraded to a new version of MSAL

Repro

var ClientApplication = PublicClientApplicationBuilder
    .Create(clientId)
    .WithB2CAuthority(signInAuthority)
    .WithRedirectUri($"msal{ClientId}://auth")
    .WithIosKeychainSecurityGroup(securityGroup)
    .WithDebugLoggingCallback(LogLevel.Verbose, true, true)
    .Build();
var account = await GetAccountAsync();
if (account == null)
    return null;

return await ClientApplication.AcquireTokenSilentAsync(
    scopes, 
    account, 
    signInAuthority, 
    true);

Expected behavior
Should return a token, if one is present in the token cache.

Actual behavior
On a Samsung Galaxy S9 we get the following exception thrown:

System.ArgumentException: An item with the same key has already been added. Key: User-Agent
at System.Collections.Generic.Dictionary`2[TKey,TValue].TryInsert (TKey key, TValue value, System.Collections.Generic.InsertionBehavior behavior)
at System.Collections.Generic.Dictionary`2[TKey,TValue].Add (TKey key, TValue value)
at System.Net.Http.Headers.HttpHeaders.GetValues[T] (System.String name)
at System.Net.Http.Headers.HttpRequestHeaders.get_UserAgent ()
at Microsoft.Identity.Client.Http.HttpManager+<ExecuteAsync>d__9.MoveNext ()
at Microsoft.Identity.Client.Http.HttpManager+<ExecuteWithRetryAsync>d__8.MoveNext ()
at Microsoft.Identity.Client.Http.HttpManager+<SendGetAsync>d__5.MoveNext ()
at Microsoft.Identity.Client.OAuth2.OAuth2Client+<ExecuteRequestAsync>d__10`1[T].MoveNext ()
at Microsoft.Identity.Client.OAuth2.OAuth2Client+<DiscoverAadInstanceAsync>d__8.MoveNext ()
at Microsoft.Identity.Client.Instance.AadInstanceDiscovery+<SendInstanceDiscoveryRequestAsync>d__12.MoveNext ()
at Microsoft.Identity.Client.Instance.AadInstanceDiscovery+<DoInstanceDiscoveryAndCacheAsync>d__7.MoveNext ()
at Microsoft.Identity.Client.Instance.AadInstanceDiscovery+<GetMetadataEntryAsync>d__6.MoveNext ()
at Microsoft.Identity.Client.TokenCache+<GetCachedOrDiscoverAuthorityMetaDataAsync>d__59.MoveNext ()
at Microsoft.Identity.Client.TokenCache+<Microsoft-Identity-Client-ITokenCacheInternal-FindRefreshTokenAsync>d__55.MoveNext ()
at Microsoft.Identity.Client.Internal.Requests.SilentRequest+<FindRefreshTokenOrFailAsync>d__6.MoveNext ()
at Microsoft.Identity.Client.Internal.Requests.SilentRequest+<ExecuteAsync>d__3.MoveNext ()
at Microsoft.Identity.Client.Internal.Requests.RequestBase+<RunAsync>d__13.MoveNext ()
at Microsoft.Identity.Client.ApiConfig.Executors.ClientApplicationBaseExecutor+<ExecuteAsync>d__2.MoveNext ()
at Microsoft.Identity.Client.ClientApplicationBase+<AcquireTokenSilentAsync>d__22.MoveNext ()

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

@jennyf19
Copy link
Collaborator

@rickykaare Which version of MSAL were you using before you updated?

@rickykaare
Copy link
Author

We were using v2.7.1 before the upgrade.

After some testing today, we found out that AcquireTokenSilentAsync was being called from multiple places at the same time. Apparently the method is not thread-safe, as it is calling HttpRequestHeaders.UserAgent on a shared header.

@jmprieur
Copy link
Contributor

@MarkZuber @bgavrilMS

@bgavrilMS
Copy link
Member

Good catch @rickykaare, we are calling DefaultRequestHeaders which apparently is not thread safe dotnet/dotnet-api-docs#1085

@bgavrilMS
Copy link
Member

Fixed by 00c070a

@bgavrilMS bgavrilMS added the Fixed label Apr 3, 2019
@bgavrilMS bgavrilMS added this to the 3.0.3 milestone Apr 3, 2019
@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 8, 2019

included in msal 3.0.3-preview

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

No branches or pull requests

4 participants