phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.77k stars 1.97k forks source link

[BUG]: New method ->validate() from JWT\Token internally calls undefined method #16115

Closed rayanlevert closed 1 year ago

rayanlevert commented 1 year ago

Hello guys, ggs by the way for the stable release of v5 o/

I saw your new version of handling validation of JWT tokens, so i checked a bit of it and changed my code to call ->validate() from the Token like this

$now = (new \DateTimeImmutable())->getTimestamp();

$oValidator = (new \Phalcon\Encryption\Security\JWT\Validator($this->oToken, 100))
      ->validateIssuedAt($now)
      ->validateIssuer($this->oConfig->application->baseUri)
      ->validateAudience($this->oConfig->application->baseUri)
      ->validateExpiration($now)
      ->validateSignature(new Hmac('sha256'), $signerKey ?: $this->oConfig->security->cryptKey)
      ->validateId($id);

$this->oToken->validate($oValidator);

And i got this RuntimeException 'Call to undefined method Phalcon\Encryption\Security\JWT\Validator::aud()'

After checking the source code in Encryption/Security/JWT/Token/Token.zep, I might see the error but never programmed in Zephir, the way you loop into the array of [method => claimId], didn't you invert claimId and method in the for..in ? It would call validator->Enum::AUDIENCE instead of validator->validateAudience()

let methods = [
    "validateAudience"   : Enum::AUDIENCE,
    "validateExpiration" : Enum::EXPIRATION_TIME,
    "validateId"         : Enum::ID,
    "validateIssuedAt"   : Enum::ISSUED_AT,
    "validateIssuer"     : Enum::ISSUER,
    "validateNotBefore"  : Enum::NOT_BEFORE
];

for claimId, method in methods {
    validator->{method}(this->claims->get(claimId));
}

I might have seen another 'bug' in the validationExpiration() from the Validator, the validationExpiration(timestamp) method would add now an error if timestamp is before the timestamp in exp claim, if I set the timestamp after the exp claim, i got no error.

$oToken = (new \Phalcon\Encryption\Security\JWT\Builder(new Hmac('sha256')))
    ->setIssuer('test')
    ->setAudience('test')
    ->setExpirationTime(strtotime('+100 seconds'))
    ->setPassphrase($this->di->getConfig()->security->cryptKey)
    ->getToken();

$oValidator = (new \Phalcon\Encryption\Security\JWT\Validator($oToken))
    ->validateExpiration((strtotime('now')));

// Validation: the token has expired
print_r($oValidator->getErrors());

$oValidator = (new \Phalcon\Encryption\Security\JWT\Validator($oToken))
    ->validateExpiration((strtotime('+101 seconds')));

// empty array
print_r($oValidator->getErrors());

Thanks again o/

niden commented 1 year ago

Thank you @levertr for reporting this. It has been addressed

https://github.com/phalcon/cphalcon/pull/16116

(and new v5.0.1 release)