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

Authentication logic could permit inactive/deleted users to continue using their account #571

Open
brassy-endomorph opened this issue Sep 10, 2024 · 0 comments

Comments

@brassy-endomorph
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

When we login a user, we set their session to contain the user_id here:

session["user_id"] = user.id

Our auth function is defined here:

def authentication_required(f: Callable[..., Any]) -> Callable[..., Any]:
@wraps(f)
def decorated_function(*args: Any, **kwargs: Any) -> Any:
if "user_id" not in session:
flash("👉 Please complete authentication.")
return redirect(url_for("login"))
if not session.get("is_authenticated", False):
return redirect(url_for("verify_2fa_login"))
return f(*args, **kwargs)

Because the auth function does not hit the database on every request, it could be possible for a user be deleted from the database and that user to still actively use the session. Currently this does not happen as the bug #570 (infinite redirect) is hit instead.

Further, since the session uses the user's primary key (users.id) as the identifier, even if we fixed the logic to do a lookup on that value for every request, it would be impossible in invalidate the session without some additional logic being added to the app and DB.

Describe the solution you'd like

  1. Just use Flask-Login
  2. Add a session_id field to the users table (or a separate related sessions table). Use the session_id in the session token. Look up the user on every request, and add the User object to Flask.g so that only the session_id needs to be stored in the cookie.
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

No branches or pull requests

1 participant