dvsekhvalnov / jose-jwt

Ultimate Javascript Object Signing and Encryption (JOSE), JSON Web Token (JWT) and Json Web Keys (JWK) Implementation for .NET and .NET Core
MIT License
936 stars 184 forks source link

Allow easy subclassing default IJsonMapper implementations: to let adjust settings easily. #214

Open RufusJWB opened 1 year ago

RufusJWB commented 1 year ago

Is it possible to tell the default JSON serializer to ignore empty fields of a JWK and don't serialize them?

Currently my JWS header looks like this after serialization:

{
  "alg": "ES256",
  "JWK": {
    "kty": "EC",
    "use": null,
    "alg": null,
    "keyId": null,
    "keyOps": null,
    "k": null,
    "n": null,
    "e": null,
    "d": null,
    "p": null,
    "dp": null,
    "q": null,
    "dq": null,
    "qi": null,
    "crv": null,
    "x": "mS9EPo_7ZJGgva3NJMAMFBrYj_a65y7wNc6dXLVgAho",
    "y": "3JB6m2MWaMvcIJVqV2WSH59SLtPsa2MtGrCRTsgPPo8",
    "x5U": null,
    "x5T": null,
    "x5TSha256": null,
    "x5C": [
      "MIIBqTCCAU6gAwIBAgIUTSEzQzGXgJRGrQF5U6OVMO2ku2MwCgYIKoZIzj0EAwIwKzEpMCcGCSqGSIb3DQEJARYacnVmdXMuYnVzY2hhcnRAc2llbWVucy5jb20wHhcNMjMwMTEzMTQ1MDAwWhcNMjUxMDA5MTQ1MDAwWjArMSkwJwYJKoZIhvcNAQkBFhpydWZ1cy5idXNjaGFydEBzaWVtZW5zLmNvbTBWMBAGByqGSM49AgEGBSuBBAAKA0IABJkvRD6P+2SRoL2tzSTADBQa2I/2uucu8DXOnVy1YAIa3JB6m2MWaMvcIJVqV2WSH59SLtPsa2MtGrCRTsgPPo+jUzBRMB0GA1UdDgQWBBQtKe/ByzbXhDmuHleGcfSg6GiSdzAfBgNVHSMEGDAWgBQtKe/ByzbXhDmuHleGcfSg6GiSdzAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49BAMCA0kAMEYCIQD/9uGdD2zKI3kl8PFT/kGhi79uIakNyNY6mRfeTEAhNAIhALR79ZA5n4M4J4ky4iHfCQU9at+GU8V0V8dmZDrnpj1v"
    ],
    "otherParams": null
  }
}

And I'd like to have it look like this:

{
  "alg": "ES256",
  "JWK": {
    "kty": "EC",
    "x": "mS9EPo_7ZJGgva3NJMAMFBrYj_a65y7wNc6dXLVgAho",
    "y": "3JB6m2MWaMvcIJVqV2WSH59SLtPsa2MtGrCRTsgPPo8",
    "x5C": [
      "MIIBqTCCAU6gAwIBAgIUTSEzQzGXgJRGrQF5U6OVMO2ku2MwCgYIKoZIzj0EAwIwKzEpMCcGCSqGSIb3DQEJARYacnVmdXMuYnVzY2hhcnRAc2llbWVucy5jb20wHhcNMjMwMTEzMTQ1MDAwWhcNMjUxMDA5MTQ1MDAwWjArMSkwJwYJKoZIhvcNAQkBFhpydWZ1cy5idXNjaGFydEBzaWVtZW5zLmNvbTBWMBAGByqGSM49AgEGBSuBBAAKA0IABJkvRD6P+2SRoL2tzSTADBQa2I/2uucu8DXOnVy1YAIa3JB6m2MWaMvcIJVqV2WSH59SLtPsa2MtGrCRTsgPPo+jUzBRMB0GA1UdDgQWBBQtKe/ByzbXhDmuHleGcfSg6GiSdzAfBgNVHSMEGDAWgBQtKe/ByzbXhDmuHleGcfSg6GiSdzAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49BAMCA0kAMEYCIQD/9uGdD2zKI3kl8PFT/kGhi79uIakNyNY6mRfeTEAhNAIhALR79ZA5n4M4J4ky4iHfCQU9at+GU8V0V8dmZDrnpj1v"
    ]
  }
}
dvsekhvalnov commented 1 year ago

