diff --git a/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs b/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs index 9ab5770c8..ef0f50a86 100644 --- a/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs +++ b/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs @@ -24,7 +24,7 @@ public async System.Threading.Tasks.Task MicrosoftAuthentication_GetAccessTokenA var msAuth = new MicrosoftAuthentication(context); await Assert.ThrowsAsync( - () => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName)); + () => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName, false)); } } } diff --git a/src/shared/Core/Authentication/MicrosoftAuthentication.cs b/src/shared/Core/Authentication/MicrosoftAuthentication.cs index 900cb8d65..06bd7330d 100644 --- a/src/shared/Core/Authentication/MicrosoftAuthentication.cs +++ b/src/shared/Core/Authentication/MicrosoftAuthentication.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net.Http; using System.Threading.Tasks; using GitCredentialManager.Interop.Windows.Native; @@ -23,7 +24,7 @@ namespace GitCredentialManager.Authentication public interface IMicrosoftAuthentication { Task GetTokenAsync(string authority, string clientId, Uri redirectUri, - string[] scopes, string userName); + string[] scopes, string userName, bool msaPt = false); } public interface IMicrosoftAuthenticationResult @@ -59,7 +60,7 @@ public MicrosoftAuthentication(ICommandContext context) #region IMicrosoftAuthentication public async Task GetTokenAsync( - string authority, string clientId, Uri redirectUri, string[] scopes, string userName) + string authority, string clientId, Uri redirectUri, string[] scopes, string userName, bool msaPt) { // Check if we can and should use OS broker authentication bool useBroker = CanUseBroker(); @@ -67,10 +68,15 @@ public MicrosoftAuthentication(ICommandContext context) ? "OS broker is available and enabled." : "OS broker is not available or enabled."); + if (msaPt) + { + Context.Trace.WriteLine("MSA passthrough is enabled."); + } + try { // Create the public client application for authentication - IPublicClientApplication app = await CreatePublicClientApplicationAsync(authority, clientId, redirectUri, useBroker); + IPublicClientApplication app = await CreatePublicClientApplicationAsync(authority, clientId, redirectUri, useBroker, msaPt); AuthenticationResult result = null; @@ -78,7 +84,7 @@ public MicrosoftAuthentication(ICommandContext context) bool hasExistingUser = !string.IsNullOrWhiteSpace(userName); if (hasExistingUser) { - result = await GetAccessTokenSilentlyAsync(app, scopes, userName); + result = await GetAccessTokenSilentlyAsync(app, scopes, userName, msaPt); } // @@ -116,7 +122,7 @@ public MicrosoftAuthentication(ICommandContext context) // account then the user may become stuck in a loop of authentication failures. if (!hasExistingUser && Context.Settings.UseMsAuthDefaultAccount) { - result = await GetAccessTokenSilentlyAsync(app, scopes, null); + result = await GetAccessTokenSilentlyAsync(app, scopes, null, msaPt); if (result is null || !await UseDefaultAccountAsync(result.Account.Username)) { @@ -281,23 +287,45 @@ internal MicrosoftAuthenticationFlowType GetFlowType() /// /// Obtain an access token without showing UI or prompts. /// - private async Task GetAccessTokenSilentlyAsync(IPublicClientApplication app, string[] scopes, string userName) + private async Task GetAccessTokenSilentlyAsync( + IPublicClientApplication app, string[] scopes, string userName, bool msaPt) { try { if (userName is null) { - Context.Trace.WriteLine("Attempting to acquire token silently for current operating system account..."); + Context.Trace.WriteLine( + "Attempting to acquire token silently for current operating system account..."); - return await app.AcquireTokenSilent(scopes, PublicClientApplication.OperatingSystemAccount).ExecuteAsync(); + return await app.AcquireTokenSilent(scopes, PublicClientApplication.OperatingSystemAccount) + .ExecuteAsync(); } else { Context.Trace.WriteLine($"Attempting to acquire token silently for user '{userName}'..."); - // We can either call `app.GetAccountsAsync` and filter through the IAccount objects for the instance with the correct user name, - // or we can just pass the user name string we have as the `loginHint` and let MSAL do exactly that for us instead! - return await app.AcquireTokenSilent(scopes, loginHint: userName).ExecuteAsync(); + // Enumerate all accounts and find the one matching the user name + IEnumerable accounts = await app.GetAccountsAsync(); + IAccount account = accounts.FirstOrDefault(x => + StringComparer.OrdinalIgnoreCase.Equals(x.Username, userName)); + if (account is null) + { + Context.Trace.WriteLine($"No cached account found for user '{userName}'..."); + return null; + } + + var atsBuilder = app.AcquireTokenSilent(scopes, account); + + // Is we are operating with an MSA passthrough app we need to ensure that we target the + // special MSA 'transfer' tenant explicitly. This is a workaround for MSAL issue: + // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/3077 + if (msaPt && Guid.TryParse(account.HomeAccountId.TenantId, out Guid homeTenantId) && + homeTenantId == Constants.MsaHomeTenantId) + { + atsBuilder = atsBuilder.WithTenantId(Constants.MsaTransferTenantId.ToString("D")); + } + + return await atsBuilder.ExecuteAsync(); } } catch (MsalUiRequiredException) @@ -305,10 +333,16 @@ private async Task GetAccessTokenSilentlyAsync(IPublicClie Context.Trace.WriteLine("Failed to acquire token silently; user interaction is required."); return null; } + catch (Exception ex) + { + Context.Trace.WriteLine("Failed to acquire token silently."); + Context.Trace.WriteException(ex); + return null; + } } private async Task CreatePublicClientApplicationAsync( - string authority, string clientId, Uri redirectUri, bool enableBroker) + string authority, string clientId, Uri redirectUri, bool enableBroker, bool msaPt) { var httpFactoryAdaptor = new MsalHttpClientFactoryAdaptor(Context.HttpClientFactory); @@ -370,7 +404,7 @@ private async Task GetAccessTokenSilentlyAsync(IPublicClie new BrokerOptions(BrokerOptions.OperatingSystems.Windows) { Title = "Git Credential Manager", - MsaPassthrough = true, + MsaPassthrough = msaPt, } ); #endif diff --git a/src/shared/Core/Constants.cs b/src/shared/Core/Constants.cs index 9b82d0673..4c9c4f730 100644 --- a/src/shared/Core/Constants.cs +++ b/src/shared/Core/Constants.cs @@ -18,6 +18,17 @@ public static class Constants public static readonly Guid DevBoxPartnerId = new("e3171dd9-9a5f-e5be-b36c-cc7c4f3f3bcf"); + /// + /// Home tenant ID for Microsoft Accounts (MSA). + /// + public static readonly Guid MsaHomeTenantId = new("9188040d-6c67-4c5b-b112-36a304b66dad"); + + /// + /// Special tenant ID for transferring between Microsoft Account (MSA) native tokens + /// and AAD tokens. Only required for MSA-Passthrough applications. + /// + public static readonly Guid MsaTransferTenantId = new("f8cdef31-a31e-4b4a-93e4-5f571e91255a"); + public static class CredentialStoreNames { public const string WindowsCredentialManager = "wincredman"; diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs index 80aef5dee..d7fc916e1 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs @@ -170,7 +170,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); @@ -219,7 +219,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); @@ -268,7 +268,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); @@ -315,7 +315,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ var azDevOpsMock = new Mock(MockBehavior.Strict); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); @@ -363,7 +363,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ var azDevOpsMock = new Mock(MockBehavior.Strict); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, account)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, account, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); @@ -413,7 +413,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthorit azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); @@ -462,7 +462,7 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_Ge .ReturnsAsync(personalAccessToken); var msAuthMock = new Mock(MockBehavior.Strict); - msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, true)) .ReturnsAsync(authResult); var authorityCacheMock = new Mock(MockBehavior.Strict); diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 353bf4a09..e696e504d 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -202,7 +202,8 @@ private async Task GeneratePersonalAccessTokenAsync(InputArguments GetClientId(), GetRedirectUri(), AzureDevOpsConstants.AzureDevOpsDefaultScopes, - null); + null, + msaPt: true); _context.Trace.WriteLineSecrets( $"Acquired Azure access token. Account='{result.AccountUpn}' Token='{{0}}'", new object[] {result.AccessToken}); @@ -293,7 +294,8 @@ private async Task GetAzureAccessTokenAsync(Inpu GetClientId(), GetRedirectUri(), AzureDevOpsConstants.AzureDevOpsDefaultScopes, - userName); + userName, + msaPt: true); _context.Trace.WriteLineSecrets( $"Acquired Azure access token. Account='{result.AccountUpn}' Token='{{0}}'", new object[] {result.AccessToken});