TYPO3 11 compatibility #8
Conversation
TYPO3 11 compatibility
Fatal error: During inheritance of JsonSerializable: Uncaught TYPO3\CMS\Core\Error\Exception: PHP Deprecation Notice: Return type of FGTCLB\OAuth2Server\Domain\Entity\Scope::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /var/www/html/public/typo3conf/ext/oauth2_server/Classes/Domain/Entity/Scope.php line 16 in /var/www/html/vendor/helhum/typo3-console/Classes/Console/Error/ErrorHandler.php:88
Class serialization is not allowed anymore: TYPO3/typo3@733353c#diff-1d396fe456cb8cc5db6a383afa0925cdd834074fa83d38a4f71b770410fa2a95R229
|
@schliesser @OleksandrMatsko @wandoliver Thanks for pushing this forward! I personally never dug deep into this ext, I guess I'll do that now and see if I can further improve this MR (and finally test it standalone on a v11 system). |
| @@ -13,7 +13,7 @@ final class Scope implements ScopeEntityInterface | |||
| { | |||
| use EntityTrait; | |||
|
|
|||
| public function jsonSerialize() | |||
| public function jsonSerialize(): mixed | |||
There was a problem hiding this comment.
mixed is not available in PHP 7.4.
I'm also open to drop 7.4-compatibility, since it's EOL anyways.
There was a problem hiding this comment.
missed that. Remove it, if it's the only php 8 only stuff
| @@ -15,10 +15,11 @@ final class Client implements ClientEntityInterface | |||
| use EntityTrait; | |||
| use ClientTrait; | |||
|
|
|||
| public function __construct($identifier, $name, $redirectUri) | |||
| public function __construct($identifier, $name, $redirectUri, $isConfidential) | |||
There was a problem hiding this comment.
We should at least add type hints for the new parameters.
| * | ||
| * @param string $clientIdentifier The client's identifier | ||
| * @param null|string $clientSecret The client's secret (if sent) | ||
| * @param null|string $grantType The type of grant the client is using (if sent) |
There was a problem hiding this comment.
If we're introducing new methods/opening them to the public, the type hints should IMO be part of the method signature.
And I also think a (public) method with this name does not belong to a repository.
There was a problem hiding this comment.
I just realized that this method comes from the interface. Gnah.
| $authorizationRequest = $userSession->getData('oauth2.authorizationRequest'); | ||
|
|
||
| $this->bootFrontendController(); | ||
| $router = $this->siteFinder->getSiteByPageId($this->configuration->getLoginPage())->getRouter($this->context); |
There was a problem hiding this comment.
If the site is not found, this will fail due to a null-pointer. I think we should either do this step by step, or use the ?-> operator if we decide to drop 7.4.
In any case, we need to check if $router is actually available and raise an error if not.
| $redirectUri = $this->getContentObjectRenderer()->typoLink_URL([ | ||
| 'parameter' => sprintf('t3://page?uid=%d&redirect_url=%s', $this->configuration->getLoginPage(), $request->getUri()->getPath()), | ||
| ]); | ||
| // With TYPO3 11.5.17 it takes 3 loops to unserialize the AuthorizationRequest |
There was a problem hiding this comment.
This sounds a bit weird. I would love to have a more detailed comment here how the data looks like and why it is encoded multiple times (?)
There was a problem hiding this comment.
I'm not 100% sure where it does come from. But the session Data serialization in the core was changed from 10.4 to 11.5. An the core serializes the data also. So it seems to be serialized multiple times.
| @@ -50,45 +44,62 @@ public function __construct() | |||
| */ | |||
| public function getClientEntity($clientIdentifier, $grantType = null, $clientSecret = null, $mustValidateSecret = true) | |||
There was a problem hiding this comment.
The parameters $grantType, $clientSecret, $mustValidateSecret were dropped in league/oauth2-server 8.3.
I'm not sure how that should influence the behaviour of this method, but I would assume that we don't have to do any validation here now, but just return the client, and move the validation to validateClient() below.
Based on the compatibility improvements from sitegeist I created a TYPO3 11 compatible version of this extension. The main problem caused by different session handling of the TYPO3 core. They don't allow the serialization of any classes anymore: TYPO3/typo3@733353c#diff-1d396fe456cb8cc5db6a383afa0925cdd834074fa83d38a4f71b770410fa2a95R229
Beside this the TSFE is not bootet that early anymore. Therefore the Redirect uri building was rewritten.
Didn't test it with TYPO3 10.