msauth: return MSAL wrapped result rather than JWT AT

Change the MSAuth GetToken method signature to return a wrapper result object rather than just the JWT access token. The result wrapper object includes the account UPN and the wrap AT as returned by MSAL. We do not need to inspect the claims of the AT (which might not even by a JWT in some cases anyway) since we have the account info elsewhere.
git-ecosystem · Feb 2, 2021 · 216b2a8 · 216b2a8
1 parent 683c668
commit 216b2a8
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 85 deletions.
diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureDevOpsApiTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureDevOpsApiTests.cs
@@ -9,10 +9,8 @@
 using System.Threading.Tasks;
 using Microsoft.Git.CredentialManager;
 using Microsoft.Git.CredentialManager.Tests.Objects;
-using Microsoft.IdentityModel.JsonWebTokens;
 using Newtonsoft.Json;
 using Xunit;
-using static Microsoft.Git.CredentialManager.Tests.TestHelpers;
 
 namespace Microsoft.AzureRepos.Tests
 {
@@ -228,7 +226,7 @@ public async Task AzureDevOpsRestApi_CreatePersonalAccessTokenAsync_ReturnsPAT()
             var orgUri = new Uri("https://dev.azure.com/org/");
 
             const string expectedPat = "PERSONAL-ACCESS-TOKEN";
-            JsonWebToken accessToken = CreateJwt();
+            string accessToken = "ACCESS-TOKEN";
             IEnumerable<string> scopes = new[] {AzureDevOpsConstants.PersonalAccessTokenScopes.ReposWrite};
 
             var identityServiceUri = new Uri("https://identity.example.com/");
@@ -267,7 +265,7 @@ public async Task AzureDevOpsRestApi_CreatePersonalAccessTokenAsync_LocSvcReturn
             var context = new TestCommandContext();
             var orgUri = new Uri("https://dev.azure.com/org/");
 
-            JsonWebToken accessToken = CreateJwt();
+            string accessToken = "ACCESS-TOKEN";
             IEnumerable<string> scopes = new[] {AzureDevOpsConstants.PersonalAccessTokenScopes.ReposWrite};
 
             var locSvcRequestUri = new Uri(orgUri, ExpectedLocationServicePath);
@@ -287,7 +285,7 @@ public async Task AzureDevOpsRestApi_CreatePersonalAccessTokenAsync_IdentSvcRetu
             var context = new TestCommandContext();
             var orgUri = new Uri("https://dev.azure.com/org/");
 
-            JsonWebToken accessToken = CreateJwt();
+            string accessToken = "ACCESS-TOKEN";
             IEnumerable<string> scopes = new[] {AzureDevOpsConstants.PersonalAccessTokenScopes.ReposWrite};
 
             var identityServiceUri = new Uri("https://identity.example.com/");
@@ -320,7 +318,7 @@ public async Task AzureDevOpsRestApi_CreatePersonalAccessTokenAsync_IdentSvcRetu
             var context = new TestCommandContext();
             var orgUri = new Uri("https://dev.azure.com/org/");
 
-            JsonWebToken accessToken = CreateJwt();
+            string accessToken = "ACCESS-TOKEN";
             IEnumerable<string> scopes = new[] {AzureDevOpsConstants.PersonalAccessTokenScopes.ReposWrite};
 
             var identityServiceUri = new Uri("https://identity.example.com/");
@@ -405,12 +403,12 @@ private static void AssertAcceptJson(HttpRequestMessage request)
             Assert.Contains(Constants.Http.MimeTypeJson, acceptMimeTypes);
         }
 
-        private static void AssertBearerToken(HttpRequestMessage request, JsonWebToken bearerToken)
+        private static void AssertBearerToken(HttpRequestMessage request, string bearerToken)
         {
             AuthenticationHeaderValue authHeader = request.Headers.Authorization;
             Assert.NotNull(authHeader);
             Assert.Equal("Bearer", authHeader.Scheme);
-            Assert.Equal(bearerToken.EncodedToken, authHeader.Parameter);
+            Assert.Equal(bearerToken, authHeader.Parameter);
         }
 
         private static HttpResponseMessage CreateLocationServiceResponse(Uri identityServiceUri)

diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs
@@ -9,7 +9,6 @@
 using Microsoft.Git.CredentialManager.Tests.Objects;
 using Moq;
 using Xunit;
