django-rest-auth
django-rest-auth copied to clipboard
Tivix
Fix Logout Anonymous User
Insert a proper response when the user is anonymous on the logout view
Fixes https://github.com/Tivix/django-rest-auth/issues/332
Coverage increased (+0.01%) to 96.944% when pulling 8a09037cb3b5873b285386de870d9d26571e9b3c on NikosVlagoidis:322-log-out-view into 9d1f65eedcd437003f5bef13fdc1f27d0fb0ee1a on Tivix:master.
Coverage increased (+0.01%) to 96.944% when pulling 8a09037cb3b5873b285386de870d9d26571e9b3c on NikosVlagoidis:322-log-out-view into 9d1f65eedcd437003f5bef13fdc1f27d0fb0ee1a on Tivix:master.
I think that not necessary, especially in next case:
- User logout(but not get response, because trouble in internet connection)
- User try logout one more time(but he can't logout because api return error)
And after that we have two ways for solving that trouble:
- Write client side logic based on text of response message, and check that logic every time when update django-rest-auth
- Write client side logic for check logged user, and test logged user every time before make request to logout api.
HTTP status codes
The HTTP standard provides over 70 status codes to describe the return values.
200 – OK – Everything is working 400 – Bad Request – The request was invalid or cannot be served. The exact error should be explained in the error payload. E.g. „The JSON is not valid“
/rest-auth/logout/ endpoint should have a proper response in my opinion even this has nothing to do with business logic.
If you want an example I can think that a developer consuming my API depends on the responses and he makes a mistake when he calls log out view and does not include a correct Token on the Headers. Then when we test his app he sees that he log out correctly but the token still stays on the server. That's, of course, his mistake but it leads to hours of debugging because my API is not responding correctly.
As far as your point about connection
User logout(but not get response, because trouble in internet connection) User try logout one more time(but he can't logout because api return error)
This will happen if we return "Successfully logged out." too so that is not a valid argument.
I'm still not get how that can be useful. And I don't see any real life example about that logic.
This will happen if we return "Successfully logged out." too so that is not a valid argument.
That will not happened, because client side will get 200 status code from logout api, and user can continue work in application like anonymous user.
User logout(but not get response, because trouble in internet connection) .
You say he is not getting a response. So how is he getting 200 status code?
You can see all best Practices for REST-API that your responses need to describe what happened. This is only enough for a better response.
User logout(but not get response, because trouble in internet connection)
Client app will get error of request timeout or some other kind of error. But actually user can be logout on server side in that moment(especially token can deleted)
User try logout one more time(but he can't logout because api return error)
Right now, this app will return 200 status code in any case(yes it will try delete token, but if token already not exist that is not raise any exception).
With your fix user will get 400 error from server, and for check why we get that response we must have to look at status message of that response(which will depend from language of user) or make some other logic for checking why we get 400 error.
You can see all best Practices for REST-API that your responses need to describe what happened. This is only enough for a better response.
Can you give an example of those best practice?
Right now I though that app corresponds to next next bullets:
- Keep all logic as simple as possible, especially for most of responses can get just two statuses, success(200) and fail(400, 402 with detail descriptions)
- Simple is better than complex.(pep8) Don't have any not necessary logic(not check credentials of user for logout, not check authenticated status of logged user for login and etc.)
- Those app will work correct with many different auth backends, especially I'm have successful experience of use it in production with Session, oAuth2 Tokens, DRF tokens(and anyone from developers in my team don't get any trouble of debugging logout api).
In this case, the user logged out correctly so the client has a token that does not exist. If he gets a 400 you can redirect him to the login page. Don't see any flaw. I guess you would redirect him anyway there after log out.
So saying that > Client Sends a bad request without a token. Servers do not delete the token on the DB. The client thinks the token is deleted and deletes the token. The token is still valid in the database.
- I don't see how logic has to do with misleading again I told you.
- Read the above line of PEP8 -> Explicit is better than implicit. Explicit by definition -- > Fully and clearly expressed; leaving nothing implied. Right now you implied that "Successfully logged out". This means that session is flushed and the token is deleted from the database which neither happens.
- I don't say it does not work. I say it is confusing. It confused me and another person who created the issue. Congrats on your developer team for beeing so awesome but me and another person had this confusion so I think in the future someone else can be in the same place.
As I'm told you can't proof any of your words:
-
You don't know how it can be useful, just write something very synthetic test and thinking about that is real use-case.
-
You don't know at which cases new behavior can be more useful then old behavior
-
You told about "best practices" but you can't proof that too(can't give any link to any article from which I can see new behavior more preferable then old according list of rules).
-
Read the above line of PEP8 -> Explicit is better than implicit. Explicit by definition -- > Fully and clearly expressed; leaving nothing implied. Right now you implied that "Successfully logged out". This means that session is flushed and the token is deleted from the database which neither happens.
Feel free write in docs line: if you send request from anonymous user to logout, you will not get any changes in DB
And also:
- I still sure more better get 200 response from logout and be sure about any tokens for those user not exists on server, then any 40? error response and some unreasonable client side workaround.
- Server should not try to understand which token is correct and which is not, it must just try to delete everything around those credentials.
You mentioned about after I get unsuccessful response response from logout view, I can redirect to login view, but why I'm must do that? How I can recognize those 40? error about those user not logged or something else wrong? Or you would like I'm redirect him in any case to login view?
And last question how you can explain to all users who would like update their django-rest-auth to new version why you broke backward compatibility?
The user does not see the API messages. The developer does. 2 Developers got confused from your Packages. I suggested a solution. It is as simple as that. As far as best practices are concerned you can google it. Let me google it for you does not work at the moment for some reason. If you still can't get it and you are the only person responsible for the request then just close it. Else I can wait and listen to some other opinions about it. Maybe the response should be 200 and say nothing about logged out so it is compatible or it should be 200 but state that user was anonymous. The thing is that there was a problem that you refuse to solve and this is a bad attitude in my opinion.
The user does not see the API messages. The developer does.
You will get message inside response, that code will do that
return Response({"detail": _("Cannot log out AnonymousUser")},`
, and client side can process it in different ways for example your app will not show anything, but some other will.
And in those app errors usually come inside 'non_field_errors' as array.
If you pass API responses to the user and don't implement yours then something wrong is happening the first place.
If you pass API responses to the user and don't implement yours then something wrong is happening the first place.
Can you give an example?
What I am saying is self-explanatory.
I found another case that this implementation may be wrong:
We are using Token base authentication. The user is logged in in 2 clients which mean 2 clients have a valid Token. Now user tries to log out from every device on a panel on one of the clients. Database fails to execute the query. This fails silently now by passing an exception. Now he is logged in in a device he does not want to. What about that?
Hi, as a user of rest-auth, thanks for the contribution! This repo is not maintained anymore, so the development moved to dj-rest-auth. (reference: https://github.com/Tivix/django-rest-auth/issues/568) It may be best, if you move this PR there. (and upgrade to using dj_rest_auth)
new repo link: https://github.com/jazzband/dj-rest-auth (I'm not the upkeeper of that repo, it just makes sense for me to help you merge your PR)
Many Thanks, Barney
