Skip to content

Commit 3e13383

Browse files
gladjohnGladwinJohnsonpmaytak
authored
concurrent token refresh fix for managed identity and app token provider (cc) (#4309)
* mi * app token * Apply suggestions from code review Co-authored-by: Peter <[email protected]> * pr comments * pr comments * pr comments * verbose logging * pr comments * Update src/client/Microsoft.Identity.Client/Internal/Requests/ManagedIdentityAuthRequest.cs Co-authored-by: Peter <[email protected]> * mi fix * few more edits * app token provider * edits * Apply suggestions from code review Co-authored-by: Peter <[email protected]> * pr comments * pr comments * ProactivelyRefreshed * move around --------- Co-authored-by: Gladwin Johnson <[email protected]> Co-authored-by: Peter <[email protected]>
1 parent 208aa58 commit 3e13383

File tree

7 files changed

+512
-112
lines changed

7 files changed

+512
-112
lines changed

LibsAndSamples.sln

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WAMMauiApp", "tests\devapps
241241
EndProject
242242
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Identity.Test.Integration.NetCore", "tests\Microsoft.Identity.Test.Integration.netcore\Microsoft.Identity.Test.Integration.NetCore.csproj", "{C5902FA4-CC11-43E8-B7E3-192B5FFA1C40}"
243243
EndProject
244+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NetCoreTestApp", "tests\devapps\NetCoreTestApp\NetCoreTestApp.csproj", "{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}"
245+
EndProject
244246
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Identity.Client.Extensions", "src\client\Microsoft.Identity.Client.Extensions\Microsoft.Identity.Client.Extensions.csproj", "{870F5BB7-68E4-44D5-B50A-71F41B1E726E}"
245247
EndProject
246248
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{1A37FD75-94E9-4D6F-953A-0DABBD7B49E9}"
@@ -2649,6 +2651,48 @@ Global
26492651
{C5902FA4-CC11-43E8-B7E3-192B5FFA1C40}.Release|x64.Build.0 = Release|Any CPU
26502652
{C5902FA4-CC11-43E8-B7E3-192B5FFA1C40}.Release|x86.ActiveCfg = Release|Any CPU
26512653
{C5902FA4-CC11-43E8-B7E3-192B5FFA1C40}.Release|x86.Build.0 = Release|Any CPU
2654+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|Any CPU.ActiveCfg = Debug + MobileApps|Any CPU
2655+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|Any CPU.Build.0 = Debug + MobileApps|Any CPU
2656+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|ARM.ActiveCfg = Debug + MobileApps|Any CPU
2657+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|ARM.Build.0 = Debug + MobileApps|Any CPU
2658+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|ARM64.ActiveCfg = Debug + MobileApps|Any CPU
2659+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|ARM64.Build.0 = Debug + MobileApps|Any CPU
2660+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|iPhone.ActiveCfg = Debug + MobileApps|Any CPU
2661+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|iPhone.Build.0 = Debug + MobileApps|Any CPU
2662+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|iPhoneSimulator.ActiveCfg = Debug + MobileApps|Any CPU
2663+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|iPhoneSimulator.Build.0 = Debug + MobileApps|Any CPU
2664+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|x64.ActiveCfg = Debug + MobileApps|Any CPU
2665+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|x64.Build.0 = Debug + MobileApps|Any CPU
2666+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|x86.ActiveCfg = Debug + MobileApps|Any CPU
2667+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug + MobileApps|x86.Build.0 = Debug + MobileApps|Any CPU
2668+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
2669+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|Any CPU.Build.0 = Debug|Any CPU
2670+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|ARM.ActiveCfg = Debug|Any CPU
2671+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|ARM.Build.0 = Debug|Any CPU
2672+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|ARM64.ActiveCfg = Debug|Any CPU
2673+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|ARM64.Build.0 = Debug|Any CPU
2674+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|iPhone.ActiveCfg = Debug|Any CPU
2675+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|iPhone.Build.0 = Debug|Any CPU
2676+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU
2677+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU
2678+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|x64.ActiveCfg = Debug|Any CPU
2679+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|x64.Build.0 = Debug|Any CPU
2680+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|x86.ActiveCfg = Debug|Any CPU
2681+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Debug|x86.Build.0 = Debug|Any CPU
2682+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|Any CPU.ActiveCfg = Release|Any CPU
2683+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|Any CPU.Build.0 = Release|Any CPU
2684+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|ARM.ActiveCfg = Release|Any CPU
2685+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|ARM.Build.0 = Release|Any CPU
2686+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|ARM64.ActiveCfg = Release|Any CPU
2687+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|ARM64.Build.0 = Release|Any CPU
2688+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|iPhone.ActiveCfg = Release|Any CPU
2689+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|iPhone.Build.0 = Release|Any CPU
2690+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
2691+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
2692+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|x64.ActiveCfg = Release|Any CPU
2693+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|x64.Build.0 = Release|Any CPU
2694+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|x86.ActiveCfg = Release|Any CPU
2695+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572}.Release|x86.Build.0 = Release|Any CPU
26522696
{870F5BB7-68E4-44D5-B50A-71F41B1E726E}.Debug + MobileApps|Any CPU.ActiveCfg = Debug|Any CPU
26532697
{870F5BB7-68E4-44D5-B50A-71F41B1E726E}.Debug + MobileApps|Any CPU.Build.0 = Debug|Any CPU
26542698
{870F5BB7-68E4-44D5-B50A-71F41B1E726E}.Debug + MobileApps|ARM.ActiveCfg = Debug|Any CPU
@@ -2808,6 +2852,7 @@ Global
28082852
{4EA542D2-D4C9-403C-B615-0047D0A62790} = {5FAAD966-36B8-4C19-A5FA-5410DD53063D}
28092853
{1B047736-9325-4F59-906B-89A3E12AC8FB} = {5FAAD966-36B8-4C19-A5FA-5410DD53063D}
28102854
{C5902FA4-CC11-43E8-B7E3-192B5FFA1C40} = {9B0B5396-4D95-4C15-82ED-DC22B5A3123F}
2855+
{4BEDCC7F-C027-4326-BAE4-AD62C1CD5572} = {384BA371-F17F-4A70-9423-D54F71BB3FCB}
28112856
{870F5BB7-68E4-44D5-B50A-71F41B1E726E} = {1A37FD75-94E9-4D6F-953A-0DABBD7B49E9}
28122857
{74805FE3-2E0D-4EAB-8CFE-A9D46F8D5972} = {34BE693E-3496-45A4-B1D2-D3A0E068EEDB}
28132858
{92064C48-0136-48CD-AE8D-C6FEDBC7B639} = {74805FE3-2E0D-4EAB-8CFE-A9D46F8D5972}

