Locking

src/Microsoft.Identity.Web/AppServicesAuth/AppServicesAuthenticationTokenAcquisition.cs

@@ -18,7 +18,12 @@ namespace Microsoft.Identity.Web
     /// </summary>
     public class AppServicesAuthenticationTokenAcquisition : ITokenAcquisition
     {
-        private IConfidentialClientApplication? _confidentialClientApplication;
+        private readonly object _applicationSyncObj = new object();
+
+        /// <summary>
+        ///  Please call GetOrCreateApplication instead of accessing this field directly.
+        /// </summary>
+        private IConfidentialClientApplication? _application;
         private readonly IHttpContextAccessor _httpContextAccessor;
         private readonly IMsalHttpClientFactory _httpClientFactory;
         private readonly IMsalTokenCacheProvider _tokenCacheProvider;
@@ -70,22 +75,28 @@ public AppServicesAuthenticationTokenAcquisition(
 
         private IConfidentialClientApplication GetOrCreateApplication()
         {
-            if (_confidentialClientApplication == null)
+            if (_application == null)
             {
-                ConfidentialClientApplicationOptions options = new ConfidentialClientApplicationOptions()
+                lock (_applicationSyncObj)
                 {
-                    ClientId = AppServicesAuthenticationInformation.ClientId,
-                    ClientSecret = AppServicesAuthenticationInformation.ClientSecret,
-                    Instance = AppServicesAuthenticationInformation.Issuer,
-                };
-                _confidentialClientApplication = ConfidentialClientApplicationBuilder.CreateWithApplicationOptions(options)
-                    .WithHttpClientFactory(_httpClientFactory)
-                    .Build();
-                _tokenCacheProvider.Initialize(_confidentialClientApplication.AppTokenCache);
-                _tokenCacheProvider.Initialize(_confidentialClientApplication.UserTokenCache);
+                    if (_application == null)
+                    {
+                        var options = new ConfidentialClientApplicationOptions()
+                        {
+                            ClientId = AppServicesAuthenticationInformation.ClientId,
+                            ClientSecret = AppServicesAuthenticationInformation.ClientSecret,
+                            Instance = AppServicesAuthenticationInformation.Issuer,
+                        };
+                        _application = ConfidentialClientApplicationBuilder.CreateWithApplicationOptions(options)
+                            .WithHttpClientFactory(_httpClientFactory)
+                            .Build();
+                        _tokenCacheProvider.Initialize(_application.AppTokenCache);
+                        _tokenCacheProvider.Initialize(_application.UserTokenCache);
+                    }
+                }
             }
 
-            return _confidentialClientApplication;
+            return _application;
         }
 
         /// <inheritdoc/>
@@ -109,16 +120,29 @@ public async Task<string> GetAccessTokenForAppAsync(
         }
 
         /// <inheritdoc/>
-        public async Task<string> GetAccessTokenForUserAsync(
+        public Task<string> GetAccessTokenForUserAsync(
             IEnumerable<string> scopes,
             string? tenantId = null,
             string? userFlow = null,
             ClaimsPrincipal? user = null,
             TokenAcquisitionOptions? tokenAcquisitionOptions = null)
         {
-            string accessToken = GetAccessToken(CurrentHttpContext?.Request.Headers);
+            var httpContext = CurrentHttpContext;
+            string accessToken;
+            if (httpContext != null)
+            {
+                // Need to lock due to https://docs.microsoft.com/en-us/aspnet/core/performance/performance-best-practices?#do-not-access-httpcontext-from-multiple-threads
+                lock (httpContext)
+                {
+                    accessToken = GetAccessToken(httpContext.Request.Headers);
+                }
+            }
+            else
+            {
+                accessToken = string.Empty;
+            }
 
-            return await Task.FromResult(accessToken).ConfigureAwait(false);
+            return Task.FromResult(accessToken);
         }
 
         private string GetAccessToken(IHeaderDictionary? headers)
@@ -145,7 +169,6 @@ private string GetAccessToken(IHeaderDictionary? headers)
         }
 
         /// <inheritdoc/>
-#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
         public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
             IEnumerable<string> scopes,
             string? tenantId = null,
@@ -209,6 +232,5 @@ public Task<AuthenticationResult> GetAuthenticationResultForAppAsync(string scop
         {
             throw new NotImplementedException();
         }
-#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
     }
 }

src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Globalization;
+using System.Security.Claims;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
@@ -101,17 +102,27 @@ public override void OnException(ExceptionContext context)
                         incrementalConsentScopes = Scopes;
                     }
 
