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

Fixes Invalid JWT token using a personal account #2388

Merged
merged 3 commits into from
Oct 30, 2023
Merged

Fixes Invalid JWT token using a personal account #2388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c3f1dda
fix: Decode valid JWT tokens.
peombwa Oct 25, 2023
8f78ddd
chore: Add unit tests.
peombwa Oct 25, 2023
0593b6b
chore: Fix typo.
peombwa Oct 26, 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
26 changes: 10 additions & 16 deletions src/Authentication/Authentication.Core/Utilities/JwtHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,15 @@ internal static class JwtHelpers
internal static void DecodeJWT(string jwToken, IAccount account, ref IAuthContext authContext)
{
var jwtPayload = DecodeToObject<JwtPayload>(jwToken);
if (authContext.AuthType == AuthenticationType.UserProvidedAccessToken)
if (authContext.AuthType == AuthenticationType.UserProvidedAccessToken &&
jwtPayload != null &&
jwtPayload.Exp <= ConvertToUnixTimestamp(DateTime.UtcNow + TimeSpan.FromMinutes(Constants.TokenExpirationBufferInMinutes)))
{
if (jwtPayload == null)
{
throw new Exception(string.Format(
CultureInfo.CurrentCulture,
ErrorConstants.Message.InvalidUserProvidedToken,
"AccessToken"));
}

if (jwtPayload.Exp <= ConvertToUnixTimestamp(DateTime.UtcNow + TimeSpan.FromMinutes(Constants.TokenExpirationBufferInMinutes)))
{
throw new Exception(string.Format(
// Throw exception if access token is expired or is about to expire with a 5 minutes buffer.
throw new Exception(string.Format(
CultureInfo.CurrentCulture,
ErrorConstants.Message.ExpiredUserProvidedToken,
"AccessToken"));
}
}

authContext.ClientId = jwtPayload?.Appid ?? authContext.ClientId;
Expand Down Expand Up @@ -74,9 +66,11 @@ internal static T DecodeToObject<T>(string jwtString)

internal static JwtContent DecodeJWT(string jwtString)
{
// See https://tools.ietf.org/html/rfc7519
if (string.IsNullOrWhiteSpace(jwtString) || !jwtString.Contains(".") || !jwtString.StartsWith("eyJ"))
if (string.IsNullOrWhiteSpace(jwtString))
throw new ArgumentException("Invalid JSON Web Token (JWT).");
// See JWT RFC spec: https://tools.ietf.org/html/rfc7519.
if (!jwtString.Contains(".") || !jwtString.StartsWith("eyJ", StringComparison.OrdinalIgnoreCase))
return null; // Personal account access token are not JWT and cannot be decoded. See https://github.com/microsoftgraph/msgraph-sdk-powershell/issues/2386.

var jwtSegments = jwtString.Split('.');

Expand Down Expand Up @@ -116,4 +110,4 @@ internal static long ConvertToUnixTimestamp(DateTime time)
return (long)timeDiff.TotalSeconds;
}
}
}
}
27 changes: 13 additions & 14 deletions src/Authentication/Authentication.Test/Helpers/AuthenticationHelpersTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ public AuthenticationHelpersTests()
public async Task ShouldUseDelegateAuthProviderWhenUserAccessTokenIsProvidedAsync()
{
// Arrange
string dummyAccessToken = "eyJhbGciOiJIUzI1NiJ9.eyJSb2xlIjoiVGVzdCIsIklzc3VlciI6Iklzc3VlciIsIlVzZXJuYW1lIjoiVGVzdCIsImV4cCI6MTY3ODQ4ODgxNiwiaWF0IjoxNjc4NDg4ODE2fQ.hpYypwHAV8H3jb4KuTiLpgLWy9A8H2d9HG7SxJ8Kpn0";
GraphSession.Instance.InMemoryTokenCache = new InMemoryTokenCache(Encoding.UTF8.GetBytes(dummyAccessToken));
GraphSession.Instance.InMemoryTokenCache = new InMemoryTokenCache(Encoding.UTF8.GetBytes(MockConstants.DummyAccessToken));
AuthContext userProvidedAuthContext = new AuthContext
{
AuthType = AuthenticationType.UserProvidedAccessToken,
Expand All @@ -48,8 +47,8 @@ public async Task ShouldUseDelegateAuthProviderWhenUserAccessTokenIsProvidedAsyn
var accessToken = await authProvider.GetAuthorizationTokenAsync(requestMessage.RequestUri);

// Assert
Assert.IsType<AzureIdentityAccessTokenProvider>(authProvider);
Assert.Equal(dummyAccessToken, accessToken);
_ = Assert.IsType<AzureIdentityAccessTokenProvider>(authProvider);
Assert.Equal(MockConstants.DummyAccessToken, accessToken);
Assert.Equal(GraphEnvironmentConstants.EnvironmentName.Global, userProvidedAuthContext.Environment);

// reset static instance.
Expand All @@ -72,7 +71,7 @@ public async Task ShouldUseDeviceCodeWhenSpecifiedByUserAsync()
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(delegatedAuthContext, default);

// Assert
Assert.IsType<DeviceCodeCredential>(tokenCredential);
_ = Assert.IsType<DeviceCodeCredential>(tokenCredential);

// reset static instance.
GraphSession.Reset();
Expand All @@ -93,7 +92,7 @@ public async Task ShouldUseDeviceCodeWhenFallbackAsync()
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(delegatedAuthContext, default);

// Assert
Assert.IsType<DeviceCodeCredential>(tokenCredential);
_ = Assert.IsType<DeviceCodeCredential>(tokenCredential);

// reset static instance.
GraphSession.Reset();
Expand All @@ -113,7 +112,7 @@ public async Task ShouldUseInteractiveProviderWhenDelegatedAsync()
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(delegatedAuthContext, default);

// Assert
Assert.IsType<InteractiveBrowserCredential>(tokenCredential);
_ = Assert.IsType<InteractiveBrowserCredential>(tokenCredential);

// reset static instance.
GraphSession.Reset();
Expand All @@ -135,7 +134,7 @@ public async Task ShouldUseInteractiveAuthenticationProviderWhenDelegatedContext
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(delegatedAuthContext, default);

// Assert
Assert.IsType<InteractiveBrowserCredential>(tokenCredential);
_ = Assert.IsType<InteractiveBrowserCredential>(tokenCredential);

// reset static instance.
GraphSession.Reset();
Expand Down Expand Up @@ -167,13 +166,13 @@ public async Task ShouldUseClientCredentialProviderWhenAppOnlyContextIsProvidedA
ContextScope = ContextScope.Process,
TenantId = mockAuthRecord.TenantId
};
CreateAndStoreSelfSignedCert(appOnlyAuthContext.CertificateSubjectName);
_ = CreateAndStoreSelfSignedCert(appOnlyAuthContext.CertificateSubjectName);

// Act
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(appOnlyAuthContext, default);

// Assert
Assert.IsType<ClientCertificateCredential>(tokenCredential);
_ = Assert.IsType<ClientCertificateCredential>(tokenCredential);

// reset
DeleteSelfSignedCertByName(appOnlyAuthContext.CertificateSubjectName);
Expand All @@ -198,7 +197,7 @@ public async Task ShouldUseInMemoryCertificateWhenProvidedAsync()
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(appOnlyAuthContext, default);

// Assert
Assert.IsType<ClientCertificateCredential>(tokenCredential);
_ = Assert.IsType<ClientCertificateCredential>(tokenCredential);

GraphSession.Reset();
}
Expand All @@ -209,7 +208,7 @@ public async Task ShouldUseCertNameInsteadOfPassedInCertificateWhenBothAreSpecif
// Arrange
var dummyCertName = "CN=dummycert";
var inMemoryCertName = "CN=inmemorycert";
CreateAndStoreSelfSignedCert(dummyCertName);
_ = CreateAndStoreSelfSignedCert(dummyCertName);
var inMemoryCertificate = CreateSelfSignedCert(inMemoryCertName);
AuthContext appOnlyAuthContext = new AuthContext
{
Expand All @@ -225,7 +224,7 @@ public async Task ShouldUseCertNameInsteadOfPassedInCertificateWhenBothAreSpecif
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(appOnlyAuthContext, default);

// Assert
Assert.IsType<ClientCertificateCredential>(tokenCredential);
_ = Assert.IsType<ClientCertificateCredential>(tokenCredential);

//CleanUp
DeleteSelfSignedCertByName(appOnlyAuthContext.CertificateSubjectName);
Expand Down Expand Up @@ -254,7 +253,7 @@ public async Task ShouldUseCertThumbPrintInsteadOfPassedInCertificateWhenBothAre
TokenCredential tokenCredential = await AuthenticationHelpers.GetTokenCredentialAsync(appOnlyAuthContext, default);

// Assert
Assert.IsType<ClientCertificateCredential>(tokenCredential);
_ = Assert.IsType<ClientCertificateCredential>(tokenCredential);

//CleanUp
DeleteSelfSignedCertByThumbprint(appOnlyAuthContext.CertificateThumbprint);
Expand Down
68 changes: 68 additions & 0 deletions src/Authentication/Authentication.Test/Helpers/JwtHelpersTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
using Microsoft.Graph.PowerShell.Authentication;
using Microsoft.Graph.PowerShell.Authentication.Core.Utilities;
using Microsoft.Identity.Client;
using Moq;
using System;
using Xunit;

namespace Microsoft.Graph.Authentication.Test.Helpers
{
public class JwtHelpersTests
{
private readonly IAccount _mockIAccount;
public JwtHelpersTests()
{
_mockIAccount = GetIAccountMock();
}

[Fact]
public void DecodeJWTStringShouldReturnAuthContextWithClaims()
{
IAuthContext authContext = new AuthContext
{
AuthType = AuthenticationType.Delegated
};

JwtHelpers.DecodeJWT(MockConstants.DummyAccessToken, _mockIAccount, ref authContext);

Assert.Equal("mockAppId", authContext.ClientId);
Assert.Equal("mockTid", authContext.TenantId);
Assert.Equal("[email protected]", authContext.Account);
Assert.Equal(2, authContext.Scopes.Length);
}

[Fact]
public void DecodeJWTStringShouldReturnNullAuthContextWhenTokenIsNotJWT()
{
IAuthContext authContext = new AuthContext
{
AuthType = AuthenticationType.Delegated
};

JwtHelpers.DecodeJWT("EwCQA_NOT_JWT", _mockIAccount, ref authContext);

Assert.Null(authContext.Scopes);
Assert.Equal("mockUsername", authContext.Account);
}

[Fact]
public void DecodeJWTStringShouldThrowExceptionWhenTokenIsExpired()
{
IAuthContext authContext = new AuthContext
{
AuthType = AuthenticationType.UserProvidedAccessToken
};

_ = Assert.Throws<Exception>(() => JwtHelpers.DecodeJWT(MockConstants.DummyAccessToken, _mockIAccount, ref authContext));
}

private IAccount GetIAccountMock()
{
var accountId = new AccountId("mockId", "mockObjectId", "mockTenantId");
Mock<IAccount> accountMock = new Mock<IAccount>();
_ = accountMock.SetupGet(account => account.HomeAccountId).Returns(accountId);
_ = accountMock.SetupGet(account => account.Username).Returns("mockUsername");
return accountMock.Object;
}
}
}
3 changes: 2 additions & 1 deletion src/Authentication/Authentication.Test/Microsoft.Graph.Authentication.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
<PropertyGroup>
<TargetFrameworks>net6.0;net472</TargetFrameworks>
<IsPackable>false</IsPackable>
<Version>2.6.1</Version>
<Version>2.8.0</Version>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.2" />
<!-- As described in this post https://devblogs.microsoft.com/powershell/depending-on-the-right-powershell-nuget-package-in-your-net-project, reference the SDK for dotnetcore-->
<PackageReference Include="Microsoft.PowerShell.SDK" Version="7.2.2" PrivateAssets="all" Condition="'$(TargetFramework)' == 'net6.0'" />
<PackageReference Include="Moq" Version="4.20.69" />
<PackageReference Include="xunit" Version="2.4.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
<PrivateAssets>all</PrivateAssets>
Expand Down
12 changes: 12 additions & 0 deletions src/Authentication/Authentication.Test/MockConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. See License in the project root for license information.
// ------------------------------------------------------------------------------

namespace Microsoft.Graph.Authentication.Test
{
internal class MockConstants
{
// This is a dummy access token that is used for testing purposes only. Expired at 2023-10-25T22:23:50.265Z.
internal const string DummyAccessToken = "eyJhbGciOiJIUzI1NiJ9.eyJSb2xlIjoiVGVzdCIsInNjcCI6Im9wZW5pZCBSZXBvcnRzLlJlYWQiLCJ1cG4iOiJ1cG5AY29udG9zby5jb20iLCJJc3N1ZXIiOiJJc3N1ZXIiLCJVc2VybmFtZSI6IlRlc3QiLCJhcHBpZCI6Im1vY2tBcHBJZCIsImFwcF9kaXNwbGF5bmFtZSI6Im1vY2tOYW1lIiwiZXhwIjoxNjk4MjcyNjMwLCJpYXQiOjE2OTgyNzI2MzAsInRpZCI6Im1vY2tUaWQifQ.sA7eX8PRxhUTRnXHYZyFB095jszZX75NeIjUae8oGic";
}
}