From 5e20344e719ae988d895313c76b16e2b6bf0b966 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Mon, 9 Oct 2023 16:00:17 -0300 Subject: [PATCH 1/4] Add sendX5c parameter in certificate auth factory --- ...tificateServiceClientCredentialsFactory.cs | 19 ++++++++++++- .../Authentication/MsalAppCredentials.cs | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs b/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs index 0b52de3b27..0506aefd4d 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs @@ -17,6 +17,7 @@ namespace Microsoft.Bot.Connector.Authentication public class CertificateServiceClientCredentialsFactory : ServiceClientCredentialsFactory { private readonly X509Certificate2 _certificate; + private readonly bool _sendX5c = false; private readonly string _appId; private readonly string _tenantId; private readonly HttpClient _httpClient; @@ -45,6 +46,22 @@ public CertificateServiceClientCredentialsFactory(X509Certificate2 certificate, _logger = logger; } + /// + /// Initializes a new instance of the class. + /// + /// The certificate to use for authentication. + /// If true will send the public certificate to Azure AD along with the token request, so that + /// Azure AD can use it to validate the subject name based on a trusted issuer policy. + /// Microsoft application Id related to the certificate. + /// The oauth token tenant. + /// A custom httpClient to use. + /// A logger instance to use. + public CertificateServiceClientCredentialsFactory(X509Certificate2 certificate, bool sendX5c, string appId, string tenantId = null, HttpClient httpClient = null, ILogger logger = null) + : this(certificate, appId, tenantId, httpClient, logger) + { + _sendX5c = sendX5c; + } + /// public override Task IsValidAppIdAsync(string appId, CancellationToken cancellationToken) { @@ -68,7 +85,7 @@ public override Task CreateCredentialsAsync( } return Task.FromResult( - new CertificateAppCredentials(_certificate, _appId, _tenantId, _httpClient, _logger)); + new CertificateAppCredentials(_certificate, _sendX5c, _appId, _tenantId, _httpClient, _logger)); } } } diff --git a/libraries/Microsoft.Bot.Connector/Authentication/MsalAppCredentials.cs b/libraries/Microsoft.Bot.Connector/Authentication/MsalAppCredentials.cs index 63524e8194..aaa048d5f6 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/MsalAppCredentials.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/MsalAppCredentials.cs @@ -106,6 +106,33 @@ public MsalAppCredentials(string appId, X509Certificate2 certificate, string aut .Build(); } + /// + /// Initializes a new instance of the class. + /// + /// The Microsoft application id. + /// The certificate to use for authentication. + /// If true will send the public certificate to Azure AD along with the token request, so that + /// Azure AD can use it to validate the subject name based on a trusted issuer policy. + /// Optional switch for whether to validate the authority. + /// Optional authority. + /// Optional custom scope. + /// Optional . + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2234:Pass system uri objects instead of strings", Justification = "Using string overload for legacy compatibility.")] + public MsalAppCredentials(string appId, X509Certificate2 certificate, bool sendX5c, string authority = null, string scope = null, bool validateAuthority = true, ILogger logger = null) + : this( + clientApplication: null, + appId: appId, + authority: authority, + scope: scope, + validateAuthority: validateAuthority, + logger: logger) + { + _clientApplication = ConfidentialClientApplicationBuilder.Create(appId) + .WithAuthority(authority ?? OAuthEndpoint, validateAuthority) + .WithCertificate(certificate, sendX5c) + .Build(); + } + async Task IAuthenticator.GetTokenAsync(bool forceRefresh) { var watch = Stopwatch.StartNew(); From 4c7288f8d7b478d258df3322b2487b7910e870ef Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Mon, 9 Oct 2023 16:01:47 -0300 Subject: [PATCH 2/4] Add unit tests --- ...ateServiceClientCredentialsFactoryTests.cs | 4 +- ...salServiceClientCredentialsFactoryTests.cs | 100 ++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs index e1bf5ed6bc..25ea221f0d 100644 --- a/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs @@ -64,7 +64,7 @@ public void IsAuthenticationDisabledTest() [Fact] public async void CanCreateCredentials() { - var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, TestAppId); + var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, false, TestAppId); var credentials = await factory.CreateCredentialsAsync( TestAppId, TestAudience, "https://login.microsoftonline.com", true, CancellationToken.None); @@ -76,7 +76,7 @@ public async void CanCreateCredentials() [Fact] public void CannotCreateCredentialsWithInvalidAppId() { - var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, TestAppId); + var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, false, TestAppId); Assert.ThrowsAsync(() => factory.CreateCredentialsAsync( "InvalidAppId", TestAudience, "https://login.microsoftonline.com", true, CancellationToken.None)); diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs new file mode 100644 index 0000000000..99e86e6f57 --- /dev/null +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs @@ -0,0 +1,100 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Bot.Connector.Authentication; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Microsoft.Identity.Client; +using Moq; +using Xunit; + +namespace Microsoft.Bot.Connector.Tests.Authentication +{ + public class MsalServiceClientCredentialsFactoryTests + { + private const string TestAppId = nameof(TestAppId); + private const string TestTenantId = nameof(TestTenantId); + private const string TestAudience = nameof(TestAudience); + private const string LoginEndpoint = "https://login.microsoftonline.com"; + private const string LoginEndpointGov = "https://login.microsoftonline.us/MicrosoftServices.onmicrosoft.us"; + private readonly Mock logger = new Mock(); + private readonly Mock configuration = new Mock(); + private readonly Mock clientApplication = new Mock(); + + // private readonly bool sendX5c = false; + + [Fact] + public void ConstructorTests() + { + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + + Assert.NotNull(factory); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public async Task ShouldReturnEmptyCredentialsWithoutAppId(string appId) + { + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + var credentials = await factory.CreateCredentialsAsync(appId, TestAudience, LoginEndpoint, true, CancellationToken.None); + + Assert.Equal(MsalAppCredentials.Empty, credentials); + } + + [Fact] + public void ShouldThrowIfAppIdDoesNotMatch() + { + configuration.Setup(x => x.GetSection(MicrosoftAppCredentials.MicrosoftAppIdKey).Value).Returns(TestAppId); + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + + Assert.ThrowsAsync(() => factory.CreateCredentialsAsync( + "InvalidAppId", TestAudience, LoginEndpoint, true, CancellationToken.None)); + } + + [Fact] + public void ShouldCreateCredentials() + { + configuration.Setup(x => x.GetSection(MicrosoftAppCredentials.MicrosoftAppIdKey).Value).Returns(TestAppId); + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + var credentials = factory.CreateCredentialsAsync(TestAppId, TestAudience, LoginEndpoint, true, CancellationToken.None).GetAwaiter().GetResult(); + + Assert.NotNull(credentials); + Assert.IsType(credentials); + } + + [Fact] + public void ShouldCreateCredentialsForGoverment() + { + configuration.Setup(x => x.GetSection(MicrosoftAppCredentials.MicrosoftAppIdKey).Value).Returns(TestAppId); + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + var credentials = factory.CreateCredentialsAsync(TestAppId, TestAudience, LoginEndpointGov, true, CancellationToken.None).GetAwaiter().GetResult(); + + Assert.NotNull(credentials); + Assert.IsType(credentials); + } + + [Fact] + public void IsValidAppIdTest() + { + configuration.Setup(x => x.GetSection(MicrosoftAppCredentials.MicrosoftAppIdKey).Value).Returns(TestAppId); + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + + Assert.True(factory.IsValidAppIdAsync(TestAppId, CancellationToken.None).GetAwaiter().GetResult()); + Assert.False(factory.IsValidAppIdAsync("InvalidAppId", CancellationToken.None).GetAwaiter().GetResult()); + } + + [Fact] + public void IsAuthenticationDisabledTest() + { + configuration.Setup(x => x.GetSection(MicrosoftAppCredentials.MicrosoftAppIdKey).Value).Returns(string.Empty); + var factory = new MsalServiceClientCredentialsFactory(configuration.Object, clientApplication.Object, logger.Object); + + Assert.True(factory.IsAuthenticationDisabledAsync(CancellationToken.None).GetAwaiter().GetResult()); + } + } +} From 9bc3eeebec6efa552065a519af399f4a5f56cdb5 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Tue, 10 Oct 2023 09:11:52 -0300 Subject: [PATCH 3/4] Remove commented property. --- .../Authentication/MsalServiceClientCredentialsFactoryTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs index 99e86e6f57..bd89bffb04 100644 --- a/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/MsalServiceClientCredentialsFactoryTests.cs @@ -24,8 +24,6 @@ public class MsalServiceClientCredentialsFactoryTests private readonly Mock configuration = new Mock(); private readonly Mock clientApplication = new Mock(); - // private readonly bool sendX5c = false; - [Fact] public void ConstructorTests() { From bbfe78615a7abfe7b00de790a6754654e7647867 Mon Sep 17 00:00:00 2001 From: CeciliaAvila Date: Thu, 12 Oct 2023 10:27:43 -0300 Subject: [PATCH 4/4] Revert changes in CertificateServiceClientCredentialsFactory --- ...tificateServiceClientCredentialsFactory.cs | 19 +------------------ ...ateServiceClientCredentialsFactoryTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs b/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs index 0506aefd4d..0b52de3b27 100644 --- a/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs +++ b/libraries/Microsoft.Bot.Connector/Authentication/CertificateServiceClientCredentialsFactory.cs @@ -17,7 +17,6 @@ namespace Microsoft.Bot.Connector.Authentication public class CertificateServiceClientCredentialsFactory : ServiceClientCredentialsFactory { private readonly X509Certificate2 _certificate; - private readonly bool _sendX5c = false; private readonly string _appId; private readonly string _tenantId; private readonly HttpClient _httpClient; @@ -46,22 +45,6 @@ public CertificateServiceClientCredentialsFactory(X509Certificate2 certificate, _logger = logger; } - /// - /// Initializes a new instance of the class. - /// - /// The certificate to use for authentication. - /// If true will send the public certificate to Azure AD along with the token request, so that - /// Azure AD can use it to validate the subject name based on a trusted issuer policy. - /// Microsoft application Id related to the certificate. - /// The oauth token tenant. - /// A custom httpClient to use. - /// A logger instance to use. - public CertificateServiceClientCredentialsFactory(X509Certificate2 certificate, bool sendX5c, string appId, string tenantId = null, HttpClient httpClient = null, ILogger logger = null) - : this(certificate, appId, tenantId, httpClient, logger) - { - _sendX5c = sendX5c; - } - /// public override Task IsValidAppIdAsync(string appId, CancellationToken cancellationToken) { @@ -85,7 +68,7 @@ public override Task CreateCredentialsAsync( } return Task.FromResult( - new CertificateAppCredentials(_certificate, _sendX5c, _appId, _tenantId, _httpClient, _logger)); + new CertificateAppCredentials(_certificate, _appId, _tenantId, _httpClient, _logger)); } } } diff --git a/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs b/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs index 25ea221f0d..e1bf5ed6bc 100644 --- a/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs +++ b/tests/Microsoft.Bot.Connector.Tests/Authentication/CertificateServiceClientCredentialsFactoryTests.cs @@ -64,7 +64,7 @@ public void IsAuthenticationDisabledTest() [Fact] public async void CanCreateCredentials() { - var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, false, TestAppId); + var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, TestAppId); var credentials = await factory.CreateCredentialsAsync( TestAppId, TestAudience, "https://login.microsoftonline.com", true, CancellationToken.None); @@ -76,7 +76,7 @@ public async void CanCreateCredentials() [Fact] public void CannotCreateCredentialsWithInvalidAppId() { - var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, false, TestAppId); + var factory = new CertificateServiceClientCredentialsFactory(certificate.Object, TestAppId); Assert.ThrowsAsync(() => factory.CreateCredentialsAsync( "InvalidAppId", TestAudience, "https://login.microsoftonline.com", true, CancellationToken.None));