diff --git a/cista/api.py b/cista/api.py index 0828726..12e28a7 100644 --- a/cista/api.py +++ b/cista/api.py @@ -126,12 +126,10 @@ async def update_public(request): await auth.verify(request, privileged=True) try: public = request.json["public"] - if not isinstance(public, bool): - raise ValueError("public must be a boolean") except KeyError: raise BadRequest("Missing public field") from None - except ValueError as e: - raise BadRequest(str(e)) from None + if not isinstance(public, bool): + raise BadRequest("public must be a boolean") config.update_config({"public": public}) return json({"message": "Public access setting updated", "public": public}) @@ -141,12 +139,10 @@ async def update_name(request): await auth.verify(request, privileged=True) try: name = request.json["name"] - if not isinstance(name, str): - raise ValueError("name must be a string") except KeyError: raise BadRequest("Missing name field") from None - except ValueError as e: - raise BadRequest(str(e)) from None + if not isinstance(name, str): + raise BadRequest("name must be a string") config.update_config({"name": name}) # Return the effective name (fallback to path.name if empty) effective_name = name or config.config.path.name diff --git a/cista/app.py b/cista/app.py index c71793e..45b2b45 100644 --- a/cista/app.py +++ b/cista/app.py @@ -56,8 +56,8 @@ configure_main_logging() @app.on_request async def use_session(req): - req.ctx._log_start = time.perf_counter() - req.ctx._auth_flow = ["session: start"] + req.ctx.log_start = time.perf_counter() + req.ctx.auth_flow = ["session: start"] auth.hydrate_request_auth_context(req, source="app.on_request") # CSRF protection if req.method == "GET" and req.headers.upgrade != "websocket": @@ -74,7 +74,7 @@ async def log_access(req, res): """Log HTTP access in a clean single-line format.""" if req.headers.get("upgrade", "").lower() == "websocket": return res - start = getattr(req.ctx, "_log_start", None) + start = getattr(req.ctx, "log_start", None) duration_ms = (time.perf_counter() - start) * 1000 if start is not None else 0.0 client = req.client_ip or "-" host = req.host or "-" @@ -84,7 +84,7 @@ async def log_access(req, res): if isinstance(qs, bytes): qs = qs.decode(errors="replace") path = f"{path}?{qs}" - extra = getattr(req.ctx, "_log_extra", None) + extra = getattr(req.ctx, "log_extra", None) line = format_access_log( client, res.status, req.method, host, path, duration_ms, extra=extra ) @@ -103,7 +103,7 @@ async def forward_sso_cookies(req, res): @app.on_response async def persist_auth_session(req, res): """Persist a session cookie after successful Authorization-based auth.""" - username = getattr(req.ctx, "_create_session_username", None) + username = getattr(req.ctx, "create_session_username", None) if not username or res.status >= 400: return existing = getattr(req.ctx, "session", None) diff --git a/cista/auth.py b/cista/auth.py index f3b427b..427fba1 100644 --- a/cista/auth.py +++ b/cista/auth.py @@ -193,13 +193,13 @@ def _set_auth_failure_log(request, auth_flow: list[str]) -> None: value = request.headers.get(header) if value: parts.append(f"{label}={value}") - request.ctx._log_extra = " | ".join(parts) + request.ctx.log_extra = " | ".join(parts) def hydrate_request_auth_context(request, *, source: str) -> None: - auth_flow = getattr(request.ctx, "_auth_flow", None) + auth_flow = getattr(request.ctx, "auth_flow", None) if auth_flow is None: - auth_flow = request.ctx._auth_flow = [] + auth_flow = request.ctx.auth_flow = [] if hasattr(request.ctx, "session"): # Already hydrated by an earlier caller (e.g., use_session middleware) @@ -832,7 +832,7 @@ async def _ntlm_auth_login(request, *, privileged=False): request.ctx.user = user request.ctx.auth_token_id = tid request.ctx.auth_token = token - request.ctx._create_session_username = token.username + request.ctx.create_session_username = token.username logger.debug( "NTLM auth success for local user %s (token=%s...)", token.username, @@ -881,7 +881,7 @@ async def verify(request, *, privileged=False): scheme = auth_header.split()[0].lower() if has_auth_header else None # Concise auth flow for diagnostics (populated by use_session + verify) - auth_flow = list(getattr(request.ctx, "_auth_flow", ["session:skipped"])) + auth_flow = list(getattr(request.ctx, "auth_flow", ["session:skipped"])) tried: list[str] = [] sso = _get_sso() @@ -954,10 +954,10 @@ async def verify(request, *, privileged=False): user = None else: if user is not None: - if getattr(request.ctx, "_create_session_username", None) is None: + if getattr(request.ctx, "create_session_username", None) is None: username = getattr(request.ctx, "username", None) if username: - request.ctx._create_session_username = username + request.ctx.create_session_username = username return # Auth header present but invalid → try session fallback tried.append("session") @@ -1095,13 +1095,16 @@ async def login_post(request): else: username = request.form["username"][0] password = request.form["password"][0] - if not username or not password: - raise KeyError except KeyError: raise BadRequest( "Missing username or password", context={"redirect": "/login"}, ) from None + if not username or not password: + raise BadRequest( + "Missing username or password", + context={"redirect": "/login"}, + ) try: user = login(username, password) except ValueError as e: @@ -1140,12 +1143,12 @@ async def change_password(request): username = request.form["username"][0] pwchange = request.form["passwordChange"][0] password = request.form["password"][0] - if not username or not password: - raise KeyError except KeyError: raise BadRequest( "Missing username, passwordChange or password", ) from None + if not username or not password: + raise BadRequest("Missing username, passwordChange or password") try: user = login(username, password) set_password(user, pwchange) @@ -1188,10 +1191,10 @@ async def create_user(request): username = request.form["username"][0] password = request.form.get("password", [None])[0] privileged = request.form.get("privileged", ["false"])[0].lower() == "true" - if not username or not username.isidentifier(): - raise ValueError("Invalid username") - except (KeyError, ValueError) as e: - raise BadRequest(str(e)) from e + except KeyError as e: + raise BadRequest("Missing fields") from e + if not username or not username.isidentifier(): + raise BadRequest("Invalid username") if username in config.config.users: raise BadRequest("User already exists") if not password: diff --git a/cista/fileserver.py b/cista/fileserver.py index a65ea57..855ee2c 100644 --- a/cista/fileserver.py +++ b/cista/fileserver.py @@ -76,7 +76,7 @@ async def upload_file_chunk(request, name): size_after = upload_info.get("size_after") if size_before is not None and size_after is not None and size_before != size_after: extras.append("resized") - request.ctx._log_extra = " ".join(extras) if extras else None + request.ctx.log_extra = " ".join(extras) if extras else None real_rel = PurePosixPath(path.relative_to(config.config.path.resolve()).as_posix()) watching.notify_change(real_rel, *real_rel.parents) return json( @@ -197,38 +197,18 @@ async def copy_or_move(request, name=""): def _apply(): for op_name, op_keys in (("cp", cp_keys), ("mv", mv_keys)): - op_multi = len(op_keys) > 1 for key in op_keys: try: src_rel = key_paths[key] src_abs = _resolve_from_relpath(src_rel, request=request) - if op_multi: - if not dst_is_dir: - raise BadRequest( - "Destination must be an existing directory for multiple keys" - ) - dst_item_rel = ( - dst_rel / src_rel.name - if dst_rel.parts - else PurePosixPath(src_rel.name) - ) - elif dst_is_dir: + if dst_is_dir: dst_item_rel = ( dst_rel / src_rel.name if dst_rel.parts else PurePosixPath(src_rel.name) ) else: - if not dst_rel.parts: - raise BadRequest("Destination file path is required") - parent_abs = dst_abs.parent - if not parent_abs.is_dir(): - raise BadRequest("Destination parent folder does not exist") - if src_abs.is_dir() and dst_exists and dst_abs.is_file(): - raise BadRequest( - "Cannot move/copy a directory to an existing file" - ) dst_item_rel = dst_rel dst_item_abs = _resolve_from_relpath(dst_item_rel, request=request) @@ -362,7 +342,7 @@ async def dav_copy(request, name=""): dst_rel, dst_abs = _parse_webdav_destination(dest_header, request=request) if auth.request_share_token(request) is not None and not dst_rel.parts: raise BadRequest("Destination cannot be virtual root") - request.ctx._log_extra = f"→ {dst_rel}" + request.ctx.log_extra = f"→ {dst_rel}" if not src_abs.exists(): raise NotFound(f"Source not found: {name}") if src_abs == dst_abs: @@ -401,7 +381,7 @@ async def dav_move(request, name=""): dst_rel, dst_abs = _parse_webdav_destination(dest_header, request=request) if auth.request_share_token(request) is not None and not dst_rel.parts: raise BadRequest("Destination cannot be virtual root") - request.ctx._log_extra = f"→ {dst_rel}" + request.ctx.log_extra = f"→ {dst_rel}" if not src_abs.exists(): raise NotFound(f"Source not found: {name}") if src_abs == dst_abs: diff --git a/cista/preview.py b/cista/preview.py index f5ca953..a2f71db 100644 --- a/cista/preview.py +++ b/cista/preview.py @@ -615,20 +615,20 @@ async def preview(req, path): logger.warning("Preview worker timeout for %s", filepath) return empty(503) except httpx.HTTPStatusError: - req.ctx._log_extra = "onlyoffice N/A" + req.ctx.log_extra = "onlyoffice N/A" return empty(503) except httpx.RequestError: - req.ctx._log_extra = "onlyoffice N/A" + req.ctx.log_extra = "onlyoffice N/A" return empty(503) except RuntimeError as e: detail = str(e) if detail.startswith("OnlyOffice"): - req.ctx._log_extra = _onlyoffice_error_short_text(detail) + req.ctx.log_extra = _onlyoffice_error_short_text(detail) return empty(503) raise except PreviewError as e: if e.backend: - req.ctx._log_extra = e.backend + req.ctx.log_extra = e.backend detail = str(e) if detail == "preview worker error" and e.stderr: captured = e.stderr.strip() @@ -637,7 +637,7 @@ async def preview(req, path): logger.error("%s preview: %s", filepath, detail) return empty(422) except asyncio.CancelledError: - req.ctx._log_extra = "preview cancelled" + req.ctx.log_extra = "preview cancelled" return empty(503) except Exception: logger.exception("Unhandled preview error for %s", filepath) @@ -647,9 +647,9 @@ async def preview(req, path): timing_detail = "/".join( str(round(value)) for value in preview_resp.timings ) - req.ctx._log_extra = f"{preview_resp.backend} {timing_detail} ➛" + req.ctx.log_extra = f"{preview_resp.backend} {timing_detail} ➛" else: - req.ctx._log_extra = preview_resp.backend + req.ctx.log_extra = preview_resp.backend if not img: # Preview generation failed, redirect to the file itself return redirect(f"/files/{path}", status=303) diff --git a/cista/preview_worker.py b/cista/preview_worker.py index c31dc30..315dd24 100644 --- a/cista/preview_worker.py +++ b/cista/preview_worker.py @@ -414,7 +414,7 @@ def process_video(path, *, maxsize, quality): }, ) if not isinstance(ostream, av.VideoStream): - raise RuntimeError("failed to initialize AV1 video stream") + raise TypeError("failed to initialize AV1 video stream") ostream.width = frame.width ostream.height = frame.height ostream.pix_fmt = frame.format.name diff --git a/cista/serve.py b/cista/serve.py index 79fff6b..79e1439 100644 --- a/cista/serve.py +++ b/cista/serve.py @@ -4,22 +4,11 @@ from pathlib import Path from fastapi_vue.hostutil import parse_endpoint from sanic import Sanic -from sanic.worker.loader import AppLoader from cista import config, server80 from cista.app import app -def load_app() -> Sanic: - """Load the primary app in spawned worker processes. - - Sanic's default multiprocess fallback looks up apps from the in-memory - registry, but that registry starts empty under the `spawn` start method. - Importing this module rebuilds the registry before returning the app. - """ - return app - - def run(*, dev=False): """Run Sanic main process that spawns worker processes to serve HTTP requests.""" _url, opts = parse_listen(config.config.listen) @@ -40,7 +29,7 @@ def run(*, dev=False): access_log=False, ) # type: ignore[call-arg] if dev: - Sanic.serve(app_loader=AppLoader(factory=load_app)) + Sanic.serve() else: Sanic.serve_single() diff --git a/cista/session.py b/cista/session.py index d0fa4b2..445d31d 100644 --- a/cista/session.py +++ b/cista/session.py @@ -36,7 +36,7 @@ def get(request): def create(request, res, username, **kwargs): _purge_expired() token = _token() - _sessions[token] = {"exp": int(time()) + max_age, "username": username, **kwargs} + put(token, username, **kwargs) secure = request.scheme == "https" res.cookies.add_cookie( SESSION_COOKIE_NAME, @@ -49,10 +49,17 @@ def create(request, res, username, **kwargs): def delete(request, res): + token = request.cookies.get(SESSION_COOKIE_NAME) + if token is not None: + _sessions.pop(token, None) secure = request.scheme == "https" res.cookies.delete_cookie(SESSION_COOKIE_NAME, host_prefix=secure) +def put(token: str, username: str, **kwargs) -> None: + _sessions[token] = {"exp": int(time()) + max_age, "username": username, **kwargs} + + def flash(res, message: str | None): if message is None: res.cookies.delete_cookie("message") diff --git a/cista/util/asynclink.py b/cista/util/asynclink.py index 37ca33d..5af2442 100644 --- a/cista/util/asynclink.py +++ b/cista/util/asynclink.py @@ -23,7 +23,7 @@ class AsyncLink: @property def to_sync(self): """Yield SyncRequests from async caller when called from worker thread.""" - while (req := self._await(self._get())) is not None: + while (req := self.await_sync(self._get())) is not None: yield SyncRequest(self, req) async def _get(self): @@ -33,7 +33,7 @@ class AsyncLink: self.queue.task_done() return ret - def _await(self, coro): + def await_sync(self, coro): """Run coroutine in main thread and return result; called from worker.""" return asyncio.run_coroutine_threadsafe(coro, self.loop).result() @@ -87,9 +87,9 @@ class SyncRequest: def set_result(self, value): """Set result value; mark as done.""" self.done = True - self.alink._await(set_result(self.future, value)) + self.alink.await_sync(set_result(self.future, value)) def set_exception(self, exc): """Set exception; mark as done.""" self.done = True - self.alink._await(set_result(self.future, exception=exc)) + self.alink.await_sync(set_result(self.future, exception=exc)) diff --git a/cista/watching.py b/cista/watching.py index ea64164..63f46b0 100644 --- a/cista/watching.py +++ b/cista/watching.py @@ -49,6 +49,10 @@ pubsub = {} sortkey = natsort_keygen(alg=ns.LOCALE) +class FormatUpdateLoopError(RuntimeError): + pass + + class State: def __init__(self): self.lock = threading.RLock() @@ -301,7 +305,7 @@ def format_update(old, new): logger.error( f"format_update potential infinite loop! iteration={iteration_count}, oidx={oidx}, nidx={nidx}" ) - raise Exception( + raise FormatUpdateLoopError( f"format_update infinite loop detected at iteration {iteration_count}" ) @@ -656,16 +660,7 @@ def watcher(loop): while not stop_event.is_set(): if use_inotify: - try: - inotify_tree = inotify.adapters.InotifyTree(rootpath.as_posix()) - except OSError as e: - inotify_tree = None - use_inotify = False - logger.warning( - "Inotify watcher unavailable for %s; falling back to polling: %r", - rootpath, - e, - ) + inotify_tree = inotify.adapters.InotifyTree(rootpath.as_posix()) # Initialize the tree from filesystem update_root(loop) diff --git a/pyproject.toml b/pyproject.toml index 6e4d7e9..85b8541 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -158,11 +158,7 @@ ignore = [ "PLR0915", # legacy complexity; keep other correctness rules enabled "PLR2004", # legacy comparisons use inline constants "PLW0603", # module-level shared state exists in server runtime code - "SLF001", # cohesive modules occasionally need private-member access - "TRY002", # exception-class strictness too noisy on legacy handlers "TRY003", # exception-message strictness too noisy on legacy handlers - "TRY004", # type-check strictness too noisy on legacy handlers - "TRY301", # stylistic raise-in-try preference ] isort.known-first-party = ["cista"] per-file-ignores."tests/*" = ["S", "ANN", "D", "INP", "PLR2004"] diff --git a/tests/test_auth_builtin_flows.py b/tests/test_auth_builtin_flows.py new file mode 100644 index 0000000..b005d97 --- /dev/null +++ b/tests/test_auth_builtin_flows.py @@ -0,0 +1,220 @@ +from http.cookies import SimpleCookie +from pathlib import Path +from uuid import uuid4 + +import pytest +import pytest_asyncio +from sanic import Sanic + +from cista import auth, config +from cista.app import use_session +from cista.auth import bp as auth_bp + + +def _set_cookie_headers(response) -> list[str]: + return list(response.headers.get_list("set-cookie")) + + +def _cookie_header(response, name: str = "cista") -> dict[str, str]: + for header in _set_cookie_headers(response): + cookie = SimpleCookie() + cookie.load(header) + morsel = cookie.get(name) + if morsel is not None and morsel.value: + return {"Cookie": f"{name}={morsel.value}"} + raise AssertionError(f"response did not set cookie {name!r}") + + +@pytest.fixture +def setup_auth_config(tmp_path: Path): + alice = config.User() + auth.set_password(alice, "secret") + admin = config.User(privileged=True) + auth.set_password(admin, "admin-secret") + config.config = config.Config( + path=tmp_path, + listen=":0", + public=False, + users={"alice": alice, "admin": admin}, + ) + return tmp_path + + +@pytest_asyncio.fixture() +async def client(setup_auth_config: Path): + app = Sanic(f"auth-builtins-test-{uuid4().hex}", strict_slashes=True) + + @app.on_request + async def load_auth_context(request): + await use_session(request) + + app.blueprint(auth_bp) + yield app.asgi_client + + +@pytest.mark.asyncio +async def test_restricted_page_renders_login_form_when_logged_out(client): + _, res = await client.get("/auth/restricted/") + + assert res.status_code == 200 + assert "Authentication Required" in res.text + assert "Username:" in res.text + assert "Password:" in res.text + assert "/auth/login" in res.text + + +@pytest.mark.asyncio +async def test_restricted_page_with_invalid_session_clears_cookie(client): + _, res = await client.get( + "/auth/restricted/", + headers={"Cookie": "cista=missing-session"}, + ) + + assert res.status_code == 200 + assert "Authentication Required" in res.text + assert any("cista=" in header.lower() for header in _set_cookie_headers(res)) + + +@pytest.mark.asyncio +async def test_json_login_sets_session_cookie_and_allows_session_authenticated_api_access( + client, +): + _, res = await client.post( + "/auth/login", + json={"username": "alice", "password": "secret"}, + ) + + assert res.status_code == 200 + assert res.json == {"data": {"username": "alice", "privileged": False}} + + session_cookie = _cookie_header(res) + + _, tokens_res = await client.get("/auth/tokens", headers=session_cookie) + assert tokens_res.status_code == 200 + assert tokens_res.json == {"tokens": []} + + _, restricted_res = await client.get("/auth/restricted/", headers=session_cookie) + assert restricted_res.status_code == 200 + assert "auth-success" in restricted_res.text + + +@pytest.mark.asyncio +async def test_json_login_rejects_missing_fields(client): + _, res = await client.post( + "/auth/login", + json={"username": "alice"}, + ) + + assert res.status_code == 400 + assert "Missing username or password" in res.json["message"] + + +@pytest.mark.asyncio +async def test_json_login_rejects_invalid_password(client): + _, res = await client.post( + "/auth/login", + json={"username": "alice", "password": "wrong"}, + ) + + assert res.status_code == 403 + assert "Invalid password" in res.json["message"] + + +@pytest.mark.asyncio +async def test_html_login_redirects_and_sets_flash_and_session_cookies(client): + _, res = await client.post( + "/auth/login", + data={"username": "alice", "password": "secret"}, + headers={"Accept": "text/html"}, + follow_redirects=False, + ) + + assert res.status_code == 302 + assert res.headers["location"] == "/" + headers = _set_cookie_headers(res) + assert any("cista=" in header.lower() for header in headers) + assert any("message=" in header.lower() for header in headers) + + +@pytest.mark.asyncio +async def test_logout_json_revokes_the_existing_session(client): + _, login_res = await client.post( + "/auth/login", + json={"username": "alice", "password": "secret"}, + ) + session_cookie = _cookie_header(login_res) + + _, logout_res = await client.post("/auth/api/logout", headers=session_cookie) + + assert logout_res.status_code == 200 + assert logout_res.json == {"message": "Logged out"} + assert any("cista=" in header.lower() for header in _set_cookie_headers(logout_res)) + + _, retry_res = await client.get("/auth/tokens", headers=session_cookie) + assert retry_res.status_code == 401 + + +@pytest.mark.asyncio +async def test_logout_without_session_reports_not_logged_in(client): + _, res = await client.post("/auth/api/logout") + + assert res.status_code == 200 + assert res.json == {"message": "Not logged in"} + + +@pytest.mark.asyncio +async def test_password_change_updates_credentials_and_reissues_session(client): + _, change_res = await client.post( + "/auth/password-change", + json={ + "username": "alice", + "password": "secret", + "passwordChange": "fresh-secret", + }, + ) + + assert change_res.status_code == 200 + assert change_res.json == {"message": "Password updated"} + + session_cookie = _cookie_header(change_res) + _, tokens_res = await client.get("/auth/tokens", headers=session_cookie) + assert tokens_res.status_code == 200 + + _, old_login_res = await client.post( + "/auth/login", + json={"username": "alice", "password": "secret"}, + ) + assert old_login_res.status_code == 403 + + _, new_login_res = await client.post( + "/auth/login", + json={"username": "alice", "password": "fresh-secret"}, + ) + assert new_login_res.status_code == 200 + assert new_login_res.json == {"data": {"username": "alice", "privileged": False}} + + +@pytest.mark.asyncio +async def test_password_change_rejects_wrong_current_password(client): + _, res = await client.post( + "/auth/password-change", + json={ + "username": "alice", + "password": "wrong", + "passwordChange": "fresh-secret", + }, + ) + + assert res.status_code == 403 + assert "Invalid password" in res.json["message"] + + +@pytest.mark.asyncio +async def test_password_change_rejects_missing_fields(client): + _, res = await client.post( + "/auth/password-change", + json={"username": "alice", "password": "secret"}, + ) + + assert res.status_code == 400 + assert "Missing username, passwordChange or password" in res.json["message"] diff --git a/tests/test_files_auth.py b/tests/test_files_auth.py index b3494a4..b9e723c 100644 --- a/tests/test_files_auth.py +++ b/tests/test_files_auth.py @@ -3,7 +3,6 @@ import hashlib import hmac import struct from pathlib import Path -from time import time from uuid import uuid4 import pytest @@ -93,10 +92,7 @@ def _ntlm_type3( def _session_cookie_header(username: str) -> dict[str, str]: token = "test-" + username - session._sessions[token] = { - "exp": int(time()) + session.max_age, - "username": username, - } + session.put(token, username) return {"Cookie": f"cista={token}"}