search

auth0 / java-jwt

Java implementation of JSON Web Token (JWT)
MIT License
5.84k stars 921 forks source link

Preserve insertion order for claims #656

Closed snago closed 1 year ago

snago commented 1 year ago

Changes

Please describe both what is changing and why this is important. Include:

Make header and payload fields in the resulting JWT have the same order as they were added to the JWTCreator.Builder. When looking at a decoded JWT it's easier to see if all expected claims are included when they aren't scrambled.

  • Endpoints added, deleted, deprecated, or changed
  • Classes and methods added, deleted, deprecated, or changed

Very small change, only changed HashMap to LinkedHashMap for headerClaims and payloadClaims in JWTCreator.Builder.

  • Any alternative designs or approaches considered

A more complicated variant could allow selection of different orderings, e.g. scrambled (current), insertion order (this) and alphabetically sorted (e.g. by using TreeMap).

  • Screenshots of new or changed UI, if applicable

  • A summary of usage if this is a new feature or change to a public API (this should also be added to relevant documentation once released)

Order was unspecified before (since HashMap was used), now it's deterministic.

References

Please include relevant links supporting this change such as a:

  • support ticket
  • community post
  • StackOverflow post
  • support forum thread

None that I know of.

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

I've added a test that verifies that both header and payload claims are in insertion order. The test doesn't care about where any other header fields are inserted, such as alg or typ (they will be last since they're added in the sign method).

Checklist

snago commented 1 year ago

Oh, there's some interaction with the other PR that was just merged. I'll take a look at that and make sure that order is preserved for the new JSON-string methods as well...

snago commented 1 year ago

Fixed! All that was needed was to use LinkedHashMap in those methods as well. I made the unit test check the result of JSON claims as well, in both header and payload.