500 internal server error when retrieve_user returns None

[shaljam]

As seen here:

sanic-jwt/sanic_jwt/authentication.py

Line 43 in a6fcb7f

raise exceptions.InvalidRetrieveUserObject()

when user is None, BaseAuthentication raises InvalidRetrieveUserObject. In my case when a /auth/refresh request arrives with a token created for a user not present, this problem happens.

[ahopkins]

What is the expected response? You can always catch that and provide a response yourself.

[shaljam]

@ahopkins I would say 401 Unauthorized would fit better. Also as None is expected to be returned from retrieve_user, I think 500 Internal Server Error is not appropriate.
Could you please point me to samples regarding catching the exception?

[ahopkins]

I am thinking maybe 400. Not sure I agree it is Unauthorized.

Regardless, you can catch exceptions like this:

@app.exception(InvalidRetrieveUserObject)
async def invalid_user(request, exception):
    return text("Looks like we got another bad user... 😒")

https://sanic.readthedocs.io/en/latest/sanic/exceptions.html#handling-exceptions

[superosku]

Having the same issue here. @ahopkins is there any other way to handle deleted users?

[ahopkins]

What do you have in mind? Also, not to be one to tell you how to structure your app, but in my experience typically you do not want to delete user records, but instead deactivate them.

Does the above exception handler not work for you?

[superosku]

Maybe deactivating the user instead is the way to go. But also it would be nice if sanic-jwt supported removing users.

At first this solution was not working for me since the exception was handled by this function that handles the exception and returns an 500 error:
https://github.com/ahopkins/sanic-jwt/blob/master/sanic_jwt/responses.py#L94

But I got it working by adding the exception handler before calling the Initialize method of sanic_jwt.

[ahopkins]

The "problem" with that solution is that sanic-jwt is not meant to get involved in how your application handles users. All it should need to know is how to get them, because anything else would be highly domain specific.

[superosku]

Yeah that makes sense. But there should be a way to use sanic-jwt so that one can delete users. And there should be a documented way to implement that. It seems reasonable to me that the correct way to implement that using sanic-jwt would be to make it so that the retrieve_user function returns None when the user is removed (or disabled? **). That seems more appropriate way than adding the exception handler as described in this issue.

On somewhat related note: This issue was really hard to debug since sanic-jwt seems to hide the error and just return an 500 error. This issue of debugging being hard happened to me a few times actually when my retrieve_user and authenticate functions had bugs in them. So at least that could be improved.

** I did not check how the disabled user use case would work. I guess retrieve_user would return the user data but what would then handle that the refresh endpoint does not then return a new token? Should it be handled in the retrieve_refresh_token function? I could then check if the user is still valid there and return None if not. That would then mean I would need to query the users table twice in one request.

[ahopkins]

I am not clear if you are looking for another way to catch the None or if you are looking for an endpoint to be able to take an action (like removal of a user), sort of like how there is a method for retrieving and setting tokens.