Exception nazi and other suppression removals. Added tests on auth flows that were simplified to linter requirements.
This commit is contained in:
+4
-8
@@ -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
|
||||
|
||||
+5
-5
@@ -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)
|
||||
|
||||
+18
-15
@@ -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:
|
||||
|
||||
+4
-24
@@ -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:
|
||||
|
||||
+7
-7
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
+1
-12
@@ -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()
|
||||
|
||||
|
||||
+8
-1
@@ -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")
|
||||
|
||||
@@ -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))
|
||||
|
||||
+6
-11
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user