Fix defaultValidator's time being static#149
Conversation
It should be dynamic so that the same instance can be reused over time.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes JwtValidator.defaultValidator so its notion of “current time” is evaluated dynamically, allowing a single validator instance to be reused over time without using a stale now.
Changes:
- Make
nowdynamic indefaultValidatorby switching from a fixedvalto a method-based lookup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def now = Instant.now(clock) | ||
| JwtValidator.fromSync[Any] { | ||
| case Jwt(_, claims) if claims.exp.exists(_.isBefore(now)) => | ||
| s"Token expired at ${claims.exp.get}.${iatMessage(claims.iat)} It is now $now." |
There was a problem hiding this comment.
now is a def, so it’s evaluated multiple times per validation (in the guard and again when interpolating the error message). That can lead to slightly inconsistent “It is now …” timestamps and performs multiple clock reads. Consider computing val now = Instant.now(clock) once per JWT validation (inside the returned validator function) and reusing it for both the checks and message formatting (e.g., via fromSyncLifted / a total function that returns Option[String]).
nrktkt
left a comment
nrktkt
left a comment
There was a problem hiding this comment.
interesting, didn't know that would work.
seems like the type signature would become Jwt => PartialFunciton[Jwt, ...]
It should be dynamic so that the same instance can be reused over time.