fix: support token blacklist feature in rest_auth

[bartvanandel]

Add correct serializer when rest_framework_simplejwt.token_blacklist is detected, mirroring the approach taken by dj-rest-auth. This serializer currently requires the token to be sent along with the request.

Note: the code in dj-rest-auth currently marks the body as optional, but:

a) I could not find a way to specify that the request body is entirely optional in drf-spectacular (I think drf-spectacular currently forces required: true when the request body is not None); and
b) dj-rest-auth will currently definitely fail anyway when the blacklist app is enabled and /logout is called without a proper body.

We rely on a code generator to generate out front-end auth client. Without this patch, we can't use the /logout endpoint in that client.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.58%. Comparing base (d952b47) to head (fe3ce1e).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1392   +/-   ##
=======================================
  Coverage   98.58%   98.58%           
=======================================
  Files          76       76           
  Lines        9176     9191   +15     
=======================================
+ Hits         9046     9061   +15     
  Misses        130      130           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bartvanandel]

@tfranzel could you have a look, and enable the workflow that requires approval?

[tfranzel]

Hi @bartvanandel,

sry for the delay. Could you please rebase your change because the the GH worker image got retired and rescheduling the actions won't make use of the new one.

[bartvanandel]

@tfranzel Rebase done!

[tfranzel]

Finally had some time to review. I think this looks good. However, I believe there is another conditional missing. Shouldn't this also be dependant on USE_JWT?

https://github.com/iMerica/dj-rest-auth/blob/545fd1f4c9ae570216989a8f5a563dd938e038af/dj_rest_auth/views.py#L171

[bartvanandel]

Yeah probably a good idea. One can have SIMPLE_JWT in their settings, but choose not to enable it for everything.

Updated and rebased on top of current master.

[bartvanandel]

@tfranzel Anything else you want to change? Need more testing?

[tfranzel]

Hey @bartvanandel sry for this delay, but I am finally catching up. Got 2 more issues left for a release.
I'll try to resolve this with you in a speedy manner. No more delays.

On revisiting this, I noticed another thing. I think its basically three conditions to end up with that POST body

• api_settings.USE_JWT
• 'rest_framework_simplejwt.token_blacklist' in settings.INSTALLED_APPS
• not api_settings.JWT_AUTH_HTTPONLY

https://github.com/iMerica/dj-rest-auth/blob/c4213c58ad931ea4a27756fd2b38facef1383d38/dj_rest_auth/views.py#L187

Also your condition getattr(settings, 'SIMPLE_JWT', {}).get('BLACKLIST_AFTER_ROTATION', False) somewhat puzzled me. Isn't this setting unrelated to the actual logout process or am I missing something?

---

Note: the code in dj-rest-auth currently marks the body as optional, but:

I don't think that is correct. If we end up in the branch with those 3 conditions the 'refresh' field is mandatory, not optional. Otherwise it will just be ignored. you agree?

---

I could not find a way to specify that the request body is entirely optional in drf-spectacular (I think drf-spectacular currently forces required: true when the request body is not None)

Afair there is not a direct way to turn it off, but if you have a serializer with zero required "write" fields, the whole payload becomes optional and that check turns it off then. So constructing the serializer in that way should remove required: True.

drf-spectacular/drf_spectacular/openapi.py

Line 1371 in 39cb3d5

request_body_required = any(req not in readonly_props for req in required_props)

[bartvanandel]

Hi @tfranzel,

We've moved on from Python to C# in the meantime, so I'm not actively using your project anymore. Having said that...

I think its basically three conditions to end up with that POST body [...]

From what I remember, yes, it's a combination of settings. Mind you, since you took a while to respond, we were using a private fork to get around the issues we were experiencing. This PR is basically the code that we were using in production.

Also your condition getattr(settings, 'SIMPLE_JWT', {}).get('BLACKLIST_AFTER_ROTATION', False) somewhat puzzled me. Isn't this setting unrelated to the actual logout process or am I missing something?

I'd have to retrace my steps to get the exact details, but I think I remember that with settings['SIMPLE_JWT']['BLACKLIST_AFTER_ROTATION'] enabled, it is required to send your current token to the logout endpoint (because the backend then requires blacklisting it). So you have to account for that.

Note: the code in dj-rest-auth currently marks the body as optional, but:

I don't think that is correct. If we end up in the branch with those 3 conditions the 'refresh' field is mandatory, not optional. Otherwise it will just be ignored. you agree?

I really can't remember the detail. This is just what I ran into, at the time, I did some research into the issue, and this is what I noticed.

I could not find a way to specify that the request body is entirely optional in drf-spectacular (I think drf-spectacular currently forces required: true when the request body is not None)

Afair there is not a direct way to turn it off, but if you have a serializer with zero required "write" fields, the whole payload becomes optional and that check turns it off then. So constructing the serializer in that way should remove required: True.

I really can't remember, sorry. However, I'd be careful to consider 'zero required "write" fields' the same as None for a body. Posting an empty object vs None can mean 2 different things, no?

Anyway, like I said, we have moved away. I guess it can still be beneficial to fix this issue, especially while other projects (not to mention Django itself) point to drf-spectacular as the go-to way for rendering OpenAPI specs.

[tfranzel]

@bartvanandel

Thanks for getting back at me even though you moved on. Much appreciated. I wish you good luck with C# 😄

Posting an empty object vs None can mean 2 different things, no?

I guess so, but I don't think it matter much here. DRF might also accept {} for empty but I don't remember exactly either. In any case this patch is an improvement either way.

closed by 5221afd