lingthio / Flask-User

Customizable User Authorization & User Management: Register, Confirm, Login, Change username/password, Forgot password and more.
http://flask-user.readthedocs.io/
MIT License
1.06k stars 292 forks source link

Fix crash on non-existant user ID #293

Closed rubenwardy closed 3 years ago

rubenwardy commented 4 years ago

Fixes #238

If a user is deleted then any logged-in sessions will cause an exception due to a NoneType user

rubenwardy commented 4 years ago

on a semi-related note, this function would read a lot better like this:

# Verifies a token and decrypts a User ID and parts of a User password hash
user_manager = current_app.user_manager
data_items = user_manager.verify_token(token, expiration_in_seconds)
if not data_items:
    return None

# Load user by User ID
user_id = data_items[0]
password_ends_with = data_items[1]
user = user_manager.db_manager.get_user_by_id(user_id)
if not user:
    return None

# Make sure that last 8 characters of user password matches
user_password = '' if user_manager.USER_ENABLE_AUTH0 else user.password[-8:]

return user if user_password==password_ends_with else None

you remove the unnecessary token_is_valid variable, and have a simpler control flow (ie: if data_items is None, then it always returns None)

divad1196 commented 4 years ago

Based on @rubenwardy reply, here is the fix i did in my code ` from flask_user import UserMixin

@classmethod def get_user_by_token(cls, token, expiration_in_seconds=None):

This function works in tandem with UserMixin.get_id()

# Token signatures and timestamps are verified.
# user_id and password_ends_with are decrypted.

# Verifies a token and decrypts a User ID and parts of a User password hash
user_manager = current_app.user_manager
data_items = user_manager.verify_token(token, expiration_in_seconds)

# Verify password_ends_with
if data_items:

    # Load user by User ID
    user_id = data_items[0]
    password_ends_with = data_items[1]
    user = user_manager.db_manager.get_user_by_id(user_id)
    if not user:
        return None
    user_password = '' if user_manager.USER_ENABLE_AUTH0 else user.password[-8:]

    # Make sure that last 8 characters of user password matches
    token_is_valid = user and user_password==password_ends_with
    return user if token_is_valid else None

return None

UserMixin.get_user_by_token = get_user_by_token `

It can be called anywhere, but i recommand to keep it before the first use of UserMixin.

I also reorganised the conditions

jaredthecoder commented 4 years ago

Is there anything we can do to get this merged?

rubenwardy commented 4 years ago

@lingthio would be nice to have a review