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

Added fix for Attribute error when ROTATE_REFRESH_TOKENS=True and blacklist app is not installed. #881

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rohitrhmn1
Copy link

When the blacklist app is not installed, the Refresh Token endpoint fails with Attribute error.

Moving the outstand method inside the if block fixes the issue.

@MarcoGlauser
Copy link

Related to this issue with the 5.5.0 release
#875

@@ -130,6 +130,8 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, str]:
try:
# Attempt to blacklist the given refresh token
refresh.blacklist()
# Outstand the token only if the `blacklist` app is installed
refresh.outstand()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Token needs to be added to OutstandingTable after the JTI claim is re-set, otherwise it will end up with the old claim in the table.

TBH i am not sure what is the best approach here, probably some small refactor can make this better, or worst case scenario another try-catch and checking of the BLACKLIST_AFTER_ROTATION setting needs to be done

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably adding a check inside outstand method can help fix the issue.

Copy link
Author

@rohitrhmn1 rohitrhmn1 Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the blacklist function already has the method to Outstand the revoked token:

def blacklist(self) -> BlacklistedToken:
            """
            Ensures this token is included in the outstanding token list and
            adds it to the blacklist.
            """
            jti = self.payload[api_settings.JTI_CLAIM]
            exp = self.payload["exp"]
            user_id = self.payload.get(api_settings.USER_ID_CLAIM)
            User = get_user_model()
            try:
                user = User.objects.get(**{api_settings.USER_ID_FIELD: user_id})
            except User.DoesNotExist:
                user = None
            
            # Ensure outstanding token exists with given jti
            token, _ = OutstandingToken.objects.get_or_create(
                jti=jti,
                defaults={
                    "user": user,
                    "created_at": self.current_time,
                    "token": str(self),
                    "expires_at": datetime_from_epoch(exp),
                },
            )

            return BlacklistedToken.objects.get_or_create(token=token)

Comparing it with Outstand method:

def outstand(self) -> OutstandingToken:
         """
         Ensures this token is included in the outstanding token list and
         adds it to the outstanding token list if not.
         """
         jti = self.payload[api_settings.JTI_CLAIM]
         exp = self.payload["exp"]
         user_id = self.payload.get(api_settings.USER_ID_CLAIM)
         User = get_user_model()
         try:
             user = User.objects.get(**{api_settings.USER_ID_FIELD: user_id})
         except User.DoesNotExist:
             user = None
 
         # Ensure outstanding token exists with given jti
         return OutstandingToken.objects.get_or_create(
             jti=jti,
             defaults={
                 "user": user,
                 "created_at": self.current_time,
                 "token": str(self),
                 "expires_at": datetime_from_epoch(exp),
             },
         )

Both the functions have the similar functionality. Better suggestion would be to keep the OutstandingToken query in the outstand method and remove it from blacklist method.

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

Successfully merging this pull request may close these issues.

3 participants