Hi @RufusJWB , can you please share code that producing it?

And also environment details, .net version, core or framework, OS, json serializer (newtonsoft vs. system.text.json), e.t.c.

I haven't seen it in my setup, may be env specific issue or json serializer specific.

RufusJWB commented 1 year ago

Hi @dvsekhvalnov !

Thank you for your support!

Please find following the sources, that generate this output:

using Jose;
using NLog;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;

NLog.LogManager.Setup().LoadConfiguration(builder =>
{
    builder.ForLogger().FilterMinLevel(LogLevel.Trace).WriteToConsole();
});

NLog.Logger Logger = NLog.LogManager.GetCurrentClassLogger();

var payload = new Dictionary<string, object>()
{
    { "termsOfServiceAgreed", true },
    { "contact", new []{ "mailto:rufus.buschart@siemens.com"} }
};

X509Certificate2 selfSignedIDevID = new X509Certificate2("C:\\Users\\z002m76a\\Desktop\\certificate.pem");

var publicKey = selfSignedIDevID.GetECDsaPublicKey();

Jwk jwk = new Jwk(publicKey,isPrivate:false);
jwk.Add(selfSignedIDevID);

var extraHeaders = new Dictionary<string, object>()
{
    {"nonce","6S8IqOGY7eL2lsGoTZYifg" },
    {"url","https://acme-v02.api.letsencrypt.org/acme/new-acct"},
    {"JWK",jwk }
};

var eccPrivPem = File.ReadAllText("C:\\Users\\z002m76a\\Desktop\\private_key.pem");
var privateKey = ECDsa.Create();
privateKey.ImportFromPem(eccPrivPem);

var mapper = Jose.JWT.DefaultSettings.JsonMapper;

Logger.Debug("Mapper: {@typof}", mapper.GetType());

string token = Jose.JWT.Encode(payload:payload, key: privateKey, algorithm:JwsAlgorithm.ES256, extraHeaders:extraHeaders);

Logger.Debug("Token: {@token}",token);

image

I'm using Visual Studio Professional 2022, Version 17.5.0 Preview 2.0. The application is a console application targeting .net 7.

dvsekhvalnov commented 1 year ago

@RufusJWB , try:

{"JWK", jwk.ToDictionary() } // instead of just 'jwk'
RufusJWB commented 1 year ago

@RufusJWB , try:

{"JWK", jwk.ToDictionary() } // instead of just 'jwk'

I can confirm, that this works. But it seems to be a rather ugly work-around. I would like to propose, to expose the serializer options of the serializer, as there is no global way of setting them: https://github.com/dotnet/runtime/issues/31094#issuecomment-1235744986

dvsekhvalnov commented 1 year ago

Your question - is exactly why library is trying to avoid serialization/deserialization of objects at all :)

Because there is no guarantee that you will use System.Text.Json. It can be Newtonsoft or something else, and then suddenly library have to deal with all those options: skip nulls, avoid defaults, e.t.c. multiplied by number of json parser implementation.

Trying to stick to bare minimum which more or less working same across everything: primites, maps and lists :)

RufusJWB commented 1 year ago

I totally understand that, but it would help a lot, if you would make the Serializer / Deserializer property publicly available: https://github.com/dvsekhvalnov/jose-jwt/blob/master/jose-jwt/json/JsonMapper.cs#L13

dvsekhvalnov commented 1 year ago

But you can always register your own JsonMapper with any settings you like: https://github.com/dvsekhvalnov/jose-jwt#settings

isn't it what you looking for if you want to adjust default behavior?

i can make SerializeOptions/DeserializeOptions protected for instance, will be easier to inherit default impl?

RufusJWB commented 1 year ago

i can make SerializeOptions/DeserializeOptions protected for instance, will be easier to inherit default impl?

That's something I'd appreciate as I want to avoid implementing my own JsonMapper just for changing some basic setting. You know, programmers are lazy people :-)

dvsekhvalnov commented 1 year ago

Okay, should be easy. How badly you want it @RufusJWB ? Like right now or can wait?

RufusJWB commented 1 year ago

Okay, should be easy. How badly you want it @RufusJWB ? Like right now or can wait?

It can wait. It's just about code aesthetics. The proposed work around works fine.

dvsekhvalnov commented 1 year ago

Okay, you can also just submit PR for this any time, can speed up things :)