+                    HttpRequest httpRequest;
+                    ClaimsPrincipal user;
+                    HttpContext httpContext = context.HttpContext;
+
+                    lock (httpContext)
+                    {
+                        httpRequest = httpContext.Request;
+                        user = httpContext.User;
+                    }
+
                     AuthenticationProperties properties = IncrementalConsentAndConditionalAccessHelper.BuildAuthenticationProperties(
                         incrementalConsentScopes,
                         msalUiRequiredException,
-                        context.HttpContext.User,
+                        user,
                         UserFlow);
 
-                    if (IsAjaxRequest(context.HttpContext.Request) && (!string.IsNullOrEmpty(context.HttpContext.Request.Headers[Constants.XReturnUrl])
-                        || !string.IsNullOrEmpty(context.HttpContext.Request.Query[Constants.XReturnUrl])))
+                    if (IsAjaxRequest(httpRequest) && (!string.IsNullOrEmpty(httpRequest.Headers[Constants.XReturnUrl])
+                        || !string.IsNullOrEmpty(httpRequest.Query[Constants.XReturnUrl])))
                     {
-                        string redirectUri = !string.IsNullOrEmpty(context.HttpContext.Request.Headers[Constants.XReturnUrl]) ? context.HttpContext.Request.Headers[Constants.XReturnUrl]
-                            : context.HttpContext.Request.Query[Constants.XReturnUrl];
+                        string redirectUri = !string.IsNullOrEmpty(httpRequest.Headers[Constants.XReturnUrl]) ? httpRequest.Headers[Constants.XReturnUrl]
+                            : httpRequest.Query[Constants.XReturnUrl];
 
                         UrlHelper urlHelper = new UrlHelper(context);
                         if (urlHelper.IsLocalUrl(redirectUri))

src/Microsoft.Identity.Web/HttpContextExtensions.cs

@@ -16,7 +16,11 @@ internal static class HttpContextExtensions
         /// it can be used in the actions.</param>
         internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, JwtSecurityToken? token)
         {
-            httpContext.Items.Add(Constants.JwtSecurityTokenUsedToCallWebApi, token);
+            // lock due to https://docs.microsoft.com/en-us/aspnet/core/performance/performance-best-practices?#do-not-access-httpcontext-from-multiple-threads
+            lock (httpContext)
+            {
+                httpContext.Items.Add(Constants.JwtSecurityTokenUsedToCallWebApi, token);
+            }
         }
 
         /// <summary>
@@ -26,7 +30,11 @@ internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, Jw
         /// <returns><see cref="JwtSecurityToken"/> used to call the web API.</returns>
         internal static JwtSecurityToken? GetTokenUsedToCallWebAPI(this HttpContext httpContext)
         {
-            return httpContext.Items[Constants.JwtSecurityTokenUsedToCallWebApi] as JwtSecurityToken;
+            // lock due to https://docs.microsoft.com/en-us/aspnet/core/performance/performance-best-practices?#do-not-access-httpcontext-from-multiple-threads
+            lock (httpContext)
+            {
+                return httpContext.Items[Constants.JwtSecurityTokenUsedToCallWebApi] as JwtSecurityToken;
+            }
         }
     }
 }

src/Microsoft.Identity.Web/MicrosoftIdentityConsentAndConditionalAccessHandler.cs