-using static Microsoft.Git.CredentialManager.Tests.TestHelpers;
 
 namespace Microsoft.AzureRepos.Tests
 {
@@ -155,8 +154,9 @@ public async Task AzureReposProvider_GetCredentialAsync_ReturnsCredential()
             var expectedClientId = AzureDevOpsConstants.AadClientId;
             var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri;
             var expectedResource = AzureDevOpsConstants.AadResourceId;
-            var accessToken = CreateJwt("john.doe");
+            var accessToken = "ACCESS-TOKEN";
             var personalAccessToken = "PERSONAL-ACCESS-TOKEN";
+            var authResult = CreateAuthResult("john.doe", accessToken);
 
             var context = new TestCommandContext();
 
@@ -167,8 +167,8 @@ public async Task AzureReposProvider_GetCredentialAsync_ReturnsCredential()
                         .ReturnsAsync(personalAccessToken);
 
             var msAuthMock = new Mock<IMicrosoftAuthentication>();
-            msAuthMock.Setup(x => x.GetAccessTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedResource, remoteUri, null))
-                      .ReturnsAsync(accessToken);
+            msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedResource, remoteUri, null))
+                      .ReturnsAsync(authResult);
 
             var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object);
 
@@ -280,5 +280,21 @@ public async Task AzureReposHostProvider_UnconfigureAsync_User_Windows_UseHttpPa
 
             Assert.False(context.Git.GlobalConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out _));
         }
+
+        private static IMicrosoftAuthenticationResult CreateAuthResult(string upn, string token)
+        {
+            return new MockMsAuthResult
+            {
+                AccountUpn = upn,
+                AccessToken = token,
+            };
+        }
+
+        private class MockMsAuthResult : IMicrosoftAuthenticationResult
+        {
+            public string AccessToken { get; set; }
+            public string AccountUpn { get; set; }
+            public string TokenSource { get; set; }
+        }
     }
 }
diff --git a/src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs b/src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs
@@ -9,14 +9,13 @@
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Microsoft.Git.CredentialManager;
-using Microsoft.IdentityModel.JsonWebTokens;
 
 namespace Microsoft.AzureRepos
 {
     public interface IAzureDevOpsRestApi : IDisposable
     {
         Task<string> GetAuthorityAsync(Uri organizationUri);
-        Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, JsonWebToken accessToken, IEnumerable<string> scopes);
+        Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, string accessToken, IEnumerable<string> scopes);
     }
 
     public class AzureDevOpsRestApi : IAzureDevOpsRestApi
@@ -91,7 +90,7 @@ public async Task<string> GetAuthorityAsync(Uri organizationUri)
             return commonAuthority;
         }
 
