From 8aeac51124b4e69cba904cf6cbb61754855b34d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Wed, 9 Sep 2015 16:13:10 +0200 Subject: [PATCH] Fixed an issue that cause user sessions to not be properly associated Sessions could get duplicated, wrongly saved etc. The reason was not persisting the actual user object to the internal session map (but the LocalProxy instead). That could lead to multiple sessions being created for one login, or the session user being set to an anonymous user, or various other odd effects depending on timing. --- src/octoprint/server/util/flask.py | 6 ++++-- src/octoprint/users.py | 24 +++++++++++++++--------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/octoprint/server/util/flask.py b/src/octoprint/server/util/flask.py index e14ab1ba..4dfa3200 100644 --- a/src/octoprint/server/util/flask.py +++ b/src/octoprint/server/util/flask.py @@ -227,8 +227,10 @@ def passive_login(): user = flask.ext.login.current_user if user is not None and not user.is_anonymous(): - flask.g.user = user flask.ext.principal.identity_changed.send(flask.current_app._get_current_object(), identity=flask.ext.principal.Identity(user.get_id())) + if hasattr(user, "get_session"): + flask.session["usersession.id"] = user.get_session() + flask.g.user = user return flask.jsonify(user.asDict()) elif settings().getBoolean(["accessControl", "autologinLocal"]) \ and settings().get(["accessControl", "autologinAs"]) is not None \ @@ -252,7 +254,7 @@ def passive_login(): logger = logging.getLogger(__name__) logger.exception("Could not autologin user %s for networks %r" % (autologinAs, localNetworks)) - return ("", 204) + return "", 204 #~~ cache decorator for cacheable views diff --git a/src/octoprint/users.py b/src/octoprint/users.py index 6d844612..8e48f5b0 100644 --- a/src/octoprint/users.py +++ b/src/octoprint/users.py @@ -28,13 +28,18 @@ class UserManager(object): def login_user(self, user): self._cleanup_sessions() - if user is None \ - or (isinstance(user, LocalProxy) and not isinstance(user._get_current_object(), User)) \ - or (not isinstance(user, LocalProxy) and not isinstance(user, User)): + if user is None: + return + + if isinstance(user, LocalProxy): + user = user._get_current_object() + + if not isinstance(user, User): return None if not isinstance(user, SessionUser): user = SessionUser(user) + self._session_users_by_session[user.get_session()] = user if not user.get_name() in self._session_users_by_username: @@ -49,6 +54,9 @@ class UserManager(object): if user is None: return + if isinstance(user, LocalProxy): + user = user._get_current_object() + if not isinstance(user, SessionUser): return @@ -146,12 +154,10 @@ class UserManager(object): del self._session_users_by_username[username] def findUser(self, username=None, session=None): - if session is not None: - for session in self._session_users_by_session: - user = self._session_users_by_session[session] - if username is None or username == user.get_id(): - return user - break + if session is not None and session in self._session_users_by_session: + user = self._session_users_by_session[session] + if username is None or username == user.get_id(): + return user return None