Add constraint for private claim validation

[james-bw]

Re: #826

[Slamdunk]

@Ocramius https://github.com/lcobucci/jwt/runs/5749877948?check_suite_focus=true#step:4:250

[BC] ADDED: Method hasClaimWithValue() was added to interface Lcobucci\JWT\UnencryptedToken
[BC] ADDED: Method hasClaimWithValue() was added to interface Lcobucci\JWT\Token
2 backwards-incompatible changes detected

Why adding a method to an interface should be considered a BC Break?

[SvenRtbg]

Adding new methods to an interface requires implementation of it in every class implementing the interface. This is incompatible with "I just update and everything works". :)

[Slamdunk]

Ok, now I see the need for https://wiki.php.net/rfc/sealed_classes as in this context Lcobucci\JWT\UnencryptedToken and Lcobucci\JWT\Token are not meant to be implemented outside this library

[Ocramius]

@lcobucci before adding this, please check the premise in #826 - the entire idea (so far) smells, IMO

[lcobucci]

@lcobucci before adding this, please check the premise in #826 - the entire idea (so far) smells, IMO

@Ocramius I saw it... I'm honestly abstracting that page because I think it's useful to have a generic constraint for custom claims in the lib.

[lcobucci]

Just to expand on the reason for a closure... the initial motivation to introduce this is related to hash comparison and doing that operation with === opens a security issue (timing attacks) - that's why we use hash_equals().

@james-bw have you thought about that? This PR does NOT add a safe way to perform hash comparison.

[lcobucci]

Also, you can run make to execute most of the checks and solve the problems locally.

[james-bw]

Just to expand on the reason for a closure... the initial motivation to introduce this is related to hash comparison and doing that operation with === opens a security issue (timing attacks) - that's why we use hash_equals().

@james-bw have you thought about that? This PR does NOT add a safe way to perform hash comparison.

I did not. I am not aware of how === leads to a timing attack. I'll have to investigate further.

...

OK I understand the timing attack now, and understand why === is not to be used for cryptographic stuff. However, custom claim validation is no different to existing code:

    public function isIdentifiedBy(string $id): bool
    {
        return $this->claims->get(RegisteredClaims::ID) === $id;
    }

    public function isRelatedTo(string $subject): bool
    {
        return $this->claims->get(RegisteredClaims::SUBJECT) === $subject;
    }

I think this validation should be consistent.

[james-bw]

Also, you can run make to execute most of the checks and solve the problems locally.

I'm developing on Windows/git bash, so don't have make natively. I've been trying to get my IDE working with the equivalent tests. At this stage I'm about to walk away and leave you to it, I was just trying to help with a quick win and this is turning into ben hur.

Changes addressed