-        public async Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, JsonWebToken accessToken, IEnumerable<string> scopes)
+        public async Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, string accessToken, IEnumerable<string> scopes)
         {
             const string sessionTokenUrl = "_apis/token/sessiontokens?api-version=1.0&tokentype=compact";
 
@@ -141,7 +140,7 @@ public async Task<string> CreatePersonalAccessTokenAsync(Uri organizationUri, Js
 
         #region Private Methods
 
-        private async Task<Uri> GetIdentityServiceUriAsync(Uri organizationUri, JsonWebToken accessToken)
+        private async Task<Uri> GetIdentityServiceUriAsync(Uri organizationUri, string accessToken)
         {
             const string locationServicePath = "_apis/ServiceDefinitions/LocationService2/951917AC-A960-4999-8464-E3F0AA25B381";
             const string locationServiceQuery = "api-version=1.0";
@@ -280,7 +279,7 @@ private static StringContent CreateAccessTokenRequestJson(Uri organizationUri, I
         /// <param name="content">Optional request content.</param>
         /// <param name="bearerToken">Optional bearer token for authorization.</param>
         /// <returns>HTTP request message.</returns>
-        private static HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri uri, HttpContent content = null, JsonWebToken bearerToken = null)
+        private static HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri uri, HttpContent content = null, string bearerToken = null)
         {
             var request = new HttpRequestMessage(method, uri);
 
@@ -291,7 +290,7 @@ private static HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri ur
 
             if (bearerToken != null)
             {
-                request.Headers.Authorization = new AuthenticationHeaderValue(Constants.Http.WwwAuthenticateBearerScheme, bearerToken.EncodedToken);
+                request.Headers.Authorization = new AuthenticationHeaderValue(Constants.Http.WwwAuthenticateBearerScheme, bearerToken);
             }
 
             return request;

diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs
@@ -7,7 +7,7 @@
 using System.Threading.Tasks;
 using Microsoft.Git.CredentialManager;
 using Microsoft.Git.CredentialManager.Authentication;
-using Microsoft.IdentityModel.JsonWebTokens;
+using Microsoft.Git.CredentialManager.Commands;
 using KnownGitCfg = Microsoft.Git.CredentialManager.Constants.GitConfiguration;
 
 namespace Microsoft.AzureRepos
@@ -151,15 +151,15 @@ private async Task<ICredential> GenerateCredentialAsync(InputArguments input)
 
             // Get an AAD access token for the Azure DevOps SPS
             _context.Trace.WriteLine("Getting Azure AD access token...");
-            JsonWebToken accessToken = await _msAuth.GetAccessTokenAsync(
+            IMicrosoftAuthenticationResult result = await _msAuth.GetTokenAsync(
                 authAuthority,
                 AzureDevOpsConstants.AadClientId,
                 AzureDevOpsConstants.AadRedirectUri,
                 AzureDevOpsConstants.AadResourceId,
                 remoteUri,
                 null);
-            string atUser = accessToken.GetAzureUserName();
-            _context.Trace.WriteLineSecrets($"Acquired Azure access token. User='{atUser}' Token='{{0}}'", new object[] {accessToken.EncodedToken});
+            _context.Trace.WriteLineSecrets(
+                $"Acquired Azure access token. Account='{result.AccountUpn}' Token='{{0}}'", new object[] {result.AccessToken});
 
             // Ask the Azure DevOps instance to create a new PAT
             var patScopes = new[]
@@ -170,11 +170,11 @@ private async Task<ICredential> GenerateCredentialAsync(InputArguments input)
             _context.Trace.WriteLine($"Creating Azure DevOps PAT with scopes '{string.Join(", ", patScopes)}'...");
             string pat = await _azDevOps.CreatePersonalAccessTokenAsync(
                 orgUri,
-                accessToken,
+                result.AccessToken,
                 patScopes);
             _context.Trace.WriteLineSecrets("PAT created. PAT='{0}'", new object[] {pat});
 
-            return new GitCredential(atUser, pat);
+            return new GitCredential(result.AccountUpn, pat);
         }
 
         /// <remarks>

diff --git a/...ared/Microsoft.Git.CredentialManager.Tests/Authentication/MicrosoftAuthenticationTests.cs b/...ared/Microsoft.Git.CredentialManager.Tests/Authentication/MicrosoftAuthenticationTests.cs
@@ -27,7 +27,7 @@ public async System.Threading.Tasks.Task MicrosoftAuthentication_GetAccessTokenA
             var msAuth = new MicrosoftAuthentication(context);
 
             await Assert.ThrowsAsync<InvalidOperationException>(
-                () => msAuth.GetAccessTokenAsync(authority, clientId, redirectUri, resource, remoteUri, userName));
+                () => msAuth.GetTokenAsync(authority, clientId, redirectUri, resource, remoteUri, userName));
         }
     }
 }
diff --git a/src/shared/Microsoft.Git.CredentialManager/Authentication/MicrosoftAuthentication.cs b/src/shared/Microsoft.Git.CredentialManager/Authentication/MicrosoftAuthentication.cs
@@ -8,16 +8,21 @@
 using System.Threading.Tasks;
 using Microsoft.Identity.Client;
 using Microsoft.Identity.Client.Extensions.Msal;
