From ad4729f77250ce7022dc99cd7ccec20735436300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Fri, 19 Mar 2021 15:15:34 +0100 Subject: [PATCH 1/2] Guard against precision issues on time fractions --- test/functional/TimeFractionPrecisionTest.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 test/functional/TimeFractionPrecisionTest.php diff --git a/test/functional/TimeFractionPrecisionTest.php b/test/functional/TimeFractionPrecisionTest.php new file mode 100644 index 00000000..4cb0534d --- /dev/null +++ b/test/functional/TimeFractionPrecisionTest.php @@ -0,0 +1,61 @@ +format('U.u')); + + $token = $config->builder() + ->issuedAt($issuedAt) + ->getToken($config->signer(), $config->signingKey()); + + $parsedToken = $config->parser()->parse($token->toString()); + + self::assertInstanceOf(Plain::class, $parsedToken); + self::assertSame($timeFraction, $parsedToken->claims()->get('iat')->format('U.u')); + } + + /** @return iterable */ + public function datesWithPotentialRoundingIssues(): iterable + { + yield ['1613938511.017448']; + yield ['1613938511.023691']; + yield ['1613938511.018045']; + yield ['1616074725.008455']; + } +} From 9a961f4541ba13ac27dcebaaa222ad8e342ce14b Mon Sep 17 00:00:00 2001 From: rais0005 Date: Fri, 19 Mar 2021 01:49:08 +0100 Subject: [PATCH 2/2] Fix usage of non JSON numeric values for time fractions The RFC-7519 states that the `NumericDate` type is: > JSON numeric value representing the number of seconds from > 1970-01-01T00:00:00Z UTC until the specified UTC date/time, ignoring > leap seconds. Then also mentions that time fractions (as covered by RFC-3339) are supported: > Seconds Since the Epoch", in which each day is accounted for by > exactly 86400 seconds, other than that non-integer values can be > represented. While adding support for time fractions we've interpreted the "non-integer" really as any "non-integer" value, and used strings to guard against precision issues. That causes issues, since a string isn't a "JSON numeric value" according to the JSON specs. We observed that the 6-digit precision is not lost when doing JSON encode/decode operations, this applies that technique to make sure we comply to the specs and have "rounding issues" when dealing with floats. --- src/Encoding/MicrosecondBasedDateConversion.php | 11 ++++------- src/Token/Parser.php | 8 +++++++- test/unit/Encoding/ChainedFormatterTest.php | 2 +- .../Encoding/MicrosecondBasedDateConversionTest.php | 6 +++--- test/unit/Token/ParserTest.php | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Encoding/MicrosecondBasedDateConversion.php b/src/Encoding/MicrosecondBasedDateConversion.php index 0f10cf0b..c8fc68c8 100644 --- a/src/Encoding/MicrosecondBasedDateConversion.php +++ b/src/Encoding/MicrosecondBasedDateConversion.php @@ -25,16 +25,13 @@ public function formatClaims(array $claims): array return $claims; } - /** @return int|string */ + /** @return int|float */ private function convertDate(DateTimeImmutable $date) { - $seconds = $date->format('U'); - $microseconds = $date->format('u'); - - if ((int) $microseconds === 0) { - return (int) $seconds; + if ($date->format('u') === '000000') { + return (int) $date->format('U'); } - return $seconds . '.' . $microseconds; + return (float) $date->format('U.u'); } } diff --git a/src/Token/Parser.php b/src/Token/Parser.php index 2d3ac688..530a727c 100644 --- a/src/Token/Parser.php +++ b/src/Token/Parser.php @@ -12,8 +12,12 @@ use function count; use function explode; use function is_array; +use function is_string; +use function json_encode; use function strpos; +use const JSON_THROW_ON_ERROR; + final class Parser implements ParserInterface { private Decoder $decoder; @@ -105,7 +109,9 @@ private function parseClaims(string $data): array continue; } - $claims[$claim] = $this->convertDate((string) $claims[$claim]); + $date = $claims[$claim]; + + $claims[$claim] = $this->convertDate(is_string($date) ? $date : json_encode($date, JSON_THROW_ON_ERROR)); } return $claims; diff --git a/test/unit/Encoding/ChainedFormatterTest.php b/test/unit/Encoding/ChainedFormatterTest.php index 3d2a856f..061999d7 100644 --- a/test/unit/Encoding/ChainedFormatterTest.php +++ b/test/unit/Encoding/ChainedFormatterTest.php @@ -34,6 +34,6 @@ public function formatClaimsShouldApplyAllConfiguredFormatters(): void $formatted = $formatter->formatClaims($claims); self::assertSame('test', $formatted[RegisteredClaims::AUDIENCE]); - self::assertSame('1487285080.123456', $formatted[RegisteredClaims::EXPIRATION_TIME]); + self::assertSame(1487285080.123456, $formatted[RegisteredClaims::EXPIRATION_TIME]); } } diff --git a/test/unit/Encoding/MicrosecondBasedDateConversionTest.php b/test/unit/Encoding/MicrosecondBasedDateConversionTest.php index 838cba0f..4be0c6b5 100644 --- a/test/unit/Encoding/MicrosecondBasedDateConversionTest.php +++ b/test/unit/Encoding/MicrosecondBasedDateConversionTest.php @@ -36,8 +36,8 @@ public function dateClaimsHaveMicrosecondsOrSeconds(): void $formatted = $formatter->formatClaims($claims); self::assertSame(1487285080, $formatted[RegisteredClaims::ISSUED_AT]); - self::assertSame('1487285080.000123', $formatted[RegisteredClaims::NOT_BEFORE]); - self::assertSame('1487285080.123456', $formatted[RegisteredClaims::EXPIRATION_TIME]); + self::assertSame(1487285080.000123, $formatted[RegisteredClaims::NOT_BEFORE]); + self::assertSame(1487285080.123456, $formatted[RegisteredClaims::EXPIRATION_TIME]); self::assertSame('test', $formatted['testing']); // this should remain untouched } @@ -62,7 +62,7 @@ public function notAllDateClaimsNeedToBeConfigured(): void $formatted = $formatter->formatClaims($claims); self::assertSame(1487285080, $formatted[RegisteredClaims::ISSUED_AT]); - self::assertSame('1487285080.123456', $formatted[RegisteredClaims::EXPIRATION_TIME]); + self::assertSame(1487285080.123456, $formatted[RegisteredClaims::EXPIRATION_TIME]); self::assertSame('test', $formatted['testing']); // this should remain untouched } } diff --git a/test/unit/Token/ParserTest.php b/test/unit/Token/ParserTest.php index 95da07b9..5c1bf90c 100644 --- a/test/unit/Token/ParserTest.php +++ b/test/unit/Token/ParserTest.php @@ -453,7 +453,7 @@ public function parseMustConvertDateClaimsToObjects(): void { $data = [ RegisteredClaims::ISSUED_AT => 1486930663, - RegisteredClaims::EXPIRATION_TIME => '1486930757.023055', + RegisteredClaims::EXPIRATION_TIME => 1486930757.023055, ]; $this->decoder->expects(self::exactly(2))