jpadilla / pyjwt

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

Add complete types to take all allowed keys into account #873

Closed Viicos closed 1 year ago

Viicos commented 1 year ago

Fixes https://github.com/jpadilla/pyjwt/issues/602, https://github.com/jpadilla/pyjwt/issues/848, https://github.com/jpadilla/pyjwt/issues/856, https://github.com/jpadilla/pyjwt/issues/864#issuecomment-1458420475

I think this is the best we can get, without having to make use of TypeVar/Generic for algorithms. Had to tweak some parts to fix mypy errors.

One possible thing (as stated in https://github.com/jpadilla/pyjwt/pull/843) could be to add overloads to encode. I'll let you decide on this one, as it can hurt code readability. Regarding the decode method, it can't be done as a list of allowed algorithms is passed.

Viicos commented 1 year ago

Regarding https://github.com/jpadilla/pyjwt/pull/873/commits/16da314831b38061611b92d36c63cf842b1453b8, maybe we could switch to from __future__ import annotations in this file as well, to be in line with the rest of the project?

There's also the mypy checks failing, because cryptography can't be found when runing in CI

Viicos commented 1 year ago

can you please check why CI is failing?

  self = <jwt.api_jwt.PyJWT object at 0x7f36e4b70610>
  payload = {'claim': 'insanity', 'exp': 1681192610, 'iss': 'jeff'}
  now = 1681192613.000076, leeway = 3

      def _validate_exp(
          self,
          payload: dict[str, Any],
          now: float,
          leeway: float,
      ) -> None:
          try:
              exp = int(payload["exp"])
          except ValueError:
              raise DecodeError("Expiration Time claim (exp) must be an" " integer.")

          if exp <= (now - leeway):
  >           raise ExpiredSignatureError("Signature has expired")
  E           jwt.exceptions.ExpiredSignatureError: Signature has expired

  jwt/api_jwt.py:304: ExpiredSignatureError

Seems like this could fail randomly

Viicos commented 1 year ago

Most of the remaining mypy errors are due to the fact that alg.prepare_key/alg.from_jwk can return either a public or a private key. Do you want to keep mypy running for tests and have an explicit assignment for the return types of these methods, or just drop mypy for tests?

auvipy commented 1 year ago

Do you want to keep mypy running for tests and have an explicit assignment for the return types of these methods

yes