Fix #46

[SystematicCZ]

Fixes #46

Fix Firebase\JWT\JWT::decode(): Argument #3 ($headers) cannot be passed by reference

[SystematicCZ]

The 3rd parameter should not be there cause method had previously only two parameters

[tjveldhuizen]

I think this is a pragmatic way to adapt my change in #25 to make this work again. @patrickbussmann do you think this is the right 'solution'? If yes, can you please merge en release this?

[ghost]

@patrickbussmann Is there any hope that it will be merged?

[ghost]

These PRs #47, #51 are also trying to solve this problem
I believe that they can be closed in favor of this decision.

[patrickbussmann]

@kksandr7 the tests are failing that's why I can't merge it

[Kreyu]

The tests are failing, because the firebase/php-jwt >=5.2.0 <=5.5.1 is still requiring an array, not an object. I've added a suggestion for a small backwards compatibility layer (supported by PHP 5.6 and up).

[Kreyu]

Third parameter of the Firebase\JWT\JWT::decode() method has changed over time:

• version 5.2.0 - 5.5.1 had array $allowed_algs = array() (as third) parameter
• version 6.0.0 removed array $allowed_algs = array() parameter (no third parameters)
• version 6.6.0 introduced stdClass &$headers = null (as third) parameter

Proposed changes are valid for current version of firebase/php-jwt, but fails on tests using version 5.2.0 - 5.5.1. For backwards compatibility, we have to check, whether the third parameter exists for the decode() method, and which type is required. Unfortunately, using ReflectionParameter::getType() is not possible, because it was introduced in PHP 7 :(

Suggested change

	$headers = (object) ["alg" => 'RS256'];
	$decoded = JWT::decode($options['id_token'], $key, $headers);
	$decodeMethodReflection = new \ReflectionMethod(JWT::class, 'decode');
	$decodeMethodParameters = $decodeMethodReflection->getParameters();
	// Backwards compatibility for firebase/php-jwt >=5.2.0 <=5.5.1 supported by PHP 5.6
	if (array_key_exists(2, $decodeMethodParameters) && 'allowed_algs' === $decodeMethodParameters[2]->getName()) {
	$decoded = JWT::decode($options['id_token'], $key, ['RS256']);
	} else {
	$headers = (object) ['alg' => 'RS256'];
	$decoded = JWT::decode($options['id_token'], $key, $headers);
	}

Couldn't think of any other way to ensure backwards compatibility with PHP 5.6, since this library test suite includes PHP 5.6. Tested with:

docker run -v .:/code phpdockerio/php56-fpm bash -c "cd code && composer update --prefer-lowest && vendor/bin/phpunit"

docker run -v .:/code phpdockerio/php80-fpm bash -c "cd code && composer update && vendor/bin/phpunit"

[stefanorosanelli]

I just ran into the same issue. This solution should pass the tests
@Kreyu can you open a new PR with this change?
Hopefully it will be merged (if tests pass there are no reasons not to)

[patrickbussmann]

If the tests are passing there is nothing against to merge it. Thats right.
And if I would find the time I can also implement it by myself but no time yet 🙁

[stefanorosanelli]

If the tests are passing there is nothing against to merge it. Thats right. And if I would find the time I can also implement it by myself but no time yet 🙁

@patrickbussmann I tried in #54 - locally unit tests seem to work

[patrickbussmann]

You can see the tests here because locally your version is different. The tests here check multiple PHP versions 👍

still failing for lowest setup. Maybe we can adjust something to passing here

[stefanorosanelli]

@patrickbussmann i see only a coding standard failure in https://github.com/patrickbussmann/oauth2-apple/actions/runs/8357382263 - should be fixed now in #54