Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rayanlevert opened this issue Sep 23, 2022 · 1 comment · Fixed by #16116
Closed
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: high High

Comments

@rayanlevert
Copy link
Sponsor

rayanlevert commented Sep 23, 2022

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());
  • Phalcon5.0.0
  • PHP8.1
  • Debian 11

Thanks again o/

@rayanlevert rayanlevert added bug A bug report status: unverified Unverified labels Sep 23, 2022
niden added a commit to niden/cphalcon that referenced this issue Sep 23, 2022
niden added a commit to niden/cphalcon that referenced this issue Sep 23, 2022
niden added a commit to niden/cphalcon that referenced this issue Sep 23, 2022
@niden niden self-assigned this Sep 23, 2022
@niden niden added status: high High 5.0 The issues we want to solve in the 5.0 release and removed status: unverified Unverified labels Sep 23, 2022
@niden niden mentioned this issue Sep 23, 2022
5 tasks
@niden niden linked a pull request Sep 23, 2022 that will close this issue
5 tasks
niden added a commit to niden/cphalcon that referenced this issue Sep 23, 2022
niden added a commit to niden/cphalcon that referenced this issue Sep 23, 2022
niden added a commit to niden/cphalcon that referenced this issue Sep 23, 2022
@niden
Copy link
Sponsor Member

niden commented Sep 23, 2022

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

#16116

(and new v5.0.1 release)

@niden niden closed this as completed Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: high High
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants