Add Security Warning while using `for_user` method about djangorestframework-simplejwt HOT 10 OPEN

I'm all for adding a logger.error and logger.critical, but outside of that is changing functionality and design principles. I'm not used to CVE reports and actionables in an open source context, but this is my understanding of general good design and consistent behavior for developers. Let me know if that's not how I should be approaching this and why.

Adding logs can help users to avoid such issues. If possible let's also add more secure examples in docs so it doesn't introduce vulns in their projects.

from djangorestframework-simplejwt.

This is an issue only if used with JWTStatelessUserAuthentication/JWTTokenUserAuthentication, while JWTAuthentication class is not affected since it queries the database to check if ther user is active.

The "bad configuration" is:

whenever the Token.for_user method is used without prior user validation and JWTStatelessUserAuthentication or JWTTokenUserAuthentication (which is an alias of ther previous one) authentication class is used.

from djangorestframework-simplejwt.

Thanks for filing. cc @robd003

This is more of a documentation thing as using .for_user directly is non-standard, but because of my very own telling to everyone to just use SimpleJWT classes directly, this mistake is attributed to me. Unfortunately, it works by design and is following DRY principles and separation of duties where the validation takes place on the serializer part, which is Django standard, but I'll add some documentation to warn users.

from djangorestframework-simplejwt.

Thanks for filing. cc @robd003

This is more of a documentation thing as using .for_user directly is non-standard, but because of my very own telling to everyone to just use SimpleJWT classes directly, this mistake is attributed to me. Unfortunately, it works by design and is following DRY principles and separation of duties where the validation takes place on the serializer part, which is Django standard, but I'll add some documentation to warn users.

I understand but there are several projects using it directly without any kind of user's active status validation which makes those projects vulnerable.

from djangorestframework-simplejwt.

I'm all for adding a logger.error and logger.critical, but outside of that is changing functionality and design principles. I'm not used to CVE reports and actionables in an open source context, but this is my understanding of general good design and consistent behavior for developers. Let me know if that's not how I should be approaching this and why.

from djangorestframework-simplejwt.

@Andrew-Chen-Wang Why don't add an exception raised if using .for_user directly?

As @dmdhrumilmistry says

there are several projects using it directly without any kind of user's active status validation which makes those projects vulnerable.

and as the vuln doc says:
CVE-2024-22513

so people doesn't have to check this manually:
if user and user.is_active:

Because this line in the vuln makes it important I think:

This feature introduces the ability to use different subclasses of the Token class, such as RefreshToken, SlidingToken, AccessToken, and UntypedToken with for_user method. However, this enhancement also expands the potential attack surface.

And it won't be shown on vuln scanners.

from djangorestframework-simplejwt.