diff --git a/README.md b/README.md index 467653c..8aa58f2 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,7 @@ ALLOW_SIGNUPS=false ``` - **First run:** there is no default account and no password in the environment. The first visitor to the login page is asked to create an account. **Do this right after starting the container**, before anyone else can reach it. - **More accounts:** sign-up is off by default. Set `ALLOW_SIGNUPS=true` to show a **Create one** link on the login page, and set it back to `false` once everyone has an account. Creating the first account is always allowed. +- **Sessions:** a login lasts 7 days. **Logout** ends that session on the server, so a copy of its cookie stops working too; other devices stay signed in. Sessions (basic and OIDC) are listed in `/config/sessions.json`, so a restart keeps you signed in. - **Changing a password:** click **Password** in the top bar. This signs out that user's other sessions. - **Storage:** accounts live in `/config/users.json`, with passwords as salted scrypt hashes and the file readable only by the container user. - **Forgotten password:** remove that user's entry from `/config/users.json` (or delete the file to start over) and restart the container. diff --git a/root/app/backend/auth.py b/root/app/backend/auth.py index 916db09..094b946 100644 --- a/root/app/backend/auth.py +++ b/root/app/backend/auth.py @@ -230,6 +230,70 @@ def check_basic_credentials(username: str, password: str) -> bool: ok = verify_password_hash(password, user["hash"] if user else _DUMMY_HASH) return user is not None and ok +# ----------------- Sessions ----------------- + +# Session cookies are signed, so the server can trust them without storing them, but +# then logging out could only delete the browser's copy: any other copy stayed valid +# until it expired. Each cookie therefore names a session id that must still be listed +# here, and logging out removes it. Kept on disk so a restart doesn't sign everyone out. +SESSIONS_FILE = CONFIG_DIR / "sessions.json" +_sessions_lock = threading.Lock() + +def _load_sessions() -> Dict[str, float]: + """{session id: expiry time}. A missing or damaged file just means no sessions.""" + try: + data = json.loads(SESSIONS_FILE.read_text()) + if isinstance(data, dict): + return {sid: float(exp) for sid, exp in data.items() if isinstance(sid, str)} + except FileNotFoundError: + pass + except (OSError, ValueError, TypeError): + logger.warning("%s is unreadable; existing sessions must log in again", SESSIONS_FILE) + return {} + +def _save_sessions(sessions: Dict[str, float]): + """Caller must hold _sessions_lock.""" + CONFIG_DIR.mkdir(parents=True, exist_ok=True) + tmp = SESSIONS_FILE.with_name(SESSIONS_FILE.name + ".tmp") + fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w") as f: + json.dump(sessions, f) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp, SESSIONS_FILE) + +_sessions = _load_sessions() + +def start_session() -> str: + """A new session id, valid for MAX_AGE.""" + global _sessions + sid = secrets.token_urlsafe(24) + now = time.time() + with _sessions_lock: + sessions = {k: v for k, v in _sessions.items() if v > now} # drop expired ones + sessions[sid] = now + MAX_AGE + _save_sessions(sessions) + _sessions = sessions + return sid + +def session_active(sid: Optional[str]) -> bool: + return bool(sid) and _sessions.get(sid, 0) > time.time() + +def end_session(token: Optional[str]): + """Revoke the session a cookie names, so no copy of that cookie works any more.""" + global _sessions + if not token: + return + try: + sid = serializer.loads(token, max_age=MAX_AGE).get("sid") + except (BadSignature, SignatureExpired, AttributeError): + return + with _sessions_lock: + if sid in _sessions: + sessions = {k: v for k, v in _sessions.items() if k != sid} + _save_sessions(sessions) + _sessions = sessions + # ----------------- Login rate limiting ----------------- LOGIN_MAX_FAILURES = 10 @@ -329,7 +393,7 @@ def _cookie_secure(request: Optional[Request]) -> bool: def create_session_cookie(response: Response, user_data: dict, request: Optional[Request] = None): if user_data.get("auth_mode") == "basic": user_data = {**user_data, "pwv": user_epoch(user_data.get("username", ""))} - token = serializer.dumps(user_data) + token = serializer.dumps({**user_data, "sid": start_session()}) response.set_cookie( key=COOKIE_NAME, value=token, @@ -397,7 +461,8 @@ def get_current_user(request: Request) -> Dict[str, Any]: # is removed from users.json epoch = user_epoch(data.get("username", "")) stale = AUTH_MODE == "basic" and (epoch is None or data.get("pwv") != epoch) - if data.get("auth_mode") == AUTH_MODE and not stale: + # Also rejects cookies issued before sessions had ids: one more login after upgrading + if data.get("auth_mode") == AUTH_MODE and not stale and session_active(data.get("sid")): return { "authenticated": True, "username": data.get("username", "user"), diff --git a/root/app/backend/main.py b/root/app/backend/main.py index 475b1c4..994a0d0 100644 --- a/root/app/backend/main.py +++ b/root/app/backend/main.py @@ -35,6 +35,8 @@ client_ip, create_session_cookie, clear_session_cookie, + end_session, + COOKIE_NAME, create_state_cookie, verify_state_cookie, clear_state_cookie @@ -332,7 +334,9 @@ async def oidc_callback(request: Request, code: Optional[str] = None, state: Opt # POST only: the session cookie is SameSite=Lax, which still rides along when another site # navigates here, so a GET logout let any page sign the user out @app.post("/auth/logout") -async def logout(): +def logout(request: Request): + # Ends the session itself, not just this browser's copy of the cookie + end_session(request.cookies.get(COOKIE_NAME)) # Cookie must be cleared on the response actually returned, not an injected one. # 303 so the browser follows up with a GET of the login page. response = RedirectResponse("/login.html", status_code=status.HTTP_303_SEE_OTHER) diff --git a/tests/test_account.py b/tests/test_account.py index 8b13152..f84079f 100644 --- a/tests/test_account.py +++ b/tests/test_account.py @@ -170,3 +170,61 @@ def test_logout_by_post_clears_the_session(app): assert res.headers["location"] == "/login.html" assert auth.COOKIE_NAME in res.headers.get("set-cookie", "") assert client.get("/auth/status").json()["authenticated"] is False + + +# ----------------- Server-side sessions ----------------- + +def test_a_copied_cookie_stops_working_after_logout(app): + """Logging out ends the session, not just this browser's copy of the cookie.""" + main, auth = app + client, res = signup(main) + cookie = client.cookies.get(auth.COOKIE_NAME) + copy = TestClient(main.app, cookies={auth.COOKIE_NAME: cookie}) + assert copy.get("/auth/status").json()["authenticated"] is True + client.post("/auth/logout", follow_redirects=False) + assert copy.get("/auth/status").json()["authenticated"] is False + + +def test_logging_out_one_session_keeps_the_others(app): + main, auth = app + signup(main) + laptop, phone = login(main), login(main) + laptop.post("/auth/logout", follow_redirects=False) + assert phone.get("/auth/status").json()["authenticated"] is True + + +def test_sessions_survive_a_restart(app): + main, auth = app + client, _ = signup(main) + cookie = client.cookies.get(auth.COOKIE_NAME) + for name in ("auth", "main"): + sys.modules.pop(name, None) + main2 = importlib.import_module("main") + sys.modules["auth"].AUTH_MODE = main2.AUTH_MODE = "basic" + copy = TestClient(main2.app, cookies={auth.COOKIE_NAME: cookie}) + assert copy.get("/auth/status").json()["authenticated"] is True + + +def test_a_validly_signed_cookie_without_a_session_is_rejected(app): + """Cookies issued before sessions had ids; also any cookie whose session ended.""" + main, auth = app + signup(main) + token = auth.serializer.dumps({"username": "alice", "auth_mode": "basic", + "pwv": auth.user_epoch("alice")}) + client = TestClient(main.app, cookies={auth.COOKIE_NAME: token}) + assert client.get("/auth/status").json()["authenticated"] is False + + +def test_expired_sessions_are_dropped_from_the_file(app, monkeypatch): + main, auth = app + signup(main) + real_time = auth.time.time + monkeypatch.setattr(auth.time, "time", lambda: real_time() + auth.MAX_AGE + 60) + login(main) + assert len(auth._load_sessions()) == 1 # only the new one + + +def test_a_damaged_sessions_file_means_no_sessions(app): + main, auth = app + auth.SESSIONS_FILE.write_text("{not json") + assert auth._load_sessions() == {}