src/client/Microsoft.Identity.Client/Internal/Requests/ClientCredentialRequest.cs

Lines changed: 118 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace Microsoft.Identity.Client.Internal.Requests
1717
internal class ClientCredentialRequest : RequestBase
1818
{
1919
private readonly AcquireTokenForClientParameters _clientParameters;
20+
private static readonly SemaphoreSlim s_semaphoreSlim = new SemaphoreSlim(1, 1);
2021

2122
public ClientCredentialRequest(
2223
IServiceBundle serviceBundle,
@@ -36,106 +37,128 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
3637
MsalErrorMessage.ScopesRequired);
3738
}
3839

39-
MsalAccessTokenCacheItem cachedAccessTokenItem = null;
40-
var logger = AuthenticationRequestParameters.RequestContext.Logger;
41-
CacheRefreshReason cacheInfoTelemetry = CacheRefreshReason.NotApplicable;
42-
43-
AuthenticationResult authResult = null;
40+
ILoggerAdapter logger = AuthenticationRequestParameters.RequestContext.Logger;
4441

4542
if (AuthenticationRequestParameters.Authority is AadAuthority aadAuthority &&
4643
aadAuthority.IsCommonOrOrganizationsTenant())
4744
{
4845
logger.Error(MsalErrorMessage.ClientCredentialWrongAuthority);
4946
}
5047

51-
if (!_clientParameters.ForceRefresh &&
52-
string.IsNullOrEmpty(AuthenticationRequestParameters.Claims))
48+
AuthenticationResult authResult = null;
49+
50+
//skip checking cache for force refresh or when claims are present
51+
if (_clientParameters.ForceRefresh || !string.IsNullOrEmpty(AuthenticationRequestParameters.Claims))
5352
{
54-
cachedAccessTokenItem = await CacheManager.FindAccessTokenAsync().ConfigureAwait(false);
53+
AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = CacheRefreshReason.ForceRefreshOrClaims;
54+
logger.Info("[ClientCredentialRequest] Skipped looking for an Access Token in the cache because ForceRefresh was set.");
55+
authResult = await GetAccessTokenAsync(cancellationToken, logger).ConfigureAwait(false);
56+
return authResult;
57+
}
5558

