-
Notifications
You must be signed in to change notification settings - Fork 682
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
base: master
Are you sure you want to change the base?
Conversation
…blacklist` app is not installed.
Related to this issue with the 5.5.0 release |
@@ -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() |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.