jumbojett / OpenID-Connect-PHP

Minimalist OpenID Connect client
https://github.com/jumbojett/OpenID-Connect-PHP
Apache License 2.0
595 stars 357 forks source link

Support for ES256 Signature Algorithm? #287

Open SAH62 opened 2 years ago

SAH62 commented 2 years ago

Yahoo signs their JWTs with the ES256 algorithm. It's not recognized in the verifyJWTsignature() function. Are there any plans to add support for it, or is there a way to set something in the code that tells Yahoo to use RS256 instead? Yahoo claims to support both algorithms:

"id_token_signing_alg_values_supported": [
"ES256",
"RS256"
],
dunglas commented 2 years ago

I monkey patched the code to support ES256 by delegating JWT validation to firebase/php-jwt:

<?php

declare(strict_types=1);

namespace App;

use Firebase\JWT\JWK;
use Firebase\JWT\JWT;
use Jumbojett\OpenIDConnectClient;
use Jumbojett\OpenIDConnectClientException;
use Lcobucci\JWT\Builder;
use Lcobucci\JWT\Signer\Key\InMemory;

/**
 * @author Kévin Dunglas <kevin@dunglas.fr>
 */
class Client extends OpenIDConnectClient
{
    public function verifyJWTsignature($jwt)
    {
        $jwks = json_decode($this->fetchURL($this->getProviderConfigValue('jwks_uri')), true);
        if (!is_array($jwks)) {
            throw new OpenIDConnectClientException('Error decoding JSON from jwks_uri');
        }

        try {
            JWT::decode($jwt, JWK::parseKeySet($jwks));
        } catch (\Exception $e) {
            throw new OpenIDConnectClientException('Error decoding JWT: '.$e->getMessage(), $e->getCode(), $e);
        }

        return true;
    }
}

For this to work, you need the experimental branch of PHP JWT adding support for ES256 to JWK (https://github.com/firebase/php-jwt/pull/399): composer require firebase/php-jwt:dev-add-ec-jwk-support.

@DeepDiver1975, if you're interested in this approach (delegating the validation to a third-party library such as PHP JWT), I can open a PR with this patch.

bshaffer commented 2 years ago

@dunglas Does this mean you've been able to verify my experimental code in a real-world situation decoding EC JWKs? And if so, can you leave a review on my PR with this information? It would speed along getting this feature merged and released. Thank you!

joostrijneveld commented 3 months ago

Kanidm defaults to disabling RS256; for anyone that ends up here because of errors trying to get ES256 to work (like me), consider that Kanidm has a switch that allows you to enable legacy crypto (like RSA). See https://kanidm.github.io/kanidm/master/integrations/oauth2.html

joostrijneveld commented 3 months ago

Reading into the issue and possible solutions a bit more, @dunglas I would be happy to support such a pull request! I think delegating JWT operations to a dedicated library makes much more sense than re-implementing it in this repository. Should be able to clean up a lot of direct calls to crypto operations, which is always a place to introduce nasty bugs.

DeepDiver1975 commented 3 months ago

I monkey patched the code to support ES256 by delegating JWT validation to firebase/php-jwt:

PR welcome! THX