diff --git a/src/octoprint/access/users.py b/src/octoprint/access/users.py index c04313b70d..c0109753eb 100644 --- a/src/octoprint/access/users.py +++ b/src/octoprint/access/users.py @@ -127,11 +127,9 @@ def _cleanup_sessions(self): for session, user in list(self._session_users_by_session.items()): if not isinstance(user, SessionUser): continue - if user.created + (24 * 60 * 60) < time.monotonic(): + if user.touched + (15 * 60) < time.monotonic(): self._logger.info( - "Cleaning up user session {} for user {}".format( - session, user.get_id() - ) + f"Cleaning up user session {session} for user {user.get_id()}" ) self.logout_user(user, stale=True) @@ -176,6 +174,9 @@ def check_password(self, username, password): # old hash doesn't match either, wrong password return False + def signature_key_for_user(self, username, salt=None): + return self.create_password_hash(username, salt=salt) + def add_user(self, username, password, active, permissions, groups, overwrite=False): pass @@ -227,21 +228,28 @@ def remove_user(self, username): del self._sessionids_by_userid[username] def validate_user_session(self, userid, session): + self._cleanup_sessions() + if session in self._session_users_by_session: user = self._session_users_by_session[session] return userid == user.get_id() return False - def find_user(self, userid=None, session=None): + def find_user(self, userid=None, session=None, fresh=False): + self._cleanup_sessions() + if session is not None and session in self._session_users_by_session: user = self._session_users_by_session[session] if userid is None or userid == user.get_id(): + user.touch() return user return None def find_sessions_for(self, matcher): + self._cleanup_sessions() + result = [] for user in self.get_all_users(): if matcher(user): @@ -249,7 +257,9 @@ def find_sessions_for(self, matcher): session_ids = self._sessionids_by_userid[user.get_id()] for session_id in session_ids: try: - result.append(self._session_users_by_session[session_id]) + session_user = self._session_users_by_session[session_id] + session_user.touch() + result.append(session_user) except KeyError: # unknown session after all continue @@ -780,6 +790,14 @@ def change_user_password(self, username, password): self._dirty = True self._save() + self._trigger_on_user_modified(user) + + def signature_key_for_user(self, username, salt=None): + if username not in self._users: + raise UnknownUser(username) + user = self._users[username] + return UserManager.create_password_hash(username + user._passwordHash, salt=salt) + def change_user_setting(self, username, key, value): if username not in self._users: raise UnknownUser(username) @@ -845,10 +863,10 @@ def remove_user(self, username): self._dirty = True self._save() - def find_user(self, userid=None, apikey=None, session=None): + def find_user(self, userid=None, apikey=None, session=None, fresh=False): user = UserManager.find_user(self, userid=userid, session=session) - if user is not None: + if user is not None or (session and fresh): return user if userid is not None: @@ -1383,8 +1401,7 @@ def __init__(self, user): wrapt.ObjectProxy.__init__(self, user) self._self_session = "".join("%02X" % z for z in bytes(uuid.uuid4().bytes)) - self._self_created = time.monotonic() - self._self_touched = time.monotonic() + self._self_created = self._self_touched = time.monotonic() @property def session(self): diff --git a/src/octoprint/server/__init__.py b/src/octoprint/server/__init__.py index 926b0c572b..133b0937e9 100644 --- a/src/octoprint/server/__init__.py +++ b/src/octoprint/server/__init__.py @@ -130,7 +130,7 @@ loginFromApiKeyRequestHandler, requireLoginRequestHandler, ) -from octoprint.server.util.flask import PreemptiveCache +from octoprint.server.util.flask import PreemptiveCache, validate_session_signature from octoprint.settings import settings VERSION = __version__ @@ -187,12 +187,25 @@ def load_user(id): else: sessionid = None + if session and "usersession.signature" in session: + sessionsig = session["usersession.signature"] + else: + sessionsig = "" + if sessionid: - user = userManager.find_user(userid=id, session=sessionid) + # session["_fresh"] is False if the session comes from a remember me cookie, + # True if it came from a use of the login dialog + user = userManager.find_user( + userid=id, session=sessionid, fresh=session.get("_fresh", False) + ) else: user = userManager.find_user(userid=id) - if user and user.is_active: + if ( + user + and user.is_active + and (not sessionid or validate_session_signature(sessionsig, id, sessionid)) + ): return user return None @@ -1366,7 +1379,9 @@ def _setup_app(self, app): app.config["TEMPLATES_AUTO_RELOAD"] = True app.config["JSONIFY_PRETTYPRINT_REGULAR"] = False + app.config["REMEMBER_COOKIE_DURATION"] = 90 * 24 * 60 * 60 # 90 days app.config["REMEMBER_COOKIE_HTTPONLY"] = True + # REMEMBER_COOKIE_SECURE will be taken care of by our custom cookie handling # we must not set this before TEMPLATES_AUTO_RELOAD is set to True or that won't take app.debug = self._debug diff --git a/src/octoprint/server/api/__init__.py b/src/octoprint/server/api/__init__.py index f63096f348..a3bc893394 100644 --- a/src/octoprint/server/api/__init__.py +++ b/src/octoprint/server/api/__init__.py @@ -38,6 +38,7 @@ limit, no_firstrun_access, passive_login, + session_signature, ) from octoprint.settings import settings as s from octoprint.settings import valid_boolean_trues @@ -312,6 +313,9 @@ def login(): user = octoprint.server.userManager.login_user(user) session["usersession.id"] = user.session + session["usersession.signature"] = session_signature( + username, user.session + ) g.user = user login_user(user, remember=remember) diff --git a/src/octoprint/server/util/flask.py b/src/octoprint/server/util/flask.py index 52356336fa..244bd8e207 100644 --- a/src/octoprint/server/util/flask.py +++ b/src/octoprint/server/util/flask.py @@ -5,11 +5,13 @@ __copyright__ = "Copyright (C) 2014 The OctoPrint Project - Released under terms of the AGPLv3 License" import functools +import hashlib +import hmac import logging import os import threading import time -from datetime import datetime +from datetime import datetime, timedelta from typing import Union import flask @@ -23,6 +25,9 @@ import webassets.updater import webassets.utils from cachelib import BaseCache +from flask import current_app +from flask_login import COOKIE_NAME as REMEMBER_COOKIE_NAME +from flask_login.utils import decode_cookie, encode_cookie from werkzeug.local import LocalProxy from werkzeug.utils import cached_property @@ -422,6 +427,49 @@ def host_to_server_and_port(host, scheme): # ~~ request and response versions +def encode_remember_me_cookie(value): + from octoprint.server import userManager + + name = value.split("|")[0] + try: + remember_key = userManager.signature_key_for_user( + name, salt=current_app.config["SECRET_KEY"] + ) + timestamp = datetime.utcnow().timestamp() + return encode_cookie(f"{name}|{timestamp}", key=remember_key) + except Exception: + pass + + return "" + + +def decode_remember_me_cookie(value): + from octoprint.server import userManager + + parts = value.split("|") + if len(parts) == 3: + name, created, _ = parts + + try: + # valid signature? + signature_key = userManager.signature_key_for_user( + name, salt=current_app.config["SECRET_KEY"] + ) + cookie = decode_cookie(value, key=signature_key) + if cookie: + # still valid? + if ( + datetime.fromtimestamp(float(created)) + + timedelta(seconds=current_app.config["REMEMBER_COOKIE_DURATION"]) + > datetime.utcnow() + ): + return encode_cookie(name) + except Exception: + pass + + raise ValueError("Invalid remember me cookie") + + class OctoPrintFlaskRequest(flask.Request): environment_wrapper = staticmethod(lambda x: x) @@ -437,10 +485,23 @@ def cookies(self): result = {} desuffixed = {} for key, value in cookies.items(): - if key.endswith(self.cookie_suffix): - desuffixed[key[: -len(self.cookie_suffix)]] = value - else: - result[key] = value + + def process_value(k, v): + if k == current_app.config.get( + "REMEMBER_COOKIE_NAME", REMEMBER_COOKIE_NAME + ): + return decode_remember_me_cookie(v) + return v + + try: + if key.endswith(self.cookie_suffix): + key = key[: -len(self.cookie_suffix)] + desuffixed[key] = process_value(key, value) + else: + result[key] = process_value(key, value) + except ValueError: + # ignore broken cookies + pass result.update(desuffixed) return result @@ -471,7 +532,7 @@ def cookie_suffix(self): class OctoPrintFlaskResponse(flask.Response): - def set_cookie(self, key, *args, **kwargs): + def set_cookie(self, key, value="", *args, **kwargs): # restrict cookie path to script root kwargs["path"] = flask.request.script_root + kwargs.get("path", "/") @@ -490,9 +551,13 @@ def set_cookie(self, key, *args, **kwargs): # set secure if necessary kwargs["secure"] = settings().getBoolean(["server", "cookies", "secure"]) + # tie account properties to remember me cookie (e.g. current password hash) + if key == current_app.config.get("REMEMBER_COOKIE_NAME", REMEMBER_COOKIE_NAME): + value = encode_remember_me_cookie(value) + # add request specific cookie suffix to name flask.Response.set_cookie( - self, key + flask.request.cookie_suffix, *args, **kwargs + self, key + flask.request.cookie_suffix, value=value, *args, **kwargs ) def delete_cookie(self, key, path="/", domain=None): @@ -608,6 +673,9 @@ def login(u): ) if hasattr(u, "session"): flask.session["usersession.id"] = u.session + flask.session["usersession.signature"] = session_signature( + u.get_id(), u.session + ) flask.g.user = u eventManager().fire(Events.USER_LOGGED_IN, payload={"username": u.get_id()}) @@ -1832,3 +1900,19 @@ def default(self, obj): return JsonEncoding.encode(obj) except TypeError: return flask.json.JSONEncoder.default(self, obj) + + +##~~ Session signing + + +def session_signature(user, session): + from octoprint.server import userManager + + key = userManager.signature_key_for_user(user, salt=current_app.config["SECRET_KEY"]) + return hmac.new( + key.encode("utf-8"), session.encode("utf-8"), hashlib.sha512 + ).hexdigest() + + +def validate_session_signature(sig, user, session): + return hmac.compare_digest(sig, session_signature(user, session)) diff --git a/src/octoprint/server/util/sockjs.py b/src/octoprint/server/util/sockjs.py index 88bcacf94a..83134540ca 100644 --- a/src/octoprint/server/util/sockjs.py +++ b/src/octoprint/server/util/sockjs.py @@ -21,9 +21,10 @@ import octoprint.vendor.sockjs.tornado.util from octoprint.access.groups import GroupChangeListener from octoprint.access.permissions import Permissions -from octoprint.access.users import LoginStatusListener +from octoprint.access.users import LoginStatusListener, SessionUser from octoprint.events import Events from octoprint.settings import settings +from octoprint.util import RepeatedTimer from octoprint.util.json import dumps as json_dumps from octoprint.util.version import get_python_version_string @@ -173,6 +174,10 @@ def __init__( self._subscriptions_active = False self._subscriptions = {"state": False, "plugins": [], "events": []} + self._keep_alive = RepeatedTimer( + 60, self._keep_alive_callback, condition=lambda: self._authed + ) + @staticmethod def _get_remote_address(info): forwarded_for = info.headers.get("X-Forwarded-For") @@ -180,6 +185,13 @@ def _get_remote_address(info): return forwarded_for.split(",")[0] return info.ip + def _keep_alive_callback(self): + if not self._authed: + return + if not isinstance(self._user, SessionUser): + return + self._user.touch() + def __str__(self): if self._remoteAddress: return f"{self!r} connected to {self._remoteAddress}" @@ -716,6 +728,8 @@ def _on_login(self, user): ) self._authed = True + self._keep_alive.start() + for name, hook in self._authed_hooks.items(): try: hook(self, self._user) diff --git a/src/octoprint/static/js/app/viewmodels/usersettings.js b/src/octoprint/static/js/app/viewmodels/usersettings.js index 4588241b6d..1ac1775b90 100644 --- a/src/octoprint/static/js/app/viewmodels/usersettings.js +++ b/src/octoprint/static/js/app/viewmodels/usersettings.js @@ -88,7 +88,7 @@ $(function () { self.userSettingsDialog.trigger("beforeSave"); - function process() { + function saveSettings() { var settings = { interface: { language: self.interface_language() @@ -110,7 +110,7 @@ $(function () { self.access_currentPassword() ) .done(function () { - process(); + saveSettings(); }) .fail(function (xhr) { if (xhr.status === 403) { @@ -118,7 +118,7 @@ $(function () { } }); } else { - process(); + saveSettings(); } }; diff --git a/src/octoprint/templates/dialogs/usersettings/access.jinja2 b/src/octoprint/templates/dialogs/usersettings/access.jinja2 index 9cd750b6ff..6bc6d9a445 100644 --- a/src/octoprint/templates/dialogs/usersettings/access.jinja2 +++ b/src/octoprint/templates/dialogs/usersettings/access.jinja2 @@ -24,6 +24,9 @@ {{ _('Passwords do not match') }} +