-using Microsoft.IdentityModel.JsonWebTokens;
 
 namespace Microsoft.Git.CredentialManager.Authentication
 {
     public interface IMicrosoftAuthentication
     {
-        Task<JsonWebToken> GetAccessTokenAsync(string authority, string clientId, Uri redirectUri, string resource,
+        Task<IMicrosoftAuthenticationResult> GetTokenAsync(string authority, string clientId, Uri redirectUri, string resource,
             Uri remoteUri, string userName);
     }
 
+    public interface IMicrosoftAuthenticationResult
+    {
+        string AccessToken { get; }
+        string AccountUpn { get; }
+    }
+
     public enum MicrosoftAuthenticationFlowType
     {
         Auto = 0,
@@ -40,12 +45,12 @@ public MicrosoftAuthentication(ICommandContext context)
 
         #region IMicrosoftAuthentication
 
-        public async Task<JsonWebToken> GetAccessTokenAsync(
+        public async Task<IMicrosoftAuthenticationResult> GetTokenAsync(
             string authority, string clientId, Uri redirectUri, string resource, Uri remoteUri, string userName)
         {
             // Try to acquire an access token in the current process
             string[] scopes = { $"{resource}/.default" };
-            return await GetAccessTokenInProcAsync(authority, clientId, redirectUri, scopes, userName);
+            return await GetTokenInProcAsync(authority, clientId, redirectUri, scopes, userName);
         }
 
         #endregion
@@ -55,7 +60,7 @@ public async Task<JsonWebToken> GetAccessTokenAsync(
         /// <summary>
         /// Obtain an access token using MSAL running inside the current process.
         /// </summary>
-        private async Task<JsonWebToken> GetAccessTokenInProcAsync(string authority, string clientId, Uri redirectUri, string[] scopes, string userName)
+        private async Task<IMicrosoftAuthenticationResult> GetTokenInProcAsync(string authority, string clientId, Uri redirectUri, string[] scopes, string userName)
         {
             IPublicClientApplication app = await CreatePublicClientApplicationAsync(authority, clientId, redirectUri);
 
@@ -136,7 +141,7 @@ private async Task<JsonWebToken> GetAccessTokenInProcAsync(string authority, str
                 }
             }
 
-            return new JsonWebToken(result.AccessToken);
+            return new MsalResult(result);
         }
 
         private MicrosoftAuthenticationFlowType GetFlowType()
@@ -335,5 +340,18 @@ private void EnsureCanUseSystemWebView(IPublicClientApplication app, Uri redirec
         }
 
         #endregion
+
+        private class MsalResult : IMicrosoftAuthenticationResult
+        {
+            private readonly AuthenticationResult _msalResult;
+
+            public MsalResult(AuthenticationResult msalResult)
+            {
+                _msalResult = msalResult;
+            }
+
+            public string AccessToken => _msalResult.AccessToken;
+            public string AccountUpn => _msalResult.Account.Username;
+        }
     }
 }
diff --git a/src/shared/Microsoft.Git.CredentialManager/JsonWebTokenExtensions.cs b/src/shared/Microsoft.Git.CredentialManager/JsonWebTokenExtensions.cs
diff --git a/src/shared/Microsoft.Git.CredentialManager/Microsoft.Git.CredentialManager.csproj b/src/shared/Microsoft.Git.CredentialManager/Microsoft.Git.CredentialManager.csproj
@@ -17,7 +17,6 @@
 
   <ItemGroup>
     <PackageReference Include="Newtonsoft.Json" Version="12.0.3" />
-    <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" Version="5.5.0" />
     <PackageReference Include="Microsoft.Identity.Client" Version="4.3.0" />
     <PackageReference Include="Microsoft.Identity.Client.Extensions.Msal" Version="2.1.0-preview" />
   </ItemGroup>

diff --git a/src/shared/TestInfrastructure/TestHelpers.cs b/src/shared/TestInfrastructure/TestHelpers.cs
diff --git a/src/windows/Installer.Windows/Setup.iss b/src/windows/Installer.Windows/Setup.iss
@@ -114,9 +114,6 @@ Source: "{#PayloadDir}\Microsoft.Git.CredentialManager.dll";           DestDir:
 Source: "{#PayloadDir}\Microsoft.Git.CredentialManager.UI.dll";        DestDir: "{app}"; Flags: ignoreversion
 Source: "{#PayloadDir}\Microsoft.Identity.Client.dll";                 DestDir: "{app}"; Flags: ignoreversion
 Source: "{#PayloadDir}\Microsoft.Identity.Client.Extensions.Msal.dll"; DestDir: "{app}"; Flags: ignoreversion
-Source: "{#PayloadDir}\Microsoft.IdentityModel.JsonWebTokens.dll";     DestDir: "{app}"; Flags: ignoreversion
-Source: "{#PayloadDir}\Microsoft.IdentityModel.Logging.dll";           DestDir: "{app}"; Flags: ignoreversion
-Source: "{#PayloadDir}\Microsoft.IdentityModel.Tokens.dll";            DestDir: "{app}"; Flags: ignoreversion
 Source: "{#PayloadDir}\Newtonsoft.Json.dll";                           DestDir: "{app}"; Flags: ignoreversion
 
 [Code]