Lexikjwtauthenticationbundle: Compatibility with lcobucci/jwt v3.4

[NicoHaase]

Today, new versions for lcobucci/jwt were released, v3.4 and v4. In v3.4, a deprecation notice has been added, such that \Lcobucci\JWT\Builder::convertToDate should no longer get called with an integer (see https://github.com/lcobucci/jwt/blob/3.4/src/Builder.php#L151). This should be fixed in \Lexik\Bundle\JWTAuthenticationBundle\Services\JWSProvider\LcobucciJWSProvider::create

[chalasr]

Thank you for the report!
Would you mind submitting a pull request? (No worries if you can't, I'll do it asap).

[NicoHaase]

I'll have a look into that...

[NicoHaase]

Phew - not good. That method supports an integer until v3.3.3, and a DateTimeImmutable from v3.4 upwards, so I see no good solution to fix this only in the code. Would it be fine for you to upgrade the dependency as well, such that your bundle depends on v3.4?

As a regular user of both these packages, I'm not that good in considering the possible problems with forcing a higher dependency :(

[chalasr]

Bumping the minimum version for this package should be the last resort.
If versions prior to 3.4 do not support passing a \DateTimeImmutable, I would use feature detection (calling class_exists/method_exists on elements introduced in the 3.4 version) to use the correct type.

[ksowa]

@chalasr versions 3.3 and 3.4 use the same methods, but in a slightly different way. It looks like the output exp property is also going to look different - timestamp vs. date string.

I'm afraid that the only reliable solution would be to stick to either of 3.3 or 3.4.

3.3:

// Lcobucci\JWT\Builder
public function setExpiration($expiration, $replicateAsHeader = false)
{
    return $this->setRegisteredClaim('exp', (int) $expiration, $replicateAsHeader);
}

3.4:

// Lcobucci\JWT\Builder
public function setExpiration($expiration, $replicateAsHeader = false)
{
    return $this->expiresAt($expiration, $replicateAsHeader);
}

public function expiresAt($expiration, $replicateAsHeader = false)
{
    return $this->setRegisteredClaim('exp', $this->convertToDate($expiration), $replicateAsHeader);
}

private function convertToDate($value)
{
    if (! $value instanceof DateTimeImmutable) {
        trigger_error('Using integers for registered date claims is deprecated, please use DateTimeImmutable objects instead.', E_USER_DEPRECATED);
        return new DateTimeImmutable('@' . $value);
    }

    return $value;
}

[chalasr]

@ksowa What I mean is that e.g. https://github.com/lcobucci/jwt/blob/3.4/src/Configuration.php seems to be available as of 3.4 only. If this class exists, we can use \DateTimeImmutable objects over integers where it's necessary (same applies for casting the exp value into an int internally).
I'll have a look to other problematic changes, the test suite should catch these issues anyways.

[NicoHaase]

https://github.com/lcobucci/jwt/issues/550 also talks about this

[lcobucci]

Hey folks 👋

Thanks for your work on integrating with my library. I'm sorry that the deprecation messages are perceived as "problematic changes", they should be harmless on a well-configured environment - but I've seen tools overriding error_reporting to -1 without mercy so that's nothing but an assumption from side.

https://github.com/lcobucci/jwt/pull/548 is centre of the "dynamic deprecations", it should provide all necessary info.

Also, in order to be fully compatible with PHP 8, you'd need to change your constraints to ^3.4 || ^4.0. Then, the deprecation messages should guide you on making the lib compatible with both versions.

I'd love help reviewing a PR, if you have any. You may ping me there and we can work together on solving this 👍

[lcobucci]

@chalasr versions 3.3 and 3.4 use the same methods, but in a slightly different way. It looks like the output exp property is also going to look different - timestamp vs. date string.

That's only if you use the new API ($token->claims()->get('exp')). When using the old API ($token->getClaim('exp')) you'll still get the timestamp.

[chalasr]

I'm going to take a deep look into all this in the next couple of days and probably ping you for review.
Thank you for your hints @lcobucci, that's much appreciated.

[lcobucci]

Glad to help! I've tried my best not to disrupt everyone's work but you sometimes can't cover all scenarions.

[chalasr]

Adding compatibility layers for v3.4 was pretty easy, no real breaking change there.
Doing so for v4 was a bit more painful, but that was expected.
Here you go #797 :)