refactor: final-review cleanup
- delete dead magic-link helpers (resolve_token, ensure_project_client, mint_link_token, provision_preview_session) + now-unused datetime import - key brute-force lockout on link_token alone (IP term only enabled a source-IP-rotation bypass; behind the proxy all clients share one IP) - drop unused PORTAL_BASE_URL from the retired CLI - add WebSocket ownership tests (unauth + cross-project both close 1008)
This commit is contained in:
@@ -23,8 +23,6 @@ only its hash is stored.
|
|||||||
|
|
||||||
# revoke a link (stops the link AND any live session it minted)
|
# revoke a link (stops the link AND any live session it minted)
|
||||||
python3 backend/portal_admin.py revoke --token-id <TID>
|
python3 backend/portal_admin.py revoke --token-id <TID>
|
||||||
|
|
||||||
The printed URL base comes from PORTAL_BASE_URL (default http://localhost:8001).
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import os
|
import os
|
||||||
@@ -40,8 +38,6 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
|||||||
from backend.database import SessionLocal
|
from backend.database import SessionLocal
|
||||||
from backend.models import Client, ClientAccessToken, Project
|
from backend.models import Client, ClientAccessToken, Project
|
||||||
|
|
||||||
PORTAL_BASE_URL = os.getenv("PORTAL_BASE_URL", "http://localhost:8001").rstrip("/")
|
|
||||||
|
|
||||||
|
|
||||||
def _get_client(db, slug):
|
def _get_client(db, slug):
|
||||||
c = db.query(Client).filter_by(slug=slug).first()
|
c = db.query(Client).filter_by(slug=slug).first()
|
||||||
|
|||||||
+5
-67
@@ -21,7 +21,6 @@ import base64
|
|||||||
import hashlib
|
import hashlib
|
||||||
import logging
|
import logging
|
||||||
import secrets
|
import secrets
|
||||||
from datetime import datetime
|
|
||||||
|
|
||||||
from fastapi import Request, Depends
|
from fastapi import Request, Depends
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
@@ -114,69 +113,6 @@ def get_current_client(request: Request, db: Session = Depends(get_db)) -> Clien
|
|||||||
return client
|
return client
|
||||||
|
|
||||||
|
|
||||||
def resolve_token(raw_token: str, db: Session):
|
|
||||||
"""Validate a raw magic-URL token. Returns (ClientAccessToken, Client) on
|
|
||||||
success, or (None, None). Also stamps last_used_at."""
|
|
||||||
tok = db.query(ClientAccessToken).filter_by(
|
|
||||||
token_hash=hash_token(raw_token), revoked_at=None
|
|
||||||
).first()
|
|
||||||
if not tok:
|
|
||||||
return None, None
|
|
||||||
client = db.query(Client).filter_by(id=tok.client_id, active=True).first()
|
|
||||||
if not client:
|
|
||||||
return None, None
|
|
||||||
tok.last_used_at = datetime.utcnow()
|
|
||||||
db.commit()
|
|
||||||
return tok, client
|
|
||||||
|
|
||||||
|
|
||||||
def ensure_project_client(project, db) -> Client:
|
|
||||||
"""Find or create the Client for a project. Reuses the project's linked client
|
|
||||||
if it has one; otherwise creates/uses a per-project 'preview-<id>' client and
|
|
||||||
sets project.client_id (only when unset, so it never clobbers a real link)."""
|
|
||||||
client = None
|
|
||||||
if project.client_id:
|
|
||||||
client = db.query(Client).filter_by(id=project.client_id, active=True).first()
|
|
||||||
if client is None:
|
|
||||||
slug = f"preview-{project.id}" # full id — an 8-char prefix can collide across projects
|
|
||||||
client = db.query(Client).filter_by(slug=slug).first()
|
|
||||||
if client is None:
|
|
||||||
client = Client(id=str(uuid.uuid4()),
|
|
||||||
name=(project.client_name or project.name or "Preview"),
|
|
||||||
slug=slug, active=True)
|
|
||||||
db.add(client)
|
|
||||||
db.flush()
|
|
||||||
if not project.client_id:
|
|
||||||
project.client_id = client.id
|
|
||||||
return client
|
|
||||||
|
|
||||||
|
|
||||||
def mint_link_token(client, db, label=None) -> str:
|
|
||||||
"""Mint a fresh access token for a client and return the RAW secret (caller
|
|
||||||
builds the /portal/enter/<raw> URL and shows it once). Only the hash is stored."""
|
|
||||||
raw = secrets.token_urlsafe(32)
|
|
||||||
db.add(ClientAccessToken(id=str(uuid.uuid4()), client_id=client.id,
|
|
||||||
token_hash=hash_token(raw), label=label))
|
|
||||||
db.commit()
|
|
||||||
return raw
|
|
||||||
|
|
||||||
|
|
||||||
def provision_preview_session(project, db) -> str:
|
|
||||||
"""Operator preview shortcut: ensure a Client + access token exist for a project
|
|
||||||
and return a token id to seal into a session cookie (no shared link). Reuses an
|
|
||||||
existing token so repeat previews don't accumulate clutter; the raw secret is
|
|
||||||
discarded (preview rides the cookie)."""
|
|
||||||
client = ensure_project_client(project, db)
|
|
||||||
tok = db.query(ClientAccessToken).filter_by(client_id=client.id, revoked_at=None).first()
|
|
||||||
if tok is None:
|
|
||||||
tok = ClientAccessToken(id=str(uuid.uuid4()), client_id=client.id,
|
|
||||||
token_hash=hash_token(secrets.token_urlsafe(32)),
|
|
||||||
label="preview")
|
|
||||||
db.add(tok)
|
|
||||||
db.commit()
|
|
||||||
return tok.id
|
|
||||||
|
|
||||||
|
|
||||||
# --- Phase-1 per-project password gate -------------------------------------------
|
# --- Phase-1 per-project password gate -------------------------------------------
|
||||||
# A portal-enabled project gets its OWN dedicated client (slug "portal-<project.id>")
|
# A portal-enabled project gets its OWN dedicated client (slug "portal-<project.id>")
|
||||||
# owning exactly that project. The project is linked to it via project.client_id so
|
# owning exactly that project. The project is linked to it via project.client_id so
|
||||||
@@ -225,9 +161,11 @@ def resolve_project_by_link_token(link_token: str, db):
|
|||||||
portal_link_token=link_token, portal_enabled=True).first()
|
portal_link_token=link_token, portal_enabled=True).first()
|
||||||
|
|
||||||
|
|
||||||
# In-memory brute-force lockout (per link_token+IP). Resets on restart; adequate for
|
# In-memory brute-force lockout, keyed per link_token (the password is shared per
|
||||||
# a read-only surface behind the UniFi edge. Single-worker dev; note multi-worker
|
# project, so per-IP granularity buys nothing and an IP term only lets an attacker
|
||||||
# would need a shared store.
|
# reset the budget by rotating source IPs). Resets on restart; adequate for a
|
||||||
|
# read-only surface behind the UniFi edge. Single-worker dev; multi-worker would
|
||||||
|
# need a shared store.
|
||||||
MAX_ATTEMPTS = 5
|
MAX_ATTEMPTS = 5
|
||||||
LOCK_SECONDS = 15 * 60
|
LOCK_SECONDS = 15 * 60
|
||||||
_failures: dict = {} # key -> (count, first_failure_epoch)
|
_failures: dict = {} # key -> (count, first_failure_epoch)
|
||||||
|
|||||||
@@ -140,7 +140,10 @@ def portal_password_submit(link_token: str, request: Request,
|
|||||||
"portal/access_required.html", {"request": request, "reason": "invalid"},
|
"portal/access_required.html", {"request": request, "reason": "invalid"},
|
||||||
status_code=404)
|
status_code=404)
|
||||||
|
|
||||||
lock_key = f"{link_token}:{request.client.host if request.client else '?'}"
|
# Shared per-project password → lock per token. (Keying on IP too only enabled a
|
||||||
|
# bypass via source-IP rotation, and behind the reverse proxy every client shares
|
||||||
|
# one IP anyway.)
|
||||||
|
lock_key = link_token
|
||||||
if is_locked(lock_key):
|
if is_locked(lock_key):
|
||||||
return templates.TemplateResponse("portal/password.html", {
|
return templates.TemplateResponse("portal/password.html", {
|
||||||
"request": request, "link_token": link_token, "project_name": project.name,
|
"request": request, "link_token": link_token, "project_name": project.name,
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
|
import pytest
|
||||||
|
from sqlalchemy.orm import sessionmaker
|
||||||
|
from starlette.testclient import WebSocketDisconnect
|
||||||
from tests.conftest import make_project
|
from tests.conftest import make_project
|
||||||
from backend import portal_auth as pa
|
from backend import portal_auth as pa
|
||||||
from backend.auth_passwords import hash_password
|
from backend.auth_passwords import hash_password
|
||||||
@@ -41,3 +44,38 @@ def test_session_can_open_its_own_location(client, db_session):
|
|||||||
assert r.status_code == 303
|
assert r.status_code == 303
|
||||||
r2 = client.get(f"/portal/location/{a_loc.id}")
|
r2 = client.get(f"/portal/location/{a_loc.id}")
|
||||||
assert r2.status_code == 200
|
assert r2.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
def test_ws_stream_rejects_unauthenticated(client, db_session):
|
||||||
|
# The live-feed WebSocket must refuse a connection with no session cookie (1008).
|
||||||
|
a = make_project(db_session, portal_enabled=True, portal_link_token="tw1",
|
||||||
|
portal_password_hash=hash_password("pw"))
|
||||||
|
a_loc = _sound_location(db_session, a)
|
||||||
|
with pytest.raises(WebSocketDisconnect) as exc:
|
||||||
|
with client.websocket_connect(f"/portal/api/location/{a_loc.id}/stream") as ws:
|
||||||
|
ws.receive_text()
|
||||||
|
assert exc.value.code == 1008
|
||||||
|
|
||||||
|
|
||||||
|
def test_ws_stream_rejects_cross_project(client, db_session, monkeypatch):
|
||||||
|
# The WebSocket enforces the SAME per-project ownership as the HTTP routes: a
|
||||||
|
# B-session opening A's stream is closed 1008 (ownership) before any device feed.
|
||||||
|
# The handler uses SessionLocal() directly (not the get_db override), so point it
|
||||||
|
# at the test DB engine so this genuinely exercises the ownership check (not a
|
||||||
|
# vacuous "client not found").
|
||||||
|
import backend.routers.portal as portal_router
|
||||||
|
monkeypatch.setattr(portal_router, "SessionLocal",
|
||||||
|
sessionmaker(bind=db_session.get_bind()))
|
||||||
|
|
||||||
|
a = make_project(db_session, portal_enabled=True, portal_link_token="tw2",
|
||||||
|
portal_password_hash=hash_password("pw"))
|
||||||
|
a_loc = _sound_location(db_session, a)
|
||||||
|
make_project(db_session, portal_enabled=True, portal_link_token="tw3",
|
||||||
|
portal_password_hash=hash_password("pw"))
|
||||||
|
# Log in as project B, then aim the stream at project A's location.
|
||||||
|
assert client.post("/portal/p/tw3", data={"password": "pw"},
|
||||||
|
follow_redirects=False).status_code == 303
|
||||||
|
with pytest.raises(WebSocketDisconnect) as exc:
|
||||||
|
with client.websocket_connect(f"/portal/api/location/{a_loc.id}/stream") as ws:
|
||||||
|
ws.receive_text()
|
||||||
|
assert exc.value.code == 1008
|
||||||
|
|||||||
Reference in New Issue
Block a user