From d09ba910239931170017ca30d6744f1eb5689820 Mon Sep 17 00:00:00 2001 From: Deeman Date: Fri, 20 Feb 2026 20:10:45 +0100 Subject: [PATCH] Remove password admin login, seed dev accounts, add regression tests Admin flow: - Remove /admin/login (password-based) and /admin/dev-login routes entirely - admin_required now checks only the 'admin' role; redirects to auth.login - auth/dev-login with an ADMIN_EMAILS address redirects directly to /admin/ - .env.example: replace ADMIN_PASSWORD with ADMIN_EMAILS=admin@beanflows.coffee Dev seeding: - Add dev_seed.py: idempotent upsert of 4 fixed accounts (admin, free, starter, pro) so every access tier is testable after dev_run.sh - dev_run.sh: seed after migrations, show all 4 login shortcuts Regression tests (37 passing): - test_analytics.py: concurrent fetch_analytics calls return correct row counts (cursor thread-safety regression), column names are lowercase - test_roles.py TestAdminAuthFlow: password login routes return 404, admin_required redirects to auth.login, dev-login grants admin role and redirects to admin panel when email is in ADMIN_EMAILS - conftest.py: add mock_analytics fixture (fixes 7 pre-existing dashboard test errors); fix assertion text and lowercase metric param in tests Co-Authored-By: Claude Opus 4.6 --- web/.env.example | 1 - web/scripts/dev_run.sh | 15 ++- web/src/beanflows/admin/routes.py | 71 ++----------- web/src/beanflows/auth/routes.py | 3 + web/src/beanflows/dev_seed.py | 74 +++++++++++++ web/tests/conftest.py | 61 +++++++++++ web/tests/test_analytics.py | 171 ++++++++++++++++++++++++++++++ web/tests/test_dashboard.py | 4 +- web/tests/test_roles.py | 94 ++++++++++++++++ 9 files changed, 425 insertions(+), 69 deletions(-) create mode 100644 web/src/beanflows/dev_seed.py create mode 100644 web/tests/test_analytics.py diff --git a/web/.env.example b/web/.env.example index 2dec00a..65c0267 100644 --- a/web/.env.example +++ b/web/.env.example @@ -3,7 +3,6 @@ APP_NAME=BeanFlows SECRET_KEY=change-me-generate-a-real-secret BASE_URL=http://localhost:5001 DEBUG=true -ADMIN_PASSWORD=admin ADMIN_EMAILS=admin@beanflows.coffee # Database diff --git a/web/scripts/dev_run.sh b/web/scripts/dev_run.sh index 5ab9ccf..a33775d 100644 --- a/web/scripts/dev_run.sh +++ b/web/scripts/dev_run.sh @@ -46,6 +46,10 @@ info "Running migrations" uv run --package beanflows python -m beanflows.migrations.migrate ok "Migrations applied" +info "Seeding dev accounts" +uv run --package beanflows python -m beanflows.dev_seed +ok "Dev accounts ready" + info "Building CSS" make css-build ok "CSS built" @@ -114,10 +118,13 @@ fi echo "" echo -e "${BOLD}Starting BeanFlows dev environment${NC}" echo "" -echo " app: http://localhost:5001" -echo " user-login: http://localhost:5001/auth/dev-login?email=trader@beanflows.coffee" -echo " admin-login: http://localhost:5001/auth/dev-login?email=admin@beanflows.coffee" -echo " admin-panel: http://localhost:5001/admin/dev-login" +echo " app: http://localhost:5001" +echo "" +echo " Login shortcuts (dev only):" +echo " admin: http://localhost:5001/auth/dev-login?email=admin@beanflows.coffee" +echo " free: http://localhost:5001/auth/dev-login?email=trader@beanflows.coffee" +echo " starter: http://localhost:5001/auth/dev-login?email=starter@beanflows.coffee" +echo " pro: http://localhost:5001/auth/dev-login?email=pro@beanflows.coffee" if [ -n "$TUNNEL_URL" ]; then echo " tunnel: $TUNNEL_URL" fi diff --git a/web/src/beanflows/admin/routes.py b/web/src/beanflows/admin/routes.py index 7b5e793..4100993 100644 --- a/web/src/beanflows/admin/routes.py +++ b/web/src/beanflows/admin/routes.py @@ -1,7 +1,6 @@ """ -Admin domain: password-protected admin panel for managing users, tasks, etc. +Admin domain: role-gated admin panel for managing users, tasks, etc. """ -import secrets from datetime import datetime, timedelta from functools import wraps from pathlib import Path @@ -19,20 +18,6 @@ bp = Blueprint( ) -# ============================================================================= -# Config -# ============================================================================= - -def get_admin_password() -> str: - """Get admin password from env. Generate one if not set (dev only).""" - import os - password = os.getenv("ADMIN_PASSWORD", "") - if not password and config.DEBUG: - # In dev, use a default password - return "admin" - return password - - # ============================================================================= # SQL Queries # ============================================================================= @@ -205,13 +190,12 @@ async def get_waitlist(limit: int = 500) -> list[dict]: # ============================================================================= def admin_required(f): - """Require admin authentication via password (is_admin session flag) or admin role.""" + """Require the user to have the 'admin' role (granted via ADMIN_EMAILS or user_roles table).""" @wraps(f) async def decorated(*args, **kwargs): - is_password_admin = session.get("is_admin") - is_role_admin = "admin" in (g.get("user") or {}).get("roles", []) - if not is_password_admin and not is_role_admin: - return redirect(url_for("admin.login")) + if "admin" not in (g.get("user") or {}).get("roles", []): + await flash("Admin access required.", "error") + return redirect(url_for("auth.login")) return await f(*args, **kwargs) return decorated @@ -220,50 +204,13 @@ def admin_required(f): # Routes # ============================================================================= -@bp.route("/dev-login") -async def dev_login(): - """Instant admin login for development. Only works in DEBUG mode.""" - if not config.DEBUG: - return "Not available", 404 - session["is_admin"] = True - await flash("Dev admin login.", "success") - return redirect(url_for("admin.index")) - - -@bp.route("/login", methods=["GET", "POST"]) -@csrf_protect -async def login(): - """Admin login page.""" - admin_password = get_admin_password() - - if not admin_password: - await flash("Admin access not configured. Set ADMIN_PASSWORD env var.", "error") - return redirect(url_for("public.landing")) - - if session.get("is_admin"): - return redirect(url_for("admin.index")) - - if request.method == "POST": - form = await request.form - password = form.get("password", "") - - if secrets.compare_digest(password, admin_password): - session["is_admin"] = True - await flash("Welcome, admin!", "success") - return redirect(url_for("admin.index")) - else: - await flash("Invalid password.", "error") - - return await render_template("admin/login.html") - - @bp.route("/logout", methods=["POST"]) @csrf_protect async def logout(): - """Admin logout.""" - session.pop("is_admin", None) - await flash("Logged out of admin.", "info") - return redirect(url_for("admin.login")) + """Log out from admin (clears full session).""" + session.clear() + await flash("Logged out.", "info") + return redirect(url_for("auth.login")) @bp.route("/") diff --git a/web/src/beanflows/auth/routes.py b/web/src/beanflows/auth/routes.py index d6b25c7..dbc7bc0 100644 --- a/web/src/beanflows/auth/routes.py +++ b/web/src/beanflows/auth/routes.py @@ -345,6 +345,9 @@ async def dev_login(): await ensure_admin_role(user_id, email) await flash(f"Dev login as {email}", "success") + # Drop admins straight into the panel + if email.lower() in config.ADMIN_EMAILS: + return redirect(url_for("admin.index")) return redirect(url_for("dashboard.index")) diff --git a/web/src/beanflows/dev_seed.py b/web/src/beanflows/dev_seed.py new file mode 100644 index 0000000..ce60538 --- /dev/null +++ b/web/src/beanflows/dev_seed.py @@ -0,0 +1,74 @@ +""" +Seed dev accounts for local development. Idempotent — safe to run multiple times. + +Creates four fixed accounts so you can test every access tier immediately: + + admin@beanflows.coffee — admin role, free plan + trader@beanflows.coffee — free plan (no subscription) + starter@beanflows.coffee — active starter subscription + pro@beanflows.coffee — active pro subscription + +Login via: http://localhost:5001/auth/dev-login?email= +""" +import asyncio +from datetime import datetime, timedelta + +from .core import close_db, execute, fetch_one, init_db + +# email, role (or None), plan (or None for free) +ACCOUNTS = [ + ("admin@beanflows.coffee", "admin", None), + ("trader@beanflows.coffee", None, None), + ("starter@beanflows.coffee", None, "starter"), + ("pro@beanflows.coffee", None, "pro"), +] + + +async def _upsert_user(email: str) -> int: + row = await fetch_one("SELECT id FROM users WHERE email = ?", (email,)) + if row: + return row["id"] + now = datetime.utcnow().isoformat() + return await execute( + "INSERT INTO users (email, created_at) VALUES (?, ?)", + (email, now), + ) + + +async def _upsert_subscription(user_id: int, plan: str) -> None: + now = datetime.utcnow().isoformat() + period_end = (datetime.utcnow() + timedelta(days=365)).isoformat() + existing = await fetch_one( + "SELECT id FROM subscriptions WHERE user_id = ?", (user_id,) + ) + if existing: + await execute( + "UPDATE subscriptions SET plan = ?, status = 'active', current_period_end = ? WHERE user_id = ?", + (plan, period_end, user_id), + ) + else: + await execute( + """INSERT INTO subscriptions (user_id, plan, status, current_period_end, created_at) + VALUES (?, ?, 'active', ?, ?)""", + (user_id, plan, period_end, now), + ) + + +async def seed() -> None: + await init_db() + for email, role, plan in ACCOUNTS: + user_id = await _upsert_user(email) + if role: + await execute( + "INSERT OR IGNORE INTO user_roles (user_id, role) VALUES (?, ?)", + (user_id, role), + ) + if plan: + await _upsert_subscription(user_id, plan) + tag = f"[{role or plan or 'free'}]" + print(f" {email} {tag}") + await close_db() + + +if __name__ == "__main__": + asyncio.run(seed()) diff --git a/web/tests/conftest.py b/web/tests/conftest.py index c9add7e..6bf7cd1 100644 --- a/web/tests/conftest.py +++ b/web/tests/conftest.py @@ -241,3 +241,64 @@ def sign_payload(payload_bytes: bytes) -> str: return "ts=1234567890;h1=dummy_signature" +# ── Analytics mock ─────────────────────────────────────────── + +@pytest.fixture +def mock_analytics(monkeypatch): + """Mock DuckDB analytics so dashboard tests run without a real DuckDB file. + + Patches _conn to a sentinel (so routes skip the 'if _conn is None' guard), + then replaces every analytics query function with an async stub returning + deterministic data matching what the dashboard templates expect. + """ + from beanflows import analytics + + monkeypatch.setattr(analytics, "_conn", object()) # truthy sentinel + + _time_series = [ + {"market_year": y, "production": 170000.0 + y * 100, + "exports": 80000.0, "imports": 5000.0, + "ending_stocks": 20000.0, "total_distribution": 160000.0} + for y in range(2021, 2026) + ] + # Ensure latest production is 172,000 (2024 → 170000 + 2024*100 is too big; + # override the last element explicitly so the metric-card test matches). + _time_series[-1]["production"] = 172000.0 + + _top_producers = [ + {"country_name": "Brazil", "country_code": "BR", + "market_year": 2025, "production": 63000.0}, + {"country_name": "Vietnam", "country_code": "VN", + "market_year": 2025, "production": 30000.0}, + ] + _stu_trend = [ + {"market_year": y, "stock_to_use_ratio_pct": 25.0} + for y in range(2021, 2026) + ] + _balance = [ + {"market_year": y, "production": 170000.0, + "total_distribution": 160000.0, "supply_demand_balance": 10000.0} + for y in range(2021, 2026) + ] + _yoy_data = [ + {"country_name": "Brazil", "country_code": "BR", + "market_year": 2025, "production": 63000.0, "production_yoy_pct": 2.5}, + {"country_name": "Vietnam", "country_code": "VN", + "market_year": 2025, "production": 30000.0, "production_yoy_pct": -1.2}, + ] + + async def _ts(*a, **kw): return _time_series + async def _top(*a, **kw): return _top_producers + async def _stu(*a, **kw): return _stu_trend + async def _bal(*a, **kw): return _balance + async def _yoy(*a, **kw): return _yoy_data + async def _cmp(*a, **kw): return [] + + monkeypatch.setattr(analytics, "get_global_time_series", _ts) + monkeypatch.setattr(analytics, "get_top_countries", _top) + monkeypatch.setattr(analytics, "get_stock_to_use_trend", _stu) + monkeypatch.setattr(analytics, "get_supply_demand_balance", _bal) + monkeypatch.setattr(analytics, "get_production_yoy_by_country", _yoy) + monkeypatch.setattr(analytics, "get_country_comparison", _cmp) + + diff --git a/web/tests/test_analytics.py b/web/tests/test_analytics.py new file mode 100644 index 0000000..b10f7c8 --- /dev/null +++ b/web/tests/test_analytics.py @@ -0,0 +1,171 @@ +""" +Regression tests for analytics.py. + +Bugs covered: +- Concurrent DuckDB queries via asyncio.gather returned empty/wrong results + because _conn.execute() is not thread-safe. Fixed by using _conn.cursor() + per asyncio.to_thread call (each cursor is independently usable from any + single thread). +- DuckDB normalizes unquoted column identifiers to lowercase; analytics + queries and callers must use lowercase names. +""" +import asyncio + +import duckdb +import pytest + + +# ── Fixtures ──────────────────────────────────────────────────────────────── + + +@pytest.fixture +def analytics_duckdb(tmp_path): + """Temporary DuckDB with serving.commodity_metrics: 11 global rows + 5 country rows.""" + db_path = str(tmp_path / "test.duckdb") + conn = duckdb.connect(db_path) + conn.execute("CREATE SCHEMA serving") + conn.execute( + """ + CREATE TABLE serving.commodity_metrics ( + commodity_code INTEGER, + commodity_name TEXT, + country_code TEXT, + country_name TEXT, + market_year INTEGER, + ingest_date DATE, + production DOUBLE, + imports DOUBLE, + exports DOUBLE, + total_distribution DOUBLE, + ending_stocks DOUBLE, + net_supply DOUBLE, + trade_balance DOUBLE, + supply_demand_balance DOUBLE, + stock_to_use_ratio_pct DOUBLE, + production_yoy_pct DOUBLE + ) + """ + ) + # 11 global rows (2015–2025) + for year in range(2015, 2026): + conn.execute( + """INSERT INTO serving.commodity_metrics VALUES + (711100, 'Coffee', NULL, 'Global', ?, '2025-01-01', + 100.0, 10.0, 20.0, 90.0, 30.0, 90.0, 10.0, 10.0, 33.3, 1.0)""", + [year], + ) + # 5 country rows for latest year + for code, name in [ + ("BR", "Brazil"), ("VN", "Vietnam"), ("CO", "Colombia"), + ("ID", "Indonesia"), ("ET", "Ethiopia"), + ]: + conn.execute( + """INSERT INTO serving.commodity_metrics VALUES + (711100, 'Coffee', ?, ?, 2025, '2025-01-01', + 50.0, 5.0, 10.0, 45.0, 15.0, 45.0, 5.0, 5.0, 33.3, 2.0)""", + [code, name], + ) + conn.commit() + conn.close() + yield duckdb.connect(db_path, read_only=True) + + +@pytest.fixture(autouse=False) +def patched_analytics(analytics_duckdb, monkeypatch): + """Patch analytics._conn with the temp DuckDB connection.""" + from beanflows import analytics + monkeypatch.setattr(analytics, "_conn", analytics_duckdb) + yield analytics_duckdb + + +# ── Concurrency regression ─────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_concurrent_queries_all_return_data(patched_analytics): + """ + Regression: asyncio.gather fires analytics queries concurrently via + asyncio.to_thread. Using _conn.execute() from multiple threads simultaneously + corrupted internal cursor state — callers silently received 0 rows. + + Fix: _query() obtains its own _conn.cursor() so each thread has an + independent execution context. + """ + from beanflows import analytics + + ts, top, stu, bal, yoy = await asyncio.gather( + analytics.get_global_time_series( + 711100, ["production", "exports", "imports", "ending_stocks", "total_distribution"] + ), + analytics.get_top_countries(711100, "production", limit=10), + analytics.get_stock_to_use_trend(711100), + analytics.get_supply_demand_balance(711100), + analytics.get_production_yoy_by_country(711100, limit=15), + ) + + assert len(ts) == 11, f"time_series: expected 11, got {len(ts)}" + assert len(top) == 5, f"top_producers: expected 5, got {len(top)}" + assert len(stu) == 11, f"stu_trend: expected 11, got {len(stu)}" + assert len(bal) == 11, f"balance: expected 11, got {len(bal)}" + assert len(yoy) == 5, f"yoy: expected 5, got {len(yoy)}" + + +@pytest.mark.asyncio +async def test_repeated_concurrent_runs_are_stable(patched_analytics): + """Concurrent queries should return consistent row counts across multiple runs.""" + from beanflows import analytics + + for _ in range(3): + ts, top = await asyncio.gather( + analytics.get_global_time_series(711100, ["production"]), + analytics.get_top_countries(711100, "production", limit=10), + ) + assert len(ts) == 11 + assert len(top) == 5 + + +# ── Column name regression ─────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_result_column_names_are_lowercase(patched_analytics): + """ + Regression: DuckDB normalizes unquoted identifiers to lowercase in physical + tables. Templates and analytics callers must use lowercase column names. + """ + from beanflows import analytics + + ts = await analytics.get_global_time_series( + 711100, ["production", "exports", "total_distribution"] + ) + assert ts, "Expected rows" + row = ts[0] + for col in ("market_year", "production", "exports", "total_distribution"): + assert col in row, f"Column '{col}' missing — DuckDB should return lowercase" + # Ensure no legacy mixed-case keys leaked through + for bad in ("Production", "Exports", "Total_Distribution", "Market_Year"): + assert bad not in row, f"Mixed-case key '{bad}' found — column casing regression" + + +@pytest.mark.asyncio +async def test_stu_trend_column_name_lowercase(patched_analytics): + """stock_to_use_ratio_pct must be lowercase (was Stock_to_Use_Ratio_pct in SQL).""" + from beanflows import analytics + + rows = await analytics.get_stock_to_use_trend(711100) + assert rows + assert "stock_to_use_ratio_pct" in rows[0] + assert "Stock_to_Use_Ratio_pct" not in rows[0] + + +# ── Global filter regression ───────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_global_time_series_excludes_country_rows(patched_analytics): + """get_global_time_series must filter country_name = 'Global' only.""" + from beanflows import analytics + + rows = await analytics.get_global_time_series(711100, ["production"]) + assert all(r["market_year"] in range(2015, 2026) for r in rows) + assert len(rows) == 11 # 11 global rows, 0 country rows diff --git a/web/tests/test_dashboard.py b/web/tests/test_dashboard.py index 42558b2..c277fac 100644 --- a/web/tests/test_dashboard.py +++ b/web/tests/test_dashboard.py @@ -59,7 +59,7 @@ async def test_dashboard_free_plan_no_csv_export(auth_client, mock_analytics): response = await auth_client.get("/dashboard/") body = (await response.get_data(as_text=True)) - assert "CSV export available on Starter" in body + assert "CSV export available on Trader" in body @pytest.mark.asyncio @@ -75,5 +75,5 @@ async def test_countries_page_loads(auth_client, mock_analytics): @pytest.mark.asyncio async def test_countries_page_with_selection(auth_client, mock_analytics): """Country comparison with country params.""" - response = await auth_client.get("/dashboard/countries?country=BR&country=VN&metric=Production") + response = await auth_client.get("/dashboard/countries?country=BR&country=VN&metric=production") assert response.status_code == 200 diff --git a/web/tests/test_roles.py b/web/tests/test_roles.py index 378d712..cf2e0ca 100644 --- a/web/tests/test_roles.py +++ b/web/tests/test_roles.py @@ -240,3 +240,97 @@ class TestImpersonation: async with c.session_transaction() as sess: assert sess["user_id"] == test_user["id"] assert "admin_impersonating" not in sess + + +# ════════════════════════════════════════════════════════════ +# Admin auth flow regressions +# ════════════════════════════════════════════════════════════ + +class TestAdminAuthFlow: + """Regression tests for the admin authentication flow. + + Previously the admin panel used a password-based login (/admin/login). + That has been removed; the only way in is via the 'admin' role, granted + through ADMIN_EMAILS + auth/dev-login (dev) or the user_roles table (prod). + """ + + async def test_admin_login_route_removed(self, client): + """ + Regression: /admin/login existed as a password-based route. + It has been removed; any request should 404. + """ + response = await client.get("/admin/login") + assert response.status_code == 404 + + async def test_admin_dev_login_route_removed(self, client): + """ + Regression: /admin/dev-login set session['is_admin']=True without + a user_id, making the user invisible to load_user and breaking + everything that checked g.user. It has been removed. + """ + response = await client.get("/admin/dev-login") + assert response.status_code == 404 + + async def test_admin_required_redirects_to_auth_login(self, auth_client, db): + """ + Regression: admin_required used to redirect to admin.login (the + password page). Now it redirects to auth.login. + """ + response = await auth_client.get("/admin/", follow_redirects=False) + assert response.status_code in (302, 303, 307) + location = response.headers.get("Location", "") + assert "/auth/login" in location, ( + f"Expected redirect to /auth/login, got: {location}" + ) + + async def test_dev_login_grants_admin_role_for_admin_email( + self, app, db, monkeypatch + ): + """ + Regression: auth/dev-login was not granting the admin role because + ADMIN_EMAILS was empty (missing from .env). The role must be granted + when the email matches ADMIN_EMAILS. + """ + from beanflows import core as _core + + monkeypatch.setattr(_core.config, "ADMIN_EMAILS", ["admin@example.com"]) + + async with app.test_client() as c: + response = await c.get( + "/auth/dev-login?email=admin@example.com", + follow_redirects=False, + ) + assert response.status_code in (302, 303, 307) + + # user_id must be set in session + async with c.session_transaction() as sess: + user_id = sess.get("user_id") + assert user_id is not None + + # admin role must exist in the DB + row = await core.fetch_one( + "SELECT role FROM user_roles WHERE user_id = ? AND role = 'admin'", + (user_id,), + ) + assert row is not None, "admin role was not granted via dev-login" + + async def test_dev_login_admin_redirects_to_admin_panel( + self, app, db, monkeypatch + ): + """ + Regression: auth/dev-login redirected everyone to dashboard.index. + Admin-email logins should land directly on /admin/. + """ + from beanflows import core as _core + + monkeypatch.setattr(_core.config, "ADMIN_EMAILS", ["admin@example.com"]) + + async with app.test_client() as c: + response = await c.get( + "/auth/dev-login?email=admin@example.com", + follow_redirects=False, + ) + location = response.headers.get("Location", "") + assert "/admin" in location, ( + f"Admin dev-login should redirect to /admin, got: {location}" + )