56-
if (cachedAccessTokenItem != null)
57-
{
58-
AuthenticationRequestParameters.RequestContext.ApiEvent.IsAccessTokenCacheHit = true;
59+
//check cache for AT
60+
MsalAccessTokenCacheItem cachedAccessTokenItem = await GetCachedAccessTokenAsync().ConfigureAwait(false);
5961

60-
Metrics.IncrementTotalAccessTokensFromCache();
61-
authResult = new AuthenticationResult(
62-
cachedAccessTokenItem,
63-
null,
64-
AuthenticationRequestParameters.AuthenticationScheme,
65-
AuthenticationRequestParameters.RequestContext.CorrelationId,
66-
TokenSource.Cache,
67-
AuthenticationRequestParameters.RequestContext.ApiEvent,
68-
account: null,
69-
spaAuthCode: null,
70-
additionalResponseParameters: null);
71-
}
72-
else
62+
if (cachedAccessTokenItem != null)
63+
{
64+
//return the token in the cache and check if it needs to be proactively refreshed
65+
authResult = CreateAuthenticationResultFromCache(cachedAccessTokenItem);
66+
67+
try
7368
{
74-
if (AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo != CacheRefreshReason.Expired)
69+
var proactivelyRefresh = SilentRequestHelper.NeedsRefresh(cachedAccessTokenItem);
70+
71+
// may fire a request to get a new token in the background when AT needs to be refreshed
72+
if (proactivelyRefresh)
7573
{
76-
cacheInfoTelemetry = CacheRefreshReason.NoCachedAccessToken;
74+
AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = CacheRefreshReason.ProactivelyRefreshed;
75+
76+
SilentRequestHelper.ProcessFetchInBackground(
77+
cachedAccessTokenItem,
78+
() => GetAccessTokenAsync(cancellationToken, logger), logger);
7779
}
7880
}
81+
catch (MsalServiceException e)
82+
{
83+
return await HandleTokenRefreshErrorAsync(e, cachedAccessTokenItem).ConfigureAwait(false);
84+
}
7985
}
8086
else
8187
{
82-
cacheInfoTelemetry = CacheRefreshReason.ForceRefreshOrClaims;
83-
logger.Info("Skipped looking for an Access Token in the cache because ForceRefresh or Claims were set. ");
88+
// No AT in the cache
89+
if (AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo != CacheRefreshReason.Expired)
90+
{
91+
AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = CacheRefreshReason.NoCachedAccessToken;
92+
}
93+
94+
authResult = await GetAccessTokenAsync(cancellationToken, logger).ConfigureAwait(false);
8495
}
8596

86-
if (AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo == CacheRefreshReason.NotApplicable)
97+
return authResult;
98+
}
99+
100+
private async Task<AuthenticationResult> GetAccessTokenAsync(
101+
CancellationToken cancellationToken,
102+
ILoggerAdapter logger)
103+
{
104+
await ResolveAuthorityAsync().ConfigureAwait(false);
105+
106+
//calls sent to AAD
107+
if (ServiceBundle.Config.AppTokenProvider == null)
87108
{
88-
AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = cacheInfoTelemetry;
109+
MsalTokenResponse msalTokenResponse = await SendTokenRequestAsync(GetBodyParameters(), cancellationToken).ConfigureAwait(false);
110+
return await CacheTokenResponseAndCreateAuthenticationResultAsync(msalTokenResponse).ConfigureAwait(false);
89111
}
90112

91-
// No AT in the cache or AT needs to be refreshed
113+
//calls sent to app token provider
114+
AuthenticationResult authResult;
115+
MsalAccessTokenCacheItem cachedAccessTokenItem = null;
116+
92117
try
93118
{
94-
if (cachedAccessTokenItem == null)
119+
//allow only one call to the provider
120+
logger.Verbose(() => "[ClientCredentialRequest] Entering token acquire for client credential request semaphore.");
121+
await s_semaphoreSlim.WaitAsync(cancellationToken).ConfigureAwait(false);
122+
logger.Verbose(() => "[ClientCredentialRequest] Entered token acquire for client credential request semaphore.");
123+
124+
// Bypass cache and send request to token endpoint, when
125+
// 1. Force refresh is requested, or
126+
// 2. Claims are passed, or
127+
// 3. If the AT needs to be refreshed pro-actively
128+
if (_clientParameters.ForceRefresh ||
129+
AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo == CacheRefreshReason.ProactivelyRefreshed ||
130+
!string.IsNullOrEmpty(AuthenticationRequestParameters.Claims))
95131
{
96-
authResult = await FetchNewAccessTokenAsync(cancellationToken).ConfigureAwait(false);
132+
authResult = await SendTokenRequestToAppTokenProviderAsync(logger, cancellationToken).ConfigureAwait(false);
97133
}
98134
else
99135
{
100-
var shouldRefresh = SilentRequestHelper.NeedsRefresh(cachedAccessTokenItem);
136+
cachedAccessTokenItem = await GetCachedAccessTokenAsync().ConfigureAwait(false);
101137

102-
// may fire a request to get a new token in the background
103-
if (shouldRefresh)
138+
if (cachedAccessTokenItem == null)
104139
{
105-
AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = CacheRefreshReason.ProactivelyRefreshed;
106-
107-
SilentRequestHelper.ProcessFetchInBackground(
108-
cachedAccessTokenItem,
109-
() => FetchNewAccessTokenAsync(cancellationToken), logger);
140+
authResult = await SendTokenRequestToAppTokenProviderAsync(logger, cancellationToken).ConfigureAwait(false);
141+
}
142+
else
143+
{
144+
logger.Verbose(() => "[ClientCredentialRequest] Getting Access token from cache ...");
145+
authResult = CreateAuthenticationResultFromCache(cachedAccessTokenItem);
110146
}
111147
}
112-
148+
113149
return authResult;
114150
}
115-
catch (MsalServiceException e)
151+
finally
116152
{
117-
return await HandleTokenRefreshErrorAsync(e, cachedAccessTokenItem).ConfigureAwait(false);
153+
s_semaphoreSlim.Release();
154+
logger.Verbose(() => "[ClientCredentialRequest] Released token acquire for client credential request semaphore.");
118155
}
119156
}
120157

