jpadilla / pyjwt

JSON Web Token implementation in Python
https://pyjwt.readthedocs.io
MIT License
5.05k stars 676 forks source link

Add compute_hash_digest to Algorithm objects #819

Closed sirosen closed 1 year ago

sirosen commented 1 year ago

Algorithm.compute_hash_digest is defined as a method which inspects the object to see that it has the requisite attributes, hash_alg.

If hash_alg is not set, then the method raises a NotImplementedError. This applies to classes like NoneAlgorithm.

If hash_alg is set, then it is checked for

has_crypto  # is cryptography available?
and isinstance(hash_alg, type)
and issubclass(hash_alg, hashes.HashAlgorithm)

to see which API for computing a digest is appropriate -- hashlib vs cryptography.hazmat.primitives.hashes.

These checks could be avoided at runtime if it were necessary to optimize further (e.g. attach compute_hash_digest methods to classes with a class decorator) but this is not clearly a worthwhile optimization. Such perf tuning is intentionally omitted for now.


This is pulled from #775 . I can't tell why that PR didn't get reviewed, but I'll assume it was the doc addition. So I've rebased and taken just the functional part.

resolves #314

auvipy commented 1 year ago

bot automatically closed that

sirosen commented 1 year ago

I'm aware that the bot automatically closed it. But like I said, I have no information about why that never got reviewed.

If you can't get a reply to PRs in a few months (which, believe me, I understand -- I'm a package maintainer too), consider increasing your stalebot window. It doesn't serve anyone to have it closing things which are relatively fresh and unreviewed.

By a similar token, I've been trying to get #314 solved for 5 years now. I get that I seem to be the only person who cares about this, but it's demoralizing to leave a well-intentioned, polite comment like this one and then have stalebot come back and close it.

For low-touch and legacy projects I maintain, I do not use stalebot. It does more harm than good in those cases.

In any case, #775 now seems poised to merge, so I'm closing this as a duplicate.