SWI-12789 Add Lock to OAuth Token Fetch

custom_templates/auth/OAuthAuthenticator.mustache

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Concurrent;
+using System.Threading;
 using System.Threading.Tasks;
 using Newtonsoft.Json;
 using RestSharp;
@@ -32,6 +33,11 @@ namespace {{packageName}}.Client.Auth
         /// </summary>
         static readonly ConcurrentDictionary<string, CachedToken> _tokenCache = new ConcurrentDictionary<string, CachedToken>();
 
+        /// <summary>
+        /// One lock per cache key so concurrent cache misses share a single token request instead of each fetching.
+        /// </summary>
+        static readonly ConcurrentDictionary<string, Lazy<SemaphoreSlim>> _fetchLocks = new ConcurrentDictionary<string, Lazy<SemaphoreSlim>>();
+
         sealed class CachedToken
         {
             public string Token { get; set; }
@@ -92,13 +98,40 @@ namespace {{packageName}}.Client.Auth
         /// </summary>
         /// <returns>An authentication token.</returns>
         async Task<string> GetCachedOrFetchToken()
+        {
+            if (TryGetCachedToken(out var token))
+            {
+                return token;
+            }
+
+            var fetchLock = _fetchLocks.GetOrAdd(CacheKey, _ => new Lazy<SemaphoreSlim>(() => new SemaphoreSlim(1, 1))).Value;
+            await fetchLock.WaitAsync().ConfigureAwait(false);
+            try
+            {
+                // Re-check: a caller that held the lock before us may have already populated the cache.
+                if (TryGetCachedToken(out token))
+                {
+                    return token;
+                }
+
+                return await GetToken().ConfigureAwait(false);
+            }
+            finally
+            {
+                fetchLock.Release();
+            }
+        }
+
+        bool TryGetCachedToken(out string token)
         {
             if (_tokenCache.TryGetValue(CacheKey, out var cached) && cached.ExpiresAtUtc > DateTime.UtcNow)
             {
-                return cached.Token;
+                token = cached.Token;
+                return true;
             }
 
-            return await GetToken().ConfigureAwait(false);
+            token = null;
+            return false;
         }
 
         /// <summary>

src/Bandwidth.Standard.Test/Unit/Client/OAuthAuthenticatorTests.cs

@@ -10,6 +10,7 @@
 
 
 using System;
+using System.Linq;
 using System.Net;
 using System.Net.Sockets;
 using System.Text;
@@ -42,6 +43,9 @@ public class OAuthAuthenticatorTests : IDisposable
         private int _expiresInSeconds = 3600;
         private bool _includeExpiresIn = true;
 
+        // Artificial latency on the fake endpoint so concurrent callers overlap on a cache miss.
+        private int _responseDelayMs = 0;
+
         public OAuthAuthenticatorTests()
         {
             // Unique client id per test keeps the authenticator's process-wide static
@@ -110,6 +114,22 @@ public async Task GetAuthenticationParameter_WithoutExpiresIn_FetchesEachTime()
             Assert.Equal(2, _tokenRequestCount);
         }
 
+        /// <summary>
+        /// Concurrent callers hitting a cold cache should share a single token request rather than each fetching.
+        /// </summary>
+        [Fact]
+        public async Task GetAuthenticationParameter_ConcurrentColdCache_FetchesOnlyOnce()
+        {
+            _responseDelayMs = 200;
+            var authenticator = CreateAuthenticator();
+
+            var results = await Task.WhenAll(
+                Enumerable.Range(0, 50).Select(_ => authenticator.GetAuthHeaderAsync()));
+
+            Assert.All(results, r => Assert.Equal($"Bearer {AccessToken}", r));
+            Assert.Equal(1, _tokenRequestCount);
+        }
+
         private TestableOAuthAuthenticator CreateAuthenticator()
         {
             return new TestableOAuthAuthenticator(
@@ -139,6 +159,9 @@ private async Task ServeTokenRequestsAsync()
                 Interlocked.Increment(ref _tokenRequestCount);
                 _lastAuthorizationHeader = context.Request.Headers["Authorization"];
 
+                if (_responseDelayMs > 0)
+                    await Task.Delay(_responseDelayMs);
+
                 string body = _includeExpiresIn
                     ? $"{{\"token_type\":\"Bearer\",\"access_token\":\"{AccessToken}\",\"expires_in\":{_expiresInSeconds}}}"
                     : $"{{\"token_type\":\"Bearer\",\"access_token\":\"{AccessToken}\"}}";

src/Bandwidth.Standard/Client/Auth/OAuthAuthenticator.cs

@@ -11,6 +11,7 @@
 
 using System;
 using System.Collections.Concurrent;
+using System.Threading;
 using System.Threading.Tasks;
 using Newtonsoft.Json;
 using RestSharp;
@@ -41,6 +42,11 @@ public class OAuthAuthenticator : AuthenticatorBase
         /// </summary>
         static readonly ConcurrentDictionary<string, CachedToken> _tokenCache = new ConcurrentDictionary<string, CachedToken>();
 
+        /// <summary>
+        /// One lock per cache key so concurrent cache misses share a single token request instead of each fetching.
+        /// </summary>
+        static readonly ConcurrentDictionary<string, Lazy<SemaphoreSlim>> _fetchLocks = new ConcurrentDictionary<string, Lazy<SemaphoreSlim>>();
+
         sealed class CachedToken
         {
             public string Token { get; set; }
@@ -101,13 +107,40 @@ protected override async ValueTask<Parameter> GetAuthenticationParameter(string
         /// </summary>
         /// <returns>An authentication token.</returns>
         async Task<string> GetCachedOrFetchToken()
+        {
+            if (TryGetCachedToken(out var token))
+            {
+                return token;
+            }
+
+            var fetchLock = _fetchLocks.GetOrAdd(CacheKey, _ => new Lazy<SemaphoreSlim>(() => new SemaphoreSlim(1, 1))).Value;
+            await fetchLock.WaitAsync().ConfigureAwait(false);
+            try
+            {
+                // Re-check: a caller that held the lock before us may have already populated the cache.
+                if (TryGetCachedToken(out token))
+                {
+                    return token;
+                }
+
+                return await GetToken().ConfigureAwait(false);
+            }
+            finally
+            {
+                fetchLock.Release();
+            }
+        }
+
+        bool TryGetCachedToken(out string token)
         {
             if (_tokenCache.TryGetValue(CacheKey, out var cached) && cached.ExpiresAtUtc > DateTime.UtcNow)
             {
-                return cached.Token;
+                token = cached.Token;
+                return true;
             }
 
-            return await GetToken().ConfigureAwait(false);
+            token = null;
+            return false;
         }
 
         /// <summary>