Cookie transport must return empty json and not null in response.data on login

[caniko]

This causes issues when integrating the openapi schema into openapi-generator. Because the code generator expects the response to be a JSON, as it should be.

[frankie567]

I understand the issue here. I was wondering: will the OpenAPI generator be happy if we properly define a 204 response with no content? Sounds like the most correct way to define this response to me.

[caniko]

I understand the issue here. I was wondering: will the OpenAPI generator be happy if we properly define a 204 response with no content? Sounds like the most correct way to define this response to me.

Right! That is aesthetically more pleasing.

However, it didn't work back in January 31st this year for this guy. He came up wirh some hacky middleware solution.

We would have to test, and see. I do think 204 makes the most sense, but it will be irritating if the openapi-generator is bugged.

I'll do some more digging and testing today. And post my results here.

Just fyi, everything ran fine when it was empty json

[frankie567]

Hmm I guess it's still around: OpenAPITools/openapi-generator#7720 Those 204 responses definitely cause lot of issues 😅

If you have the chance to try, and confirm it's not working, it could be nice. Otherwise, let's go for the empty JSON. The unit test will need to be updated.

Another thing: could you revert the version bumping? This is something I do automatically with a script when I create a release.

[caniko]

I am done testing, and everything ran fine with 204 on my end; however, I only tested for dart-dio with the openapi-generator, there may be issues with other languages. Pedantically speaking, it isn't an issue on our side, but I am not sure if it is OK to neglect for practical reasons. I reverted my empty JSON change for now, I can add it back if you would like.

We should require Fastapi 0.79, coincidentally the latest version 😅, as this is when the 204 issue was fixed on the fastapi side. I made the change to pyproject.toml.

Ready to merge!

[codecov]

Codecov Report

Merging #1037 (b1db927) into master (d0c295e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master     #1037    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           20        35    +15     
  Lines          717       957   +240     
==========================================
+ Hits           717       957   +240     

Impacted Files	Coverage Δ
fastapi_users/__init__.py	100.00% <100.00%> (ø)
fastapi_users/authentication/__init__.py	100.00% <100.00%> (ø)
fastapi_users/authentication/authenticator.py	100.00% <100.00%> (ø)
fastapi_users/authentication/backend.py	100.00% <100.00%> (ø)
fastapi_users/authentication/strategy/__init__.py	100.00% <100.00%> (ø)
fastapi_users/authentication/strategy/base.py	100.00% <100.00%> (ø)
...stapi_users/authentication/strategy/db/__init__.py	100.00% <100.00%> (ø)
...astapi_users/authentication/strategy/db/adapter.py	100.00% <100.00%> (ø)
fastapi_users/authentication/strategy/db/models.py	100.00% <100.00%> (ø)
...stapi_users/authentication/strategy/db/strategy.py	100.00% <100.00%> (ø)
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

[frankie567]

Great! I consider we are doing the right thing here, so let's go with it and we'll see if some OpenAPI generators cause issues to some people.

I took the liberty to slightly change your approach by building a full response manually. This way, we still be compatible with older FastAPI.

Thank you for your raising this point and your help 👍

[frankie567]

@all-contributors add @caniko for bug

[allcontributors]

@frankie567

I've put up a pull request to add @caniko! 🎉

[caniko]

Thank you for this awesome package!

@frankie567 you reverted the versioning on pyproject.toml from fastapi=0.79.0 to the original. Was this a mistake? I can make a new PR!

[frankie567]

Thanks!

No, I did it on purpose: since we build a custom raw Response, we can use older FastAPI version, it won't provoke the 204 response bug.

[caniko]

I took the liberty to slightly change your approach by building a full response manually. This way, we still be compatible with older FastAPI.

I see, I misunderstood this statement! Have a good weekend 😁