{% trans %} + Please note that you will be logged out immediately after changing your password and asked to login again. + {% endtrans %}

{{ _('API Key') }} diff --git a/tests/server/util/test_flask.py b/tests/server/util/test_flask.py index 114220c26d..b7c8f6a44d 100644 --- a/tests/server/util/test_flask.py +++ b/tests/server/util/test_flask.py @@ -10,6 +10,7 @@ import unittest from unittest import mock +import flask from ddt import data, ddt, unpack from octoprint.server.util.flask import ( @@ -424,6 +425,9 @@ class OctoPrintFlaskRequestTest(unittest.TestCase): def setUp(self): self.orig_environment_wrapper = OctoPrintFlaskRequest.environment_wrapper + self.app = flask.Flask("testapp") + self.app.config["SECRET_KEY"] = "secret" + def tearDown(self): OctoPrintFlaskRequest.environment_wrapper = staticmethod( self.orig_environment_wrapper @@ -470,7 +474,8 @@ def test_cookies(self): request = OctoPrintFlaskRequest(environ) - cookies = request.cookies + with self.app.app_context(): + cookies = request.cookies self.assertDictEqual( { "postfixed": "postfixed_value", @@ -495,6 +500,9 @@ def setUp(self): self.settings = mock.MagicMock() self.settings_getter.return_value = self.settings + self.app = flask.Flask("testapp") + self.app.config["SECRET_KEY"] = "secret" + def tearDown(self): self.settings_patcher.stop() @@ -545,13 +553,14 @@ def test_cookie_set_and_delete( # test set_cookie with mock.patch("flask.Response.set_cookie") as set_cookie_mock: - response.set_cookie("some_key", "some_value", **kwargs) + with self.app.app_context(): + response.set_cookie("some_key", "some_value", **kwargs) # set_cookie should have key and path values adjusted set_cookie_mock.assert_called_once_with( response, "some_key" + expected_suffix, - "some_value", + value="some_value", path=expected_path_set, secure=secure, samesite=expected_samesite, @@ -560,7 +569,8 @@ def test_cookie_set_and_delete( # test delete_cookie with mock.patch("flask.Response.set_cookie") as set_cookie_mock: with mock.patch("flask.Response.delete_cookie") as delete_cookie_mock: - response.delete_cookie("some_key", **kwargs) + with self.app.app_context(): + response.delete_cookie("some_key", **kwargs) # delete_cookie internally calls set_cookie - so our delete_cookie call still uses the non modified # key and path values, set_cookie will translate those (as tested above)