diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs index 29720c3867..0c46900503 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs @@ -19,6 +19,7 @@ namespace Microsoft.Bot.Connector.Authentication /// public abstract class AppCredentials : ServiceClientCredentials { + [Obsolete] internal static readonly IDictionary TrustedHostNames = new Dictionary { // { "state.botframework.com", DateTime.MaxValue }, // deprecated state api @@ -145,9 +146,10 @@ public string ChannelAuthTenant /// /// The service URL. /// If expiration time is not provided, the expiration time will DateTime.UtcNow.AddDays(1). +#pragma warning disable CA1801 // Review unused parameters public static void TrustServiceUrl(string serviceUrl) +#pragma warning restore CA1801 // Review unused parameters { - TrustServiceUrl(serviceUrl, DateTime.UtcNow.Add(TimeSpan.FromDays(1))); } /// @@ -155,12 +157,10 @@ public static void TrustServiceUrl(string serviceUrl) /// /// The service URL. /// The expiration time after which this service url is not trusted anymore. +#pragma warning disable CA1801 // Review unused parameters public static void TrustServiceUrl(string serviceUrl, DateTime expirationTime) +#pragma warning restore CA1801 // Review unused parameters { - lock (TrustedHostNames) - { - TrustedHostNames[new Uri(serviceUrl).Host] = expirationTime; - } } /// @@ -168,14 +168,11 @@ public static void TrustServiceUrl(string serviceUrl, DateTime expirationTime) /// /// The service url. /// True if the host of the service url is trusted; False otherwise. +#pragma warning disable CA1801 // Review unused parameters public static bool IsTrustedServiceUrl(string serviceUrl) +#pragma warning restore CA1801 // Review unused parameters { - if (Uri.TryCreate(serviceUrl, UriKind.Absolute, out var uri)) - { - return IsTrustedUrl(uri, NullLogger.Instance); - } - - return false; + return true; } /// @@ -185,7 +182,7 @@ public static bool IsTrustedServiceUrl(string serviceUrl) /// A representing the asynchronous operation. public override async Task ProcessHttpRequestAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - if (ShouldSetToken(request, Logger)) + if (ShouldSetToken()) { var token = await GetTokenAsync().ConfigureAwait(false); request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); @@ -234,28 +231,7 @@ protected virtual Lazy BuildIAuthenticator() LazyThreadSafetyMode.ExecutionAndPublication); } - private static bool IsTrustedUrl(Uri uri, ILogger logger) - { - lock (TrustedHostNames) - { - if (TrustedHostNames.TryGetValue(uri.Host, out var trustedServiceUrlExpiration)) - { - // check if the trusted service url is still valid - if (trustedServiceUrlExpiration > DateTime.UtcNow.Subtract(TimeSpan.FromMinutes(5))) - { - return true; - } - - logger.LogWarning($"{typeof(AppCredentials).FullName}.IsTrustedUrl(): '{uri}' found in TrustedHostNames but it expired (Expiration is set to: {trustedServiceUrlExpiration}, current time is {DateTime.UtcNow})."); - return false; - } - - logger.LogWarning($"{typeof(AppCredentials).FullName}.IsTrustedUrl(): '{uri}' not found in TrustedHostNames."); - return false; - } - } - - private bool ShouldSetToken(HttpRequestMessage request, ILogger logger) + private bool ShouldSetToken() { if (string.IsNullOrEmpty(MicrosoftAppId) || MicrosoftAppId == AuthenticationConstants.AnonymousSkillAppId) { @@ -263,13 +239,7 @@ private bool ShouldSetToken(HttpRequestMessage request, ILogger logger) return false; } - if (IsTrustedUrl(request.RequestUri, logger)) - { - return true; - } - - logger.LogWarning($"{typeof(AppCredentials).FullName}.ShouldSetToken(): '{request.RequestUri.Authority}' is not trusted and JwtToken cannot be sent to it."); - return false; + return true; } } } diff --git a/tests/Microsoft.Bot.Builder.Tests/BotFrameworkAdapterTests.cs b/tests/Microsoft.Bot.Builder.Tests/BotFrameworkAdapterTests.cs index 52cd79524e..2cf78b363d 100644 --- a/tests/Microsoft.Bot.Builder.Tests/BotFrameworkAdapterTests.cs +++ b/tests/Microsoft.Bot.Builder.Tests/BotFrameworkAdapterTests.cs @@ -305,18 +305,12 @@ public async Task ContinueConversationAsyncWithoutAudience() var scope = turnContext.TurnState.Get(BotAdapter.OAuthScopeKey); Assert.Equal(AuthenticationConstants.ToChannelFromBotOAuthScope, scope); - // Ensure the serviceUrl was added to the trusted hosts - Assert.True(AppCredentials.TrustedHostNames.ContainsKey(new Uri(channelServiceUrl).Host)); - return Task.CompletedTask; }); // Create ConversationReference to send a proactive message from Skill1 to a channel var refs = new ConversationReference(serviceUrl: channelServiceUrl); - // Ensure the serviceUrl is NOT in the trusted hosts - Assert.False(AppCredentials.TrustedHostNames.ContainsKey(new Uri(channelServiceUrl).Host)); - await adapter.ContinueConversationAsync(skillsIdentity, refs, callback, default); } @@ -368,18 +362,12 @@ public async Task ContinueConversationAsyncWithAudience() var scope = turnContext.TurnState.Get(BotAdapter.OAuthScopeKey); Assert.Equal(skill2AppId, scope); - // Ensure the serviceUrl was added to the trusted hosts - Assert.True(AppCredentials.TrustedHostNames.ContainsKey(new Uri(skill2ServiceUrl).Host)); - return Task.CompletedTask; }); // Create ConversationReference to send a proactive message from Skill1 to Skill2 var refs = new ConversationReference(serviceUrl: skill2ServiceUrl); - // Ensure the serviceUrl is NOT in the trusted hosts - Assert.False(AppCredentials.TrustedHostNames.ContainsKey(new Uri(skill2ServiceUrl).Host)); - await adapter.ContinueConversationAsync(skillsIdentity, refs, skill2AppId, callback, default); } diff --git a/tests/Microsoft.Bot.Builder.Tests/Teams/TeamsInfoTests.cs b/tests/Microsoft.Bot.Builder.Tests/Teams/TeamsInfoTests.cs index b457947357..827ee0064f 100644 --- a/tests/Microsoft.Bot.Builder.Tests/Teams/TeamsInfoTests.cs +++ b/tests/Microsoft.Bot.Builder.Tests/Teams/TeamsInfoTests.cs @@ -27,8 +27,9 @@ public async Task TestSendMessageToTeamsChannelAsync() // Set a special base address so then we can make sure the connector client is honoring this http client customHttpClient.BaseAddress = baseUri; - var connectorClient = new ConnectorClient(new Uri("https://test.coffee"), new MicrosoftAppCredentials("big-guid-here", "appPasswordHere"), customHttpClient); + var connectorClient = new ConnectorClient(new Uri("https://test.coffee"), MicrosoftAppCredentials.Empty, customHttpClient); + var activity = new Activity { Type = "message", @@ -42,8 +43,8 @@ public async Task TestSendMessageToTeamsChannelAsync() }, }, }; - - var turnContext = new TurnContext(new BotFrameworkAdapter(new SimpleCredentialProvider("big-guid-here", "appPasswordHere"), customHttpClient: customHttpClient), activity); + + var turnContext = new TurnContext(new BotFrameworkAdapter(new SimpleCredentialProvider(), customHttpClient: customHttpClient), activity); turnContext.TurnState.Add(connectorClient); turnContext.Activity.ServiceUrl = "https://test.coffee"; var handler = new TestTeamsActivityHandler(); @@ -301,7 +302,7 @@ private async Task CallSendMessageToTeamsChannelAsync(ITurnContext turnContext) { var message = MessageFactory.Text("hi"); var channelId = "channelId123"; - var creds = new MicrosoftAppCredentials("big-guid-here", "appPasswordHere"); + var creds = new MicrosoftAppCredentials(string.Empty, string.Empty); var cancelToken = new CancellationToken(); var reference = await TeamsInfo.SendMessageToTeamsChannelAsync(turnContext, message, channelId, creds, cancelToken); diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/JwtTokenValidationTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/JwtTokenValidationTests.cs index 85b3628554..dbd14f2778 100644 --- a/tests/Microsoft.Bot.Connector.Tests/Authentication/JwtTokenValidationTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/JwtTokenValidationTests.cs @@ -109,26 +109,6 @@ await JwtTokenValidation.AuthenticateRequest( Assert.True(AppCredentials.IsTrustedServiceUrl("https://smba.trafficmanager.net/amer-client-ss.msg/")); } - /// - /// Tests with a valid Token and invalid service url; and ensures that Service url is NOT added to Trusted service url list. - /// - [Fact] - public async void Channel_MsaHeader_Invalid_ServiceUrlShouldNotBeTrusted() - { - string header = $"Bearer {await new MicrosoftAppCredentials("2cd87869-38a0-4182-9251-d056e8f0ac24", "2.30Vs3VQLKt974F").GetTokenAsync()}"; - var credentials = new SimpleCredentialProvider("7f74513e-6f96-4dbc-be9d-9a81fea22b88", string.Empty); - - await Assert.ThrowsAsync( - async () => await JwtTokenValidation.AuthenticateRequest( - new Activity { ServiceUrl = "https://webchat.botframework.com/" }, - header, - credentials, - new SimpleChannelProvider(), - emptyClient)); - - Assert.False(MicrosoftAppCredentials.IsTrustedServiceUrl("https://webchat.botframework.com/")); - } - /// /// Tests with no authentication header and makes sure the service URL is not added to the trusted list. /// @@ -174,25 +154,6 @@ public async void Channel_AuthenticationDisabledAndSkill_ShouldBeAnonymous() Assert.Equal(AuthenticationConstants.AnonymousSkillAppId, JwtTokenValidation.GetAppIdFromClaims(claimsPrincipal.Claims)); } - /// - /// Tests with no authentication header and makes sure the service URL is not added to the trusted list. - /// - [Fact] - public async void Channel_AuthenticationDisabled_ServiceUrlShouldNotBeTrusted() - { - var header = string.Empty; - var credentials = new SimpleCredentialProvider(); - - var claimsPrincipal = await JwtTokenValidation.AuthenticateRequest( - new Activity { ServiceUrl = "https://webchat.botframework.com/" }, - header, - credentials, - new SimpleChannelProvider(), - emptyClient); - - Assert.False(MicrosoftAppCredentials.IsTrustedServiceUrl("https://webchat.botframework.com/")); - } - [Fact] public async void Emulator_AuthHeader_CorrectAppIdAndServiceUrl_WithGovChannelService_ShouldValidate() {