diff --git a/libraries/Microsoft.Bot.Connector/AttachmentsEx.cs b/libraries/Microsoft.Bot.Connector/AttachmentsEx.cs index e095448b77..926d3ce54a 100644 --- a/libraries/Microsoft.Bot.Connector/AttachmentsEx.cs +++ b/libraries/Microsoft.Bot.Connector/AttachmentsEx.cs @@ -32,7 +32,9 @@ public partial class Attachments /// id of the attachment. /// default is "original". /// uri. +#pragma warning disable CA1055 // Uri return values should not be strings (we can't change this without breaking binary compat) public string GetAttachmentUri(string attachmentId, string viewId = "original") +#pragma warning restore CA1055 // Uri return values should not be strings { if (attachmentId == null) { @@ -41,7 +43,7 @@ public string GetAttachmentUri(string attachmentId, string viewId = "original") // Construct URL var baseUrl = this.Client.BaseUri.AbsoluteUri; - var url = new Uri(new Uri(baseUrl + (baseUrl.EndsWith("/") ? string.Empty : "/")), "v3/attachments/{attachmentId}/views/{viewId}").ToString(); + var url = new Uri(new Uri(baseUrl + (baseUrl.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/", StringComparison.OrdinalIgnoreCase)), "v3/attachments/{attachmentId}/views/{viewId}").ToString(); url = url.Replace("{attachmentId}", Uri.EscapeDataString(attachmentId)); url = url.Replace("{viewId}", Uri.EscapeDataString(viewId)); return url; diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs b/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs index 6167241265..671983d813 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AdalAuthenticator.cs @@ -90,7 +90,7 @@ public async Task GetTokenAsync(bool forceRefresh = false) async Task IAuthenticator.GetTokenAsync(bool forceRefresh) { - var result = await GetTokenAsync(forceRefresh); + var result = await GetTokenAsync(forceRefresh).ConfigureAwait(false); return new AuthenticatorResult() { AccessToken = result.AccessToken, @@ -98,102 +98,7 @@ async Task IAuthenticator.GetTokenAsync(bool forceRefresh) }; } - 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.Wait(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 - AuthenticationResult authResult = null; - - // Password based auth - if (clientCredential != null) - { - authResult = await authContext.AcquireTokenAsync(authConfig.Scope, this.clientCredential).ConfigureAwait(false); - } - - // Certificate based auth - else if (clientCertificate != null) - { - authResult = await authContext.AcquireTokenAsync(authConfig.Scope, clientCertificate, sendX5c: this.clientCertSendX5c).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 authResult; - } - 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) - { - ReleaseSemaphore(); - } - } - } - - private void ReleaseSemaphore() - { - try - { - tokenRefreshSemaphore.Release(); - } - catch (SemaphoreFullException) - { - // We should not be hitting this after switching to SemaphoreSlim, but if we do hit it everything will keep working. - // Logging to have clear knowledge of whether this is happening. - logger?.LogWarning("Attempted to release a full semaphore."); - } - - // Any exception other than SemaphoreFullException should be thrown right away - } - - private RetryParams HandleAdalException(Exception ex, int currentRetryCount) + private static RetryParams HandleAdalException(Exception ex, int currentRetryCount) { if (IsAdalServiceUnavailable(ex)) { @@ -220,7 +125,7 @@ private RetryParams HandleAdalException(Exception ex, int currentRetryCount) } } - private bool IsAdalServiceUnavailable(Exception ex) + private static bool IsAdalServiceUnavailable(Exception ex) { AdalServiceException adalServiceException = ex as AdalServiceException; if (adalServiceException == null) @@ -238,7 +143,7 @@ private bool IsAdalServiceUnavailable(Exception ex) /// /// Exception. /// True if the exception represents an invalid request. - private bool IsAdalServiceInvalidRequest(Exception ex) + private static bool IsAdalServiceInvalidRequest(Exception ex) { if (ex is AdalServiceException adal) { @@ -253,7 +158,7 @@ private bool IsAdalServiceInvalidRequest(Exception ex) return false; } - private RetryParams ComputeAdalRetry(Exception ex) + private static RetryParams ComputeAdalRetry(Exception ex) { if (ex is AdalServiceException) { @@ -282,6 +187,22 @@ private RetryParams ComputeAdalRetry(Exception ex) return RetryParams.DefaultBackOff(0); } + + private void ReleaseSemaphore() + { + try + { + tokenRefreshSemaphore.Release(); + } + catch (SemaphoreFullException) + { + // We should not be hitting this after switching to SemaphoreSlim, but if we do hit it everything will keep working. + // Logging to have clear knowledge of whether this is happening. + logger?.LogWarning("Attempted to release a full semaphore."); + } + + // Any exception other than SemaphoreFullException should be thrown right away + } private void Initialize(OAuthConfiguration configurationOAuth, HttpClient customHttpClient) { @@ -296,9 +217,83 @@ private void Initialize(OAuthConfiguration configurationOAuth, HttpClient custom } } - private bool UseCertificate() + private async Task AcquireTokenAsync(bool forceRefresh = false) { - return this.clientCertificate != null; + 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.Wait(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 + AuthenticationResult authResult = null; + + // Password based auth + if (clientCredential != null) + { + authResult = await authContext.AcquireTokenAsync(authConfig.Scope, this.clientCredential).ConfigureAwait(false); + } + + // Certificate based auth + else if (clientCertificate != null) + { + authResult = await authContext.AcquireTokenAsync(authConfig.Scope, clientCertificate, sendX5c: this.clientCertSendX5c).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 authResult; + } + 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) + { + ReleaseSemaphore(); + } + } } } } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs index 2d2597678e..9120d30ee2 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Net.Http; using System.Net.Http.Headers; using System.Threading; @@ -29,7 +30,7 @@ public abstract class AppCredentials : ServiceClientCredentials /// /// Authenticator abstraction used to obtain tokens through the Client Credentials OAuth 2.0 flow. /// - private readonly Lazy authenticator; + private Lazy authenticator; /// /// Initializes a new instance of the class. @@ -52,7 +53,6 @@ public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClie public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClient = null, ILogger logger = null, string oAuthScope = null) { OAuthScope = string.IsNullOrWhiteSpace(oAuthScope) ? AuthenticationConstants.ToChannelFromBotOAuthScope : oAuthScope; - authenticator = BuildIAuthenticator(); ChannelAuthTenant = channelAuthTenant; CustomHttpClient = customHttpClient; Logger = logger; @@ -84,7 +84,7 @@ public string ChannelAuthTenant set { // Advanced user only, see https://aka.ms/bots/tenant-restriction - var endpointUrl = string.Format(AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, value); + var endpointUrl = string.Format(CultureInfo.InvariantCulture, AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, value); if (Uri.TryCreate(endpointUrl, UriKind.Absolute, out var result)) { @@ -103,7 +103,7 @@ public string ChannelAuthTenant /// /// The OAuth endpoint to use. /// - public virtual string OAuthEndpoint => string.Format(AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, ChannelAuthTenant); + public virtual string OAuthEndpoint => string.Format(CultureInfo.InvariantCulture, AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, ChannelAuthTenant); /// /// Gets the OAuth scope to use. @@ -200,6 +200,7 @@ public override async Task ProcessHttpRequestAsync(HttpRequestMessage request, C /// If the task is successful, the result contains the access token string. public async Task GetTokenAsync(bool forceRefresh = false) { + authenticator ??= BuildIAuthenticator(); var token = await authenticator.Value.GetTokenAsync(forceRefresh).ConfigureAwait(false); return token.AccessToken; } @@ -242,7 +243,7 @@ private static bool IsTrustedUrl(Uri uri) } } - private bool ShouldSetToken(HttpRequestMessage request) + private static bool ShouldSetToken(HttpRequestMessage request) { if (IsTrustedUrl(request.RequestUri)) { diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConfiguration.cs b/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConfiguration.cs index 60aafc579e..edd740af7c 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConfiguration.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AuthenticationConfiguration.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; + namespace Microsoft.Bot.Connector.Authentication { /// @@ -12,7 +14,9 @@ namespace Microsoft.Bot.Connector.Authentication /// public class AuthenticationConfiguration { - public string[] RequiredEndorsements { get; set; } = { }; +#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat) + public string[] RequiredEndorsements { get; set; } = Array.Empty(); +#pragma warning restore CA1819 // Properties should not return arrays /// /// Gets or sets an instance used to validate the identity claims. diff --git a/libraries/Microsoft.Bot.Connector/Authentication/ChannelValidation.cs b/libraries/Microsoft.Bot.Connector/Authentication/ChannelValidation.cs index f4c0f57a9f..138ed4c9cc 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/ChannelValidation.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/ChannelValidation.cs @@ -35,7 +35,9 @@ public static class ChannelValidation /// /// The default endpoint that is used for Open ID Metadata requests. /// +#pragma warning disable CA1056 // Uri properties should not be strings (we can't change this without breaking binary compat) public static string OpenIdMetadataUrl { get; set; } = AuthenticationConstants.ToBotFromChannelOpenIdMetadataUrl; +#pragma warning restore CA1056 // Uri properties should not be strings /// /// Validate the incoming Auth Header as a token sent from the Bot Framework Service. @@ -86,7 +88,7 @@ public static async Task AuthenticateChannelToken(string authHea OpenIdMetadataUrl, AuthenticationConstants.AllowedSigningAlgorithms); - var identity = await tokenExtractor.GetIdentityAsync(authHeader, channelId, authConfig.RequiredEndorsements); + var identity = await tokenExtractor.GetIdentityAsync(authHeader, channelId, authConfig.RequiredEndorsements).ConfigureAwait(false); if (identity == null) { // No valid identity. Not Authorized. @@ -123,7 +125,7 @@ public static async Task AuthenticateChannelToken(string authHea throw new UnauthorizedAccessException(); } - if (!await credentials.IsValidAppIdAsync(appIdFromClaim)) + if (!await credentials.IsValidAppIdAsync(appIdFromClaim).ConfigureAwait(false)) { // The AppId is not valid. Not Authorized. throw new UnauthorizedAccessException($"Invalid AppId passed on token: {appIdFromClaim}"); @@ -167,7 +169,7 @@ public static async Task AuthenticateChannelToken(string authHea throw new ArgumentNullException(nameof(authConfig)); } - var identity = await AuthenticateChannelToken(authHeader, credentials, httpClient, channelId, authConfig); + var identity = await AuthenticateChannelToken(authHeader, credentials, httpClient, channelId, authConfig).ConfigureAwait(false); var serviceUrlClaim = identity.Claims.FirstOrDefault(claim => claim.Type == AuthenticationConstants.ServiceUrlClaim)?.Value; if (string.IsNullOrWhiteSpace(serviceUrlClaim)) @@ -176,7 +178,7 @@ public static async Task AuthenticateChannelToken(string authHea throw new UnauthorizedAccessException(); } - if (!string.Equals(serviceUrlClaim, serviceUrl)) + if (!string.Equals(serviceUrlClaim, serviceUrl, StringComparison.OrdinalIgnoreCase)) { // Claim must match. Not Authorized. throw new UnauthorizedAccessException(); diff --git a/libraries/Microsoft.Bot.Connector/Authentication/EndorsementsRetriever.cs b/libraries/Microsoft.Bot.Connector/Authentication/EndorsementsRetriever.cs index 9b69b43634..8aeaf04d75 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/EndorsementsRetriever.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/EndorsementsRetriever.cs @@ -73,7 +73,7 @@ public async Task>> GetConfigurationAsync(st throw new ArgumentNullException(nameof(retriever)); } - var jsonDocument = await retriever.GetDocumentAsync(address, cancellationToken); + var jsonDocument = await retriever.GetDocumentAsync(address, cancellationToken).ConfigureAwait(false); var configurationRoot = JObject.Parse(jsonDocument); var keys = configurationRoot["keys"]?.Value(); @@ -121,14 +121,14 @@ public async Task GetDocumentAsync(string address, CancellationToken can throw new ArgumentNullException(nameof(address)); } - using (var documentResponse = await _httpClient.GetAsync(address, cancellationToken)) + using (var documentResponse = await _httpClient.GetAsync(address, cancellationToken).ConfigureAwait(false)) { if (!documentResponse.IsSuccessStatusCode) { throw new Exception($"An non-success status code of {documentResponse.StatusCode} was received while fetching the endorsements document."); } - var json = await documentResponse.Content.ReadAsStringAsync(); + var json = await documentResponse.Content.ReadAsStringAsync().ConfigureAwait(false); if (string.IsNullOrWhiteSpace(json)) { @@ -143,14 +143,14 @@ public async Task GetDocumentAsync(string address, CancellationToken can return string.Empty; } - using (var keysResponse = await _httpClient.GetAsync(keysUrl, cancellationToken)) + using (var keysResponse = await _httpClient.GetAsync(keysUrl, cancellationToken).ConfigureAwait(false)) { if (!keysResponse.IsSuccessStatusCode) { throw new Exception($"An non-success status code of {keysResponse.StatusCode} was received while fetching the web key set document."); } - return await keysResponse.Content.ReadAsStringAsync(); + return await keysResponse.Content.ReadAsStringAsync().ConfigureAwait(false); } } } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/EnterpriseChannelValidation.cs b/libraries/Microsoft.Bot.Connector/Authentication/EnterpriseChannelValidation.cs index f24f13a369..b6492a0c46 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/EnterpriseChannelValidation.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/EnterpriseChannelValidation.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Globalization; using System.Linq; using System.Net.Http; using System.Security.Claims; @@ -71,7 +72,7 @@ public static async Task AuthenticateChannelToken(string authHea var tokenExtractor = new JwtTokenExtractor( httpClient, ToBotFromEnterpriseChannelTokenValidationParameters, - string.Format(AuthenticationConstants.ToBotFromEnterpriseChannelOpenIdMetadataUrlFormat, channelService), + string.Format(CultureInfo.InvariantCulture, AuthenticationConstants.ToBotFromEnterpriseChannelOpenIdMetadataUrlFormat, channelService), AuthenticationConstants.AllowedSigningAlgorithms); var identity = await tokenExtractor.GetIdentityAsync(authHeader, channelId, authConfig.RequiredEndorsements).ConfigureAwait(false); @@ -119,7 +120,7 @@ public static async Task ValidateIdentity(ClaimsIdentity identity, ICredentialPr throw new UnauthorizedAccessException(); } - if (!await credentials.IsValidAppIdAsync(appIdFromClaim)) + if (!await credentials.IsValidAppIdAsync(appIdFromClaim).ConfigureAwait(false)) { // The AppId is not valid. Not Authorized. throw new UnauthorizedAccessException($"Invalid AppId passed on token: {appIdFromClaim}"); @@ -134,7 +135,7 @@ public static async Task ValidateIdentity(ClaimsIdentity identity, ICredentialPr throw new UnauthorizedAccessException(); } - if (!string.Equals(serviceUrlClaim, serviceUrl)) + if (!string.Equals(serviceUrlClaim, serviceUrl, StringComparison.OrdinalIgnoreCase)) { // Claim must match. Not Authorized. throw new UnauthorizedAccessException(); diff --git a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentChannelValidation.cs b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentChannelValidation.cs index 533508e14c..a7e0da3777 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentChannelValidation.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentChannelValidation.cs @@ -29,7 +29,9 @@ public sealed class GovernmentChannelValidation ValidateIssuerSigningKey = true, }; +#pragma warning disable CA1056 // Uri properties should not be strings (we can't change this without breaking binary compat) public static string OpenIdMetadataUrl { get; set; } = GovernmentAuthenticationConstants.ToBotFromChannelOpenIdMetadataUrl; +#pragma warning restore CA1056 // Uri properties should not be strings /// /// Validate the incoming Auth Header as a token sent from a Bot Framework Government Channel Service. @@ -124,7 +126,7 @@ public static async Task ValidateIdentity(ClaimsIdentity identity, ICredentialPr throw new UnauthorizedAccessException(); } - if (!await credentials.IsValidAppIdAsync(appIdFromClaim)) + if (!await credentials.IsValidAppIdAsync(appIdFromClaim).ConfigureAwait(false)) { // The AppId is not valid. Not Authorized. throw new UnauthorizedAccessException($"Invalid AppId passed on token: {appIdFromClaim}"); @@ -139,7 +141,7 @@ public static async Task ValidateIdentity(ClaimsIdentity identity, ICredentialPr throw new UnauthorizedAccessException(); } - if (!string.Equals(serviceUrlClaim, serviceUrl)) + if (!string.Equals(serviceUrlClaim, serviceUrl, StringComparison.OrdinalIgnoreCase)) { // Claim must match. Not Authorized. throw new UnauthorizedAccessException(); diff --git a/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenExtractor.cs b/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenExtractor.cs index 5c2eaf8709..920016d3ea 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenExtractor.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenExtractor.cs @@ -118,7 +118,7 @@ public JwtTokenExtractor( public async Task GetIdentityAsync(string authorizationHeader, string channelId) { - return await GetIdentityAsync(authorizationHeader, channelId, new string[] { }).ConfigureAwait(false); + return await GetIdentityAsync(authorizationHeader, channelId, Array.Empty()).ConfigureAwait(false); } public async Task GetIdentityAsync(string authorizationHeader, string channelId, string[] requiredEndorsements) @@ -139,7 +139,7 @@ public async Task GetIdentityAsync(string authorizationHeader, s public async Task GetIdentityAsync(string scheme, string parameter, string channelId) { - return await GetIdentityAsync(scheme, parameter, channelId, new string[] { }).ConfigureAwait(false); + return await GetIdentityAsync(scheme, parameter, channelId, Array.Empty()).ConfigureAwait(false); } public async Task GetIdentityAsync(string scheme, string parameter, string channelId, string[] requiredEndorsements) @@ -231,7 +231,7 @@ private async Task ValidateTokenAsync(string jwtToken, string c // Validate Channel / Token Endorsements. For this, the channelID present on the Activity // needs to be matched by an endorsement. var keyId = (string)parsedJwtToken?.Header?[AuthenticationConstants.KeyIdHeader]; - var endorsements = await _endorsementsData.GetConfigurationAsync(); + var endorsements = await _endorsementsData.GetConfigurationAsync().ConfigureAwait(false); // Note: On the Emulator Code Path, the endorsements collection is empty so the validation code // below won't run. This is normal. diff --git a/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenValidation.cs b/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenValidation.cs index 8dba51dfdf..fa22135d94 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenValidation.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenValidation.cs @@ -121,7 +121,7 @@ public static async Task ValidateAuthHeader(string authHeader, I httpClient = httpClient ?? _httpClient; - var identity = await AuthenticateToken(authHeader, credentials, channelProvider, channelId, authConfig, serviceUrl, httpClient); + var identity = await AuthenticateToken(authHeader, credentials, channelProvider, channelId, authConfig, serviceUrl, httpClient).ConfigureAwait(false); await ValidateClaimsAsync(authConfig, identity.Claims).ConfigureAwait(false); diff --git a/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs b/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs index c7a340d5e6..30d0a29315 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/Retry.cs @@ -21,10 +21,11 @@ public static async Task Run(Func> task, Func /// Validates JWT tokens sent to and from a Skill. /// +#pragma warning disable CA1052 // Static holder types should be Static or NotInheritable (we can't change this without breaking binary compat) public class SkillValidation +#pragma warning restore CA1052 // Static holder types should be Static or NotInheritable { /// /// TO SKILL FROM BOT and TO BOT FROM SKILL: Token validation parameters when connecting a bot to a skill. diff --git a/libraries/Microsoft.Bot.Connector/Authentication/ThrottleException.cs b/libraries/Microsoft.Bot.Connector/Authentication/ThrottleException.cs index abbfe785d2..c0289bac92 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/ThrottleException.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/ThrottleException.cs @@ -7,6 +7,20 @@ namespace Microsoft.Bot.Connector.Authentication { public class ThrottleException : Exception { + public ThrottleException() + { + } + + public ThrottleException(string message) + : base(message) + { + } + + public ThrottleException(string message, Exception innerException) + : base(message, innerException) + { + } + public RetryParams RetryParams { get; set; } } } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/TimeSpanExtensions.cs b/libraries/Microsoft.Bot.Connector/Authentication/TimeSpanExtensions.cs index 1420de0404..49ccaf2c8a 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/TimeSpanExtensions.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/TimeSpanExtensions.cs @@ -9,7 +9,9 @@ public static class TimeSpanExtensions { private static readonly Random _random = new Random(); +#pragma warning disable CA1801 // Review unused parameters (we can't change this without breaking binary compat) public static TimeSpan WithJitter(this TimeSpan delay, double multiplier = 0.1) +#pragma warning restore CA1801 // Review unused parameters { // 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 diff --git a/libraries/Microsoft.Bot.Connector/Channels.cs b/libraries/Microsoft.Bot.Connector/Channels.cs index 46ca6b0241..511792d1b7 100644 --- a/libraries/Microsoft.Bot.Connector/Channels.cs +++ b/libraries/Microsoft.Bot.Connector/Channels.cs @@ -6,7 +6,11 @@ namespace Microsoft.Bot.Connector /// /// Ids of channels supported by the Bot Builder. /// +#pragma warning disable CA1052 // Static holder types should be Static or NotInheritable (we can't change this without breaking binary compat) +#pragma warning disable CA1724 // Type names should not match namespaces (we can't change this without breaking binary compat) public class Channels +#pragma warning restore CA1724 // Type names should not match namespaces +#pragma warning restore CA1052 // Static holder types should be Static or NotInheritable { /// /// Console channel. diff --git a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs index db8802ecdc..d0e8117f3f 100644 --- a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs +++ b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs @@ -70,7 +70,9 @@ public ConnectorClient(Uri baseUri, MicrosoftAppCredentials credentials, HttpCli /// The HTTP client to use for this connector client. /// Optional, an array of objects to /// add to the HTTP client pipeline. +#pragma warning disable CA1801 // Review unused parameters (we can't change this without breaking binary compat) public ConnectorClient(Uri baseUri, ServiceClientCredentials credentials, HttpClient customHttpClient, bool addJwtTokenRefresher = true, params DelegatingHandler[] handlers) +#pragma warning restore CA1801 // Review unused parameters : this(baseUri, handlers) { this.Credentials = credentials; @@ -94,7 +96,9 @@ public ConnectorClient(Uri baseUri, ServiceClientCredentials credentials, HttpCl /// The HTTP client to use for this connector client. /// Optional, an array of objects to /// add to the HTTP client pipeline. +#pragma warning disable CA1801 // Review unused parameters (we can't remove the addJwtTokenRefresher parameter without breaking binary compat) public ConnectorClient(Uri baseUri, MicrosoftAppCredentials credentials, HttpClientHandler httpClientHandler, bool addJwtTokenRefresher = true, HttpClient customHttpClient = null, params DelegatingHandler[] handlers) +#pragma warning restore CA1801 // Review unused parameters : this(baseUri, httpClientHandler, handlers) { this.Credentials = credentials; diff --git a/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj b/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj index eec44969c4..80c84348cd 100644 --- a/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj +++ b/libraries/Microsoft.Bot.Connector/Microsoft.Bot.Connector.csproj @@ -1,10 +1,10 @@  - - $(LocalPackageVersion) - $(ReleasePackageVersion) - $(LocalPackageVersion) - $(ReleasePackageVersion) - Debug;Release + + $(LocalPackageVersion) + $(ReleasePackageVersion) + $(LocalPackageVersion) + $(ReleasePackageVersion) + Debug;Release true bin\$(Configuration)\netstandard2.0\Microsoft.Bot.Connector.xml true @@ -28,29 +28,32 @@ - - - - + + + + - - - - - - - - - + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + - - + \ No newline at end of file diff --git a/libraries/Microsoft.Bot.Connector/OAuthClientConfig.cs b/libraries/Microsoft.Bot.Connector/OAuthClientConfig.cs index bb2d21cb61..34ec524601 100644 --- a/libraries/Microsoft.Bot.Connector/OAuthClientConfig.cs +++ b/libraries/Microsoft.Bot.Connector/OAuthClientConfig.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Net; using System.Net.Http; using System.Threading; @@ -10,6 +11,7 @@ using Microsoft.Bot.Connector.Authentication; using Microsoft.Bot.Schema; using Microsoft.Rest; +using Microsoft.Rest.Serialization; using Newtonsoft.Json; namespace Microsoft.Bot.Connector @@ -45,7 +47,7 @@ public static async Task SendEmulateOAuthCardsAsync(OAuthClient client, bool emu string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("emulateOAuthCards", emulateOAuthCards); ServiceClientTracing.Enter(invocationId, client, "GetToken", tracingParameters); @@ -53,84 +55,82 @@ public static async Task SendEmulateOAuthCardsAsync(OAuthClient client, bool emu // Construct URL var baseUrl = client.BaseUri.AbsoluteUri; - var url = new Uri(new Uri(baseUrl + (baseUrl.EndsWith("/") ? string.Empty : "/")), "api/usertoken/emulateOAuthCards?emulate={emulate}").ToString(); + var url = new Uri(new Uri(baseUrl + (baseUrl.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/usertoken/emulateOAuthCards?emulate={emulate}").ToString(); url = url.Replace("{emulate}", emulateOAuthCards.ToString()); // Create HTTP transport objects - var httpRequest = new HttpRequestMessage(); - HttpResponseMessage httpResponse = null; - httpRequest.Method = new HttpMethod("POST"); - httpRequest.RequestUri = new System.Uri(url); - - var cancellationToken = CancellationToken.None; - - // Serialize Request - string requestContent = null; - - // Set Credentials - if (client.Credentials != null) + using (var httpRequest = new HttpRequestMessage()) { - cancellationToken.ThrowIfCancellationRequested(); - await client.Credentials.ProcessHttpRequestAsync(httpRequest, cancellationToken).ConfigureAwait(false); - } + httpRequest.Method = new HttpMethod("POST"); + httpRequest.RequestUri = new Uri(url); - // Send Request - if (shouldTrace) - { - ServiceClientTracing.SendRequest(invocationId, httpRequest); - } + var cancellationToken = CancellationToken.None; - cancellationToken.ThrowIfCancellationRequested(); - httpResponse = await client.HttpClient.SendAsync(httpRequest, cancellationToken).ConfigureAwait(false); - if (shouldTrace) - { - ServiceClientTracing.ReceiveResponse(invocationId, httpResponse); - } + // Serialize Request + string requestContent = null; - HttpStatusCode statusCode = httpResponse.StatusCode; - cancellationToken.ThrowIfCancellationRequested(); - string responseContent = null; - if (statusCode != HttpStatusCode.OK && statusCode != HttpStatusCode.NotFound) - { - var ex = new ErrorResponseException(string.Format("Operation returned an invalid status code '{0}'", statusCode)); - try + // Set Credentials + if (client.Credentials != null) { - responseContent = await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(false); - ErrorResponse errorBody = Rest.Serialization.SafeJsonConvert.DeserializeObject(responseContent, client.DeserializationSettings); - if (errorBody != null) - { - ex.Body = errorBody; - } - } - catch (JsonException) - { - // Ignore the exception + cancellationToken.ThrowIfCancellationRequested(); + await client.Credentials.ProcessHttpRequestAsync(httpRequest, cancellationToken).ConfigureAwait(false); } - ex.Request = new HttpRequestMessageWrapper(httpRequest, requestContent); - ex.Response = new HttpResponseMessageWrapper(httpResponse, responseContent); + // Send Request if (shouldTrace) { - ServiceClientTracing.Error(invocationId, ex); + ServiceClientTracing.SendRequest(invocationId, httpRequest); } - httpRequest.Dispose(); - if (httpResponse != null) + cancellationToken.ThrowIfCancellationRequested(); + using (var httpResponse = await client.HttpClient.SendAsync(httpRequest, cancellationToken).ConfigureAwait(false)) { - httpResponse.Dispose(); - } - - throw ex; - } + if (shouldTrace) + { + ServiceClientTracing.ReceiveResponse(invocationId, httpResponse); + } - // Create Result - var result = new HttpOperationResponse(); - result.Request = httpRequest; - result.Response = httpResponse; + var statusCode = httpResponse.StatusCode; + cancellationToken.ThrowIfCancellationRequested(); + string responseContent = null; + if (statusCode != HttpStatusCode.OK && statusCode != HttpStatusCode.NotFound) + { + var ex = new ErrorResponseException(string.Format(CultureInfo.InvariantCulture, "Operation returned an invalid status code '{0}'", statusCode)); + try + { + responseContent = await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(false); + var errorBody = SafeJsonConvert.DeserializeObject(responseContent, client.DeserializationSettings); + if (errorBody != null) + { + ex.Body = errorBody; + } + } + catch (JsonException) + { + // Ignore the exception + } + + ex.Request = new HttpRequestMessageWrapper(httpRequest, requestContent); + ex.Response = new HttpResponseMessageWrapper(httpResponse, responseContent); + if (shouldTrace) + { + ServiceClientTracing.Error(invocationId, ex); + } + + throw ex; + } - if (shouldTrace) - { - ServiceClientTracing.Exit(invocationId, result); + if (shouldTrace) + { + // Create and log result + using (var result = new HttpOperationResponse()) + { + result.Request = httpRequest; + result.Response = httpResponse; + ServiceClientTracing.Exit(invocationId, result); + } + } + } } } } diff --git a/libraries/Microsoft.Bot.Connector/OAuthClientOld.cs b/libraries/Microsoft.Bot.Connector/OAuthClientOld.cs index b570e696bb..cca40dbabb 100644 --- a/libraries/Microsoft.Bot.Connector/OAuthClientOld.cs +++ b/libraries/Microsoft.Bot.Connector/OAuthClientOld.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; @@ -18,8 +19,11 @@ namespace Microsoft.Bot.Connector /// /// Service client to handle requests to the Bot Framework API service. /// + [Obsolete("This class is deprecated, us OAuthClientConfig instead", error: true)] public class OAuthClientOld : ServiceClient { +#pragma warning disable CA2000 // Dispose objects before losing scope (this class is deprecated, we won't fix FxCop issues) +#pragma warning disable CA1801 // Review unused parameters (this class is deprecated, we won't fix FxCop issues) private readonly ConnectorClient _client; private readonly string _uri; @@ -83,7 +87,7 @@ public OAuthClientOld(ConnectorClient client, string uri) string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("userId", userId); tracingParameters.Add("connectionName", connectionName); @@ -93,7 +97,7 @@ public OAuthClientOld(ConnectorClient client, string uri) } // Construct URL - var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/") ? string.Empty : "/")), "api/usertoken/GetToken?userId={userId}&connectionName={connectionName}{magicCodeParam}").ToString(); + var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/usertoken/GetToken?userId={userId}&connectionName={connectionName}{magicCodeParam}").ToString(); tokenUrl = tokenUrl.Replace("{connectionName}", Uri.EscapeDataString(connectionName)); tokenUrl = tokenUrl.Replace("{userId}", Uri.EscapeDataString(userId)); if (!string.IsNullOrEmpty(magicCode)) @@ -107,6 +111,7 @@ public OAuthClientOld(ConnectorClient client, string uri) // Create HTTP transport objects var httpRequest = new HttpRequestMessage(); + HttpResponseMessage httpResponse = null; httpRequest.Method = new HttpMethod("GET"); httpRequest.RequestUri = new Uri(tokenUrl); @@ -184,7 +189,7 @@ public OAuthClientOld(ConnectorClient client, string uri) string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("userId", userId); tracingParameters.Add("connectionName", connectionName); @@ -193,7 +198,7 @@ public OAuthClientOld(ConnectorClient client, string uri) } // Construct URL - var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/") ? string.Empty : "/")), "api/usertoken/SignOut?&userId={userId}{connectionNameParam}").ToString(); + var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/usertoken/SignOut?&userId={userId}{connectionNameParam}").ToString(); var connectionNameUri = $"&connectionName={Uri.EscapeDataString(connectionName)}"; tokenUrl = tokenUrl.Replace( "{connectionNameParam}", @@ -259,7 +264,7 @@ public OAuthClientOld(ConnectorClient client, string uri) string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("state", state); tracingParameters.Add("finalRedirect", finalRedirect); @@ -268,7 +273,7 @@ public OAuthClientOld(ConnectorClient client, string uri) } // Construct URL - var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/") ? string.Empty : "/")), "api/botsignin/getsigninurl?&state={state}{finalRedirectParam}").ToString(); + var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/botsignin/getsigninurl?&state={state}{finalRedirectParam}").ToString(); tokenUrl = tokenUrl.Replace("{state}", state); tokenUrl = tokenUrl.Replace( "{finalRedirectParam}", @@ -330,7 +335,7 @@ public OAuthClientOld(ConnectorClient client, string uri) string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("userId", userId); tracingParameters.Add("includeFilter", includeFilter); @@ -339,7 +344,7 @@ public OAuthClientOld(ConnectorClient client, string uri) } // Construct URL - var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/") ? string.Empty : "/")), "api/usertoken/gettokenstatus?userId={userId}{includeFilterParam}").ToString(); + var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/usertoken/gettokenstatus?userId={userId}{includeFilterParam}").ToString(); tokenUrl = tokenUrl.Replace("{userId}", Uri.EscapeDataString(userId)); if (!string.IsNullOrEmpty(includeFilter)) { @@ -451,7 +456,7 @@ public OAuthClientOld(ConnectorClient client, string uri) string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("userId", userId); tracingParameters.Add("connectionName", connectionName); @@ -461,7 +466,7 @@ public OAuthClientOld(ConnectorClient client, string uri) } // Construct URL - var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/") ? string.Empty : "/")), "api/usertoken/GetAadTokens?userId={userId}&connectionName={connectionName}").ToString(); + var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/usertoken/GetAadTokens?userId={userId}&connectionName={connectionName}").ToString(); tokenUrl = tokenUrl.Replace("{userId}", Uri.EscapeDataString(userId)); tokenUrl = tokenUrl.Replace("{connectionName}", Uri.EscapeDataString(connectionName)); @@ -520,7 +525,7 @@ public OAuthClientOld(ConnectorClient client, string uri) } else { - var ex = new ErrorResponseException(string.Format("Operation returned an invalid status code '{0}'", statusCode)); + var ex = new ErrorResponseException(string.Format(CultureInfo.InvariantCulture, "Operation returned an invalid status code '{0}'", statusCode)); string responseContent = null; try { @@ -567,7 +572,7 @@ public async Task SendEmulateOAuthCardsAsync(bool emulateOAuthCards) string invocationId = null; if (shouldTrace) { - invocationId = ServiceClientTracing.NextInvocationId.ToString(); + invocationId = ServiceClientTracing.NextInvocationId.ToString(CultureInfo.InvariantCulture); Dictionary tracingParameters = new Dictionary(); tracingParameters.Add("emulateOAuthCards", emulateOAuthCards); ServiceClientTracing.Enter(invocationId, this, "SendEmulateOAuthCards", tracingParameters); @@ -576,7 +581,7 @@ public async Task SendEmulateOAuthCardsAsync(bool emulateOAuthCards) var cancellationToken = default(CancellationToken); // Construct URL - var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/") ? string.Empty : "/")), "api/usertoken/emulateOAuthCards?emulate={emulate}").ToString(); + var tokenUrl = new Uri(new Uri(_uri + (_uri.EndsWith("/", StringComparison.OrdinalIgnoreCase) ? string.Empty : "/")), "api/usertoken/emulateOAuthCards?emulate={emulate}").ToString(); tokenUrl = tokenUrl.Replace("{emulate}", emulateOAuthCards.ToString()); // Create HTTP transport objects @@ -602,5 +607,7 @@ public async Task SendEmulateOAuthCardsAsync(bool emulateOAuthCards) ServiceClientTracing.ReceiveResponse(invocationId, httpResponse); } } +#pragma warning restore CA2000 // Dispose objects before losing scope +#pragma warning restore CA1801 // Review unused parameters } }