Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash on non-existant user ID #293

Closed
wants to merge 1 commit into from

Conversation

rubenwardy
Copy link

Fixes #238

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

@rubenwardy
Copy link
Author

rubenwardy commented Jan 19, 2020

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
Copy link

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
Copy link

Is there anything we can do to get this merged?

@rubenwardy
Copy link
Author

rubenwardy commented Aug 8, 2020

@lingthio would be nice to have a review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: 'NoneType' object has no attribute 'password'
3 participants