Added support for JSON Web key sets by CADBIMDeveloper · Pull Request #489 · jwt-dotnet/jwt

CADBIMDeveloper · 2024-01-27T11:35:47Z

Description

Based on #488 , adds JWKS support

Breaking changes

None

Checklist

Tests added
Version bumped
Changelog updated

CADBIMDeveloper · 2024-01-27T11:35:55Z

Привет, @abatishchev,

It's currently far away from fully ready, anyway, take a look please. Would like to hear from you, if you agree or not about namespaces, naming and the approach itself

CADBIMDeveloper · 2024-02-08T05:48:06Z

Александр, @abatishchev, do I need to add/fix something here? Ping :-)

abatishchev · 2024-02-08T05:58:49Z

Thank you so much for your contribution! It's a lot and more than I expected. Please allow me some time to review it. Will do :)

abatishchev · 2024-04-16T14:22:51Z

My sincere apologies! It felt through the cracks of my memory, but now it's back and I'll review the changes as part of the upcoming major version release. Once again, thanks for your contribution!

with an ability to search keys by ids

to create an algorithms based on JSON Web key sets

to jwt validator

to make a decoder with JWKS

JSON Web key

abatishchev · 2024-04-29T20:50:26Z

hey @CADBIMDeveloper, I'm back to the PR, as promised :) can you please check why all tests are failing? and rebase on the latest main please too.

CADBIMDeveloper · 2024-04-30T06:48:53Z

Hey @abatishchev ,

Hypothesis: You've removed src/Directory.Build.targets where MODERN_DOTNET constant was defined. I will check and fix it this week.

abatishchev · 2024-04-30T14:08:13Z

Yeah, it was removed in #497, the .NET team has confirmed that it's better not to reference it until strictly necessary, e.g. in lower target frameworks which don't ship it within

replace MODERN_DOTNET which is not presented any more

CADBIMDeveloper · 2024-05-01T08:56:17Z

Hey @abatishchev , it's working now :-)

abatishchev · 2024-05-02T22:58:05Z

src/JWT/Algorithms/HMACSHAAlgorithmFactory.cs

    {
+        private readonly byte[] _key;
+
+        public HMACSHAAlgorithmFactory()


Why do all classes need an empty ctor? Both the algos and the factories?

I didn't want to introduce breaking changes for the user code like:

JwtBuilder.Create().WithAlgorithm(...)...; JwtBuilder.Create().WithAlgorithm(...).WithSecret(...).Encode(...); JwtBuilder.Create().WithAlgorithmFactory(...)...;

I can remove empty ctors if you want to

abatishchev · 2024-05-02T23:04:38Z

src/JWT/Jwk/JwtWebKeyPropertyValuesEncoder.cs

+            }
+        }
+
+        private static char DecodeCharacter(char ch)


Is this a good name for this method?

To be honest, I took it from Microsoft.AspNetCore.WebUtilities.WebEncoders and didn't pay a lot of attention to it. Will it be okay for you, if I rename it to Transform(char symbol)?

abatishchev · 2024-05-02T23:05:13Z

src/JWT/Jwk/IJwtWebKeysCollectionFactory.cs

@@ -0,0 +1,6 @@
+namespace JWT.Jwk;


Please convert implicit namespace into explicit (here and elsewhere too), for the consistency of the codebase

Oops. sorry. This is the only place with implicit namespace (my bad, miss it). Fixed

src/JWT/Jwk/JwtWebKeySet.cs

abatishchev · 2024-05-02T23:06:06Z

src/JWT/Jwk/JwtWebKeysCollection.cs

