PHP 8 Support

[driesvints]

This adds a PHP 8 build. The current php: >=7.2.0 constraint already allows for PHP 8 installations.

I've also update lcobucci/jwt to allow ^4.0 which allows for PHP 8 installs (currently in alpha and unreleased). I haven't checked into the migration path for that one yet and will do updates based on what the test suite says. In draft until tests pass and this dependency is released. Related issue: lcobucci/jwt#300

This PR is needed to provide PHP 8 support in Laravel Passport: laravel/passport#1373

[driesvints]

I've taken the liberty to move the build to Github Actions because Travis is extremely slow and shutting down travis-ci.org soon. It won't show up here yet because this is the first time it's run but it's running on my fork. I'll fix the build first before marking the PR as ready (needs lcobucci/jwt v4 to be released first).

Builds on my fork: https://github.com/driesvints/oauth2-server/actions

[Sephster]

Looks good to me @driesvints. Thanks for putting together and cheers for the Travis CI change. Primary blocker for us will be @lcobucci's support for PHP 8. As soon as version 4 is out of alpha, I'm happy to merge this and tag a new release.

@lcobucci do you have a rough time schedule for this? Is there any risk you won't make GA?

[lcobucci]

@lcobucci do you have a rough time schedule for this? Is there any risk you won't make GA?

@Sephster I've assigned due dates to the milestones. I don't foresee any risk but you know how software goes 😆

@driesvints I'd suggest splitting the GH actions into another PR so we can use this one to ensure your tests pass for both v3.4 and v4.0. It'd be really lovely if we could use this PR to test against the dev versions (I'll review it to address any compatibility risk).

[driesvints]

Hey @lcobucci that all sounds good! I'd definitely be keen to revert the GH Actions changes but I mostly did that because the Travis builds are currently taking up several hours to run which makes it super hard to do changes to this PR...

[driesvints]

I extracted the Github Actions migration into a separate PR: #1148

As soon as it's merged on master and I can get to it I'll rebase and add a PHP 8 build on it in this PR.

[lcobucci]

The first thing that comes to my mind regarding compatibility here is the configuration object. It's not mandatory, however, it provides more flexibility and reduces the coupling between the two libraries.

It's documented here and its idea is to allow users to inject a configuration object, which provides an API to retrieve the necessary object to issue/parse a JWT.

This means that the AccessTokenTrait could be updated to something like this (skipping the unrelated stuff):

<?php
  trait AccessTokenTrait
  {
      /**
-      * @var CryptKey
+      * @var Configuration
       */
      private $jwtConfiguration;

-     /**
-      * Set the private key used to encrypt this access token.
-      */
-     public function setPrivateKey(CryptKey $privateKey)
-     {
-         $this->privateKey = $privateKey;
-     }
+     /**
+      * Set the configuration to issue the JWT
+      */
+     public function setJwtConfiguration(Configuration $jwtConfiguration)
+     {
+         $this->jwtConfiguration = $jwtConfiguration;
+     }

      /**
       * Generate a JWT from the access token
       *
-      * @param CryptKey $privateKey
+      * @param Configuration $config
       *
       * @return Token
       */
      private function convertToJWT(Configuration $config)
      {
-         return (new Builder())
+         return $config->createBuilder()
              ->permittedFor($this->getClient()->getIdentifier())
              ->identifiedBy($this->getIdentifier())
-             ->issuedAt(\time())
-             ->canOnlyBeUsedAfter(\time())
+             ->issuedAt(new DateTimeImmutable())
+             ->canOnlyBeUsedAfter(new DateTimeImmutable())
              ->expiresAt($this->getExpiryDateTime()->getTimestamp())
              ->relatedTo((string) $this->getUserIdentifier())
              ->withClaim('scopes', $this->getScopes())
-             ->getToken(new Sha256(), new Key($privateKey->getKeyPath(), $privateKey->getPassPhrase()));
+             ->getToken($config->getSigner(), $config->getSigningKey());
      }

      /**
       * Generate a string representation from the access token
       */
      public function __toString()
      {
-         return (string) $this->convertToJWT($this->privateKey);
+         return (string) $this->convertToJWT($this->jwtConfiguration);
      }
  }

And the same goes for BearerTokenValidator, which can get quite simpler.

You can still rely on the objects there, however, we're introducing some extension points (and BC-breaks) in v4 and pointing to the correct types to instantiate will possibly make this code more complicated.

[driesvints]

@lcobucci sorry I haven't been of much help yet.. I'll try to check into all this again on Thursday.

[lcobucci]

@lcobucci sorry I haven't been of much help yet.. I'll try to check into all this again on Thursday.

@driesvints don't worry, this is OSS ❤️

[Sephster]

Sorry folks, I haven't looked at this much but got a chance to properly this eve. It looks like we will need to drop the 3.x branch and move to 4.x fully from what I can tell due to the differences between these versions (although happy to be corrected by @lcobucci). I haven't looked deeply into this but PHPStan is flagging a whole load of errors for v4 which is to be expected if there has been significant API changes.

The downside of this is that the 4.x branch doesn't appear to support PHP 7.3. I've always tried to keep the minimum to the security supported releases of PHP but will need to drop this if we are to upgrade to the 4.x version.

@driesvints were you planning on dropping PHP 7.3 support for Passport when 8 is released?

[lcobucci]

@Sephster we'll be shipping a forward compatibility layer on v3.4. So, this doesn't need to drop PHP versions but will need to adjust the code to work with both versions.

[Sephster]

Brilliant, thanks @lcobucci. If I aim to get v4 compat am I right in assuming it should be fairly easy to retrofit 3.4 when finalised? If so, will get going on that. Thanks for your help

[driesvints]

were you planning on dropping PHP 7.3 support for Passport when 8 is released?

@Sephster heya, no we can't do that yet unfortunately. Earliest would be the end of next year when PHP 7.3 is EoL.

@lcobucci that's brilliant!

[driesvints]

@lcobucci is there anything specific in JWT where I can help out? I've tried looking at the issues on the open milestones but I'm not really sure where to begin. The 4.0.0-beta1 milestone seems to require some very specific work and I think the tasks for 3.4 are waiting until 4.0.0-beta1 is finalised?

[lcobucci]

@lcobucci is there anything specific in JWT where I can help out? I've tried looking at the issues on the open milestones but I'm not really sure where to begin. The 4.0.0-beta1 milestone seems to require some very specific work and I think the tasks for 3.4 are waiting until 4.0.0-beta1 is finalised?

4.0.0-beta1 is nearly done and I'll release it today. The items assigned to 3.4.0 aren't blocked by that release and I could definitely use some help there.

Brilliant, thanks @lcobucci. If I aim to get v4 compat am I right in assuming it should be fairly easy to retrofit 3.4 when finalised? If so, will get going on that. Thanks for your help

@Sephster that's exactly the idea 👍 I'll be writing some documentation on how to upgrade the code and we can apply the steps in this PR to see if everything works as expected.

[driesvints]

@lcobucci @Sephster thanks for your help with this one! :)