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

Incorrect type hints for several classes/methods #787

Open
mlazar-endear opened this issue Feb 29, 2024 · 4 comments
Open

Incorrect type hints for several classes/methods #787

mlazar-endear opened this issue Feb 29, 2024 · 4 comments

Comments

@mlazar-endear
Copy link

Hi!

I just updated my version to djangorestframework-simplejwt==5.3.1 and I'm running into several mypy errors that were introduced with the recent typehints PR here: #683

I've looked into it, and there are several incorrect types in this library. Many of them have are arguments/return values which are annotated as Token, but in reality should be str or bytes. Some of them are easy to spot (and should have been caught by mypy when adding the types in the first place). Some of them are more difficult because of the dynamic nature of the library (e.g. AUTH_TOKEN_CLASSES), but they raise errors when you start writing subclasses for the tokens and backends.

For example, this line passes a Token object into the underlying jwt library, which is incorrect (should be str or bytes).

return jwt.decode(
token,

Same thing here, passes a Token object but it should be str or bytes.

if self.jwks_client:
try:
return self.jwks_client.get_signing_key_from_jwt(token).key

Here, the raw_token is correctly annotated, but then it's being passed into an AuthToken initializer which is expecting the argument to be Token.

def get_validated_token(self, raw_token: bytes) -> Token:
"""
Validates an encoded JSON web token and returns a validated token
wrapper object.
"""
messages = []
for AuthToken in api_settings.AUTH_TOKEN_CLASSES:
try:
return AuthToken(raw_token)

This initializer is wrong (Why would the token class be initialized with an instance of itself?)

class Token:
"""
A class which validates and wraps an existing JWT or can be used to build a
new JWT.
"""
token_type: Optional[str] = None
lifetime: Optional[timedelta] = None
def __init__(self, token: Optional["Token"] = None, verify: bool = True) -> None:

Unfortunately, I think fixing these type hints will require significant effort to go through and untangle everything.

@mlazar-endear
Copy link
Author

For anyone else who runs into this, you can configure mypy to ignore the all of the typehints provided by this library.

Here's what I added in my pyproject.toml file:

[[tool.mypy.overrides]]
# https://github.com/jazzband/djangorestframework-simplejwt/issues/787
module = "rest_framework_simplejwt.*"
follow_imports = "skip"

@Andrew-Chen-Wang
Copy link
Member

@mlazar-endear do you have django-stubs enabled? I believe that's a requirement for type checking simplejwt

@mlazar-endear
Copy link
Author

@Andrew-Chen-Wang Yes I do, but I don't see any reason why that would matter.

django-stubs==4.2.7
django-stubs-ext==4.2.7
djangorestframework-stubs==3.14.5

django==4.2.10
djangorestframework-simplejwt==5.3.1

@Andrew-Chen-Wang
Copy link
Member

Andrew-Chen-Wang commented Mar 19, 2024

just for diagnosing. yes i see the issues too thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants