From a805fcaa4950e318213119eee412f04348171374 Mon Sep 17 00:00:00 2001 From: Bogdan Gavril Date: Mon, 9 Sep 2024 12:25:27 +0100 Subject: [PATCH] Fix for 4913 - sendX5C for ROPC CCA (#4917) * Fix for 4913 - sendX5C for ROPC CCA * fix --- ...AndPasswordConfidentialParameterBuilder.cs | 13 ++++++- .../Executors/ConfidentialClientExecutor.cs | 2 ++ ...cquireTokenConfidentialClientParameters.cs | 5 --- ...quireTokenByAuthorizationCodeParameters.cs | 5 +++ ...cquireTokenByUsernamePasswordParameters.cs | 4 +++ ...UsernamePasswordIntegrationTests.NetFwk.cs | 17 ++++++--- .../ClientCredentialWithCertTest.cs | 35 +++++++++++++++++++ 7 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenByUsernameAndPasswordConfidentialParameterBuilder.cs b/src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenByUsernameAndPasswordConfidentialParameterBuilder.cs index 801e6409cd..04995d6614 100644 --- a/src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenByUsernameAndPasswordConfidentialParameterBuilder.cs +++ b/src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenByUsernameAndPasswordConfidentialParameterBuilder.cs @@ -18,7 +18,7 @@ namespace Microsoft.Identity.Client { /// - /// Parameter builder for the + /// Parameter builder for the /// operation. See https://aka.ms/msal-net-up /// public sealed class AcquireTokenByUsernameAndPasswordConfidentialParameterBuilder : @@ -67,5 +67,16 @@ internal override ApiEvent.ApiIds CalculateApiEventId() { return ApiEvent.ApiIds.AcquireTokenByUsernamePassword; } + + /// + protected override void Validate() + { + base.Validate(); + + if (Parameters.SendX5C == null) + { + Parameters.SendX5C = ServiceBundle.Config?.SendX5C ?? false; + } + } } } diff --git a/src/client/Microsoft.Identity.Client/ApiConfig/Executors/ConfidentialClientExecutor.cs b/src/client/Microsoft.Identity.Client/ApiConfig/Executors/ConfidentialClientExecutor.cs index 97bfb4a884..6ae2e3858d 100644 --- a/src/client/Microsoft.Identity.Client/ApiConfig/Executors/ConfidentialClientExecutor.cs +++ b/src/client/Microsoft.Identity.Client/ApiConfig/Executors/ConfidentialClientExecutor.cs @@ -142,6 +142,8 @@ public async Task ExecuteAsync( commonParameters, requestContext, _confidentialClientApplication.UserTokenCacheInternal).ConfigureAwait(false); + + requestParams.SendX5C = usernamePasswordParameters.SendX5C ?? false; var handler = new UsernamePasswordRequest( ServiceBundle, diff --git a/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AbstractAcquireTokenConfidentialClientParameters.cs b/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AbstractAcquireTokenConfidentialClientParameters.cs index db63a4fb9e..6237f45345 100644 --- a/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AbstractAcquireTokenConfidentialClientParameters.cs +++ b/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AbstractAcquireTokenConfidentialClientParameters.cs @@ -20,10 +20,5 @@ internal abstract class AbstractAcquireTokenConfidentialClientParameters /// This overrides application config settings. /// public bool? SendX5C { get; set; } - - /// - /// if true then Spa code param will be sent via AcquireTokenByAuthorizeCode - /// - public bool SpaCode { get; set; } } } diff --git a/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByAuthorizationCodeParameters.cs b/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByAuthorizationCodeParameters.cs index 31228b55a9..3e41c579c8 100644 --- a/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByAuthorizationCodeParameters.cs +++ b/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByAuthorizationCodeParameters.cs @@ -11,6 +11,11 @@ internal class AcquireTokenByAuthorizationCodeParameters : AbstractAcquireTokenC public string PkceCodeVerifier { get; set; } + /// + /// if true then Spa code param will be sent via AcquireTokenByAuthorizeCode + /// + public bool SpaCode { get; set; } + public void LogParameters(ILoggerAdapter logger) { } diff --git a/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByUsernamePasswordParameters.cs b/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByUsernamePasswordParameters.cs index bbcaf230a0..17ec6e79f4 100644 --- a/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByUsernamePasswordParameters.cs +++ b/src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByUsernamePasswordParameters.cs @@ -2,14 +2,18 @@ // Licensed under the MIT License. using System.Security; +using System.Text; using Microsoft.Identity.Client.Core; namespace Microsoft.Identity.Client.ApiConfig.Parameters { + internal class AcquireTokenByUsernamePasswordParameters : AbstractAcquireTokenByUsernameParameters, IAcquireTokenParameters { public string Password { get; set; } + public bool? SendX5C { get; set; } // CCA only + /// public void LogParameters(ILoggerAdapter logger) { diff --git a/tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/UsernamePasswordIntegrationTests.NetFwk.cs b/tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/UsernamePasswordIntegrationTests.NetFwk.cs index e9238b5460..d4d1f7f0b8 100644 --- a/tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/UsernamePasswordIntegrationTests.NetFwk.cs +++ b/tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/UsernamePasswordIntegrationTests.NetFwk.cs @@ -275,13 +275,22 @@ private async Task RunHappyPathTestAsync(LabResponse labResponse, string federat else { IConfidentialAppSettings settings = ConfidentialAppSettings.GetSettings(cloud); - clientApp = ConfidentialClientApplicationBuilder + var clientAppBuilder = ConfidentialClientApplicationBuilder .Create(settings.ClientId) .WithTestLogging() .WithHttpClientFactory(factory) - .WithAuthority(labResponse.Lab.Authority, "organizations") - .WithClientSecret(settings.GetSecret()) - .Build(); + .WithAuthority(labResponse.Lab.Authority, "organizations"); + + if (cloud == Cloud.Arlington) + { + clientAppBuilder.WithClientSecret(settings.GetSecret()); + } + else + { + clientAppBuilder.WithCertificate(settings.GetCertificate(), true); + } + + clientApp = clientAppBuilder.Build(); } AuthenticationResult authResult diff --git a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithCertTest.cs b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithCertTest.cs index 453a33e361..96f6198245 100644 --- a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithCertTest.cs +++ b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ClientCredentialWithCertTest.cs @@ -669,6 +669,41 @@ await app.AcquireTokenByAuthorizationCode(TestConstants.s_scope, TestConstants.D } } + + // regression test for https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4913 + [DataTestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task RopcCcaSendsX5CAsync(bool sendX5C) + { + using (var harness = CreateTestHarness()) + { + var certificate = CertHelper.GetOrCreateTestCert(); + var exportedCertificate = Convert.ToBase64String(certificate.Export(X509ContentType.Cert)); + + var app = ConfidentialClientApplicationBuilder + .Create(TestConstants.ClientId) + .WithHttpManager(harness.HttpManager) + .WithCertificate(certificate, sendX5C) + .Build(); + + harness.HttpManager.AddInstanceDiscoveryMockHandler(); + + harness.HttpManager.AddMockHandler( + CreateTokenResponseHttpHandlerWithX5CValidation( + clientCredentialFlow: false, + expectedX5C: sendX5C ? exportedCertificate: null)); + + var result = await (app as IByUsernameAndPassword) + .AcquireTokenByUsernamePassword( + TestConstants.s_scope, + TestConstants.Username, + TestConstants.DefaultPassword) + .ExecuteAsync() + .ConfigureAwait(false); + } + } + private static string ComputeCertThumbprint(X509Certificate2 certificate, bool useSha2) { string thumbprint = null;