Skip to content

Workaround MSAL.NET issue with MSA-PT account silent auth #1321

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

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Workaround MSAL.NET issue with MSA-PT account silent auth #1321

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99dea8e
msauth: let caller select if MSA-PT behaviour is used
mjcheetham Jul 10, 2023
68bcc34
msauth: workaround MSAL.NET issue with MSA-PT accounts
mjcheetham Jul 10, 2023
725ab49
msauth: add extra tracing of MSA-PT and ATS
mjcheetham Jul 31, 2023
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
Next Next commit
msauth: let caller select if MSA-PT behaviour is used 
Let the caller in to the `IMicrosoftAuthentication` component decide if
Microsoft Account Passthrough (MSA-PT) behaviour should be used.

Azure DevOps requires MSA-PT, so set that to `true` in usages.
  • Loading branch information
@mjcheetham
mjcheetham committed Jul 10, 2023
commit 99dea8e638af4b9e30a9cf4f1d83dea0cd686862
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));
}
}
}
10 changes: 5 additions & 5 deletions src/shared/Core/Authentication/MicrosoftAuthentication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,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 msaPt = false);
}

public interface IMicrosoftAuthenticationResult
Expand Down Expand Up @@ -59,7 +59,7 @@ 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 msaPt)
{
// Check if we can and should use OS broker authentication
bool useBroker = CanUseBroker();
Expand All @@ -70,7 +70,7 @@ public async Task<IMicrosoftAuthenticationResult> GetTokenAsync(
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;

Expand Down Expand Up @@ -308,7 +308,7 @@ private async Task<AuthenticationResult> GetAccessTokenSilentlyAsync(IPublicClie
}

private async Task<IPublicClientApplication> 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);

Expand Down Expand Up @@ -370,7 +370,7 @@ private async Task<IPublicClientApplication> CreatePublicClientApplicationAsync(
new BrokerOptions(BrokerOptions.OperatingSystems.Windows)
{
Title = "Git Credential Manager",
MsaPassthrough = true,
MsaPassthrough = msaPt,
}
);
#endif
Expand Down
14 changes: 7 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 @@ -170,7 +170,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -219,7 +219,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -268,7 +268,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -315,7 +315,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -363,7 +363,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -413,7 +413,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down Expand Up @@ -462,7 +462,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, true))
.ReturnsAsync(authResult);

var authorityCacheMock = new Mock<IAzureDevOpsAuthorityCache>(MockBehavior.Strict);
Expand Down
6 changes: 4 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 @@ -202,7 +202,8 @@ private async Task<ICredential> 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});

Expand Down Expand Up @@ -293,7 +294,8 @@ private async Task<IMicrosoftAuthenticationResult> 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});

Expand Down