@@ -43,11 +43,21 @@ public ClaimsPrincipal User
         {
             get
             {
-                return _user ??
-#pragma warning disable CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case.
-                    (!IsBlazorServer ? _httpContextAccessor.HttpContext.User :
-#pragma warning restore CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case.
-                    throw new InvalidOperationException(IDWebErrorMessage.BlazorServerUserNotSet));
+                if (_user != null)
+                {
+                    return _user;
+                }
+
+                HttpContext httpContext = _httpContextAccessor!.HttpContext!;
+                ClaimsPrincipal user;
+
+                lock (httpContext)
+                {
+                    user = httpContext.User;
+                }
+
+                return !IsBlazorServer ? user :
+                    throw new InvalidOperationException(IDWebErrorMessage.BlazorServerUserNotSet);
             }
             set
             {
@@ -62,11 +72,21 @@ public string? BaseUri
         {
             get
             {
-                return _baseUri ??
-#pragma warning disable CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case
-                    (!IsBlazorServer ? CreateBaseUri(_httpContextAccessor.HttpContext.Request) :
-#pragma warning restore CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case
-                    throw new InvalidOperationException(IDWebErrorMessage.BlazorServerBaseUriNotSet));
+                if (_baseUri != null)
+                {
+                    return _baseUri;
+                }
+
+                HttpRequest httpRequest;
+                HttpContext httpContext = _httpContextAccessor!.HttpContext!;
+
+                lock (httpContext)
+                {
+                    httpRequest = httpContext.Request;
+                }
+
+                return !IsBlazorServer ? CreateBaseUri(httpRequest) :
+                    throw new InvalidOperationException(IDWebErrorMessage.BlazorServerBaseUriNotSet);
             }
             set
             {
@@ -160,14 +180,19 @@ public void ChallengeUser(
             }
             else
             {
-#pragma warning disable CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case.
-                var request = _httpContextAccessor.HttpContext.Request;
-#pragma warning restore CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case.
+                HttpRequest httpRequest;
+                HttpContext httpContext = _httpContextAccessor!.HttpContext!;
+
+                lock (httpContext)
+                {
+                    httpRequest = httpContext.Request;
+                }
+
                 redirectUri = string.Format(
                     CultureInfo.InvariantCulture,
                     "{0}/{1}",
-                    CreateBaseUri(request),
-                    request.Path.ToString().TrimStart('/'));
+                    CreateBaseUri(httpRequest),
+                    httpRequest.Path.ToString().TrimStart('/'));
             }
 
             string url = $"{BaseUri}/{Constants.BlazorChallengeUri}{redirectUri}"
@@ -181,9 +206,12 @@ public void ChallengeUser(
             }
             else
             {
-#pragma warning disable CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case.
-                _httpContextAccessor.HttpContext.Response.Redirect(url);
-#pragma warning restore CS8602 // Dereference of a possibly null reference. HttpContext will not be null in this case.
+                HttpContext httpContext = _httpContextAccessor!.HttpContext!;
+
+                lock (httpContext)
+                {
+                    httpContext.Response.Redirect(url);
+                }
             }
         }
 

src/Microsoft.Identity.Web/Resource/RequiredScopeFilter.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using System.Net;
@@ -55,31 +56,51 @@ private void ValidateEffectiveScopes(AuthorizationFilterContext context)
                 throw new InvalidOperationException(IDWebErrorMessage.MissingRequiredScopesForAuthorizationFilter);
             }
 
-            if (context.HttpContext.User == null || context.HttpContext.User.Claims == null || !context.HttpContext.User.Claims.Any())
+            IEnumerable<Claim> userClaims;
+            ClaimsPrincipal user;
+            HttpContext httpContext = context.HttpContext;
+
+            // Need to lock due to https://docs.microsoft.com/en-us/aspnet/core/performance/performance-best-practices?#do-not-access-httpcontext-from-multiple-threads
+            lock (httpContext)
+            {
+                user = httpContext.User;
+                userClaims = user.Claims;
+            }
+
+            if (user == null || userClaims == null || !userClaims.Any())
             {
-                context.HttpContext.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
+                lock (httpContext)
+                {
+                    httpContext.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
+                }
+
                 throw new UnauthorizedAccessException(IDWebErrorMessage.UnauthenticatedUser);
             }
             else
             {
                 // Attempt with Scp claim
-                Claim? scopeClaim = context.HttpContext.User.FindFirst(ClaimConstants.Scp);
+                Claim? scopeClaim = user.FindFirst(ClaimConstants.Scp);
 
                 // Fallback to Scope claim name
                 if (scopeClaim == null)
                 {
-                    scopeClaim = context.HttpContext.User.FindFirst(ClaimConstants.Scope);
+                    scopeClaim = user.FindFirst(ClaimConstants.Scope);
                 }
 
                 if (scopeClaim == null || !scopeClaim.Value.Split(' ').Intersect(_effectiveAcceptedScopes).Any())
                 {
-                    context.HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
                     string message = string.Format(
                         CultureInfo.InvariantCulture,
                         IDWebErrorMessage.MissingScopes,
                         string.Join(",", _effectiveAcceptedScopes));
-                    context.HttpContext.Response.WriteAsync(message);
-                    context.HttpContext.Response.CompleteAsync();
+
+                    lock (httpContext)
+                    {
+                        httpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
+                        httpContext.Response.WriteAsync(message);
+                        httpContext.Response.CompleteAsync();
+                    }
+
                     throw new UnauthorizedAccessException(message);
                 }
             }