Count the visits, and say whether anyone got anywhere
A hosted demo raises a question a local app never does: is anyone using it, and do they reach the part that matters? `/analytics` answers it — visitors, pages, referrers, countries, devices, which shared scenarios get played, turns and demo-key spend, API and turn errors, and a funnel from visited to played a turn to signed up. Not a third-party script, for reasons specific to this one. The CSP allows `script-src 'self'`, so a tracker means loosening it; adblockers eat the popular ones, which silently biases exactly the technical audience this project gets shown to; and none of them can see the measurement that actually matters here, which is a turn, not a pageview. **A visit is a write and never a read.** After the 189x egress fix it would be perverse to add a feature that reads rows per request, so counts accumulate in a process-local dict and flush every 60s as UPSERTs. Storage is a generic `(day, metric, label) -> hits` counter, so measuring something new later costs a constant rather than a migration, plus one row per visitor per day for the funnel flags. Every dashboard query is a GROUP BY returning tens of rows however much traffic sits behind it; a month reads back in a few kilobytes. The buffer's cost is that a hard restart can lose up to a minute — the flusher also runs on shutdown, and a tier that sleeps when idle sleeps on an empty buffer anyway. **The counters are anonymous; the access log beside them is not, on purpose.** A visitor is `HMAC(secret, "visitor:<user id>")` truncated to 32 chars — one-way, so `analytics_daily` and `analytics_visitor_days` cannot be joined back to `users`, and keyed, so no client can compute one. Story content never reaches that module, and the only content it ever names is a seeded public scenario's title; a player's own titles are theirs. `accesslog.py` is the identifying half and is a separate module writing a separate table so that separation is a property of the code rather than a convention: `access_events` records sessions, sign-ins, registrations and failed attempts with address, email and device, read on a second tab of the same page behind the same gate. Both halves are gated on `AIDND_ANALYTICS_EMAILS`, not `POWER_USERS`. An unmetered tester is not automatically someone who should see the traffic. The route 404s and the nav link is absent for everyone else, the same treatment AI Chat gets; unset in a hosted deploy means nobody sees it, including me. Three things came out of building it that a test would not have suggested. **A failed turn is an HTTP 200 with a bad ending.** The status-code middleware cannot see one, so a demo whose model had started refusing every request would look perfectly healthy from outside. All five SSE error paths in `_generate_turn` now go through a `turn_error()` helper that counts on the way out. Error buckets elsewhere are labelled by the matched route template rather than the requested path — one bucket per endpoint instead of one per adventure id, and, the reason it isn't merely tidier, an unmatched path is entirely attacker-chosen, so labelling by it would let anyone mint rows. **The funnel counts people, not clicks.** A player who starts six adventures is one person who started an adventure. That is the whole reason the per-visitor-day table exists; its flags only ever turn on, and `is_new` is settled by the first write of a visitor's first day. **The tests run on SQLite and production is Neon.** A flush that raises is caught and logged, so a dialect mistake in the UPSERTs would have stayed invisible until the dashboard quietly never filled. `test_the_upserts_compile_for_postgres` compiles both statements against the Postgres dialect without connecting to one. Two things this leans on elsewhere. `limits._client_ip` is now public `client_ip`: the access log needs the same answer, and two functions both deciding which hop is the caller's is how one of them ends up trusting a header it shouldn't. And the cleanup sweeper now starts if *either* job has work — a deployment can keep every guest forever and still want its visitor-day rows aged out. No migration. Both tables are new and `bootstrap()` calls `create_all` on existing databases too, the route `branches` took in Phase 14, so `LATEST_VERSION` is still 64. 497 tests green, frontend lint and build clean, driven by hand against a synthetic 90-day fixture at 1568px. The narrow-screen layout follows the existing 720px block but is unverified: `resize_window` is ignored on a maximized Chrome and `frame-ancestors 'none'` rules out checking it in a sized iframe. Also repaired here: a rename in test_ratelimit_hardening.py had run through the test names themselves, leaving `testclient_ip_*` — still collected by pytest, which is why it passed unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfMCsN1KBLsTqMkj5hSgrY
This commit is contained in:
parththakkar106
co-authored by
Claude Opus 5
Claude Opus 5
parent
3b9e6b3d50
commit
041f9e25f3
@@ -0,0 +1,215 @@
|
||||
"""The access log — app/accesslog.py and GET /api/analytics/access.
|
||||
|
||||
This is the half of the analytics work that identifies people on purpose, so
|
||||
the things worth pinning are the ones that would quietly make it wrong: that
|
||||
the address recorded is the hardened one and not a header a client chose, that
|
||||
session rows are thinned instead of written per page load, and that a row
|
||||
outlives the account it describes — guest cleanup runs on a schedule, and a log
|
||||
that deletes itself is not a log.
|
||||
|
||||
python -m pytest tests/test_accesslog.py -v
|
||||
"""
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
_tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
|
||||
_tmp.close()
|
||||
os.environ["AIDND_DB_PATH"] = _tmp.name
|
||||
os.environ.pop("AIDND_DATABASE_URL", None)
|
||||
os.environ.pop("DATABASE_URL", None)
|
||||
|
||||
import pytest
|
||||
from fastapi import Depends
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app import accesslog, auth, limits, models, security
|
||||
from app.database import Base, SessionLocal, engine, get_db
|
||||
from app.main import app
|
||||
|
||||
EDGE = "198.51.100.77" # what the trusted proxy appended
|
||||
SPOOF = "10.0.0.1" # what a client put in front of it
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_state():
|
||||
accesslog._last_session.clear()
|
||||
yield
|
||||
accesslog._last_session.clear()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client(monkeypatch):
|
||||
Base.metadata.create_all(bind=engine)
|
||||
setup = SessionLocal()
|
||||
owner = models.User(is_guest=False, email="owner@example.com")
|
||||
member = models.User(
|
||||
is_guest=False, email="player@example.com",
|
||||
password_hash=security.hash_password("hunter2long"),
|
||||
)
|
||||
setup.add_all([owner, member])
|
||||
setup.commit()
|
||||
ids = {"owner": owner.id, "member": member.id}
|
||||
setup.close()
|
||||
|
||||
monkeypatch.setattr(limits, "rate_limit", lambda *a, **k: None)
|
||||
monkeypatch.setattr(limits, "check_login_allowed", lambda *a, **k: None)
|
||||
monkeypatch.setattr(auth, "MULTI_USER", True)
|
||||
monkeypatch.setattr(auth, "ANALYTICS_EMAILS", {"owner@example.com"})
|
||||
|
||||
# /auth/me resolves its own session, so the cookie flow below is the real
|
||||
# one; every other endpoint goes through get_current_user, and `act_as`
|
||||
# decides who that is.
|
||||
acting = {"id": ids["owner"]}
|
||||
|
||||
def _current(db=Depends(get_db)):
|
||||
return db.get(models.User, acting["id"])
|
||||
|
||||
app.dependency_overrides[auth.get_current_user] = _current
|
||||
try:
|
||||
test_client = TestClient(app)
|
||||
test_client.ids = ids
|
||||
test_client.act_as = lambda user_id: acting.update(id=user_id)
|
||||
yield test_client
|
||||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
Base.metadata.drop_all(bind=engine)
|
||||
|
||||
|
||||
def visit(client, ip=EDGE, ua="Mozilla/5.0 (Windows NT 10.0; Win64; x64)"):
|
||||
return client.get(
|
||||
"/api/auth/me",
|
||||
headers={"x-forwarded-for": f"{SPOOF}, {ip}", "user-agent": ua},
|
||||
)
|
||||
|
||||
|
||||
def rows(kind=None):
|
||||
db = SessionLocal()
|
||||
try:
|
||||
query = db.query(models.AccessEvent).order_by(models.AccessEvent.id)
|
||||
if kind:
|
||||
query = query.filter_by(kind=kind)
|
||||
return query.all()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def read_log(client, **params):
|
||||
return client.get("/api/analytics/access", params=params)
|
||||
|
||||
|
||||
# ---------- Writing ----------
|
||||
|
||||
def test_a_new_session_is_logged(client):
|
||||
visit(client)
|
||||
logged = rows()
|
||||
assert len(logged) == 1
|
||||
entry = logged[0]
|
||||
assert entry.kind == accesslog.SESSION
|
||||
assert entry.is_guest and entry.who.startswith("Guest #")
|
||||
assert entry.device == "desktop"
|
||||
|
||||
|
||||
def test_the_address_is_the_hardened_one_not_the_clients(client):
|
||||
visit(client)
|
||||
# The client prepended its own value; only the hop the edge appended counts.
|
||||
# Recording the leftmost would make every row forgeable, which for a log is
|
||||
# worse than having no log.
|
||||
assert rows()[0].ip == EDGE
|
||||
|
||||
|
||||
def test_session_rows_are_thinned_to_one_per_day_per_address(client):
|
||||
for _ in range(4):
|
||||
visit(client)
|
||||
assert len(rows(accesslog.SESSION)) == 1
|
||||
|
||||
|
||||
def test_a_changed_address_writes_a_new_row(client):
|
||||
visit(client)
|
||||
visit(client, ip="203.0.113.9")
|
||||
logged = rows(accesslog.SESSION)
|
||||
assert [entry.ip for entry in logged] == [EDGE, "203.0.113.9"]
|
||||
# Same session throughout, so both rows name the same visitor.
|
||||
assert logged[0].who == logged[1].who
|
||||
|
||||
|
||||
def test_sign_in_and_failure_are_both_logged(client):
|
||||
client.post("/api/auth/login", json={"email": "player@example.com", "password": "wrong"},
|
||||
headers={"x-forwarded-for": EDGE})
|
||||
client.post("/api/auth/login", json={"email": "player@example.com", "password": "hunter2long"},
|
||||
headers={"x-forwarded-for": EDGE})
|
||||
kinds = [entry.kind for entry in rows()]
|
||||
assert accesslog.LOGIN_FAILED in kinds and accesslog.LOGIN in kinds
|
||||
|
||||
failure = rows(accesslog.LOGIN_FAILED)[0]
|
||||
# The address tried, not the account that owns it: a run against an address
|
||||
# with no account behind it is exactly what this row is for.
|
||||
assert failure.who == "player@example.com"
|
||||
assert failure.user_id is None
|
||||
assert rows(accesslog.LOGIN)[0].user_id == client.ids["member"]
|
||||
|
||||
|
||||
def test_registering_is_logged_against_the_upgraded_account(client):
|
||||
visit(client) # mints the guest whose session registers
|
||||
client.act_as(rows()[0].user_id)
|
||||
client.post("/api/auth/register", json={"email": "new@example.com", "password": "hunter2long"})
|
||||
entry = rows(accesslog.REGISTER)[0]
|
||||
assert entry.who == "new@example.com" and not entry.is_guest
|
||||
|
||||
|
||||
def test_a_row_outlives_the_account_it_describes(client):
|
||||
visit(client)
|
||||
entry = rows()[0]
|
||||
db = SessionLocal()
|
||||
try:
|
||||
db.delete(db.get(models.User, entry.user_id))
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
# No foreign key, and `who` is a snapshot — guest cleanup deletes accounts
|
||||
# on a schedule, and a log that vanishes with them is not a log.
|
||||
survivor = rows()[0]
|
||||
assert survivor.who == entry.who and survivor.ip == EDGE
|
||||
|
||||
|
||||
def test_a_long_user_agent_is_truncated(client):
|
||||
visit(client, ua="Mozilla/" + "x" * 500)
|
||||
assert len(rows()[0].user_agent) == accesslog.MAX_UA
|
||||
|
||||
|
||||
def test_a_logging_failure_does_not_break_the_request(client, monkeypatch):
|
||||
monkeypatch.setattr(accesslog, "_client_ip", lambda request: 1 / 0)
|
||||
# The log watches sign-in; it must not be able to stand in its way.
|
||||
assert visit(client).status_code == 200
|
||||
|
||||
|
||||
# ---------- Reading ----------
|
||||
|
||||
def test_the_log_is_invisible_to_everyone_but_the_owner(client):
|
||||
visit(client)
|
||||
assert read_log(client).status_code == 200
|
||||
client.act_as(client.ids["member"])
|
||||
assert read_log(client).status_code == 404
|
||||
|
||||
|
||||
def test_the_log_reads_newest_first_and_pages_backwards(client):
|
||||
for index in range(5):
|
||||
visit(client, ip=f"203.0.113.{index}")
|
||||
first = read_log(client, limit=2).json()
|
||||
assert [event["ip"] for event in first["events"]] == ["203.0.113.4", "203.0.113.3"]
|
||||
assert first["has_more"]
|
||||
|
||||
older = read_log(client, limit=2, before_id=first["events"][-1]["id"]).json()
|
||||
assert [event["ip"] for event in older["events"]] == ["203.0.113.2", "203.0.113.1"]
|
||||
|
||||
|
||||
def test_the_log_filters_by_kind_and_searches(client):
|
||||
visit(client)
|
||||
client.post("/api/auth/login", json={"email": "player@example.com", "password": "hunter2long"},
|
||||
headers={"x-forwarded-for": "203.0.113.44"})
|
||||
|
||||
assert len(read_log(client, kind="login").json()["events"]) == 1
|
||||
by_email = read_log(client, q="player@example.com").json()["events"]
|
||||
assert len(by_email) == 1 and by_email[0]["kind"] == "login"
|
||||
by_ip = read_log(client, q="203.0.113.44").json()["events"]
|
||||
assert len(by_ip) == 1
|
||||
assert read_log(client, q="nobody@example.com").json()["events"] == []
|
||||
@@ -0,0 +1,392 @@
|
||||
"""Visit analytics — app/analytics.py and the two endpoints in front of it.
|
||||
|
||||
Three things are worth testing here and the rest is arithmetic. That the
|
||||
counters survive the buffer/UPSERT round trip (a flush must add to what is
|
||||
already stored, not replace it, or every number is only ever the last minute).
|
||||
That the funnel counts *people* rather than clicks, which is the only reason
|
||||
the visitor-day table exists. And that the gate holds: a stranger cannot read
|
||||
the dashboard, and cannot inflate what it says beyond hitting the page.
|
||||
|
||||
python -m pytest tests/test_analytics.py -v
|
||||
"""
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
_tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
|
||||
_tmp.close()
|
||||
os.environ["AIDND_DB_PATH"] = _tmp.name
|
||||
os.environ.pop("AIDND_DATABASE_URL", None)
|
||||
os.environ.pop("DATABASE_URL", None)
|
||||
|
||||
from datetime import timedelta
|
||||
|
||||
import pytest
|
||||
from fastapi import Depends
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app import analytics, auth, limits, models
|
||||
from app.database import Base, SessionLocal, engine, get_db
|
||||
from app.main import app
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_buffer():
|
||||
"""The buffer is process-wide, so a test that leaves counts in it would
|
||||
show up inside the next one's flush."""
|
||||
analytics._counts.clear()
|
||||
analytics._visits.clear()
|
||||
analytics._labels_seen.clear()
|
||||
yield
|
||||
analytics._counts.clear()
|
||||
analytics._visits.clear()
|
||||
analytics._labels_seen.clear()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def db():
|
||||
Base.metadata.create_all(bind=engine)
|
||||
session = SessionLocal()
|
||||
try:
|
||||
yield session
|
||||
finally:
|
||||
session.close()
|
||||
Base.metadata.drop_all(bind=engine)
|
||||
|
||||
|
||||
def counter(db, metric, label):
|
||||
row = (
|
||||
db.query(models.AnalyticsDaily)
|
||||
.filter_by(metric=metric, label=label)
|
||||
.one_or_none()
|
||||
)
|
||||
return row.hits if row else 0
|
||||
|
||||
|
||||
def make_user(db, email=None):
|
||||
user = models.User(is_guest=email is None, email=email)
|
||||
db.add(user)
|
||||
db.commit()
|
||||
return user
|
||||
|
||||
|
||||
# ---------- The buffer and its flush ----------
|
||||
|
||||
def test_counts_accumulate_across_flushes(db):
|
||||
analytics.record(analytics.M_PAGE, "/")
|
||||
analytics.record(analytics.M_PAGE, "/")
|
||||
analytics.flush(db)
|
||||
analytics.record(analytics.M_PAGE, "/")
|
||||
analytics.flush(db)
|
||||
# The second flush has to find the existing row and add to it. Replacing it
|
||||
# would leave every counter showing only the newest minute of traffic.
|
||||
assert counter(db, analytics.M_PAGE, "/") == 3
|
||||
|
||||
|
||||
def test_flush_is_a_no_op_when_nothing_happened(db):
|
||||
analytics.flush(db)
|
||||
assert db.query(models.AnalyticsDaily).count() == 0
|
||||
|
||||
|
||||
def test_a_failed_flush_keeps_the_counts(db, monkeypatch):
|
||||
analytics.record(analytics.M_PAGE, "/")
|
||||
monkeypatch.setattr(analytics, "_write_counts", lambda *a: 1 / 0)
|
||||
analytics.flush(db) # must not raise
|
||||
monkeypatch.undo()
|
||||
analytics.flush(db)
|
||||
assert counter(db, analytics.M_PAGE, "/") == 1
|
||||
|
||||
|
||||
def test_label_cardinality_is_capped(db):
|
||||
for i in range(analytics.MAX_LABELS_PER_METRIC + 25):
|
||||
analytics.record(analytics.M_REFERRER, f"host{i}.example")
|
||||
analytics.flush(db)
|
||||
labels = db.query(models.AnalyticsDaily).filter_by(metric=analytics.M_REFERRER).count()
|
||||
# Everything past the cap is folded into one bucket, so a referrer flood
|
||||
# cannot mint rows without limit.
|
||||
assert labels == analytics.MAX_LABELS_PER_METRIC + 1
|
||||
assert counter(db, analytics.M_REFERRER, analytics.OTHER) == 25
|
||||
|
||||
|
||||
# ---------- Visitors ----------
|
||||
|
||||
def test_visitor_id_is_stable_and_keyed(db, monkeypatch):
|
||||
user = make_user(db)
|
||||
handle = analytics.visitor_id(user)
|
||||
assert handle == analytics.visitor_id(user) # a returning visitor
|
||||
assert handle != analytics.visitor_id(make_user(db)) # is still one visitor
|
||||
assert len(handle) == 32 and int(handle, 16) >= 0 # opaque hex, not an id
|
||||
# Keyed on the app secret, not a bare hash of the user id: otherwise anyone
|
||||
# holding this table could rebuild the mapping by hashing 1, 2, 3, …
|
||||
monkeypatch.setattr(analytics.security, "SECRET_KEY", b"a-different-secret")
|
||||
assert analytics.visitor_id(user) != handle
|
||||
|
||||
|
||||
def test_a_repeat_visitor_is_new_only_once(db):
|
||||
user = make_user(db)
|
||||
analytics.record_visit(user)
|
||||
analytics.flush(db)
|
||||
rows = db.query(models.AnalyticsVisitorDay).all()
|
||||
assert len(rows) == 1 and rows[0].is_new
|
||||
|
||||
# Same visitor, a later day: seen before, so not new — and not merged into
|
||||
# the first day's row either.
|
||||
tomorrow = (models.utcnow().date() + timedelta(days=1)).isoformat()
|
||||
analytics._visits[(tomorrow, analytics.visitor_id(user))] = set()
|
||||
analytics.flush(db)
|
||||
rows = db.query(models.AnalyticsVisitorDay).order_by(models.AnalyticsVisitorDay.day).all()
|
||||
assert [row.is_new for row in rows] == [True, False]
|
||||
|
||||
|
||||
def test_one_row_per_visitor_per_day_however_much_they_do(db):
|
||||
user = make_user(db)
|
||||
for _ in range(5):
|
||||
analytics.record_event(analytics.EV_ADVENTURE, user)
|
||||
analytics.flush(db)
|
||||
assert db.query(models.AnalyticsVisitorDay).count() == 1
|
||||
assert counter(db, analytics.M_EVENT, analytics.EV_ADVENTURE) == 5
|
||||
|
||||
|
||||
def test_funnel_flags_only_ever_turn_on(db):
|
||||
user = make_user(db)
|
||||
analytics.record_event(analytics.EV_TURN, user)
|
||||
analytics.flush(db)
|
||||
# A later visit that reaches no funnel step must not clear the earlier one.
|
||||
analytics.record_visit(user)
|
||||
analytics.flush(db)
|
||||
row = db.query(models.AnalyticsVisitorDay).one()
|
||||
assert row.played and not row.created
|
||||
|
||||
|
||||
def test_purge_drops_only_rows_past_the_horizon(db):
|
||||
old = (models.utcnow().date() - timedelta(days=analytics.RETENTION_DAYS + 1)).isoformat()
|
||||
db.add(models.AnalyticsVisitorDay(day=old, visitor="a" * 32))
|
||||
db.add(models.AnalyticsVisitorDay(day=analytics._today(), visitor="b" * 32))
|
||||
db.commit()
|
||||
assert analytics.purge_old_visitor_days(db) == 1
|
||||
assert [r.visitor for r in db.query(models.AnalyticsVisitorDay)] == ["b" * 32]
|
||||
|
||||
|
||||
# ---------- Normalizing what a browser claims ----------
|
||||
|
||||
@pytest.mark.parametrize("path, expected", [
|
||||
("/", "/"),
|
||||
("/adventures", "/adventures"),
|
||||
("/adventures/", "/adventures"),
|
||||
("/play/12?x=1", "/play/:id"),
|
||||
("/scenarios/9#top", "/scenarios/:id"),
|
||||
("/wp-admin", "(other)"),
|
||||
("/play/../../etc", "(other)"),
|
||||
("", "/"),
|
||||
])
|
||||
def test_route_normalization(path, expected):
|
||||
assert analytics.normalize_route(path) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize("referrer, expected", [
|
||||
("", "(direct)"),
|
||||
("https://news.ycombinator.com/item?id=1", "news.ycombinator.com"),
|
||||
("https://www.google.com/", "google.com"),
|
||||
("https://ai-dnd.example/scenarios", ""), # our own host: not a referral
|
||||
("javascript:alert(1)", "(other)"),
|
||||
("https://" + "x" * 200 + ".com", "(other)"),
|
||||
])
|
||||
def test_referrer_normalization(referrer, expected):
|
||||
assert analytics.normalize_referrer(referrer, "ai-dnd.example") == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ua, expected", [
|
||||
("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0) AppleWebKit", "mobile"),
|
||||
("Mozilla/5.0 (iPad; CPU OS 17_0) AppleWebKit", "tablet"),
|
||||
("Mozilla/5.0 (Windows NT 10.0; Win64; x64)", "desktop"),
|
||||
("Googlebot/2.1", "bot"),
|
||||
("", "(unknown)"),
|
||||
])
|
||||
def test_device_detection(ua, expected):
|
||||
assert analytics.device_of(ua) == expected
|
||||
|
||||
|
||||
def test_only_iso_looking_country_headers_are_trusted():
|
||||
assert analytics.country_of({"cf-ipcountry": "de"}) == "DE"
|
||||
assert analytics.country_of({"cf-ipcountry": "Norway"}) == analytics.UNKNOWN
|
||||
assert analytics.country_of({"cf-ipcountry": "XX"}) == analytics.UNKNOWN
|
||||
assert analytics.country_of({}) == analytics.UNKNOWN
|
||||
|
||||
|
||||
def test_error_labels_use_the_route_not_the_path():
|
||||
class Route:
|
||||
path = "/api/adventures/{adventure_id}"
|
||||
|
||||
assert analytics.api_route_label({"route": Route()}, 500) == "500 /api/adventures/{adventure_id}"
|
||||
# An unmatched path is entirely attacker-chosen, so it never becomes a label.
|
||||
assert analytics.api_route_label({}, 404) == "404 (unmatched)"
|
||||
|
||||
|
||||
# ---------- The summary ----------
|
||||
|
||||
def test_summary_counts_people_once_per_step(db):
|
||||
one, two = make_user(db), make_user(db)
|
||||
for _ in range(3):
|
||||
analytics.record_event(analytics.EV_SCENARIO_OPEN, one)
|
||||
analytics.record_event(analytics.EV_TURN, one)
|
||||
analytics.record_event(analytics.EV_SCENARIO_OPEN, two)
|
||||
|
||||
result = analytics.summary(db, days=7)
|
||||
steps = {row["step"]: row["count"] for row in result["funnel"]}
|
||||
assert steps["Visited"] == 2
|
||||
assert steps["Opened a scenario"] == 2
|
||||
assert steps["Played a turn"] == 1 # not 3 — one person, three turns
|
||||
assert steps["Signed up"] == 0
|
||||
# Raw event totals still count every occurrence.
|
||||
assert result["totals"]["turns"] == 3
|
||||
assert result["totals"]["visitors"] == 2
|
||||
|
||||
|
||||
def test_summary_series_covers_every_day_including_empty_ones(db):
|
||||
analytics.record(analytics.M_PAGE, "/")
|
||||
result = analytics.summary(db, days=7)
|
||||
assert len(result["series"]) == 7
|
||||
assert result["series"][-1]["day"] == models.utcnow().date().isoformat()
|
||||
assert result["series"][-1]["pageviews"] == 1
|
||||
assert result["series"][0]["pageviews"] == 0
|
||||
|
||||
|
||||
def test_summary_flushes_before_reading(db):
|
||||
analytics.record(analytics.M_EVENT, analytics.EV_TURN)
|
||||
# Never flushed by hand: the dashboard must not be up to a minute stale.
|
||||
assert analytics.summary(db, days=1)["totals"]["turns"] == 1
|
||||
|
||||
|
||||
def test_summary_reports_pages_referrers_and_errors(db):
|
||||
analytics.record(analytics.M_PAGE, "/play/:id", n=4)
|
||||
analytics.record(analytics.M_REFERRER, "news.ycombinator.com", n=2)
|
||||
analytics.record(analytics.M_ERROR, "500 /api/adventures/{adventure_id}")
|
||||
result = analytics.summary(db, days=30)
|
||||
assert result["pages"][0] == {"label": "/play/:id", "hits": 4}
|
||||
assert result["referrers"][0]["label"] == "news.ycombinator.com"
|
||||
assert result["totals"]["errors"] == 1
|
||||
|
||||
|
||||
# ---------- The endpoints ----------
|
||||
|
||||
@pytest.fixture()
|
||||
def client(monkeypatch):
|
||||
Base.metadata.create_all(bind=engine)
|
||||
setup = SessionLocal()
|
||||
visitor = models.User(is_guest=True)
|
||||
owner = models.User(is_guest=False, email="owner@example.com")
|
||||
setup.add_all([visitor, owner])
|
||||
setup.commit()
|
||||
ids = {"visitor": visitor.id, "owner": owner.id}
|
||||
setup.close()
|
||||
|
||||
monkeypatch.setattr(limits, "rate_limit", lambda *a, **k: None)
|
||||
# Multi-user is what makes the gate mean anything: local mode trusts
|
||||
# whoever is at the keyboard, because it is the operator's own machine.
|
||||
monkeypatch.setattr(auth, "MULTI_USER", True)
|
||||
monkeypatch.setattr(auth, "ANALYTICS_EMAILS", {"owner@example.com"})
|
||||
|
||||
current = {"id": ids["visitor"]}
|
||||
|
||||
def _current_user(db=Depends(get_db)):
|
||||
return db.get(models.User, current["id"])
|
||||
|
||||
app.dependency_overrides[auth.get_current_user] = _current_user
|
||||
monkeypatch.setattr(
|
||||
auth, "resolve_session_user", lambda request, db: db.get(models.User, current["id"])
|
||||
)
|
||||
try:
|
||||
client = TestClient(app)
|
||||
client.ids, client.current = ids, current
|
||||
yield client
|
||||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
Base.metadata.drop_all(bind=engine)
|
||||
|
||||
|
||||
def read_summary(client, days=30):
|
||||
return client.get(f"/api/analytics/summary?days={days}")
|
||||
|
||||
|
||||
def test_dashboard_is_invisible_to_everyone_but_the_owner(client):
|
||||
assert read_summary(client).status_code == 404
|
||||
client.current["id"] = client.ids["owner"]
|
||||
assert read_summary(client).status_code == 200
|
||||
|
||||
|
||||
def test_collect_records_a_pageview_and_the_visit(client):
|
||||
resp = client.post("/api/analytics/collect", json={"path": "/play/7", "first": True,
|
||||
"referrer": "https://news.ycombinator.com/"})
|
||||
assert resp.status_code == 204
|
||||
client.current["id"] = client.ids["owner"]
|
||||
body = read_summary(client).json()
|
||||
assert body["pages"][0] == {"label": "/play/:id", "hits": 1}
|
||||
assert body["referrers"][0]["label"] == "news.ycombinator.com"
|
||||
assert body["totals"]["visitors"] == 1
|
||||
|
||||
|
||||
def test_referrer_and_device_are_recorded_once_per_visit_not_per_view(client):
|
||||
for path in ("/", "/scenarios", "/adventures"):
|
||||
client.post("/api/analytics/collect", json={"path": path, "first": path == "/"})
|
||||
client.current["id"] = client.ids["owner"]
|
||||
body = read_summary(client).json()
|
||||
assert body["totals"]["pageviews"] == 3
|
||||
# Three views, one visit: the referral and the device are facts about the
|
||||
# visit, so counting them per view would multiply every one of them.
|
||||
assert sum(row["hits"] for row in body["devices"]) == 1
|
||||
|
||||
|
||||
def test_the_owners_own_visits_are_not_traffic(client):
|
||||
client.current["id"] = client.ids["owner"]
|
||||
client.post("/api/analytics/collect", json={"path": "/", "first": True})
|
||||
assert read_summary(client).json()["totals"]["pageviews"] == 0
|
||||
|
||||
|
||||
def test_a_client_cannot_invent_pages_or_events(client):
|
||||
client.post("/api/analytics/collect", json={"path": "/../../admin", "first": True})
|
||||
# There is no field for it, so a made-up event is not even expressible.
|
||||
client.post("/api/analytics/collect", json={"path": "/", "event": "signup"})
|
||||
client.current["id"] = client.ids["owner"]
|
||||
body = read_summary(client).json()
|
||||
assert {row["label"] for row in body["pages"]} == {"(other)", "/"}
|
||||
assert body["totals"]["signups"] == 0
|
||||
|
||||
|
||||
def test_api_errors_are_counted_by_route(client):
|
||||
client.get("/api/adventures/999999")
|
||||
client.current["id"] = client.ids["owner"]
|
||||
errors = read_summary(client).json()["errors"]
|
||||
assert errors and errors[0]["label"].startswith("404 /api/adventures/")
|
||||
|
||||
|
||||
# ---------- The dialect the tests never run on ----------
|
||||
|
||||
def test_the_upserts_compile_for_postgres():
|
||||
"""Prod is Neon; these tests are SQLite, and a failed flush is caught and
|
||||
logged rather than raised. A dialect mistake would therefore be invisible
|
||||
until the dashboard quietly stayed empty — so compile both statements
|
||||
against Postgres without ever connecting to one.
|
||||
"""
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.dialects import postgresql
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
session = sessionmaker(bind=create_engine("postgresql+psycopg://u:p@localhost/db"))()
|
||||
compiled = []
|
||||
|
||||
def capture(statement, *args, **kwargs):
|
||||
compiled.append(str(statement.compile(dialect=postgresql.dialect())))
|
||||
|
||||
session.execute = capture
|
||||
session.scalars = lambda *a, **k: []
|
||||
|
||||
analytics._write_counts(session, {("2026-01-01", "pageview", "/"): 2})
|
||||
analytics._write_visits(session, {("2026-01-01", "f" * 32): {"played"}})
|
||||
|
||||
counts, visits = compiled
|
||||
assert "ON CONFLICT (day, metric, label) DO UPDATE" in counts
|
||||
assert "analytics_daily.hits + excluded.hits" in counts
|
||||
assert "ON CONFLICT (day, visitor) DO UPDATE" in visits
|
||||
assert "analytics_visitor_days.played OR excluded.played" in visits
|
||||
# is_new is settled by the first write of a visitor's first day and must
|
||||
# not be in the update clause at all.
|
||||
assert "is_new" not in visits.split("DO UPDATE")[1]
|
||||
@@ -3,7 +3,7 @@ per-account login throttle added to close it.
|
||||
|
||||
Background: uvicorn's --forwarded-allow-ips "*" trusted the LEFTMOST
|
||||
X-Forwarded-For entry, which the client controls, so rotating the header
|
||||
handed out a fresh rate-limit bucket per request. _client_ip now reads the
|
||||
handed out a fresh rate-limit bucket per request. client_ip now reads the
|
||||
hop the trusted edge appends (rightmost), and login has an email-keyed throttle
|
||||
that no IP trick can dilute.
|
||||
|
||||
@@ -31,22 +31,22 @@ class _Req:
|
||||
self.client = None if peer is None else type("C", (), {"host": peer})()
|
||||
|
||||
|
||||
# ---------- _client_ip: the spoof-resistant hop ----------
|
||||
# ---------- client_ip: the spoof-resistant hop ----------
|
||||
|
||||
def test_client_ip_takes_appended_rightmost_hop(monkeypatch):
|
||||
monkeypatch.setattr(limits, "TRUSTED_PROXY_HOPS", 1)
|
||||
# Attacker prepends a fake IP; the edge appends the real one on the right.
|
||||
req = _Req("203.0.113.9, 198.51.100.77")
|
||||
assert limits._client_ip(req) == "198.51.100.77"
|
||||
assert limits.client_ip(req) == "198.51.100.77"
|
||||
|
||||
|
||||
def test_client_ip_ignores_spoofed_leftmost(monkeypatch):
|
||||
monkeypatch.setattr(limits, "TRUSTED_PROXY_HOPS", 1)
|
||||
# Whatever the client stuffs to the left, the keyed IP stays the real hop —
|
||||
# so rotating it no longer mints a new bucket.
|
||||
a = limits._client_ip(_Req("1.1.1.1, 198.51.100.77"))
|
||||
b = limits._client_ip(_Req("2.2.2.2, 198.51.100.77"))
|
||||
c = limits._client_ip(_Req("evil, junk, 198.51.100.77"))
|
||||
a = limits.client_ip(_Req("1.1.1.1, 198.51.100.77"))
|
||||
b = limits.client_ip(_Req("2.2.2.2, 198.51.100.77"))
|
||||
c = limits.client_ip(_Req("evil, junk, 198.51.100.77"))
|
||||
assert a == b == c == "198.51.100.77"
|
||||
|
||||
|
||||
@@ -54,12 +54,12 @@ def test_client_ip_honours_extra_trusted_hops(monkeypatch):
|
||||
monkeypatch.setattr(limits, "TRUSTED_PROXY_HOPS", 2)
|
||||
# Two trusted hops: real client is second from the right.
|
||||
req = _Req("9.9.9.9, 203.0.113.5, 198.51.100.77")
|
||||
assert limits._client_ip(req) == "203.0.113.5"
|
||||
assert limits.client_ip(req) == "203.0.113.5"
|
||||
|
||||
|
||||
def test_client_ip_falls_back_to_socket_peer():
|
||||
assert limits._client_ip(_Req(None, peer="172.16.0.4")) == "172.16.0.4"
|
||||
assert limits._client_ip(_Req(None, peer=None)) == "unknown"
|
||||
assert limits.client_ip(_Req(None, peer="172.16.0.4")) == "172.16.0.4"
|
||||
assert limits.client_ip(_Req(None, peer=None)) == "unknown"
|
||||
|
||||
|
||||
# ---------- per-account login throttle ----------
|
||||
|
||||
Reference in New Issue
Block a user