diff --git a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs index f36d8482ca..1f07c04f47 100644 --- a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs +++ b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs @@ -874,8 +874,8 @@ private async Task GetAppCredentialsAsync(string appId, // NOTE: we can't do async operations inside of a AddOrUpdate, so we split access pattern string appPassword = await _credentialProvider.GetAppPasswordAsync(appId).ConfigureAwait(false); appCredentials = (_channelProvider != null && _channelProvider.IsGovernment()) ? - new MicrosoftGovernmentAppCredentials(appId, appPassword) : - new MicrosoftAppCredentials(appId, appPassword); + new MicrosoftGovernmentAppCredentials(appId, appPassword, _httpClient) : + new MicrosoftAppCredentials(appId, appPassword, _httpClient); _appCredentialMap[appId] = appCredentials; return appCredentials; } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs b/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs new file mode 100644 index 0000000000..7e185d1293 --- /dev/null +++ b/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs @@ -0,0 +1,197 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.IdentityModel.Clients.ActiveDirectory; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Net.Http; +using System.Net.Http.Headers; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Bot.Connector.Authentication +{ + public class ThrottleException : Exception + { + public RetryParams RetryParams { get; set; } + } + + public class AdalAuthenticator + { + // Our ADAL context. Acquires tokens and manages token caching for us. + private readonly AuthenticationContext authContext; + + // Semaphore to control concurrency while refreshing tokens from ADAL. + // Whenever a token expires, we want only one request to retrieve a token. + // Cached requests take less than 0.1 millisecond to resolve, so the semaphore doesn't hurt performance under load tests + // unless we have more than 10,000 requests per second, but in that case other things would break first. + private static Semaphore tokenRefreshSemaphore = new Semaphore(1, 1); + private static readonly TimeSpan SemaphoreTimeout = TimeSpan.FromSeconds(10); + + // Depending on the responses we get from the service, we update a shared retry policy with the RetryAfter header + // from the HTTP 429 we receive. + // When everything seems to be OK, this retry policy will be empty. + // The reason for this is that if a request gets throttled, even if we wait to retry that, another thread will try again right away. + // With the shared retry policy, if a request gets throttled, we know that other threads have to wait as well. + // This variable is guarded by the authContextSemaphore semphore. Don't modify it outside of the semaphore scope. + private static volatile RetryParams currentRetryPolicy; + + private readonly ClientCredential clientCredential; + private readonly OAuthConfiguration oAuthConfig; + + private const string msalTemporarilyUnavailable = "temporarily_unavailable"; + + public AdalAuthenticator(ClientCredential clientCredential, OAuthConfiguration oAuthConfig, HttpClient customHttpClient = null) + { + this.oAuthConfig = oAuthConfig ?? throw new ArgumentNullException(nameof(oAuthConfig)); + this.clientCredential = clientCredential ?? throw new ArgumentNullException(nameof(clientCredential)); + + if (customHttpClient != null) + { + this.authContext = new AuthenticationContext(oAuthConfig.Authority, true, new TokenCache(), customHttpClient); + } + else + { + this.authContext = new AuthenticationContext(oAuthConfig.Authority); + } + } + + public async Task GetTokenAsync(bool forceRefresh = false) + { + return await Retry.Run( + task: () => AcquireTokenAsync(forceRefresh), + retryExceptionHandler: (ex, ct) => HandleAdalException(ex, ct)).ConfigureAwait(false); + } + + private async Task AcquireTokenAsync(bool forceRefresh = false) + { + bool acquired = false; + + if (forceRefresh) + { + authContext.TokenCache.Clear(); + } + + try + { + // The ADAL client team recommends limiting concurrency of calls. When the Token is in cache there is never + // contention on this semaphore, but when tokens expire there is some. However, after measuring performance + // with and without the semaphore (and different configs for the semaphore), not limiting concurrency actually + // results in higher response times overall. Without the use of this semaphore calls to AcquireTokenAsync can take up + // to 5 seconds under high concurrency scenarios. + acquired = tokenRefreshSemaphore.WaitOne(SemaphoreTimeout); + + // If we are allowed to enter the semaphore, acquire the token. + if (acquired) + { + // Acquire token async using MSAL.NET + // https://github.com/AzureAD/azure-activedirectory-library-for-dotnet + // Given that this is a ClientCredential scenario, it will use the cache without the + // need to call AcquireTokenSilentAsync (which is only for user credentials). + // Scenario details: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/Client-credential-flows#it-uses-the-application-token-cache + var res = await authContext.AcquireTokenAsync(oAuthConfig.Scope, this.clientCredential).ConfigureAwait(false); + // This means we acquired a valid token successfully. We can make our retry policy null. + // Note that the retry policy is set under the semaphore so no additional synchronization is needed. + if (currentRetryPolicy != null) + { + currentRetryPolicy = null; + } + + return res; + } + else + { + // If the token is taken, it means that one thread is trying to acquire a token from the server. + // If we already received information about how much to throttle, it will be in the currentRetryPolicy. + // Use that to inform our next delay before trying. + throw new ThrottleException() { RetryParams = currentRetryPolicy }; + } + } + catch (Exception ex) + { + // If we are getting throttled, we set the retry policy according to the RetryAfter headers + // that we receive from the auth server. + // Note that the retry policy is set under the semaphore so no additional synchronization is needed. + if (IsAdalServiceUnavailable(ex)) + { + currentRetryPolicy = ComputeAdalRetry(ex); + } + throw; + } + finally + { + // Always release the semaphore if we acquired it. + if (acquired) + { + tokenRefreshSemaphore.Release(); + } + } + } + + private RetryParams HandleAdalException(Exception ex, int currentRetryCount) + { + if (IsAdalServiceUnavailable(ex)) + { + return ComputeAdalRetry(ex); + } + else if (ex is ThrottleException) + { + // This is an exception that we threw, with knowledge that + // one of our threads is trying to acquire a token from the server + // Use the retry parameters recommended in the exception + ThrottleException throttlException = (ThrottleException)ex; + return throttlException.RetryParams ?? RetryParams.DefaultBackOff(currentRetryCount); + } + else + { + // We end up here is the exception is not an ADAL exception. An example, is under high traffic + // where we could have a timeout waiting to acquire a token, waiting on the semaphore. + // If we hit a timeout, we want to retry a reasonable number of times. + return RetryParams.DefaultBackOff(currentRetryCount); + } + } + + private bool IsAdalServiceUnavailable(Exception ex) + { + AdalServiceException adalServiceException = ex as AdalServiceException; + if (adalServiceException == null) + { + return false; + } + + // When the Service Token Server (STS) is too busy because of “too many requests”, + // it returns an HTTP error 429 + return adalServiceException.ErrorCode == msalTemporarilyUnavailable || adalServiceException.StatusCode == 429; + } + + private RetryParams ComputeAdalRetry(Exception ex) + { + if (ex is AdalServiceException) + { + AdalServiceException adalServiceException = (AdalServiceException)ex; + + // When the Service Token Server (STS) is too busy because of “too many requests”, + // it returns an HTTP error 429 with a hint about when you can try again (Retry-After response field) as a delay in seconds + if (adalServiceException.ErrorCode == msalTemporarilyUnavailable || adalServiceException.StatusCode == 429) + { + RetryConditionHeaderValue retryAfter = adalServiceException.Headers.RetryAfter; + + // Depending on the service, the recommended retry time may be in retryAfter.Delta or retryAfter.Date. Check both. + if (retryAfter != null && retryAfter.Delta.HasValue) + { + return new RetryParams(retryAfter.Delta.Value); + } + else if (retryAfter != null && retryAfter.Date.HasValue) + { + return new RetryParams(retryAfter.Date.Value.Offset); + } + // We got a 429 but didn't get a specific back-off time. Use the default + return RetryParams.DefaultBackOff(0); + } + } + return RetryParams.DefaultBackOff(0); + } + } +} diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConstants.cs b/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConstants.cs index 5df580ab0c..ed9f8a3cc7 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConstants.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConstants.cs @@ -13,12 +13,12 @@ public static class AuthenticationConstants /// /// TO CHANNEL FROM BOT: Login URL /// - public const string ToChannelFromBotLoginUrl = "https://login.microsoftonline.com/botframework.com/oauth2/v2.0/token"; + public const string ToChannelFromBotLoginUrl = "https://login.microsoftonline.com/botframework.com"; /// /// TO CHANNEL FROM BOT: OAuth scope to request /// - public const string ToChannelFromBotOAuthScope = "https://api.botframework.com/.default"; + public const string ToChannelFromBotOAuthScope = "https://api.botframework.com"; /// /// TO BOT FROM CHANNEL: Token issuer diff --git a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs index 5df6bd8b18..c5386a5021 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs @@ -18,12 +18,12 @@ public static class GovernmentAuthenticationConstants /// /// TO GOVERNMENT CHANNEL FROM BOT: Login URL /// - public const string ToChannelFromBotLoginUrl = "https://login.microsoftonline.us/cab8a31a-1906-4287-a0d8-4eef66b95f6e/oauth2/v2.0/token"; + public const string ToChannelFromBotLoginUrl = "https://login.microsoftonline.us/cab8a31a-1906-4287-a0d8-4eef66b95f6e"; /// /// TO GOVERNMENT CHANNEL FROM BOT: OAuth scope to request /// - public const string ToChannelFromBotOAuthScope = "https://api.botframework.us/.default"; + public const string ToChannelFromBotOAuthScope = "https://api.botframework.us"; /// /// TO BOT FROM GOVERNMENT CHANNEL: Token issuer diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs index eca5c005ea..bcf13c8f9b 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs @@ -9,6 +9,7 @@ using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; +using Microsoft.IdentityModel.Clients.ActiveDirectory; using Microsoft.Rest; using Newtonsoft.Json; @@ -34,8 +35,6 @@ public class MicrosoftAppCredentials : ServiceClientCredentials /// public const string MicrosoftAppPasswordKey = "MicrosoftAppPassword"; - private static readonly HttpClient DefaultHttpClient = new HttpClient(); - private static readonly IDictionary TrustedHostNames = new Dictionary() { // { "state.botframework.com", DateTime.MaxValue }, // deprecated state api @@ -46,38 +45,27 @@ public class MicrosoftAppCredentials : ServiceClientCredentials }; /// - /// A cache of the outstanding uncompleted or completed tasks for a given token, for ensuring that we never have more then 1 token request in flight - /// per token at a time. - /// - protected static readonly Dictionary> TokenTaskCache = new Dictionary>(); - - /// - /// The time at which we will next refresh each token. + /// Authenticator abstraction used to obtain tokens through the Client Credentials OAuth 2.0 flow /// - protected static readonly ConcurrentDictionary AutoRefreshTimes = new ConcurrentDictionary(); - - /// - /// A cache of the actual valid tokens, this is what is consumed 99.99% of the time regardless o whether there is a token refresh task in flight. - /// We refresh tokens on a schedule which is faster then their expiration, and if there is a network failure, we continue to use the good token from - /// the tokenCache while a new background refresh task gets scheduled. - /// - protected static readonly ConcurrentDictionary TokenCache = new ConcurrentDictionary(); - - /// - /// The actual key we use for the token cache. - /// - protected readonly string TokenCacheKey; + private readonly Lazy authenticator; /// /// Creates a new instance of the class. /// /// The Microsoft app ID. /// The Microsoft app password. - public MicrosoftAppCredentials(string appId, string password) + /// Optional to be used when acquiring tokens. + public MicrosoftAppCredentials(string appId, string password, HttpClient customHttpClient = null) { this.MicrosoftAppId = appId; this.MicrosoftAppPassword = password; - this.TokenCacheKey = $"{MicrosoftAppId}-cache"; + + authenticator = new Lazy(() => + new AdalAuthenticator( + new ClientCredential(MicrosoftAppId, MicrosoftAppPassword), + new OAuthConfiguration() { Authority = OAuthEndpoint, Scope = OAuthScope }, + customHttpClient), + LazyThreadSafetyMode.ExecutionAndPublication); } /// @@ -100,12 +88,6 @@ public MicrosoftAppCredentials(string appId, string password) /// public virtual string OAuthScope { get { return AuthenticationConstants.ToChannelFromBotOAuthScope; } } - - /// - /// The time window within which the token will be automatically updated. - /// - public static TimeSpan AutoTokenRefreshTimeSpan { get; set; } = TimeSpan.FromMinutes(10); - /// /// Adds the host of service url to trusted hosts. /// @@ -163,112 +145,11 @@ public override async Task ProcessHttpRequestAsync(HttpRequestMessage request, C /// True to force a refresh of the token; or false to get /// a cached token if it exists. /// A task that represents the work queued to execute. - /// If the task is successful, the result contains the access token string. + /// If the task is successful, the result contains the access token string. public async Task GetTokenAsync(bool forceRefresh = false) { - Task oAuthTokenTask = null; - OAuthResponse oAuthToken = null; - - // get tokenTask from cache - lock (TokenTaskCache) - { - // if we are being forced or don't have a token in our cache at all - if (forceRefresh || !TokenCache.TryGetValue(TokenCacheKey, out oAuthToken)) - { - // we will await this task, because we don't have a token and we need it - oAuthTokenTask = _getCurrentTokenTask(forceRefresh: forceRefresh); - } - else - { - // we have an oAuthToken - // check to see if our token is expired - if (IsTokenExpired(oAuthToken)) - { - // it is, we should await the current task (someone could have already asked for a new token) - oAuthTokenTask = _getCurrentTokenTask(forceRefresh: false); - - // if the task is completed and is the expired token, then we need to force a new one - // (This happens if bot has been 100% idle past the expiration point) - if (oAuthTokenTask.Status == TaskStatus.RanToCompletion && oAuthTokenTask.Result.access_token == oAuthToken.access_token) - { - oAuthTokenTask = _getCurrentTokenTask(forceRefresh: true); - } - } - - // always check the autorefresh - CheckAutoRefreshToken(); - } - } - - // if we have an oAuthTokenTask then we need to await it - if (oAuthTokenTask != null) - { - oAuthToken = await oAuthTokenTask; - TokenCache[TokenCacheKey] = oAuthToken; - } - - return oAuthToken?.access_token; - } - - private void CheckAutoRefreshToken() - { - // get the current autorefreshTime for this key - DateTime refreshTime; - if (AutoRefreshTimes.TryGetValue(TokenCacheKey, out refreshTime)) - { - // if we are past the refresh time - if (DateTime.UtcNow > refreshTime) - { - // set new refresh time (this keeps only one outstanding refresh Task at a time) - AutoRefreshTimes[TokenCacheKey] = DateTime.UtcNow + AutoTokenRefreshTimeSpan; - - // background task to refresh the token - // NOTE: This is not awaited, but is observed with the ContinueWith() clause. - // It is gated by the AutoRefreshTimes[] array - RefreshTokenAsync() - .ContinueWith(task => - { - // observe the background task and put in cache when done - if (task.Status == TaskStatus.RanToCompletion) - { - // update the cache with completed task so all new requests will get it - TokenTaskCache[TokenCacheKey] = task; - } - else - { - // it failed, shorten the refresh time for another task to try again in a 30s - AutoRefreshTimes[TokenCacheKey] = DateTime.UtcNow + TimeSpan.FromSeconds(30); - } - }); - } - } - } - - private Task _getCurrentTokenTask(bool forceRefresh) - { - Task oAuthTokenTask; - - // if there is not a task or we are forcing it - if (forceRefresh || TokenTaskCache.TryGetValue(TokenCacheKey, out oAuthTokenTask) == false) - { - // create it - oAuthTokenTask = RefreshTokenAsync(); - TokenTaskCache[TokenCacheKey] = oAuthTokenTask; - - // set initial refresh time - AutoRefreshTimes[TokenCacheKey] = DateTime.UtcNow + AutoTokenRefreshTimeSpan; - } - // if task is in faulted or canceled state then replace it with another attempt - else if (oAuthTokenTask.IsFaulted || oAuthTokenTask.IsCanceled) - { - oAuthTokenTask = RefreshTokenAsync(); - TokenTaskCache[TokenCacheKey] = oAuthTokenTask; - - // set initial refresh time - AutoRefreshTimes[TokenCacheKey] = DateTime.UtcNow + AutoTokenRefreshTimeSpan; - } - - return oAuthTokenTask; + var token = await authenticator.Value.GetTokenAsync(forceRefresh).ConfigureAwait(false); + return token.AccessToken; } private bool ShouldSetToken(HttpRequestMessage request) @@ -297,103 +178,5 @@ private static bool IsTrustedUrl(Uri uri) return false; } } - - /// - /// Represents an OAuth exception. - /// - public sealed class OAuthException : Exception - { - /// - /// Creates a new instance of the class. - /// - /// The OAuth response body or reason. - /// The exception thown during the OAuth request. - public OAuthException(string body, Exception inner) - : base(body, inner) - { - } - } - - private async Task RefreshTokenAsync() - { - var content = new FormUrlEncodedContent(new Dictionary() - { - { "grant_type", "client_credentials" }, - { "client_id", MicrosoftAppId }, - { "client_secret", MicrosoftAppPassword }, - { "scope", OAuthScope } - }); - - using (var response = await DefaultHttpClient.PostAsync(OAuthEndpoint, content).ConfigureAwait(false)) - { - string body = null; - try - { - response.EnsureSuccessStatusCode(); - body = await response.Content.ReadAsStringAsync().ConfigureAwait(false); - var oauthResponse = JsonConvert.DeserializeObject(body); - oauthResponse.expiration_time = DateTime.UtcNow.AddSeconds(oauthResponse.expires_in).Subtract(TimeSpan.FromSeconds(60)); - return oauthResponse; - } - catch (Exception error) - { - throw new OAuthException(body ?? response.ReasonPhrase, error); - } - } - } - - /// - /// Has the token expired? If so, then we await on every attempt to get a new token - /// - /// - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsTokenExpired(OAuthResponse token) - { - return DateTime.UtcNow > token.expiration_time; - } - - /// - /// has token reached half/life ? If so, we get more agressive about trying to refresh it in the background - /// - /// - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsTokenOld(OAuthResponse token) - { - var halfwayExpiration = (token.expiration_time - TimeSpan.FromSeconds(token.expires_in / 2)); - return DateTime.UtcNow > halfwayExpiration; - } - -#pragma warning disable IDE1006 - /// - /// Describes the structure of an OAuth access token response. - /// - /// - /// Member variables to this class follow the RFC Naming conventions, rather than C# naming conventions. - /// - protected class OAuthResponse - { - /// - /// Gets or sets the type of token. - /// - public string token_type { get; set; } - - /// - /// Gets or sets the time in seconds until the token expires. - /// - public int expires_in { get; set; } - - /// - /// Gets or sets the access token string. - /// - public string access_token { get; set; } - - /// - /// Gets or sets the time at which the token expires. - /// - public DateTime expiration_time { get; set; } - } -#pragma warning restore IDE1006 } } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs index 5bad8ead3f..c67f880897 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Net.Http; namespace Microsoft.Bot.Connector.Authentication { @@ -14,13 +15,14 @@ public class MicrosoftGovernmentAppCredentials : MicrosoftAppCredentials /// An empty set of credentials. /// public static new readonly MicrosoftGovernmentAppCredentials Empty = new MicrosoftGovernmentAppCredentials(null, null); - + /// /// Creates a new instance of the class. /// /// The Microsoft app ID. /// The Microsoft app password. - public MicrosoftGovernmentAppCredentials(string appId, string password) : base(appId, password) + /// Optional to be used when acquiring tokens. + public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClient customHttpClient = null) : base(appId, password, customHttpClient) { } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/OAuthConfiguration.cs b/libraries/Microsoft.Bot.Connector/Authentication/OAuthConfiguration.cs new file mode 100644 index 0000000000..770fcb3a14 --- /dev/null +++ b/libraries/Microsoft.Bot.Connector/Authentication/OAuthConfiguration.cs @@ -0,0 +1,22 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.Bot.Connector.Authentication +{ + /// + /// Configuration for OAuth client credential authentication. + /// + public class OAuthConfiguration + { + /// + /// OAuth Authority for authentication. + /// + public string Authority { get; set; } + + /// + /// OAuth scope for authentication. + /// + public string Scope { get; set; } + } +} diff --git a/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs b/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs new file mode 100644 index 0000000000..5008e71ec9 --- /dev/null +++ b/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs @@ -0,0 +1,54 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.Bot.Connector.Authentication +{ + public static class Retry + { + public static async Task Run(Func> task, Func retryExceptionHandler) + { + RetryParams retry = RetryParams.StopRetrying; + List exceptions = new List(); + int currentRetryCount = 0; + + do + { + try + { + return await task().ConfigureAwait(false); + } + catch (Exception ex) + { + exceptions.Add(ex); + + retry = retryExceptionHandler(ex, currentRetryCount); + } + + if (retry.ShouldRetry) + { + currentRetryCount++; + await Task.Delay(retry.RetryAfter.WithJitter()).ConfigureAwait(false); + } + + } while (retry.ShouldRetry); + + throw new AggregateException("Failed to acquire token for client credentials.", exceptions); + } + } + + public static class TimeSpanExtensions + { + private static Random random = new Random(); + + public static TimeSpan WithJitter(this TimeSpan delay, double multiplier = 0.1) + { + // Generate an uniform distribution between zero and 10% of the proposed delay and add it as + // random noise. The reason for this is that if there are multiple threads about to retry + // at the same time, it can overload the server again and trigger throttling again. + // By adding a bit of random noise, we distribute requests a across time. + return delay + TimeSpan.FromMilliseconds(random.NextDouble() * delay.TotalMilliseconds * 0.1); + } + } +} diff --git a/libraries/Microsoft.Bot.Connector/Authentication/RetryParams.cs b/libraries/Microsoft.Bot.Connector/Authentication/RetryParams.cs new file mode 100644 index 0000000000..6e563b6637 --- /dev/null +++ b/libraries/Microsoft.Bot.Connector/Authentication/RetryParams.cs @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.Bot.Connector.Authentication +{ + public class RetryParams + { + private const int maxRetries = 10; + private static readonly TimeSpan MaxDelay = TimeSpan.FromSeconds(10); + private static readonly TimeSpan DefaultBackOffTime = TimeSpan.FromMilliseconds(50); + + public static RetryParams StopRetrying { get; } = new RetryParams() { ShouldRetry = false }; + + public bool ShouldRetry { get; set; } + public TimeSpan RetryAfter { get; set; } + + public RetryParams() { } + + public RetryParams(TimeSpan retryAfter, bool shouldRetry = true) + { + ShouldRetry = shouldRetry; + RetryAfter = retryAfter; + + // We don't allow more than maxDelaySeconds seconds delay. + if (RetryAfter > MaxDelay) + { + // We don't want to throw here though - if the server asks for more delay + // than we are willing to, just enforce the upper bound for the delay + RetryAfter = MaxDelay; + } + } + + public static RetryParams DefaultBackOff(int retryCount) + { + if (retryCount < maxRetries) + { + return new RetryParams(DefaultBackOffTime); + } + else + { + return RetryParams.StopRetrying; + } + } + } +} diff --git a/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj b/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj index a575d84b33..18d5b240e4 100644 --- a/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj +++ b/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj @@ -56,6 +56,7 @@ + diff --git a/tests/Microsoft.Bot.Connector.Tests/GetTokenRefreshTests.cs b/tests/Microsoft.Bot.Connector.Tests/GetTokenRefreshTests.cs index 9ddfd0e617..9b7a0100ad 100644 --- a/tests/Microsoft.Bot.Connector.Tests/GetTokenRefreshTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/GetTokenRefreshTests.cs @@ -20,7 +20,7 @@ public GetTokenRefreshTests() [Fact] public async Task TokenTests_GetCredentialsWorks() { - MicrosoftAppCredentials credentials = new MicrosoftAppCredentials("12604f0f-bc92-4318-a6dd-aed704445ba4", "H_k}}7b75BEl+KY1"); + MicrosoftAppCredentials credentials = new MicrosoftAppCredentials("645cd89f-a83e-4af9-abb5-a454e917cbc4", "jvoMWRBA67:zjgePZ359_-_"); var result = await credentials.GetTokenAsync(); Assert.NotNull(result); } diff --git a/tests/Microsoft.Bot.Connector.Tests/MicrosoftGovernmentAppCredentialsTests.cs b/tests/Microsoft.Bot.Connector.Tests/MicrosoftGovernmentAppCredentialsTests.cs index 5af8798c29..c38345f1c6 100644 --- a/tests/Microsoft.Bot.Connector.Tests/MicrosoftGovernmentAppCredentialsTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/MicrosoftGovernmentAppCredentialsTests.cs @@ -36,14 +36,14 @@ public void GovernmentAuthenticationConstants_ChannelService_IsRight() public void GovernmentAuthenticationConstants_ToChannelFromBotLoginUrl_IsRight() { // This value should not change - Assert.Equal("https://login.microsoftonline.us/cab8a31a-1906-4287-a0d8-4eef66b95f6e/oauth2/v2.0/token", GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl); + Assert.Equal("https://login.microsoftonline.us/cab8a31a-1906-4287-a0d8-4eef66b95f6e", GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl); } [Fact] public void GovernmentAuthenticationConstants_ToChannelFromBotOAuthScope_IsRight() { // This value should not change - Assert.Equal("https://api.botframework.us/.default", GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope); + Assert.Equal("https://api.botframework.us", GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope); } [Fact] diff --git a/tests/Microsoft.Bot.Connector.Tests/RetryParamsTests.cs b/tests/Microsoft.Bot.Connector.Tests/RetryParamsTests.cs new file mode 100644 index 0000000000..70f2081eab --- /dev/null +++ b/tests/Microsoft.Bot.Connector.Tests/RetryParamsTests.cs @@ -0,0 +1,45 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Bot.Connector.Authentication; +using Xunit; + +namespace Microsoft.Bot.Connector.Tests +{ + public class RetryParamsTests + { + [Fact] + public void RetryParams_StopRetryingValidation() + { + RetryParams retryParams = RetryParams.StopRetrying; + Assert.False(retryParams.ShouldRetry); + } + + [Fact] + public void RetryParams_DefaultBackOffShouldRetryOnFirstRetry() + { + RetryParams retryParams = RetryParams.DefaultBackOff(0); + + // If this is the first time we retry, it should retry by default + Assert.True(retryParams.ShouldRetry); + Assert.Equal(TimeSpan.FromMilliseconds(50), retryParams.RetryAfter); + } + + [Fact] + public void RetryParams_DefaultBackOffShouldNotRetryAfter5Retries() + { + RetryParams retryParams = RetryParams.DefaultBackOff(10); + Assert.False(retryParams.ShouldRetry); + } + + [Fact] + public void RetryParams_DelayOutOfBounds() + { + RetryParams retryParams = new RetryParams(TimeSpan.FromSeconds(11), true); + + // RetryParams should enforce the upper bound on delay time + Assert.Equal(TimeSpan.FromSeconds(10), retryParams.RetryAfter); + } + } +} diff --git a/tests/Microsoft.Bot.Connector.Tests/RetryTests.cs b/tests/Microsoft.Bot.Connector.Tests/RetryTests.cs new file mode 100644 index 0000000000..0c9bc4752a --- /dev/null +++ b/tests/Microsoft.Bot.Connector.Tests/RetryTests.cs @@ -0,0 +1,90 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Bot.Connector.Authentication; +using Xunit; + +namespace Microsoft.Bot.Connector.Tests +{ + public class RetryTests + { + [Fact] + public async Task Retry_NoRetryWhenTaskSucceeds() + { + FaultyClass faultyClass = new FaultyClass() + { + ExceptionToThrow = null + }; + + var result = await Retry.Run( + task: () => faultyClass.FaultyTask(), + retryExceptionHandler: (ex, ct) => faultyClass.ExceptionHandler(ex, ct)); + + Assert.Null(faultyClass.ExceptionReceived); + Assert.Equal(1, faultyClass.CallCount); + } + + [Fact] + public async Task Retry_RetryThenSucceed() + { + FaultyClass faultyClass = new FaultyClass() + { + ExceptionToThrow = new ArgumentNullException(), + TriesUntilSuccess = 3 + }; + + var result = await Retry.Run( + task: () => faultyClass.FaultyTask(), + retryExceptionHandler: (ex, ct) => faultyClass.ExceptionHandler(ex, ct)); + + Assert.NotNull(faultyClass.ExceptionReceived); + Assert.Equal(3, faultyClass.CallCount); + } + + [Fact] + public async Task Retry_RetryUntilFailure() + { + FaultyClass faultyClass = new FaultyClass() + { + ExceptionToThrow = new ArgumentNullException(), + TriesUntilSuccess = 12 + }; + + await Assert.ThrowsAsync(async () => + await Retry.Run( + task: () => faultyClass.FaultyTask(), + retryExceptionHandler: (ex, ct) => faultyClass.ExceptionHandler(ex, ct))); + } + } + + public class FaultyClass + { + public Exception ExceptionToThrow { get; set; } + public Exception ExceptionReceived { get; set; } = null; + public int LatestRetryCount { get; set; } + public int CallCount { get; set; } = 0; + public int TriesUntilSuccess { get; set; } = 0; + + + public async Task FaultyTask() + { + CallCount++; + + if (CallCount < TriesUntilSuccess && ExceptionToThrow != null) + { + throw ExceptionToThrow; + } + + return string.Empty; + } + + public RetryParams ExceptionHandler(Exception ex, int currentRetryCount) + { + ExceptionReceived = ex; + LatestRetryCount = currentRetryCount; + + return RetryParams.DefaultBackOff(currentRetryCount); + } + } +} diff --git a/tests/Microsoft.Bot.Connector.Tests/SessionRecords/Connector.Tests.ConversationsTest/CreateDirectConversationAsyncWithValidData.json b/tests/Microsoft.Bot.Connector.Tests/SessionRecords/Connector.Tests.ConversationsTest/CreateDirectConversationAsyncWithValidData.json new file mode 100644 index 0000000000..16728781f9 --- /dev/null +++ b/tests/Microsoft.Bot.Connector.Tests/SessionRecords/Connector.Tests.ConversationsTest/CreateDirectConversationAsyncWithValidData.json @@ -0,0 +1,62 @@ +{ + "Entries": [ + { + "RequestUri": "/v3/conversations", + "EncodedRequestUri": "L3YzL2NvbnZlcnNhdGlvbnM=", + "RequestMethod": "POST", + "RequestBody": "{\r\n \"bot\": {\r\n \"id\": \"B21UTEF8S:T03CWQ0QB\"\r\n },\r\n \"members\": [\r\n {\r\n \"id\": \"U19KH8EHJ:T03CWQ0QB\"\r\n }\r\n ],\r\n \"activity\": {\r\n \"type\": \"message\",\r\n \"from\": {\r\n \"id\": \"B21UTEF8S:T03CWQ0QB\"\r\n },\r\n \"recipient\": {\r\n \"id\": \"U19KH8EHJ:T03CWQ0QB\"\r\n },\r\n \"membersAdded\": [],\r\n \"membersRemoved\": [],\r\n \"reactionsAdded\": [],\r\n \"reactionsRemoved\": [],\r\n \"text\": \"TEST Create Conversation\",\r\n \"attachments\": [],\r\n \"entities\": []\r\n }\r\n}", + "RequestHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Content-Length": [ + "486" + ], + "User-Agent": [ + "FxVersion/4.7.2600.0", + "OSName/Windows10Enterprise", + "OSVersion/6.3.16299", + "Microsoft.Bot.Connector.ConnectorClient/0.0.0.0", + "Microsoft-BotFramework/4.0", + "(BotBuilder .Net/0.0.0.0)" + ] + }, + "ResponseBody": "{\r\n \"activityId\": \"1516641076.000062\",\r\n \"id\": \"B21UTEF8S:T03CWQ0QB:D2369CT7C\"\r\n}", + "ResponseHeaders": { + "Content-Length": [ + "83" + ], + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Expires": [ + "-1" + ], + "Pragma": [ + "no-cache" + ], + "Request-Context": [ + "appId=cid-v1:6814484e-c0d5-40ea-9dba-74ff29ca4f62" + ], + "Strict-Transport-Security": [ + "max-age=31536000" + ], + "Cache-Control": [ + "no-cache" + ], + "Date": [ + "Mon, 22 Jan 2018 17:11:15 GMT" + ], + "Server": [ + "Microsoft-IIS/10.0" + ], + "X-Powered-By": [ + "ASP.NET" + ] + }, + "StatusCode": 200 + } + ], + "Names": {}, + "Variables": {} +} \ No newline at end of file diff --git a/tests/Microsoft.Bot.Connector.Tests/SessionRecords/Connector.Tests.ConversationsTest/CreateDirectConversationSyncWithValidData.json b/tests/Microsoft.Bot.Connector.Tests/SessionRecords/Connector.Tests.ConversationsTest/CreateDirectConversationSyncWithValidData.json new file mode 100644 index 0000000000..16728781f9 --- /dev/null +++ b/tests/Microsoft.Bot.Connector.Tests/SessionRecords/Connector.Tests.ConversationsTest/CreateDirectConversationSyncWithValidData.json @@ -0,0 +1,62 @@ +{ + "Entries": [ + { + "RequestUri": "/v3/conversations", + "EncodedRequestUri": "L3YzL2NvbnZlcnNhdGlvbnM=", + "RequestMethod": "POST", + "RequestBody": "{\r\n \"bot\": {\r\n \"id\": \"B21UTEF8S:T03CWQ0QB\"\r\n },\r\n \"members\": [\r\n {\r\n \"id\": \"U19KH8EHJ:T03CWQ0QB\"\r\n }\r\n ],\r\n \"activity\": {\r\n \"type\": \"message\",\r\n \"from\": {\r\n \"id\": \"B21UTEF8S:T03CWQ0QB\"\r\n },\r\n \"recipient\": {\r\n \"id\": \"U19KH8EHJ:T03CWQ0QB\"\r\n },\r\n \"membersAdded\": [],\r\n \"membersRemoved\": [],\r\n \"reactionsAdded\": [],\r\n \"reactionsRemoved\": [],\r\n \"text\": \"TEST Create Conversation\",\r\n \"attachments\": [],\r\n \"entities\": []\r\n }\r\n}", + "RequestHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Content-Length": [ + "486" + ], + "User-Agent": [ + "FxVersion/4.7.2600.0", + "OSName/Windows10Enterprise", + "OSVersion/6.3.16299", + "Microsoft.Bot.Connector.ConnectorClient/0.0.0.0", + "Microsoft-BotFramework/4.0", + "(BotBuilder .Net/0.0.0.0)" + ] + }, + "ResponseBody": "{\r\n \"activityId\": \"1516641076.000062\",\r\n \"id\": \"B21UTEF8S:T03CWQ0QB:D2369CT7C\"\r\n}", + "ResponseHeaders": { + "Content-Length": [ + "83" + ], + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Expires": [ + "-1" + ], + "Pragma": [ + "no-cache" + ], + "Request-Context": [ + "appId=cid-v1:6814484e-c0d5-40ea-9dba-74ff29ca4f62" + ], + "Strict-Transport-Security": [ + "max-age=31536000" + ], + "Cache-Control": [ + "no-cache" + ], + "Date": [ + "Mon, 22 Jan 2018 17:11:15 GMT" + ], + "Server": [ + "Microsoft-IIS/10.0" + ], + "X-Powered-By": [ + "ASP.NET" + ] + }, + "StatusCode": 200 + } + ], + "Names": {}, + "Variables": {} +} \ No newline at end of file