-
Notifications
You must be signed in to change notification settings - Fork 464
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
Decode<JwtHeader> not working #456
Comments
hi @thoron, what's your target platform? I'm unable to reproduce the issue. I've put your token into TestData and these Header related tests. After updating the asserts accordingly they all have passed. |
@abatishchev I encountered the issue when using a win11 computer. Now using a Linux computer (Pop 22.04) I cannot reproduce it either. Both were targeting net7.0. I could reproduce it on my Linux machine when using same target (net7.0) and dotnet version (7.0.102) [Test]
public void TestJwt()
{
var tokenString = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
var serializer = new JsonNetSerializer();
var urlEncoder = new JwtBase64UrlEncoder();
var decoder = new JwtDecoder(serializer, urlEncoder);
var header = decoder.DecodeHeader<JwtHeader>(tokenString);
Assert.AreEqual("HS256", header.Algorithm); // null
} |
I have only Windows machine but I'll try to reproduce it on different frameworks too. |
I see, so you can reproduce it now. |
Using .NET 7 SDK with target net6.0 can to reproduce the issue when using JWT 10.0.0. Downgrading fixes the issue. |
I'm running into a similar issue, accept the problem comes from using JsonProperty attributes. I made a little unit test for this: using JWT;
using JWT.Algorithms;
using JWT.Builder;
using JWT.Serializers;
using Newtonsoft.Json;
namespace JwtTokenTest;
public class UnitTest1
{
[Fact]
public void DecodeWithJsonPropertyNotWorkingTest()
{
var secret = "ahu9ar9hgiurehagh;areifuiawehfiawheuifhaewhfi;ewhiguahierug";
var myValue = "abcd";
var encoded = CreateToken(secret, new OAuthPayloadModel()
{
AccessToken = myValue,
Exp = DateTimeOffset.UtcNow.AddSeconds(26600).ToUnixTimeSeconds()
});
Assert.NotNull(encoded);
var oauth2 = DecodeToken<OAuthPayloadModel>(secret, encoded);
Assert.Null(oauth2?.AccessToken);
// Assert.Equal(myValue, oauth2.AccessToken);
}
[Fact]
public void DecodeWithoutJsonPropertyNotWorkingTest()
{
var secret = "ahu9ar9hgiurehagh;areifuiawehfiawheuifhaewhfi;ewhiguahierug";
var myValue = "abcd";
var encoded = CreateToken(secret, new OAuthPayloadWithoutJsonPropertyModel()
{
AccessToken = myValue,
Exp = DateTimeOffset.UtcNow.AddSeconds(26600).ToUnixTimeSeconds()
});
Assert.NotNull(encoded);
var oauth2 = DecodeToken<OAuthPayloadWithoutJsonPropertyModel>(secret, encoded);
Assert.NotNull(oauth2?.AccessToken);
Assert.Equal(myValue, oauth2.AccessToken);
}
/// <summary>
/// https://github.com/jwt-dotnet/jwt
/// </summary>
protected string CreateToken(string secret, object payload)
{
if (string.IsNullOrEmpty(secret))
throw new Exception("Api secret not set");
var algorithm = new HMACSHA256Algorithm();
var serializer = new JsonNetSerializer();
var urlEncoder = new JwtBase64UrlEncoder();
var encoder = new JwtEncoder(algorithm, serializer, urlEncoder);
var token = encoder.Encode(payload, secret);
return token;
}
protected T DecodeToken<T>(string secret, string token) where T : class, new()
{
return new JwtBuilder()
.WithAlgorithm(new HMACSHA256Algorithm()) // symmetric
.WithSecret(secret)
.MustVerifySignature()
.Decode<T>(token);
}
public class OAuthPayloadModel
{
[JsonProperty("HA")]
public string AccessToken { get; set; }
[JsonProperty("EX")]
public long? Exp { get; set; }
}
public class OAuthPayloadWithoutJsonPropertyModel
{
public string AccessToken { get; set; }
public long? Exp { get; set; }
}
} Both unit test will run fine, but the first one, This issue is with .Net 7 (7.0.101) on macOS and in production on Linux (latest version also) |
I found the issue. It seems that on encoding I specify the JsonSerializer, while on de decoder I didn't: protected string CreateToken(string secret, object payload)
{
if (string.IsNullOrEmpty(secret))
throw new Exception("Api secret not set");
return new JwtBuilder()
.WithJsonSerializer(new JsonNetSerializer())
.WithAlgorithm(new HMACSHA256Algorithm()) // symmetric
.WithSecret(secret)
.MustVerifySignature()
.Encode(payload);
}
protected T DecodeToken<T>(string secret, string token) where T : class, new()
{
return new JwtBuilder()
.WithJsonSerializer(new JsonNetSerializer())
.WithAlgorithm(new HMACSHA256Algorithm()) // symmetric
.WithSecret(secret)
.MustVerifySignature()
.Decode<T>(token);
} I guess that version 10 maybe default switch from json serialised, therefor breaking my code. Setting the serializer on encoding and decoding solves my problem. |
I found this issue too. It's because now it decodes using System.Text.Json instead of Json.NET for newer .net versions. @abatishchev This is the first non-beta nuget for which #401 is live, right? Previously it uses Json.NET and for that you can have a global state for the serializer settings, now from 10.0.0.0 and upwards it will use System.Text.Json as default. One thing with System.Text.Json is that it has no global state anymore, probably to make it more thread safe and so on. But it will break some previous assumptions in older code. I have this code to set the Json.NET global serializer settings to serialize as camel-case. JsonConvert.DefaultSettings = () => new JsonSerializerSettings
{
ContractResolver = new CamelCasePropertyNamesContractResolver(),
Formatting = Formatting.None
}; So previously my code just serialized my JWTs with the following code: var token = JwtBuilder.Create()
.WithAlgorithm(new NoneAlgorithm())
.AddHeader("vrsn", "1")
.WithSecret(Secret); To make it use Json.NET again you can just build your tokens with this code instead, note the last line: var token = JwtBuilder.Create()
.WithAlgorithm(new NoneAlgorithm())
.AddHeader("vrsn", "1")
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer()) |
Hi everyone, thanks for chiming in and sorry for the troubles. I hoped making STJ the serializer by default would improve things, not the other way around.
Do you know if an issue was filed on them?
Correct. Multiple beta versions were published and I hoped that would be tested by the users. But since it's a pre-release naturally some people try to avoid them (me included). WithJsonSerializer(new JsonNetSerializer()) Yes, that's the current workaround.
What I can do is to roll back the change and make Json.Net the serializer by default again, let's say in version Meanwhile, if someone could contribute to #407 it'd be very much appreciated and could have had caught this issue earlier. |
I think it's good that there is no longer any hidden global state for the serializer. This is however a new major version so some change in behaviour will be expected. So I don't it's needed to rollback to JSON.net as the default. Perhaps having it mentioned in the release notes would be good however. I have no experience with setting up pipelines but having one for Linux would be nice. |
Personally, no 'rollback' is necessary for me. Is there any reference (possible breaking change) about this change on the release notes? That should be enough, in my opinion. I have to admit, I just updated the nuget packages in my project without thinking about anything. And... no unit tests... The reason why it didn't work for me, was because of using |
Yes, I've started keeping CHANGELOG.md, per the suggestion by @cmeeren.
In your own code? Here's how it "worked around" in the library's: jwt/src/JWT/Builder/JwtHeader.cs Lines 1 to 5 in 05fce36
Just in case it'd be helpful. |
In my case it's being mixed, so using System.Text.Json and NewtonSoft's JsonProperty. So that won't work. I'm afraid I cheered a bit too early this afternoon. I now created 2 tests, where I for use JsonNetSerializer, and for the other the SystemTextSerializer. I also added the JsonProperty for both. SystemTextSerializer works, but JsonNetSerializer doesn't: using System;
using JWT.Algorithms;
using JWT.Builder;
using JWT.Serializers;
using Newtonsoft.Json;
using Xunit;
namespace WillowMedia.Insight.Tests;
public class JwtNewtonSoftTests
{
public class TokenModel
{
[Newtonsoft.Json.JsonProperty("AT")]
public string AccessToken { get; set; }
public long? Exp { get; set; }
}
[Fact]
public void NoneAlgorithmEncodeDecodeTest()
{
var Secret = "wgaeg43g35h44a3hq4w55a3";
var token = JwtBuilder.Create()
.WithAlgorithm(new NoneAlgorithm())
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer());
var encoded = token.Encode(new TokenModel()
{
AccessToken = "abc123",
Exp = DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeSeconds()
});
Assert.NotNull(encoded);
token = JwtBuilder.Create()
.WithAlgorithm(new NoneAlgorithm())
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer());
var payloadDecoded = token.Decode<TokenModel>(encoded);
Assert.NotNull(payloadDecoded?.AccessToken);
}
} Could you please try if this works for you? |
Found the issue, will fix and release an update version shortly. Long story short: the initial idea was to replace NS.J with STJ but then it evolved into allowing them side-by-side. But the header class accidentally stayed working only for the former option. This code precisely causes the issue: jwt/src/JWT/Builder/JwtHeader.cs Lines 1 to 5 in 05fce36
Fixed version: jwt/src/JWT/Builder/JwtHeader.cs Lines 1 to 5 in ebc0945
Sorry for the troubles! And thank you everyone for helping to identify the issue. |
Pushed version 10.0.1. |
Thanks, but unfortunately there is no change for me. I can't use the JsonProperty or JsonPropertyName attributes. I removed them so at least it works |
Can you describe your scenario and/or environment? I bet there is more than one way to customize/override (de)serialization behavior. |
I have this test (xunit new project) with NewtonSoft.Json: using JWT.Algorithms;
using JWT.Builder;
using JWT.Serializers;
namespace JwtTests;
public class JwtNewtonSoftTests
{
public class TokenModel
{
/// <summary>
/// Our accesstoken
/// </summary>
[Newtonsoft.Json.JsonProperty("AT")]
public string AccessToken { get; set; }
/// <summary>
/// Exp is the expiration date in seconds since epoch
/// </summary>
public long? Exp { get; set; }
}
public const string Secret = "wgaeg43g35h44a3hq4w55a3";
[Fact]
public void NoneAlgorithmEncodeDecodeTest()
{
var token = JwtBuilder.Create()
.WithAlgorithm(new NoneAlgorithm())
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer());
var model = new TokenModel()
{
AccessToken = "abc123",
Exp = DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeSeconds()
};
var encoded = token.Encode(model);
Assert.NotNull(encoded);
token = JwtBuilder.Create()
.WithAlgorithm(new NoneAlgorithm())
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer());
var payloadDecoded = token.Decode<TokenModel>(encoded);
Assert.Equal(model.AccessToken, payloadDecoded.AccessToken);
}
[Fact]
public void HMACSHA256AlgorithmEncodeDecodeTest()
{
var jwtToken = JwtBuilder.Create()
.WithAlgorithm(new HMACSHA256Algorithm())
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer());
var model = new TokenModel()
{
AccessToken = "abc123",
Exp = DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeSeconds()
};
var encoded = jwtToken.Encode(model);
Assert.NotNull(encoded);
jwtToken = JwtBuilder.Create()
.WithAlgorithm(new HMACSHA256Algorithm())
.WithSecret(Secret)
.WithJsonSerializer(new JsonNetSerializer());
var payloadDecoded = jwtToken.Decode<TokenModel>(encoded);
Assert.Equal(model.AccessToken, payloadDecoded.AccessToken);
}
} These both fail |
Even in version 10.0.1? And succeeds in version 9.0.3? |
Yes, this is with 10.0.1 |
Would you like me to publish the test project somewhere? |
Sure, please. I think this would be the right place for it: https://github.com/jwt-dotnet/jwt/blob/main/tests/JWT.Tests.Common/Builder/JwtBuilderDecodeTests.cs |
Good finding, seems when I wrote this I didn't take into account that the name of the properties might have been overriden by the attribute. @PeterHagen I used your tests as base and added them into the repo in the linked PR above |
I forgot to mention that this also applies to using System.Text.Json with the JsonPropertyNameAttribute, and the correct serializer |
Regression issue in Decode
When upgrading from 9.0.3 to 10.0.0 decode header will cease to function correctly, all fields will be
null
:For 9.0.3
For 10.0.0
The text was updated successfully, but these errors were encountered: