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.

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 JwtSecurityToken Missing Mapping When Creating a Token. #2578

Merged
merged 16 commits into from
May 20, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ namespace Microsoft.IdentityModel.Tokens
/// </summary>
public class X509EncryptingCredentials : EncryptingCredentials
{
internal const string _useShortNameForRsaOaepKey = "Switch.Microsoft.IdentityModel.UseShortNameForRsaOaepKey";
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Designed to construct <see cref="EncryptingCredentials"/> based on a x509 certificate.
/// </summary>
Expand All @@ -21,7 +23,7 @@ public class X509EncryptingCredentials : EncryptingCredentials
/// </remarks>
/// <exception cref="ArgumentNullException">if 'certificate' is null.</exception>
public X509EncryptingCredentials(X509Certificate2 certificate)
: this(certificate, SecurityAlgorithms.DefaultAsymmetricKeyWrapAlgorithm, SecurityAlgorithms.DefaultSymmetricEncryptionAlgorithm)
: this(certificate, ShouldUseShortNameForRsaOaepKey() ? SecurityAlgorithms.RsaOAEP : SecurityAlgorithms.DefaultAsymmetricKeyWrapAlgorithm, SecurityAlgorithms.DefaultSymmetricEncryptionAlgorithm)
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
{
}

Expand All @@ -48,5 +50,10 @@ public X509Certificate2 Certificate
get;
private set;
}