121-
private async Task<AuthenticationResult> FetchNewAccessTokenAsync(CancellationToken cancellationToken)
122-
{
123-
await ResolveAuthorityAsync().ConfigureAwait(false);
124-
MsalTokenResponse msalTokenResponse;
125-
if (ServiceBundle.Config.AppTokenProvider != null)
126-
{
127-
msalTokenResponse = await SendTokenRequestToProviderAsync(cancellationToken).ConfigureAwait(false);
128-
}
129-
else
130-
{
131-
msalTokenResponse = await SendTokenRequestAsync(GetBodyParameters(), cancellationToken).ConfigureAwait(false);
132-
}
133-
134-
return await CacheTokenResponseAndCreateAuthenticationResultAsync(msalTokenResponse).ConfigureAwait(false);
135-
}
136-
137-
private async Task<MsalTokenResponse> SendTokenRequestToProviderAsync(CancellationToken cancellationToken)
158+
private async Task<AuthenticationResult> SendTokenRequestToAppTokenProviderAsync(ILoggerAdapter logger,
159+
CancellationToken cancellationToken)
138160
{
161+
logger.Info("[ClientCredentialRequest] Sending Token response to client credential request endpoint ...");
139162
AppTokenProviderParameters appTokenProviderParameters = new AppTokenProviderParameters
140163
{
141164
Scopes = GetOverriddenScopes(AuthenticationRequestParameters.Scope),
@@ -150,7 +173,40 @@ private async Task<MsalTokenResponse> SendTokenRequestToProviderAsync(Cancellati
150173
var tokenResponse = MsalTokenResponse.CreateFromAppProviderResponse(externalToken);
151174
tokenResponse.Scope = appTokenProviderParameters.Scopes.AsSingleString();
152175
tokenResponse.CorrelationId = appTokenProviderParameters.CorrelationId;
153-
return tokenResponse;
176+
177+
AuthenticationResult authResult = await CacheTokenResponseAndCreateAuthenticationResultAsync(tokenResponse)
178+
.ConfigureAwait(false);
179+
180+
return authResult;
181+
}
182+
183+
private async Task<MsalAccessTokenCacheItem> GetCachedAccessTokenAsync()
184+
{
185+
MsalAccessTokenCacheItem cachedAccessTokenItem = await CacheManager.FindAccessTokenAsync().ConfigureAwait(false);
186+
187+
if (cachedAccessTokenItem != null && !_clientParameters.ForceRefresh)
188+
{
189+
AuthenticationRequestParameters.RequestContext.ApiEvent.IsAccessTokenCacheHit = true;
190+
Metrics.IncrementTotalAccessTokensFromCache();
191+
return cachedAccessTokenItem;
192+
}
193+
194+
return null;
195+
}
196+
197+
private AuthenticationResult CreateAuthenticationResultFromCache(MsalAccessTokenCacheItem cachedAccessTokenItem)
198+
{
199+
AuthenticationResult authResult = new AuthenticationResult(
200+
cachedAccessTokenItem,
201+
null,
202+
AuthenticationRequestParameters.AuthenticationScheme,
203+
AuthenticationRequestParameters.RequestContext.CorrelationId,
204+
TokenSource.Cache,
205+
AuthenticationRequestParameters.RequestContext.ApiEvent,
206+
account: null,
207+
spaAuthCode: null,
208+
additionalResponseParameters: null);
209+
return authResult;
154210
}
155211

156212
protected override SortedSet<string> GetOverriddenScopes(ISet<string> inputScopes)

0 commit comments

Comments
 (0)