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

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

Open
RufusJWB opened this issue Jan 14, 2023 · 11 comments
Labels

Comments

@RufusJWB
Copy link

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
Copy link
Owner

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
Copy link
Author

RufusJWB commented Jan 15, 2023

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:[email protected]"} }
};


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
Copy link
Owner

@RufusJWB , try:

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

@RufusJWB
Copy link
Author

@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: dotnet/runtime#31094 (comment)

@dvsekhvalnov
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

dvsekhvalnov commented Jan 16, 2023

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
Copy link
Author

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 dvsekhvalnov changed the title Serialize JWK without empty fields Allow easy subclassing default IJsonMapper implementations: to let adjust settings easily. Jan 17, 2023
@dvsekhvalnov
Copy link
Owner

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

@RufusJWB
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants