spring-projects / spring-projects/spring-security

Consider reworking Jackson modules to support nanosecond precision

Open
#9,460 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

in: oauth2 type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

As indicated in https://github.com/FasterXML/jackson-modules-java8/issues/307, ObjectMapper#readTree cannot delay the evaluation of Instant values in the same fashion as ObjectMapper#readValue, which causes a loss of precision at deserialization when Instant values have nanosecond precision.

For example, this test will fail when OAuth2ClientJacksonModule is included in the configuration of ObjectMapper:

@Test 
public void testPrecisionLoss() {
    this.mapper = new ObjectMapper();
    this.mapper.registerModules(SecurityJackson2Modules.getModules(loader));
    Instant issuedAt = Instant.ofEpochSecond(1234567890, 123456789);
    OidcIdToken  token = TestOidcIdTokens.idToken().issuedAt(issuedAt).build();
    String serialized = this.mapper.writeValueAsString(map);
    OidcIdToken deserialized = this.mapper.readValue(serialized, OidcIdToken.class);
    assertThat(deserialized.getIssuedAt()).isEqualTo(token.getIssuedAt());
}

This is because Spring Security's Jackson modules include deserializers for unmodifiable collections, each of which relies on ObjectMapper#readTree, which can only support microsecond precision.

It is not necessary for OAuth2ClientJacksonModule to use these deserializers, though since any unmodifiable collections can be converted to their modifiable counterparts at serialization time using the @JsonSerialize annotation in the corresponding mixin constructor arguments.

For example, if OidcIdTokenMixin is changed to:

// ...
@JsonProperty("claims") @JsonSerialize(converter = MapCopier.class) Map<String, Object> claims
// ...

where MapCopier is a class that takes a collection and returns a LinkedHashMap copy, then the above test passes since the UnmodifiableMapSerialzier is no longer necessary and thus ObjectMapper#parseTree is no longer employed.

This would be applied for any unmodifiable collections referred to in the OAuth2ClientJacksonModule since there are constructs like OAuth2AuthenticationToken that contains an unmodifiable list of authorities where one of those authorities is an OidcUserAuthority that contains a claim set.

Alternative

There are performance implications that still need to be investigated to determine if this is a viable option. If not, the alternative is to document that the Spring Security Jackson modules only support microsecond-level precision for Instant values, updating OAuth2AuthenticationTokenMixinTests accordingly. Applications can achieve nanosecond precision on their own in this fashion by adding:

this.mapper.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);

to their ObjectMapper configuration.

Additionally, the documentation could be updated in the future should Jackson find a way to be able to support nanosecond precision in ObjectMapper#readTree by default.

Additional Details

This can be verified on Windows using JDK 11 and modifying OAuth2AuthenticationTokenMixinTests to exclude the line:

this.mapper.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);

Occassionally, the Instant obtained from the OS contains enough precision to cause rounding, making the assertions fail.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with OAuth2AuthenticationTokenMixinTests and reproduce the precision loss without USE_BIG_DECIMAL_FOR_FLOATS. Read OAuth2ClientJacksonModule and OidcIdTokenMixin to identify the unmodifiable collection deserializers involved. Done means preserving nanosecond Instant precision without unacceptable performance costs, or documenting and testing the microsecond limitation if the alternative is retained.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.