+        }
+
+        public JwtWebKey Find(string keyId)
+        {


nit: use the => syntax for one-line methods

abatishchev · 2024-05-02T23:06:20Z

src/JWT/Jwk/JwtWebKeysCollection.cs

+
+        public JwtWebKeysCollection(JwtWebKeySet keySet) : this(keySet.Keys)
+        {
+


nit: remove line breaks from empty methods/ctors

abatishchev · 2024-05-02T23:07:15Z

src/JWT/JwtDecoder.cs

                _jwtValidator.Validate(decodedPayload, asymmAlg, bytesToSign, decodedSignature);
            }
-            else
+            else if (algorithm is ISymmetricAlgorithm symmAlg)


Should we use pattern matching switch/case here?

abatishchev · 2024-05-02T23:07:55Z

src/JWT/JwtDecoder.cs

+            else if (algorithm is ISymmetricAlgorithm symmAlg)
            {
-                ValidSymmetricAlgorithm(keys, decodedPayload, algorithm, bytesToSign, decodedSignature);
+                _jwtValidator.Validate(keys, decodedPayload, symmAlg, bytesToSign, decodedSignature);


Is the existing (non-algo specific) method Validate() still needed then?

not sure...

src/JWT/JwtValidator.cs

src/JWT/Jwk/JwtWebKeysCollection.cs

abatishchev · 2024-05-02T23:17:46Z

src/JWT/Jwk/JwtWebKeysCollection.cs

+
+namespace JWT.Jwk
+{
+    public class JwtWebKeysCollection : IJwtWebKeysCollection


Please seal all classes not intended to be overridden

Fixed. This class was the only one not sealed.

abatishchev · 2024-05-02T23:57:36Z

src/JWT/Jwk/JwtWebKeySet.cs

+    /// A JWK Set JSON data structure that represents a set of JSON Web Keys
+    /// specifed by RFC 7517, see https://datatracker.ietf.org/doc/html/rfc7517
+    /// </summary>
+    public sealed class JwtWebKeySet


Does a class need the default ctor decorated with [JsonConstructor] if it has none?

I think, it could be removed. I made it with the same rules as in JwtHeader:

jwt/src/JWT/Builder/JwtHeader.cs

Line 15 in 7ebbddf

[System.Text.Json.Serialization.JsonConstructor]

Maybe there is something, I'm not aware about, could you clarify, why we need it in JwtHeader, please?

abatishchev · 2024-05-13T17:53:07Z

hey @CADBIMDeveloper, I'm sorry again, I didn't forget - the tab is permanently open for me. It's just a bit too chaotic at work right now. I'll come back to this PR as soon as I can.

abatishchev self-assigned this Jan 30, 2024

abatishchev added the enchantment label Jan 30, 2024

abatishchev force-pushed the jwks branch from 96e1c76 to 4967267 Compare January 30, 2024 17:56

CADBIMDeveloper marked this pull request as ready for review February 4, 2024 20:08

abatishchev changed the title ~~JSON Web key sets support~~ Added support for JSON Web key sets Apr 16, 2024

CADBIMDeveloper added 21 commits April 16, 2024 07:23

add JSON Web Key definitions

9c40aa9

add Json Web keys collection

79ea39a

with an ability to search keys by ids

add a new algorithm factory

923f90c

to create an algorithms based on JSON Web key sets

add JWKS support on .net framework

72613d2

add a test

0f1d3bb

add a test

bcade65

incapsulate creating algorithm from specific JWK

9a76799

add EC algorithm specific properties

3b6b542

implement elliptic curve algorithms creation

c64bd7b

add a test

06be06c

add symmetric key property of the JWK

fc30b3f

add ISymmetircAlgorithm interface

20e3d51

move symmetric algorithm validation

528bc73

to jwt validator

hmacs sha algorithms could have built-in key

31ea350

algorithm factory reads the key from Json Web Key

f74489f

add a property to symetrical algorithm interface

e8d36db

use symmetric key from the JWKS

173a5aa

add new Jwt Builder methods

28afdb6

to make a decoder with JWKS

cleanup the test

be352ca

extend Jwt Builder with an ability to set

4ad94e9

JSON Web key

JWT can encode with symmetrical key from the JWKS

61cfe50

abatishchev force-pushed the jwks branch from fc2c3aa to 3f08528 Compare April 16, 2024 14:23

Merge branch 'main' into jwks

47e9234

CADBIMDeveloper added 2 commits May 1, 2024 11:34

fix build

7e09d4b

replace MODERN_DOTNET which is not presented any more

Merge branch 'main' into jwks

1e8cd93

abatishchev reviewed May 2, 2024

View reviewed changes

Update JwtWebKeyPropertyValuesEncoder.cs

e5f9f61

abatishchev reviewed May 2, 2024

View reviewed changes

abatishchev added 3 commits May 2, 2024 16:09

Update JwtBuilderEncodeTests.cs

a8c1651

Update JwtDecoderTests.cs

90f02d8

Update JwtValidator.cs

80eb529

abatishchev requested changes May 2, 2024

View reviewed changes

abatishchev added 2 commits May 2, 2024 16:15

Update JwtWebKeySet.cs

d48dee1

Update JwtWebKeySet.cs

1a3a717

abatishchev requested changes May 2, 2024

View reviewed changes

abatishchev added 2 commits May 2, 2024 16:33

Update JwtValidator.cs

89cc32f

Update JwtWebKeySet.cs

37aafbb

abatishchev reviewed May 2, 2024

View reviewed changes

CADBIMDeveloper added 7 commits May 3, 2024 09:37

explicit namespace

a66322b

to expression body

6dd5743

remove blank lines

e7189c1

pattern matching switch/case

c57a9a0

formatting

58e9a99

seal the class

a80e41e

rename method

0112fef

CADBIMDeveloper requested a review from abatishchev May 4, 2024 20:54


		public JwtWebKeysCollection(JwtWebKeySet keySet) : this(keySet.Keys)
		{

Conversation

CADBIMDeveloper commented Jan 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breaking changes

Checklist

Uh oh!

CADBIMDeveloper commented Jan 27, 2024

Uh oh!

CADBIMDeveloper commented Feb 8, 2024

Uh oh!

abatishchev commented Feb 8, 2024

Uh oh!

abatishchev commented Apr 16, 2024

Uh oh!

abatishchev commented Apr 29, 2024

Uh oh!

CADBIMDeveloper commented Apr 30, 2024

Uh oh!

abatishchev commented Apr 30, 2024

Uh oh!

CADBIMDeveloper commented May 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abatishchev commented May 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

CADBIMDeveloper commented Jan 27, 2024 •

edited

Loading