From 159126d62c565b71a97b89f35f98cc4d94984917 Mon Sep 17 00:00:00 2001 From: fangyangci Date: Wed, 13 Dec 2023 22:47:35 +0800 Subject: [PATCH 1/4] fixUSGovSingleTenant --- .../Authentication/AppCredentials.cs | 2 +- .../GovernmentAuthenticationConstants.cs | 12 ++++++++++++ .../MicrosoftGovernmentAppCredentials.cs | 15 ++++++++++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs index 012bb644d6..b1f5b12c49 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs @@ -74,7 +74,7 @@ public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClie /// /// Tenant to be used for channel authentication. /// - public string ChannelAuthTenant + public virtual string ChannelAuthTenant { get => string.IsNullOrEmpty(AuthTenant) ? AuthenticationConstants.DefaultChannelAuthTenant : AuthTenant; set diff --git a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs index e961808ffd..5a8d4b236d 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs @@ -18,6 +18,18 @@ public static class GovernmentAuthenticationConstants /// public const string ToChannelFromBotLoginUrl = "https://login.microsoftonline.us/MicrosoftServices.onmicrosoft.us"; + /// + /// TO CHANNEL FROM BOT: Login URL template string. Bot developer may specify + /// which tenant to obtain an access token from. By default, the channels only + /// accept tokens from "MicrosoftServices.onmicrosoft.us". For more details see https://aka.ms/bots/tenant-restriction. + /// + public const string ToChannelFromBotLoginUrlTemplate = "https://login.microsoftonline.us/{0}"; + + /// + /// The default tenant to acquire bot to channel token from. + /// + public const string DefaultChannelAuthTenant = "MicrosoftServices.onmicrosoft.us"; + /// /// TO GOVERNMENT CHANNEL FROM BOT: OAuth scope to request. /// diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs index 7839abbc11..42dc9da49b 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Globalization; using System.Net.Http; using Microsoft.Extensions.Logging; @@ -66,6 +67,18 @@ public MicrosoftGovernmentAppCredentials(string appId, string password, string t { } + /// + /// Gets or sets tenant to be used for channel authentication. + /// + /// + /// Tenant to be used for channel authentication. + /// + public override string ChannelAuthTenant + { + get => string.IsNullOrEmpty(AuthTenant) ? GovernmentAuthenticationConstants.DefaultChannelAuthTenant : AuthTenant; + set => base.ChannelAuthTenant = value; + } + /// /// Gets the OAuth endpoint to use. /// @@ -74,7 +87,7 @@ public MicrosoftGovernmentAppCredentials(string appId, string password, string t /// public override string OAuthEndpoint { - get { return GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl; } + get => string.Format(CultureInfo.InvariantCulture, GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate, ChannelAuthTenant); } } } From 810b38289f88ddb2988445666a452f7b962cd9f0 Mon Sep 17 00:00:00 2001 From: fangyangci Date: Sat, 16 Dec 2023 07:09:21 +0800 Subject: [PATCH 2/4] Add UT --- .../Authentication/AppCredentials.cs | 46 ++++++++++---- .../GovernmentAuthenticationConstants.cs | 4 +- ...vernmentCloudBotFrameworkAuthentication.cs | 2 +- .../Authentication/MicrosoftAppCredentials.cs | 62 +------------------ .../MicrosoftGovernmentAppCredentials.cs | 61 ++++-------------- .../MsalServiceClientCredentialsFactory.cs | 16 +++-- .../PasswordServiceClientCredentialFactory.cs | 4 +- .../MicrosoftAppCredentialsTests.cs | 58 ++++++++++------- .../MicrosoftGovernmentAppCredentialsTests.cs | 57 ++++++++++++++--- 9 files changed, 151 insertions(+), 159 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs index b1f5b12c49..8b618c4ee4 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs @@ -34,6 +34,10 @@ public abstract class AppCredentials : ServiceClientCredentials /// private Lazy _authenticator; + private string _oAuthScope; + + private string _channelAuthTenant; + /// /// Initializes a new instance of the class. /// @@ -54,7 +58,7 @@ public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClie /// The scope for the token. public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClient = null, ILogger logger = null, string oAuthScope = null) { - OAuthScope = string.IsNullOrWhiteSpace(oAuthScope) ? AuthenticationConstants.ToChannelFromBotOAuthScope : oAuthScope; + _oAuthScope = oAuthScope; ChannelAuthTenant = channelAuthTenant; CustomHttpClient = customHttpClient; Logger = logger ?? NullLogger.Instance; @@ -76,15 +80,17 @@ public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClie /// public virtual string ChannelAuthTenant { - get => string.IsNullOrEmpty(AuthTenant) ? AuthenticationConstants.DefaultChannelAuthTenant : AuthTenant; + get => string.IsNullOrEmpty(_channelAuthTenant) + ? DefaultChannelAuthTenant + : _channelAuthTenant; set { // Advanced user only, see https://aka.ms/bots/tenant-restriction - var endpointUrl = string.Format(CultureInfo.InvariantCulture, AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, value); + var endpointUrl = string.Format(CultureInfo.InvariantCulture, ToChannelFromBotLoginUrlTemplate, value); if (Uri.TryCreate(endpointUrl, UriKind.Absolute, out _)) { - AuthTenant = value; + _channelAuthTenant = value; } else { @@ -99,7 +105,7 @@ public virtual string ChannelAuthTenant /// /// The OAuth endpoint to use. /// - public virtual string OAuthEndpoint => string.Format(CultureInfo.InvariantCulture, AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, ChannelAuthTenant); + public virtual string OAuthEndpoint => string.Format(CultureInfo.InvariantCulture, ToChannelFromBotLoginUrlTemplate, ChannelAuthTenant); /// /// Gets a value indicating whether to validate the Authority. @@ -115,7 +121,9 @@ public virtual string ChannelAuthTenant /// /// The OAuth scope to use. /// - public virtual string OAuthScope { get; } + public virtual string OAuthScope => string.IsNullOrEmpty(_oAuthScope) + ? ToChannelFromBotOAuthScope + : _oAuthScope; /// /// Gets or sets the channel auth token tenant for this credential. @@ -123,7 +131,7 @@ public virtual string ChannelAuthTenant /// /// The channel auth token tenant for this credential. /// - protected string AuthTenant { get; set; } + protected HttpClient CustomHttpClient { get; set; } /// /// Gets or sets the channel auth token tenant for this credential. @@ -131,15 +139,27 @@ public virtual string ChannelAuthTenant /// /// The channel auth token tenant for this credential. /// - protected HttpClient CustomHttpClient { get; set; } + protected ILogger Logger { get; set; } /// - /// Gets or sets the channel auth token tenant for this credential. + /// Gets DefaultChannelAuthTenant. /// - /// - /// The channel auth token tenant for this credential. - /// - protected ILogger Logger { get; set; } + /// DefaultChannelAuthTenant. + protected virtual string DefaultChannelAuthTenant => AuthenticationConstants.DefaultChannelAuthTenant; + + /// + /// Gets ToChannelFromBotOAuthScope. + /// + /// ToChannelFromBotOAuthScope. + protected virtual string ToChannelFromBotOAuthScope => AuthenticationConstants.ToChannelFromBotOAuthScope; + + /// + /// Gets ToChannelFromBotLoginUrlTemplate. + /// + /// ToChannelFromBotLoginUrlTemplate. +#pragma warning disable CA1056 // Uri properties should not be strings + protected virtual string ToChannelFromBotLoginUrlTemplate => AuthenticationConstants.ToChannelFromBotLoginUrlTemplate; +#pragma warning restore CA1056 // Uri properties should not be strings /// /// Adds the host of service url to trusted hosts. diff --git a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs index 5a8d4b236d..6bfb1989d8 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentAuthenticationConstants.cs @@ -14,7 +14,9 @@ public static class GovernmentAuthenticationConstants public const string ChannelService = "https://botframework.azure.us"; /// - /// TO GOVERNMENT CHANNEL FROM BOT: Login URL. + /// TO CHANNEL FROM BOT: Login URL. + /// + /// DEPRECATED. For binary compat only. /// public const string ToChannelFromBotLoginUrl = "https://login.microsoftonline.us/MicrosoftServices.onmicrosoft.us"; diff --git a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentCloudBotFrameworkAuthentication.cs b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentCloudBotFrameworkAuthentication.cs index 545720488d..6403417df1 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/GovernmentCloudBotFrameworkAuthentication.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/GovernmentCloudBotFrameworkAuthentication.cs @@ -12,7 +12,7 @@ internal class GovernmentCloudBotFrameworkAuthentication : BuiltinBotFrameworkAu public GovernmentCloudBotFrameworkAuthentication(ServiceClientCredentialsFactory credentialFactory, AuthenticationConfiguration authConfiguration, IHttpClientFactory httpClientFactory, ILogger logger = null) : base( GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope, - GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl, + GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate, CallerIdConstants.USGovChannel, GovernmentAuthenticationConstants.ChannelService, GovernmentAuthenticationConstants.OAuthUrlGov, diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs index eb6b712212..7ff7a44738 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftAppCredentials.cs @@ -39,39 +39,6 @@ public class MicrosoftAppCredentials : AppCredentials /// public static readonly MicrosoftAppCredentials Empty = new MicrosoftAppCredentials(null, null); - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - public MicrosoftAppCredentials(string appId, string password) - : this(appId, password, null, null, null, null) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - /// Optional to be used when acquiring tokens. - public MicrosoftAppCredentials(string appId, string password, HttpClient customHttpClient) - : this(appId, password, null, customHttpClient) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - /// Optional to be used when acquiring tokens. - /// Optional to gather telemetry data while acquiring and managing credentials. - public MicrosoftAppCredentials(string appId, string password, HttpClient customHttpClient, ILogger logger) - : this(appId, password, null, customHttpClient, logger) - { - } - /// /// Initializes a new instance of the class. /// @@ -80,36 +47,11 @@ public MicrosoftAppCredentials(string appId, string password, HttpClient customH /// Optional to be used when acquiring tokens. /// Optional to gather telemetry data while acquiring and managing credentials. /// The scope for the token. - public MicrosoftAppCredentials(string appId, string password, HttpClient customHttpClient, ILogger logger, string oAuthScope) + public MicrosoftAppCredentials(string appId, string password, HttpClient customHttpClient = null, ILogger logger = null, string oAuthScope = null) : this(appId, password, null, customHttpClient, logger, oAuthScope) { } - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - /// Optional. The oauth token tenant. - /// Optional to be used when acquiring tokens. - public MicrosoftAppCredentials(string appId, string password, string channelAuthTenant, HttpClient customHttpClient) - : this(appId, password, channelAuthTenant, customHttpClient, null) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - /// Optional. The oauth token tenant. - /// Optional to be used when acquiring tokens. - /// Optional to gather telemetry data while acquiring and managing credentials. - public MicrosoftAppCredentials(string appId, string password, string channelAuthTenant, HttpClient customHttpClient, ILogger logger = null) - : this(appId, password, channelAuthTenant, customHttpClient, logger, null) - { - } - /// /// Initializes a new instance of the class. /// @@ -119,7 +61,7 @@ public MicrosoftAppCredentials(string appId, string password, string channelAuth /// Optional to be used when acquiring tokens. /// Optional to gather telemetry data while acquiring and managing credentials. /// The scope for the token. - public MicrosoftAppCredentials(string appId, string password, string channelAuthTenant, HttpClient customHttpClient, ILogger logger = null, string oAuthScope = null) + public MicrosoftAppCredentials(string appId, string password, string channelAuthTenant, HttpClient customHttpClient = null, ILogger logger = null, string oAuthScope = null) : base(channelAuthTenant, customHttpClient, logger, oAuthScope) { MicrosoftAppId = appId; diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs index 42dc9da49b..50df6fa1a7 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MicrosoftGovernmentAppCredentials.cs @@ -17,29 +17,6 @@ public class MicrosoftGovernmentAppCredentials : MicrosoftAppCredentials /// public static new readonly MicrosoftGovernmentAppCredentials Empty = new MicrosoftGovernmentAppCredentials(null, null, null, null, GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope); - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - /// Optional to be used when acquiring tokens. - public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClient customHttpClient = null) - : this(appId, password, customHttpClient, null, GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The Microsoft app ID. - /// The Microsoft app password. - /// Optional to be used when acquiring tokens. - /// Optional to gather telemetry data while acquiring and managing credentials. - public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClient customHttpClient, ILogger logger) - : this(appId, password, customHttpClient, logger, GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope) - { - } - /// /// Initializes a new instance of the class. /// @@ -48,8 +25,8 @@ public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClie /// Optional to be used when acquiring tokens. /// Optional to gather telemetry data while acquiring and managing credentials. /// The scope for the token (defaults to if null). - public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClient customHttpClient, ILogger logger, string oAuthScope = null) - : this(appId, password, tenantId: string.Empty, customHttpClient, logger, oAuthScope) + public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClient customHttpClient = null, ILogger logger = null, string oAuthScope = null) + : base(appId, password, customHttpClient, logger, oAuthScope) { } @@ -58,36 +35,22 @@ public MicrosoftGovernmentAppCredentials(string appId, string password, HttpClie /// /// The Microsoft app ID. /// The Microsoft app password. - /// Tenant ID of the Azure AD tenant where the bot is created. + /// Optional. The oauth token tenant. /// Optional to be used when acquiring tokens. /// Optional to gather telemetry data while acquiring and managing credentials. /// The scope for the token (defaults to if null). - public MicrosoftGovernmentAppCredentials(string appId, string password, string tenantId, HttpClient customHttpClient, ILogger logger, string oAuthScope = null) - : base(appId, password, tenantId, customHttpClient, logger, oAuthScope ?? GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope) + public MicrosoftGovernmentAppCredentials(string appId, string password, string channelAuthTenant, HttpClient customHttpClient = null, ILogger logger = null, string oAuthScope = null) + : base(appId, password, channelAuthTenant, customHttpClient, logger, oAuthScope) { } - /// - /// Gets or sets tenant to be used for channel authentication. - /// - /// - /// Tenant to be used for channel authentication. - /// - public override string ChannelAuthTenant - { - get => string.IsNullOrEmpty(AuthTenant) ? GovernmentAuthenticationConstants.DefaultChannelAuthTenant : AuthTenant; - set => base.ChannelAuthTenant = value; - } + /// + protected override string DefaultChannelAuthTenant => GovernmentAuthenticationConstants.DefaultChannelAuthTenant; - /// - /// Gets the OAuth endpoint to use. - /// - /// - /// The OAuth endpoint to use. - /// - public override string OAuthEndpoint - { - get => string.Format(CultureInfo.InvariantCulture, GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate, ChannelAuthTenant); - } + /// + protected override string ToChannelFromBotOAuthScope => GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope; + + /// + protected override string ToChannelFromBotLoginUrlTemplate => GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate; } } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MsalServiceClientCredentialsFactory.cs b/libraries/Microsoft.Bot.Connector/Authentication/MsalServiceClientCredentialsFactory.cs index 986917c0ea..7b39e34008 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MsalServiceClientCredentialsFactory.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MsalServiceClientCredentialsFactory.cs @@ -59,22 +59,28 @@ public override Task CreateCredentialsAsync(string app } // Public cloud: default authority, optional scope when authenticating for skill communication. - if (loginEndpoint.StartsWith(AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, StringComparison.OrdinalIgnoreCase)) + if (loginEndpoint.Equals(AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, StringComparison.OrdinalIgnoreCase)) { return Task.FromResult( - new MsalAppCredentials(_clientApplication, appId, authority: null, scope: audience, validateAuthority: validateAuthority, logger: _logger)); + new MsalAppCredentials( + _clientApplication, + appId, + authority: null, + scope: audience, + validateAuthority: validateAuthority, + logger: _logger)); } // Legacy gov: Set the authority (login url) to the legacy gov url, and allow for passed in scope for skill auth in // gov, or otherwise leave the default channel scope for gov. - if (loginEndpoint.Equals(GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl, StringComparison.OrdinalIgnoreCase)) + if (loginEndpoint.Equals(GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate, StringComparison.OrdinalIgnoreCase)) { return Task.FromResult( new MsalAppCredentials( _clientApplication, appId, - authority: GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl, - scope: audience ?? GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope, + authority: null, + scope: audience, validateAuthority: validateAuthority, logger: _logger)); } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/PasswordServiceClientCredentialFactory.cs b/libraries/Microsoft.Bot.Connector/Authentication/PasswordServiceClientCredentialFactory.cs index ca4050ea90..5319b78f95 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/PasswordServiceClientCredentialFactory.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/PasswordServiceClientCredentialFactory.cs @@ -93,12 +93,12 @@ public override Task CreateCredentialsAsync(string app throw new InvalidOperationException($"Invalid appId {appId} does not match expected {AppId}"); } - if (loginEndpoint.StartsWith(AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, StringComparison.OrdinalIgnoreCase)) + if (loginEndpoint.Equals(AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, StringComparison.OrdinalIgnoreCase)) { return Task.FromResult(new MicrosoftAppCredentials( appId, Password, TenantId, _httpClient, _logger, oauthScope)); } - else if (loginEndpoint.Equals(GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl, StringComparison.OrdinalIgnoreCase)) + else if (loginEndpoint.Equals(GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate, StringComparison.OrdinalIgnoreCase)) { return Task.FromResult(new MicrosoftGovernmentAppCredentials( appId, Password, TenantId, _httpClient, _logger, oauthScope)); diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftAppCredentialsTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftAppCredentialsTests.cs index 9298ee05d3..1b71835568 100644 --- a/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftAppCredentialsTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftAppCredentialsTests.cs @@ -1,10 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Net.Http; +using System.Globalization; using Microsoft.Bot.Connector.Authentication; -using Microsoft.Extensions.Logging; -using Moq; using Xunit; namespace Microsoft.Bot.Connector.Tests.Authentication @@ -15,30 +13,48 @@ public class MicrosoftAppCredentialsTests public void ConstructorTests() { var defaultScopeCase1 = new MicrosoftAppCredentials("someApp", "somePassword"); - Assert.Equal(AuthenticationConstants.ToChannelFromBotOAuthScope, defaultScopeCase1.OAuthScope); + AssertEqual(defaultScopeCase1, null, null); - using (var customHttpClient = new HttpClient()) - { - // Use with default scope - var defaultScopeCase2 = new MicrosoftAppCredentials("someApp", "somePassword", customHttpClient); - Assert.Equal(AuthenticationConstants.ToChannelFromBotOAuthScope, defaultScopeCase2.OAuthScope); + var defaultScopeCase2 = new MicrosoftAppCredentials("someApp", "somePassword", oAuthScope: "customScope"); + AssertEqual(defaultScopeCase2, null, "customScope"); - var logger = new Mock().Object; - var defaultScopeCase3 = new MicrosoftAppCredentials("someApp", "somePassword", customHttpClient, logger); - Assert.Equal(AuthenticationConstants.ToChannelFromBotOAuthScope, defaultScopeCase3.OAuthScope); + var defaultScopeCase3 = new MicrosoftAppCredentials("someApp", "somePassword", "someTenant"); + AssertEqual(defaultScopeCase3, "someTenant", null); - var defaultScopeCase4 = new MicrosoftAppCredentials("someApp", "somePassword", "someTenant", customHttpClient); - Assert.Equal(AuthenticationConstants.ToChannelFromBotOAuthScope, defaultScopeCase4.OAuthScope); + var defaultScopeCase4 = new MicrosoftAppCredentials("someApp", "somePassword", "someTenant", oAuthScope: "customScope"); + AssertEqual(defaultScopeCase4, "someTenant", "customScope"); + } - var defaultScopeCase5 = new MicrosoftAppCredentials("someApp", "somePassword", "someTenant", customHttpClient, logger); - Assert.Equal(AuthenticationConstants.ToChannelFromBotOAuthScope, defaultScopeCase5.OAuthScope); + private void AssertEqual(MicrosoftAppCredentials credential, string tenantId, string oauthScope) + { + Assert.Equal( + string.Format(CultureInfo.InvariantCulture, AuthenticationConstants.ToChannelFromBotLoginUrlTemplate, credential.ChannelAuthTenant), + credential.OAuthEndpoint); - // Use custom scope - var customScopeCase1 = new MicrosoftAppCredentials("someApp", "somePassword", customHttpClient, logger, "customScope"); - Assert.Equal("customScope", customScopeCase1.OAuthScope); + if (string.IsNullOrEmpty(oauthScope)) + { + Assert.Equal( + AuthenticationConstants.ToChannelFromBotOAuthScope, + credential.OAuthScope); + } + else + { + Assert.Equal( + oauthScope, + credential.OAuthScope); + } - var customScopeCase2 = new MicrosoftAppCredentials("someApp", "somePassword", "someTenant", customHttpClient, logger, "customScope"); - Assert.Equal("customScope", customScopeCase2.OAuthScope); + if (string.IsNullOrEmpty(tenantId)) + { + Assert.Equal( + AuthenticationConstants.DefaultChannelAuthTenant, + credential.ChannelAuthTenant); + } + else + { + Assert.Equal( + tenantId, + credential.ChannelAuthTenant); } } } diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftGovernmentAppCredentialsTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftGovernmentAppCredentialsTests.cs index 5ffda0042b..323eaac4eb 100644 --- a/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftGovernmentAppCredentialsTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/MicrosoftGovernmentAppCredentialsTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Globalization; using Microsoft.Bot.Connector.Authentication; using Xunit; @@ -39,13 +40,6 @@ public void GovernmentAuthenticationConstants_ChannelService_IsRight() Assert.Equal("https://botframework.azure.us", GovernmentAuthenticationConstants.ChannelService); } - [Fact] - public void GovernmentAuthenticationConstants_ToChannelFromBotLoginUrl_IsRight() - { - // This value should not change - Assert.Equal("https://login.microsoftonline.us/MicrosoftServices.onmicrosoft.us", GovernmentAuthenticationConstants.ToChannelFromBotLoginUrl); - } - [Fact] public void GovernmentAuthenticationConstants_ToChannelFromBotOAuthScope_IsRight() { @@ -73,5 +67,54 @@ public void GovernmentAuthenticationConstants_ToBotFromChannelOpenIdMetadataUrl_ // This value should not change Assert.Equal("https://login.botframework.azure.us/v1/.well-known/openidconfiguration", GovernmentAuthenticationConstants.ToBotFromChannelOpenIdMetadataUrl); } + + [Fact] + public void ConstructorTests() + { + var defaultScopeCase1 = new MicrosoftGovernmentAppCredentials("someApp", "somePassword"); + AssertEqual(defaultScopeCase1, null, null); + + var defaultScopeCase2 = new MicrosoftGovernmentAppCredentials("someApp", "somePassword", oAuthScope: "customScope"); + AssertEqual(defaultScopeCase2, null, "customScope"); + + var defaultScopeCase3 = new MicrosoftGovernmentAppCredentials("someApp", "somePassword", "someTenant"); + AssertEqual(defaultScopeCase3, "someTenant", null); + + var defaultScopeCase4 = new MicrosoftGovernmentAppCredentials("someApp", "somePassword", "someTenant", oAuthScope: "customScope"); + AssertEqual(defaultScopeCase4, "someTenant", "customScope"); + } + + private void AssertEqual(MicrosoftGovernmentAppCredentials credential, string tenantId, string oauthScope) + { + Assert.Equal( + string.Format(CultureInfo.InvariantCulture, GovernmentAuthenticationConstants.ToChannelFromBotLoginUrlTemplate, credential.ChannelAuthTenant), + credential.OAuthEndpoint); + + if (string.IsNullOrEmpty(oauthScope)) + { + Assert.Equal( + GovernmentAuthenticationConstants.ToChannelFromBotOAuthScope, + credential.OAuthScope); + } + else + { + Assert.Equal( + oauthScope, + credential.OAuthScope); + } + + if (string.IsNullOrEmpty(tenantId)) + { + Assert.Equal( + GovernmentAuthenticationConstants.DefaultChannelAuthTenant, + credential.ChannelAuthTenant); + } + else + { + Assert.Equal( + tenantId, + credential.ChannelAuthTenant); + } + } } } From f1239fb249aaef879cb03eeead0368ea556b34c3 Mon Sep 17 00:00:00 2001 From: fangyangci Date: Mon, 18 Dec 2023 11:06:06 +0800 Subject: [PATCH 3/4] Rollback AuthTenant Property Name --- .../Authentication/AppCredentials.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs index 8b618c4ee4..3377a3039c 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/AppCredentials.cs @@ -36,8 +36,6 @@ public abstract class AppCredentials : ServiceClientCredentials private string _oAuthScope; - private string _channelAuthTenant; - /// /// Initializes a new instance of the class. /// @@ -80,9 +78,9 @@ public AppCredentials(string channelAuthTenant = null, HttpClient customHttpClie /// public virtual string ChannelAuthTenant { - get => string.IsNullOrEmpty(_channelAuthTenant) + get => string.IsNullOrEmpty(AuthTenant) ? DefaultChannelAuthTenant - : _channelAuthTenant; + : AuthTenant; set { // Advanced user only, see https://aka.ms/bots/tenant-restriction @@ -90,7 +88,7 @@ public virtual string ChannelAuthTenant if (Uri.TryCreate(endpointUrl, UriKind.Absolute, out _)) { - _channelAuthTenant = value; + AuthTenant = value; } else { @@ -125,6 +123,14 @@ public virtual string ChannelAuthTenant ? ToChannelFromBotOAuthScope : _oAuthScope; + /// + /// Gets or sets the channel auth token tenant for this credential. + /// + /// + /// The channel auth token tenant for this credential. + /// + protected string AuthTenant { get; set; } + /// /// Gets or sets the channel auth token tenant for this credential. /// From 9de52c84cbb17b4d3411496811d171526da4d4c4 Mon Sep 17 00:00:00 2001 From: fangyangci Date: Mon, 18 Dec 2023 11:08:08 +0800 Subject: [PATCH 4/4] The Ctor do contains the old ones, Add Ctor to ApiCompatBaseline --- ApiCompatBaseline.txt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ApiCompatBaseline.txt b/ApiCompatBaseline.txt index 0895775d4c..5f057c1b72 100644 --- a/ApiCompatBaseline.txt +++ b/ApiCompatBaseline.txt @@ -1,4 +1,12 @@ # These types are no longer supported in Microsoft.Bot.Builder.Azure: TypesMustExist : Type 'Microsoft.Bot.Builder.Azure.CosmosDbCustomClientOptions' does not exist in the implementation but it does exist in the contract. TypesMustExist : Type 'Microsoft.Bot.Builder.Azure.CosmosDbStorage' does not exist in the implementation but it does exist in the contract. -TypesMustExist : Type 'Microsoft.Bot.Builder.Azure.CosmosDbStorageOptions' does not exist in the implementation but it does exist in the contract. \ No newline at end of file +TypesMustExist : Type 'Microsoft.Bot.Builder.Azure.CosmosDbStorageOptions' does not exist in the implementation but it does exist in the contract. + +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftAppCredentials..ctor(System.String, System.String)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftAppCredentials..ctor(System.String, System.String, System.Net.Http.HttpClient)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftAppCredentials..ctor(System.String, System.String, System.Net.Http.HttpClient, Microsoft.Extensions.Logging.ILogger)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftAppCredentials..ctor(System.String, System.String, System.String, System.Net.Http.HttpClient)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftAppCredentials..ctor(System.String, System.String, System.String, System.Net.Http.HttpClient, Microsoft.Extensions.Logging.ILogger)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftGovernmentAppCredentials..ctor(System.String, System.String, System.Net.Http.HttpClient)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'Microsoft.Bot.Connector.Authentication.MicrosoftGovernmentAppCredentials..ctor(System.String, System.String, System.Net.Http.HttpClient, Microsoft.Extensions.Logging.ILogger)' does not exist in the implementation but it does exist in the contract.