NetDevPack / Security.Jwt

Jwt Manager. Set of components to deal with Jwt Stuff. Automate your key rotating, add support for jwks_uri. Store your cryptography keys in a secure place.
MIT License
271 stars 38 forks source link

Remove STJ from net 6+ #62

Closed thompson-tomo closed 5 months ago

thompson-tomo commented 5 months ago

Describe the feature.

I want my application's and libraries to be as small as possible and the minimum amount of dependencies.

Is your feature related to a problem?

Additional dependencies are being added to my projects.

Describe the requested feature

When the framework is able to natively provide a dependency ie System.Text.Json that should be utilised rather than an explicit dependency. This will be the case for the net 5+ TFM.

Describe alternatives you've considered

Accept the additional dependencies

Additional Context

No response

brunobritodev commented 5 months ago

Hi @thompson-tomo

Thanks to bring this to our attention, I understand your concern.

This component is strong coupled with System.Text.Json by design choice. Firstly, this component was designed specifically for use with ASP.NET Core applications, which use System.Text.Json natively. Secondly, giving serialization choices would require a design that shifts this complexity onto developers. From my personal experience, such design decisions can be problematic as most users would either opt not to use the component or would open up support tickets, in addition it requires additional steps in the builder proccess.

However, we can explore alternatives to offer more flexibility. One option might be implementing an ISerialization interface, allowing some customization while still defaulting to System.Text.Json for those who prefer the out-of-the-box setup.

thompson-tomo commented 5 months ago

@brunobritodev i think you have missed my point. I have no issue with STJ being used for the sterilisation. What I am suggesting is having the dependency only included when the framework ie net 6 doesn't provide the functionality natively. This would be just like how we have System.IO without adding an additional dependency.

Take a look at https://github.com/ligershark/WebOptimizer/pull/307/files for an example of what I am suggesting.

brunobritodev commented 5 months ago

@thompson-tomo first, my apologies!

I appreciate your patience, and I'm working to fully understand your point as it isn't entirely clear to me.

I've looked into the dependencies of System.Text.Json and noticed it lacks compatibility with older .NET versions. Is this the issue you're addressing?

image

So In these cases, add another STJ version to work it's retro compatibility?

thompson-tomo commented 5 months ago

Not quite an example would be for https://www.nuget.org/packages/NetDevPack.Security.JwtExtensions STJ would only be listed as a dependency for net standard due to it being bundled into the framework in net 6+ with the restriction being achieved by adding a condition on the package reference.

Happy to provide a pr tomorrow to show what I mean.

brunobritodev commented 5 months ago

@thompson-tomo I see, adding a FrameworkReference System.Text.Json will not listed as a dependency, so it make more flexible.

Great, go ahead, make a PR!

thompson-tomo commented 5 months ago

All done. @brunobritodev