Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MSA silent authentication with MSA-PT apps #1358

Closed
wants to merge 1 commit into from
Closed

Fix MSA silent authentication with MSA-PT apps #1358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public async System.Threading.Tasks.Task MicrosoftAuthentication_GetAccessTokenA
var msAuth = new MicrosoftAuthentication(context);

await Assert.ThrowsAsync<Trace2InvalidOperationException>(
() => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName));
() => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName, false));
}
}
}
69 changes: 56 additions & 13 deletions src/shared/Core/Authentication/MicrosoftAuthentication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -23,7 +24,7 @@ namespace GitCredentialManager.Authentication
public interface IMicrosoftAuthentication
{
Task<IMicrosoftAuthenticationResult> GetTokenAsync(string authority, string clientId, Uri redirectUri,
string[] scopes, string userName);
string[] scopes, string userName, bool msaPassthrough);
}

public interface IMicrosoftAuthenticationResult
Expand Down Expand Up @@ -59,26 +60,32 @@ public MicrosoftAuthentication(ICommandContext context)
#region IMicrosoftAuthentication

public async Task<IMicrosoftAuthenticationResult> GetTokenAsync(
string authority, string clientId, Uri redirectUri, string[] scopes, string userName)
string authority, string clientId, Uri redirectUri, string[] scopes, string userName, bool msaPassthrough)
{
// Check if we can and should use OS broker authentication
bool useBroker = CanUseBroker();
Context.Trace.WriteLine(useBroker
? "OS broker is available and enabled."
: "OS broker is not available or enabled.");

if (msaPassthrough)
{
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, msaPassthrough);

AuthenticationResult result = null;

// Try silent authentication first if we know about an existing user
bool hasExistingUser = !string.IsNullOrWhiteSpace(userName);
if (hasExistingUser)
{
result = await GetAccessTokenSilentlyAsync(app, scopes, userName);
result = await GetAccessTokenSilentlyAsync(app, scopes, userName, msaPassthrough);
}

//
Expand Down Expand Up @@ -116,7 +123,7 @@ public async Task<IMicrosoftAuthenticationResult> GetTokenAsync(
// 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, msaPassthrough);

if (result is null || !await UseDefaultAccountAsync(result.Account.Username))
{
Expand Down Expand Up @@ -281,34 +288,70 @@ internal MicrosoftAuthenticationFlowType GetFlowType()
/// <summary>
/// Obtain an access token without showing UI or prompts.
/// </summary>
private async Task<AuthenticationResult> GetAccessTokenSilentlyAsync(IPublicClientApplication app, string[] scopes, string userName)
private async Task<AuthenticationResult> GetAccessTokenSilentlyAsync(
IPublicClientApplication app, string[] scopes, string userName, bool msaPassthrough)
{
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();
//
// Filter through the IAccount objects for the instance with the correct user name then use the
// account object to acquire the token silently. We could also just pass the user name to the
// overload that takes a login hint, but we need to make some adjustments in the case of MSAs.
//
// See this issue: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/3077
//
// Until MSAL learns to do this for us, we'll need to do it ourselves.
//
IEnumerable<IAccount> accounts = await app.GetAccountsAsync();
IAccount account = accounts.FirstOrDefault(x => StringComparer.Ordinal.Equals(x.Username, userName));
if (account is null)
{
Context.Trace.WriteLine($"Unable to locate account object for user '{userName}'.");
return null;
}

AcquireTokenSilentParameterBuilder atsBuilder = app.AcquireTokenSilent(scopes, account);

// If we need to use MSA passthrough _and_ the account is an MSA then we must target
// the special "MSA-PT transfer tenant" instead.
if (msaPassthrough &&
Guid.TryParse(account.HomeAccountId.TenantId, out Guid homeTenantId) &&
homeTenantId == Constants.MicrosoftAccountTenantId)
{
Context.Trace.WriteLine("Using MSA passthrough transfer tenant.");
atsBuilder = atsBuilder.WithTenantId(Constants.MsaTransferTenantId.ToString("D"));
}

return await atsBuilder.ExecuteAsync();
}
}
catch (MsalUiRequiredException)
{
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<IPublicClientApplication> CreatePublicClientApplicationAsync(
string authority, string clientId, Uri redirectUri, bool enableBroker)
string authority, string clientId, Uri redirectUri, bool enableBroker, bool enableMsaPassthrough)
{
var httpFactoryAdaptor = new MsalHttpClientFactoryAdaptor(Context.HttpClientFactory);

Expand Down Expand Up @@ -370,7 +413,7 @@ private async Task<IPublicClientApplication> CreatePublicClientApplicationAsync(
new BrokerOptions(BrokerOptions.OperatingSystems.Windows)
{
Title = "Git Credential Manager",
MsaPassthrough = true,
MsaPassthrough = enableMsaPassthrough,
}
);
#endif
Expand Down
3 changes: 3 additions & 0 deletions src/shared/Core/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ public static class Constants

public static readonly Guid DevBoxPartnerId = new("e3171dd9-9a5f-e5be-b36c-cc7c4f3f3bcf");

public static readonly Guid MicrosoftAccountTenantId = new("9188040d-6c67-4c5b-b112-36a304b66dad");
public static readonly Guid MsaTransferTenantId = new("f8cdef31-a31e-4b4a-93e4-5f571e91255a");

public static class CredentialStoreNames
{
public const string WindowsCredentialManager = "wincredman";
Expand Down
21 changes: 14 additions & 7 deletions src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri;
var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes;
var accessToken = "ACCESS-TOKEN";
bool msaPt = true;
var authResult = CreateAuthResult(urlAccount, accessToken);

var context = new TestCommandContext();
Expand All @@ -170,7 +171,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -207,6 +208,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri;
var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes;
var accessToken = "ACCESS-TOKEN";
bool msaPt = true;
var authResult = CreateAuthResult(urlAccount, accessToken);

var context = new TestCommandContext();
Expand All @@ -219,7 +221,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -256,6 +258,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes;
var accessToken = "ACCESS-TOKEN";
var account = "jane.doe";
bool msaPt = true;
var authResult = CreateAuthResult(account, accessToken);

var context = new TestCommandContext();
Expand All @@ -268,7 +271,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -304,6 +307,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes;
var accessToken = "ACCESS-TOKEN";
var account = "john.doe";
bool msaPt = true;
var authResult = CreateAuthResult(account, accessToken);

var context = new TestCommandContext();
Expand All @@ -315,7 +319,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var azDevOpsMock = new Mock<IAzureDevOpsRestApi>(MockBehavior.Strict);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -352,6 +356,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes;
var accessToken = "ACCESS-TOKEN";
var account = "john.doe";
bool msaPt = true;
var authResult = CreateAuthResult(account, accessToken);

var context = new TestCommandContext();
Expand All @@ -363,7 +368,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_
var azDevOpsMock = new Mock<IAzureDevOpsRestApi>(MockBehavior.Strict);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, account))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, account, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -401,6 +406,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthorit
var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes;
var accessToken = "ACCESS-TOKEN";
var account = "john.doe";
bool msaPt = true;
var authResult = CreateAuthResult(account, accessToken);

var context = new TestCommandContext();
Expand All @@ -413,7 +419,7 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthorit
azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -452,6 +458,7 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_Ge
var accessToken = "ACCESS-TOKEN";
var personalAccessToken = "PERSONAL-ACCESS-TOKEN";
var account = "john.doe";
bool msaPt = true;
var authResult = CreateAuthResult(account, accessToken);

var context = new TestCommandContext();
Expand All @@ -462,7 +469,7 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_Ge
.ReturnsAsync(personalAccessToken);

var msAuthMock = new Mock<IMicrosoftAuthentication>(MockBehavior.Strict);
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null))
msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null, msaPt))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down
12 changes: 10 additions & 2 deletions src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,18 @@ private async Task<ICredential> GeneratePersonalAccessTokenAsync(InputArguments
string authAuthority = await _azDevOps.GetAuthorityAsync(orgUri);
_context.Trace.WriteLine($"Authority is '{authAuthority}'.");

// MSA passthrough is required for Azure DevOps until the service supports 'v2' tokens
bool msaPt = true;

// Get an AAD access token for the Azure DevOps SPS
_context.Trace.WriteLine("Getting Azure AD access token...");
IMicrosoftAuthenticationResult result = await _msAuth.GetTokenAsync(
authAuthority,
GetClientId(),
GetRedirectUri(),
AzureDevOpsConstants.AzureDevOpsDefaultScopes,
null);
null,
msaPt);
_context.Trace.WriteLineSecrets(
$"Acquired Azure access token. Account='{result.AccountUpn}' Token='{{0}}'", new object[] {result.AccessToken});

Expand Down Expand Up @@ -286,14 +290,18 @@ private async Task<IMicrosoftAuthenticationResult> GetAzureAccessTokenAsync(Inpu

_context.Trace.WriteLine(string.IsNullOrWhiteSpace(userName) ? "No user found." : $"User is '{userName}'.");

// MSA passthrough is required for Azure DevOps until the service supports 'v2' tokens
bool msaPt = true;

// Get an AAD access token for the Azure DevOps SPS
_context.Trace.WriteLine("Getting Azure AD access token...");
IMicrosoftAuthenticationResult result = await _msAuth.GetTokenAsync(
authAuthority,
GetClientId(),
GetRedirectUri(),
AzureDevOpsConstants.AzureDevOpsDefaultScopes,
userName);
userName,
msaPt);
_context.Trace.WriteLineSecrets(
$"Acquired Azure access token. Account='{result.AccountUpn}' Token='{{0}}'", new object[] {result.AccessToken});

Expand Down