private static bool ShouldUseShortNameForRsaOaepKey()
{
return AppContext.TryGetSwitch("Switch.Microsoft.IdentityModel.UseShortNameForRsaOaepKey", out var useKeyWrap) && useKeyWrap;
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class JwtSecurityTokenHandler : SecurityTokenHandler
private Dictionary<string, string> _outboundAlgorithmMap = null;
private static string _shortClaimType = _namespace + "/ShortTypeName";
private bool _mapInboundClaims = DefaultMapInboundClaims;

FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// Default claim type mapping for inbound claims.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using Xunit;
pmaytak marked this conversation as resolved.
Show resolved Hide resolved
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved

namespace System.IdentityModel.Tokens.Jwt.Tests
{
[CollectionDefinition(nameof(JwtSecurityTokenHandlerNonParallelRunTests), DisableParallelization = true)]
public class JwtSecurityTokenHandlerNonParallelRunTests
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
using System.Security.Claims;
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
using Microsoft.IdentityModel.JsonWebTokens;
using Microsoft.IdentityModel.TestUtils;
using Microsoft.IdentityModel.Tokens;
using Xunit;

namespace System.IdentityModel.Tokens.Jwt.Tests
{
[Collection(nameof(JwtSecurityTokenHandlerNonParallelRunTests))]
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
public class JwtSecurityTokenHandlerTestsWithContextSwitches
{
[Fact]

FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
public void JwtSecurityTokenHandler_CreateToken_AddShortFormMappingForRsaOAEPEnabled()
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
{
AppContext.SetSwitch(X509EncryptingCredentials._useShortNameForRsaOaepKey, true);
var encryptingCredentials = new X509EncryptingCredentials(Default.Certificate);
SecurityTokenDescriptor tokenDescriptor = new SecurityTokenDescriptor
JoshLozensky marked this conversation as resolved.
Show resolved Hide resolved
{
Issuer = Default.Issuer,
IssuedAt = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Subject = new ClaimsIdentity(Default.PayloadJsonClaims),
NotBefore = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Expires = DateTime.UtcNow.Subtract(new TimeSpan(0, 10, 0)),
SigningCredentials = Default.AsymmetricSigningCredentials,
EncryptingCredentials = encryptingCredentials,
TokenType = "JWE"
};

JwtSecurityTokenHandler tokenHandler = new JwtSecurityTokenHandler();
JwtSecurityToken token = tokenHandler.CreateJwtSecurityToken(tokenDescriptor);

Assert.NotNull(token);
Assert.NotEqual(token.Header.Alg, SecurityAlgorithms.RsaOaepKeyWrap);
Assert.Equal(token.Header.Alg, SecurityAlgorithms.RsaOAEP);
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
}
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved

[Fact]
public void JwtSecurityTokenHandler_CreateToken_AddShortFormMappingForRsaOAEPDisabled()
{
AppContext.SetSwitch(X509EncryptingCredentials._useShortNameForRsaOaepKey, false);
var encryptingCredentials = new X509EncryptingCredentials(Default.Certificate);
SecurityTokenDescriptor tokenDescriptor = new SecurityTokenDescriptor
{
Issuer = Default.Issuer,
IssuedAt = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Subject = new ClaimsIdentity(Default.PayloadJsonClaims),
NotBefore = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Expires = DateTime.UtcNow.Subtract(new TimeSpan(0, 10, 0)),
SigningCredentials = Default.AsymmetricSigningCredentials,
EncryptingCredentials = encryptingCredentials,
TokenType = "JWE"
};

JwtSecurityTokenHandler tokenHandler = new JwtSecurityTokenHandler();
JwtSecurityToken token = tokenHandler.CreateJwtSecurityToken(tokenDescriptor);

Assert.NotNull(token);
Assert.NotEqual(token.Header.Alg, SecurityAlgorithms.RsaOAEP);
Assert.Equal(token.Header.Alg, SecurityAlgorithms.RsaOaepKeyWrap);
}

[Fact]
public void JsonWebTokenHandler_CreateToken_AddShortFormMappingForRsaOAEPEnabled()
{
AppContext.SetSwitch(X509EncryptingCredentials._useShortNameForRsaOaepKey, true);
var encryptingCredentials = new X509EncryptingCredentials(Default.Certificate);
SecurityTokenDescriptor tokenDescriptor = new SecurityTokenDescriptor
{
Issuer = Default.Issuer,
IssuedAt = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Subject = new ClaimsIdentity(Default.PayloadJsonClaims),
NotBefore = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Expires = DateTime.UtcNow.Subtract(new TimeSpan(0, 10, 0)),
SigningCredentials = Default.AsymmetricSigningCredentials,
EncryptingCredentials = encryptingCredentials,
TokenType = "JWE"
};

JsonWebTokenHandler tokenHandler = new JsonWebTokenHandler();
var token = tokenHandler.CreateToken(tokenDescriptor);
JsonWebToken jsonToken = tokenHandler.ReadToken(token) as JsonWebToken;

Assert.NotNull(jsonToken);
Assert.NotEqual(jsonToken._alg, SecurityAlgorithms.RsaOaepKeyWrap);
Assert.Equal(jsonToken._alg, SecurityAlgorithms.RsaOAEP);
}

[Fact]
public void JsonWebTokenHandler_CreateToken_AddShortFormMappingForRsaOAEPDisabled()
{
AppContext.SetSwitch(X509EncryptingCredentials._useShortNameForRsaOaepKey, false);
var encryptingCredentials = new X509EncryptingCredentials(Default.Certificate);
SecurityTokenDescriptor tokenDescriptor = new SecurityTokenDescriptor
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
{
Issuer = Default.Issuer,
IssuedAt = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Subject = new ClaimsIdentity(Default.PayloadJsonClaims),
NotBefore = DateTime.UtcNow.Subtract(new TimeSpan(1, 0, 0)),
Expires = DateTime.UtcNow.Subtract(new TimeSpan(0, 10, 0)),
SigningCredentials = Default.AsymmetricSigningCredentials,
EncryptingCredentials = encryptingCredentials,
TokenType = "JWE"
};

JsonWebTokenHandler tokenHandler = new JsonWebTokenHandler();
var token = tokenHandler.CreateToken(tokenDescriptor);
JsonWebToken jsonToken = tokenHandler.ReadToken(token) as JsonWebToken;
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved

Assert.NotNull(jsonToken);
FuPingFranco marked this conversation as resolved.
Show resolved Hide resolved
Assert.Equal(jsonToken._alg, SecurityAlgorithms.RsaOaepKeyWrap);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use jsonWebToken.Alg?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underscore variables should really be private...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious why were they not private when working on them? I changed it to use 'Alg' same as in JwtSecurityTokenHandler. Thanks Brent & Peter!

Assert.NotEqual(jsonToken._alg, SecurityAlgorithms.RsaOAEP);
}
}
}
Loading