feat(auth): share links open only their own widgets; unmarked routes are closed (#5033)
#5033 "Share links open only their own dashboard; every route without a role is on an explicit allow-list". Any live share token in ?s= made the caller a viewer on every viewer route — host pages, container logs, any dashboard — and the host-agent widget routes had no role check at all. - steward/dashboard/share.py: a share token opens, read-only, exactly the widget fragment URLs its own dashboard loads — same path, same configured parameters; only the time window (hours/range) may change. - require_role marks its view; public_route marks the deliberately open ones (login, setup, health, the share page, agent ingest, install.sh, agent.py) with the reason. A before_request refuses any view with neither, so a route that forgets its decorator is closed, not open. - /plugins/host_agent/widget and /widget/history now require viewer. - The history widget's range buttons keep the rest of the query (and so a share link's token) instead of rebuilding it; the uptime widget's "Full SLA" link shows to logged-in users, not to share viewers. - tests/test_route_access.py: a source scan that fails on any unmarked route in core or bundled plugins, the default-deny refusal, and the share matcher's cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,7 @@ from pathlib import Path
|
||||
|
||||
import secrets
|
||||
from quart import Blueprint, current_app, jsonify, request, Response, render_template, redirect, url_for, session
|
||||
from steward.auth.middleware import require_role
|
||||
from steward.auth.middleware import public_route, require_role
|
||||
from steward.models.users import UserRole
|
||||
from sqlalchemy import select, func, or_, and_
|
||||
from datetime import timedelta
|
||||
@@ -166,6 +166,7 @@ def _error(status: int, code: str, detail: str | None = None) -> tuple[Any, int]
|
||||
|
||||
|
||||
@host_agent_bp.post("/ingest")
|
||||
@public_route("the agent authenticates with its bearer token")
|
||||
async def ingest():
|
||||
auth = request.headers.get("Authorization", "")
|
||||
if not auth.startswith("Bearer "):
|
||||
@@ -313,6 +314,7 @@ def _agent_version() -> str:
|
||||
|
||||
|
||||
@host_agent_bp.get("/install.sh")
|
||||
@public_route("fetched by curl on the host being enrolled; carries an enrolment token")
|
||||
async def install_script():
|
||||
token = request.args.get("token", "")
|
||||
if not token:
|
||||
@@ -344,6 +346,7 @@ async def install_script():
|
||||
|
||||
|
||||
@host_agent_bp.get("/agent.py")
|
||||
@public_route("fetched by the installer and by agent self-update; holds no secrets")
|
||||
async def agent_source():
|
||||
try:
|
||||
body = AGENT_SOURCE_PATH.read_text(encoding="utf-8")
|
||||
@@ -470,6 +473,7 @@ async def _fleet_rows(session) -> list[dict]:
|
||||
|
||||
|
||||
@host_agent_bp.get("/widget")
|
||||
@require_role(UserRole.viewer)
|
||||
async def widget_table():
|
||||
"""Fleet-glance dashboard widget: one row per monitored host, latest metrics."""
|
||||
async with current_app.db_sessionmaker() as session:
|
||||
@@ -478,6 +482,7 @@ async def widget_table():
|
||||
|
||||
|
||||
@host_agent_bp.get("/widget/history")
|
||||
@require_role(UserRole.viewer)
|
||||
async def widget_history():
|
||||
host_id = request.args.get("host_id", "")
|
||||
try:
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
{% endif %}
|
||||
<span class="hist-range" style="margin-left:auto;display:inline-flex;border:1px solid var(--border-mid);border-radius:5px;overflow:hidden;">
|
||||
{% for opt in [1, 6, 24] %}
|
||||
<button type="button" onclick="histRange({{ wid }}, '{{ host.id }}', {{ opt }})"
|
||||
<button type="button" onclick="histRange({{ wid }}, {{ opt }})"
|
||||
style="border:0;cursor:pointer;padding:0.1rem 0.45rem;font-size:0.72rem;
|
||||
background:{% if opt == hours %}var(--accent){% else %}transparent{% endif %};
|
||||
color:{% if opt == hours %}#fff{% else %}var(--text-muted){% endif %};">{{ opt }}h</button>
|
||||
@@ -36,11 +36,15 @@
|
||||
<script>
|
||||
// Live time-range switch. Rewrites the parent cell's hx-get so the choice sticks
|
||||
// across polls (htmx re-reads the attribute each poll), then fetches immediately.
|
||||
window.histRange = function (wid, hostId, hours) {
|
||||
// Only `hours` changes: the rest of the query (host, widget id, a share link's
|
||||
// token) is kept, so the switch also works on a shared dashboard.
|
||||
window.histRange = function (wid, hours) {
|
||||
var cell = document.getElementById("widget-" + wid);
|
||||
if (!cell) return;
|
||||
var base = (cell.getAttribute("hx-get") || "").split("?")[0];
|
||||
var url = base + "?host_id=" + encodeURIComponent(hostId) + "&hours=" + hours + "&wid=" + wid;
|
||||
var parts = (cell.getAttribute("hx-get") || "").split("?");
|
||||
var query = new URLSearchParams(parts[1] || "");
|
||||
query.set("hours", hours);
|
||||
var url = parts[0] + "?" + query.toString();
|
||||
cell.setAttribute("hx-get", url);
|
||||
if (window.htmx) window.htmx.ajax("GET", url, { target: cell, swap: "innerHTML" });
|
||||
};
|
||||
|
||||
+11
-17
@@ -137,6 +137,9 @@ def create_app(
|
||||
app.jinja_env.globals["csrf_token"] = _csrf.csrf_token
|
||||
app.jinja_env.globals["csrf_field"] = _csrf.csrf_field
|
||||
app.before_request(_csrf.check_request)
|
||||
# Default-deny: a view with neither require_role nor public_route is refused.
|
||||
from .auth.middleware import check_view_is_marked, public_route
|
||||
app.before_request(check_view_is_marked)
|
||||
|
||||
# ── 5. Plugin migrations ───────────────────────────────────────────────────
|
||||
# Step 3 already applied all plugin migrations via discover_all_plugin_migration_dirs,
|
||||
@@ -233,33 +236,24 @@ def create_app(
|
||||
# ── 11. Share-token middleware ─────────────────────────────────────────────
|
||||
@app.before_request
|
||||
async def _validate_share_token():
|
||||
"""If ?s=<token> is present, validate it and set g.share_viewer."""
|
||||
"""A share link's token (?s=) opens, read-only, exactly the widget
|
||||
fragments its own dashboard loads (steward/dashboard/share.py)."""
|
||||
from quart import request, g
|
||||
raw = request.args.get("s")
|
||||
if not raw:
|
||||
if not raw or request.method != "GET":
|
||||
return
|
||||
try:
|
||||
import hashlib
|
||||
from datetime import datetime, timezone
|
||||
from sqlalchemy import select
|
||||
from .models.dashboard import DashboardShareToken
|
||||
token_hash = hashlib.sha256(raw.encode()).hexdigest()
|
||||
from .dashboard.share import share_grants
|
||||
async with app.db_sessionmaker() as db:
|
||||
result = await db.execute(
|
||||
select(DashboardShareToken)
|
||||
.where(DashboardShareToken.token_hash == token_hash)
|
||||
)
|
||||
share = result.scalar_one_or_none()
|
||||
if share:
|
||||
now = datetime.now(timezone.utc)
|
||||
if share.expires_at is None or share.expires_at > now:
|
||||
if await share_grants(db, raw, request.path, request.args):
|
||||
g.share_viewer = True
|
||||
g.share_dashboard_id = share.dashboard_id
|
||||
except Exception:
|
||||
pass # never block a request due to token validation failure
|
||||
# A failed check grants nothing; the request falls through to login.
|
||||
logging.getLogger(__name__).exception("Share token check failed")
|
||||
|
||||
# ── 11. Health check ───────────────────────────────────────────────────────
|
||||
@app.get("/health")
|
||||
@public_route("liveness probe for the container")
|
||||
async def health():
|
||||
return {"status": "ok"}
|
||||
|
||||
|
||||
@@ -77,10 +77,37 @@ def require_role(minimum_role: UserRole):
|
||||
if _ROLE_ORDER.index(user_role) < _ROLE_ORDER.index(minimum_role):
|
||||
abort(403)
|
||||
return await f(*args, **kwargs)
|
||||
wrapper.steward_role = minimum_role
|
||||
return wrapper
|
||||
return decorator
|
||||
|
||||
|
||||
def public_route(reason: str):
|
||||
"""Mark a view that is deliberately reachable without a role — login,
|
||||
the health probe, a bearer-authenticated endpoint. Every other view must
|
||||
carry require_role; check_view_is_marked refuses one that has neither, so
|
||||
a route that forgets its decorator is closed rather than open."""
|
||||
def decorator(f):
|
||||
f.steward_public = reason
|
||||
return f
|
||||
return decorator
|
||||
|
||||
|
||||
async def check_view_is_marked() -> Response | None:
|
||||
"""before_request hook: default-deny for views with neither require_role
|
||||
nor public_route. Static files and unmatched paths (404) pass through."""
|
||||
from quart import current_app
|
||||
endpoint = request.endpoint
|
||||
if endpoint is None or endpoint == "static" or endpoint.endswith(".static"):
|
||||
return None
|
||||
view = current_app.view_functions.get(endpoint)
|
||||
if view is None or hasattr(view, "steward_role") or hasattr(view, "steward_public"):
|
||||
return None
|
||||
current_app.logger.error("Refused %s: view %r has no require_role or public_route",
|
||||
request.path, endpoint)
|
||||
abort(403)
|
||||
|
||||
|
||||
def login_user(user) -> None:
|
||||
"""Write user identity into the session, with a fresh CSRF token."""
|
||||
from steward.auth.csrf import rotate
|
||||
|
||||
@@ -2,7 +2,7 @@ from __future__ import annotations
|
||||
import bcrypt
|
||||
from quart import Blueprint, render_template, request, redirect, url_for, session
|
||||
from sqlalchemy import select, func
|
||||
from steward.auth.middleware import login_user, logout_user, safe_next_url
|
||||
from steward.auth.middleware import login_user, logout_user, public_route, safe_next_url
|
||||
from steward.models.users import User, UserRole
|
||||
|
||||
auth_bp = Blueprint("auth", __name__)
|
||||
@@ -48,6 +48,7 @@ async def get_user_by_username(app, username: str) -> User | None:
|
||||
|
||||
|
||||
@auth_bp.get("/login")
|
||||
@public_route("the login page")
|
||||
async def login():
|
||||
from quart import current_app
|
||||
if await get_user_count(current_app) == 0:
|
||||
@@ -59,6 +60,7 @@ async def login():
|
||||
|
||||
|
||||
@auth_bp.post("/login")
|
||||
@public_route("the login form")
|
||||
async def login_post():
|
||||
from quart import current_app
|
||||
from steward.core.audit import log_audit
|
||||
@@ -100,6 +102,7 @@ async def login_post():
|
||||
|
||||
|
||||
@auth_bp.get("/login/oidc")
|
||||
@public_route("starts SSO login")
|
||||
async def login_oidc():
|
||||
from quart import current_app
|
||||
from steward.auth.oidc import get_discovery, build_authorize_url, generate_state
|
||||
@@ -125,6 +128,7 @@ async def login_oidc():
|
||||
|
||||
|
||||
@auth_bp.get("/login/oidc/callback")
|
||||
@public_route("SSO login callback")
|
||||
async def login_oidc_callback():
|
||||
from quart import current_app
|
||||
from steward.auth.oidc import get_discovery, exchange_code, get_userinfo, map_role
|
||||
@@ -179,12 +183,14 @@ async def login_oidc_callback():
|
||||
|
||||
|
||||
@auth_bp.get("/logout")
|
||||
@public_route("ends the session")
|
||||
async def logout():
|
||||
logout_user()
|
||||
return redirect(url_for("auth.login"))
|
||||
|
||||
|
||||
@auth_bp.get("/setup")
|
||||
@public_route("first-run setup (refuses once a user exists)")
|
||||
async def setup():
|
||||
from quart import current_app
|
||||
if await get_user_count(current_app) > 0:
|
||||
@@ -196,6 +202,7 @@ async def setup():
|
||||
|
||||
|
||||
@auth_bp.post("/setup")
|
||||
@public_route("first-run setup (refuses once a user exists)")
|
||||
async def setup_post():
|
||||
from quart import current_app
|
||||
from steward.core.audit import log_audit
|
||||
|
||||
@@ -6,7 +6,7 @@ from datetime import datetime, timezone, timedelta
|
||||
from urllib.parse import urlencode
|
||||
from quart import Blueprint, render_template, current_app, abort, redirect, request, session
|
||||
from sqlalchemy import select, func, update
|
||||
from steward.auth.middleware import require_role, current_user_id
|
||||
from steward.auth.middleware import public_route, require_role, current_user_id
|
||||
from steward.models.users import UserRole
|
||||
from steward.models.hosts import Host
|
||||
from steward.models.dashboard import Dashboard, DashboardWidget, DashboardShareToken
|
||||
@@ -513,6 +513,7 @@ async def revoke_share_token(dash_id: int, token_id: int):
|
||||
# ── Public share view (no login required) ────────────────────────────────────
|
||||
|
||||
@dashboard_bp.get("/share/<raw_token>")
|
||||
@public_route("a share link; the token is the credential")
|
||||
async def public_share(raw_token: str):
|
||||
token_hash = hashlib.sha256(raw_token.encode()).hexdigest()
|
||||
now = datetime.now(timezone.utc)
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
"""What a dashboard share link opens.
|
||||
|
||||
A share link's page (/share/<token>) loads each of its dashboard's widgets
|
||||
from a fragment URL with `&s=<token>` appended. The token opens exactly those
|
||||
URLs, read-only: the same path and the same configured parameters (which host,
|
||||
which container). Only the time window a viewer can switch on the widget
|
||||
itself (VIEW_PARAMS) may differ. Every other page, and the same widget aimed
|
||||
at a different host, still needs a login.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
from datetime import datetime, timezone
|
||||
from urllib.parse import parse_qsl
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from steward.models.dashboard import DashboardShareToken
|
||||
|
||||
# Parameters a shared widget may change in place: they pick a time window,
|
||||
# never which thing is shown.
|
||||
VIEW_PARAMS = frozenset({"hours", "range"})
|
||||
|
||||
|
||||
def share_allows(widget_urls: list[str], path: str, args) -> bool:
|
||||
"""True when `path` + `args` (minus the token) is one of `widget_urls`,
|
||||
allowing only VIEW_PARAMS to differ."""
|
||||
got: dict[str, str] = {}
|
||||
for key in args:
|
||||
if key == "s":
|
||||
continue
|
||||
values = args.getlist(key)
|
||||
if len(values) != 1:
|
||||
return False
|
||||
got[key] = values[0]
|
||||
for url in widget_urls:
|
||||
wpath, _, query = url.partition("?")
|
||||
if wpath != path:
|
||||
continue
|
||||
want = {k: v for k, v in parse_qsl(query, keep_blank_values=True) if k not in VIEW_PARAMS}
|
||||
if all(got.get(k) == v for k, v in want.items()) and set(got) <= set(want) | VIEW_PARAMS:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
async def share_grants(db, raw_token: str, path: str, args) -> bool:
|
||||
"""True when `raw_token` is a live share token whose dashboard loads this URL."""
|
||||
token_hash = hashlib.sha256(raw_token.encode()).hexdigest()
|
||||
share = (await db.execute(
|
||||
select(DashboardShareToken).where(DashboardShareToken.token_hash == token_hash)
|
||||
)).scalar_one_or_none()
|
||||
if share is None:
|
||||
return False
|
||||
if share.expires_at is not None and share.expires_at <= datetime.now(timezone.utc):
|
||||
return False
|
||||
from .routes import _get_widgets, _resolve_widgets
|
||||
widgets = _resolve_widgets(await _get_widgets(db, share.dashboard_id))
|
||||
return share_allows([w["hx_url"] for w in widgets], path, args)
|
||||
@@ -288,7 +288,6 @@ async def uptime_page():
|
||||
@hosts_bp.get("/uptime/widget")
|
||||
@require_role(UserRole.viewer)
|
||||
async def uptime_widget():
|
||||
share_token = request.args.get("s")
|
||||
async with current_app.db_sessionmaker() as db:
|
||||
result = await db.execute(select(Host).order_by(Host.name))
|
||||
hosts = result.scalars().all()
|
||||
@@ -297,5 +296,4 @@ async def uptime_widget():
|
||||
"hosts/uptime_widget.html",
|
||||
hosts=hosts,
|
||||
uptime=uptime,
|
||||
share_token=share_token,
|
||||
)
|
||||
|
||||
@@ -38,7 +38,7 @@
|
||||
{% endfor %}
|
||||
</div>
|
||||
|
||||
{% if share_token %}
|
||||
{% if session.user_id %}
|
||||
<div style="margin-top:0.5rem;text-align:right;">
|
||||
<a href="/hosts/uptime" style="font-size:0.75rem;color:var(--text-muted);">Full SLA →</a>
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
"""Who can reach a route: every view carries require_role or public_route,
|
||||
an unmarked one is refused, and a share link opens only its own widgets."""
|
||||
import ast
|
||||
import pathlib
|
||||
|
||||
import pytest
|
||||
from werkzeug.datastructures import MultiDict
|
||||
|
||||
import steward
|
||||
from steward.dashboard.share import share_allows
|
||||
|
||||
_REPO = pathlib.Path(steward.__file__).parent.parent
|
||||
_ROUTE_METHODS = ("get", "post", "put", "patch", "delete", "route", "websocket")
|
||||
|
||||
|
||||
def _route_decorated_views():
|
||||
files = [p for root in ("steward", "plugins") for p in (_REPO / root).rglob("*.py")
|
||||
if "migrations" not in p.parts]
|
||||
for path in files:
|
||||
tree = ast.parse(path.read_text())
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
|
||||
continue
|
||||
decorators = [ast.unparse(d) for d in node.decorator_list]
|
||||
if any(f".{m}('/" in d or f'.{m}("/' in d for d in decorators for m in _ROUTE_METHODS):
|
||||
yield path, node, decorators
|
||||
|
||||
|
||||
def test_every_route_says_who_may_reach_it():
|
||||
unmarked = [f"{p.relative_to(_REPO)}:{n.lineno} {n.name}"
|
||||
for p, n, decs in _route_decorated_views()
|
||||
if not any(d.startswith(("require_role(", "public_route(")) for d in decs)]
|
||||
assert not unmarked, "routes with neither require_role nor public_route: " + ", ".join(unmarked)
|
||||
|
||||
|
||||
def test_the_scan_sees_routes():
|
||||
names = {n.name for _, n, _ in _route_decorated_views()}
|
||||
assert {"login", "health", "ingest", "widget_table", "public_share"} <= names
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unmarked_view_is_refused(app, client):
|
||||
async def unmarked():
|
||||
return "open"
|
||||
app.add_url_rule("/__unmarked", "unmarked", unmarked)
|
||||
assert (await client.get("/__unmarked")).status_code == 403
|
||||
assert (await client.get("/health")).status_code == 200
|
||||
|
||||
|
||||
WIDGETS = ["/plugins/host_agent/widget/history?host_id=h1&hours=6&wid=7",
|
||||
"/monitors/widget?wid=8"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path,args,allowed", [
|
||||
("/plugins/host_agent/widget/history", {"host_id": "h1", "hours": "6", "wid": "7", "s": "t"}, True),
|
||||
("/plugins/host_agent/widget/history", {"host_id": "h1", "hours": "24", "wid": "7", "s": "t"}, True),
|
||||
("/monitors/widget", {"wid": "8", "s": "t"}, True),
|
||||
# the same widget aimed at another host
|
||||
("/plugins/host_agent/widget/history", {"host_id": "h2", "hours": "6", "wid": "7", "s": "t"}, False),
|
||||
# a parameter the widget was not configured with
|
||||
("/monitors/widget", {"wid": "8", "host_id": "h1", "s": "t"}, False),
|
||||
# a configured parameter left off
|
||||
("/plugins/host_agent/widget/history", {"hours": "6", "wid": "7", "s": "t"}, False),
|
||||
# any other page
|
||||
("/hosts/", {"s": "t"}, False),
|
||||
("/plugins/docker/logs", {"wid": "8", "s": "t"}, False),
|
||||
])
|
||||
def test_share_opens_only_its_widgets(path, args, allowed):
|
||||
assert share_allows(WIDGETS, path, MultiDict(args)) is allowed
|
||||
|
||||
|
||||
def test_share_refuses_a_repeated_parameter():
|
||||
args = MultiDict([("wid", "8"), ("wid", "9"), ("s", "t")])
|
||||
assert not share_allows(WIDGETS, "/monitors/widget", args)
|
||||
Reference in New Issue
Block a user