From 9038aa5c040f85e93e24a1893c43a0e9d2a058f7 Mon Sep 17 00:00:00 2001 From: webdevtodayjason Date: Mon, 21 Sep 2026 18:54:58 -0500 Subject: [PATCH 1/2] A login for people: accounts, sessions and a cookie beside the API key (#261) API-key auth protects the port and is the right credential for a machine. It is the wrong one for a person: one key for everybody, pasted into a browser and kept in localStorage, and revoking one person means rotating the key every client holds. This is the backend half of the front door. - ainode/auth/accounts.py: UsersStore over ~/.ainode/users.json (0600, temp then replace, re-read on change). Passwords are hashlib.scrypt n=2**14 r=8 p=1 over a per-password salt, compared with compare_digest, and an unknown or disabled name pays the same scrypt cost so timing is not an enumeration oracle. Sessions store only sha256(token), have NO expiry (in until you log out), and cap at 20 per account. A password change, a disable and a removal all take the open sessions with them. - ainode/auth/middleware.py: a third credential. The ainode_session cookie authenticates a request as user: with request["user"] and request["user_role"] beside it. A Bearer token is tried first, so a stale key in a browser cannot cost a good session its request, and a cookie-authenticated WRITE must carry X-AINode-Client: dashboard, which is the CSRF proof a custom header gives for free. /api/auth/login and /api/auth/me join SKIP_PATHS. - ainode/auth/session_routes.py: login (throttled at 10 failed attempts per address per 5 minutes), logout, me, the session list and revoke, a self-service password change that keeps the browser doing the changing, the account CRUD, and two fleet-key-only replication routes that move whole records including hashes. - The key routes and /api/auth/enable|disable take the same gate as the account routes: an admin session, an operator key, or the fleet key. A member gets 403. Nothing changes for a caller holding a key, and a node with no accounts behaves exactly as it did. Co-Authored-By: Claude Fable 5.1 --- README.md | 9 +- ainode/api/cluster_join.py | 5 +- ainode/api/server.py | 11 + ainode/auth/__init__.py | 12 +- ainode/auth/accounts.py | 763 +++++++++++++++++++++++++++ ainode/auth/api_routes.py | 22 + ainode/auth/middleware.py | 204 ++++++- ainode/auth/session_routes.py | 722 +++++++++++++++++++++++++ tests/conftest.py | 27 +- tests/test_accounts.py | 491 +++++++++++++++++ tests/test_auth_gate.py | 21 +- tests/test_fleet_auth.py | 21 +- tests/test_session_routes.py | 962 ++++++++++++++++++++++++++++++++++ 13 files changed, 3227 insertions(+), 43 deletions(-) create mode 100644 ainode/auth/accounts.py create mode 100644 ainode/auth/session_routes.py create mode 100644 tests/test_accounts.py create mode 100644 tests/test_session_routes.py diff --git a/README.md b/README.md index e5a257a3..6009c13d 100644 --- a/README.md +++ b/README.md @@ -1006,8 +1006,8 @@ belongs to the node whose files it writes. bytes, stored on the master as a SHA-256 hash with an expiry (30 minutes, `--ttl`) and one use, and it is the joiner's only credential: `POST /api/cluster/join` answers without an API key, because a node that has not joined cannot hold this cluster's key -yet. It is one of five keyless paths, with `/api/health`, `/api/auth/status`, -`/api/cluster/endpoint` and the static shell. A wrong, an expired and a spent token all get the same 403, +yet. It is one of seven keyless paths, with `/api/health`, `/api/auth/status`, +`/api/auth/login`, `/api/auth/me`, `/api/cluster/endpoint` and the static shell. A wrong, an expired and a spent token all get the same 403, and the handler allows five attempts a minute per source address. The joining side writes `cluster_id`, `cluster_secret`, `cluster_role`, `distributed_mode`, `master_address` and `discovery_port` into `config.json` and touches nothing else, @@ -1133,7 +1133,10 @@ With auth on, every path under `/api` and `/v1` wants the key, with these delibe exceptions: the static shell (`/` and `/static/*`), because it is what asks for the key; `/api/health`, because a liveness probe has none, which is also why `ainode update` verifies a release there; `/api/auth/status`, so the UI can say a key is -wanted instead of rendering blank; `/api/cluster/endpoint`, which carries names, +wanted instead of rendering blank; `/api/auth/login`, because the caller with no +credential is exactly who knocks on it, and `/api/auth/me`, which answers +`{"user": null}` to a caller it does not recognise so the dashboard can draw the +login page instead of guessing; `/api/cluster/endpoint`, which carries names, addresses and ports so a client stranded by its own node can find another; and `POST /api/cluster/join`, because a node joining this cluster cannot hold this cluster's key yet. That last one takes a single-use expiring join token instead, and the handler diff --git a/ainode/api/cluster_join.py b/ainode/api/cluster_join.py index 8633ddbe..82a0b237 100644 --- a/ainode/api/cluster_join.py +++ b/ainode/api/cluster_join.py @@ -1,8 +1,9 @@ """The two join routes: the one a joiner calls, and the one this node's UI calls. ``POST /api/cluster/join`` runs on the MASTER. It is the only route besides -``/`` and ``/static/*``, ``/api/health``, ``/api/auth/status`` and -``/api/cluster/endpoint`` that answers without an API key, and the +``/`` and ``/static/*``, ``/api/health``, ``/api/auth/status``, +``/api/auth/login``, ``/api/auth/me`` and ``/api/cluster/endpoint`` that answers +without an API key, and the reason is structural: the node calling it has not joined yet, so it cannot hold this cluster's key. The join token IS the credential (32 random bytes, stored hashed, single use, expiring), and everything that follows from that lives here: diff --git a/ainode/api/server.py b/ainode/api/server.py index 7ad8135c..b613771d 100644 --- a/ainode/api/server.py +++ b/ainode/api/server.py @@ -30,7 +30,9 @@ auth_middleware, is_authenticated, ) +from ainode.auth.accounts import UsersStore from ainode.auth.api_routes import register_auth_routes +from ainode.auth.session_routes import register_session_routes from ainode.ratelimit.middleware import ( RateLimitConfig, RateLimiter, @@ -125,6 +127,11 @@ def create_app( config = NodeConfig() auth_config = AuthConfig.load() + # The accounts beside the keys (#261). Built here so the middleware, the + # login routes and the CLI all read one store, and loaded the same way + # auth.json is: a malformed file raises at boot rather than letting the node + # come up with no accounts and an operator who thinks there are some. + users_store = UsersStore.load() # Order matters. The rate limiter is LAST, so it runs innermost: by then the # auth middleware has stamped the API key id, which is what the limiter keys @@ -150,6 +157,7 @@ def create_app( app["config"] = config app["auth_config"] = auth_config + app["users_store"] = users_store # Per-client limits on /v1. Off unless config.json says otherwise, so a node # that never heard of the block behaves exactly as it did before. app["rate_limiter"] = RateLimiter(config=RateLimitConfig.from_config(config)) @@ -294,6 +302,9 @@ def create_app( register_model_routes(app, models_dir=config.models_dir) register_auth_routes(app) + # The login half of auth, beside the key half: one door for people, one for + # machines (ainode/auth/session_routes.py). + register_session_routes(app) # --- Metrics routes ------------------------------------------------------ register_metrics_routes(app, collector) diff --git a/ainode/auth/__init__.py b/ainode/auth/__init__.py index e6c7d921..256c4ccd 100644 --- a/ainode/auth/__init__.py +++ b/ainode/auth/__init__.py @@ -1 +1,11 @@ -"""AINode authentication — optional API key auth for network-exposed instances.""" +"""AINode authentication: API keys for machines, logins for people. + +Two credentials live here, plus the one the fleet derives for itself: + +* ``middleware.AuthConfig`` and ``api_routes`` are the API keys (``auth.json``), + which is what the bench, the desktop app and ``curl`` present. +* ``accounts.UsersStore`` and ``session_routes`` are the named accounts and login + sessions (``users.json``), which is what a person presents (#261). +* ``fleet`` derives a node-to-node key from ``cluster_secret``, so a cluster can + run with auth on everywhere without a second credential to distribute. +""" diff --git a/ainode/auth/accounts.py b/ainode/auth/accounts.py new file mode 100644 index 00000000..ea97b6ca --- /dev/null +++ b/ainode/auth/accounts.py @@ -0,0 +1,763 @@ +"""User accounts and login sessions: the front door a PERSON walks through. + +API-key auth (``auth/middleware.py``) protects the port, and it is the right +credential for a machine: the bench, the desktop app and ``curl`` all hold a key. +It is the wrong credential for a human. A key is a 32-character hex string pasted +into a browser and kept in ``localStorage``, so "who is on this node" has one +answer for everybody, revoking one person's access means rotating the key every +client holds, and the dashboard's first screen asks a person to paste a machine +secret (#261). + +This module is the other half: named accounts with passwords, and sessions that +outlive a restart. It sits BESIDE the key store rather than replacing it. Both +files live under ``AINODE_HOME``, both are 0600, both are re-read when they +change, and a request may present either credential. + +Three decisions are worth the words: + +* **A session does not expire.** Jason's rule: once you log in, you are in until + you log out. An idle timeout on a dashboard somebody keeps open on a second + monitor all week is a login prompt in front of a node they never left, and a + short expiry is what makes people paste the API key instead. Revocation is the + control: ``ainode auth session revoke``, the dashboard's session list, + disabling the account, or changing its password. +* **Only hashes are stored, in both directions.** A password is + ``scrypt(password, salt)`` and a session token is stored as its SHA-256, so the + file a backup or a support bundle picks up cannot log anybody in. A session + token is returned exactly once, at login, the way an API key is. +* **Passwords are hashed with ``hashlib.scrypt``, from the standard library.** + The AINode container is aiohttp plus pynvml plus Rich and nothing else + (``CLAUDE.md``), so a login page must not be the reason the image grows an + argon2 wheel. scrypt at n=2**14, r=8, p=1 costs about 16 MiB and tens of + milliseconds per attempt, which is the point: it is the per-guess price an + attacker pays for a stolen ``users.json``. + +Replication is deliberately out of this module's hands. ``export_users`` and +``import_users`` move the user list between nodes as whole records, hashes +included, and the fleet endpoints in ``auth/session_routes.py`` are the only +callers. **Sessions are never replicated**: a session belongs to one node's +cookie jar, and copying one would let a revocation on the head silently not take +on a member. +""" + +from __future__ import annotations + +import hashlib +import hmac +import json +import logging +import os +import re +import secrets +import stat +from datetime import datetime, timedelta, timezone +from pathlib import Path +from typing import Optional + +from ainode.auth.fleet import FLEET_KEY_ID +from ainode.core.config import AINODE_HOME + + +logger = logging.getLogger(__name__) + +#: Where the accounts live, beside ``auth.json``. Read through +#: :attr:`UsersStore.path` at call time, never captured in ``__init__``, so a +#: test (and ``tests/conftest.py::isolate_users_store``) can redirect it the same +#: way it redirects ``AUTH_FILE``. +USERS_FILE = AINODE_HOME / "users.json" + +#: A name is lowercase, and short enough to fit a table. It ends up in +#: ``api_key_id`` as ``user:``, in log lines and in the rate limiter's +#: bucket keys, so the character set is the conservative one rather than "any +#: unicode": a name that renders two ways is a name two people read as one. +NAME_PATTERN = re.compile(r"^[a-z0-9._-]{1,64}$") +NAME_RULE = ("A name is 1 to 64 characters of lowercase letters, digits, dot, " + "underscore or hyphen.") + +#: The shortest password this node will store. Low on purpose: the throttle on +#: ``POST /api/auth/login`` is what makes guessing expensive, and a rule long +#: enough to argue with is a rule an operator works around with "ainode1". +MIN_PASSWORD_LENGTH = 8 +PASSWORD_RULE = f"A password is at least {MIN_PASSWORD_LENGTH} characters." + +#: The two roles. ``admin`` may manage users, keys and the auth switch; ``member`` +#: may use the node. Anything beyond these two is #261's explicit non-goal. +ROLE_ADMIN = "admin" +ROLE_MEMBER = "member" +ROLES = (ROLE_ADMIN, ROLE_MEMBER) + +# -- scrypt parameters. They are constants rather than per-record fields because +# every hash in the file is written by this release or a later one, and a record +# that carried its own cost parameters would let a downgrade pick the cheap ones. +SCRYPT_N = 2 ** 14 +SCRYPT_R = 8 +SCRYPT_P = 1 +SCRYPT_DKLEN = 32 +SALT_BYTES = 16 + +#: Bytes of entropy in a session token, before base64. 32 bytes is 256 bits, and +#: the token is the whole credential, so it is sized like one. +SESSION_TOKEN_BYTES = 32 + +#: Sessions kept per user before the oldest is evicted. A person accumulates one +#: per browser, per phone and per CLI, and an unbounded list is a file that grows +#: forever on a node somebody logs into from a new place every day. +MAX_SESSIONS_PER_USER = 20 + +#: How stale ``last_seen`` is allowed to get before a request rewrites it. Every +#: request would mean a 0600 file rewrite per request; never would make the +#: session list useless for "is this still somebody's laptop". +LAST_SEEN_REFRESH_SECONDS = 60 + +#: Prefix on ``request["api_key_id"]`` for a cookie-authenticated request, so +#: every reader of that field (the rate limiter, the request log) can tell a +#: person from a key without a second lookup. +USER_KEY_PREFIX = "user:" + +#: The salt an unknown or disabled name is hashed against, so a login attempt for +#: a name that does not exist costs the same scrypt work as one for a name that +#: does. Without it, response time is a user-enumeration oracle in front of a +#: login page that otherwise answers one identical message for both. +_DUMMY_SALT = b"ainode-no-such-user" + + +def _now() -> str: + """UTC, to the second, in the same spelling ``auth.json`` uses.""" + return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def _parse_stamp(value) -> Optional[datetime]: + """A stored timestamp as a datetime, or None when it cannot be read. + + None is treated by every caller as "older than any window", so a record + written by something that spelled the time differently is refreshed rather + than trusted. + """ + text = str(value or "").strip() + if not text: + return None + try: + return datetime.strptime(text, "%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=timezone.utc) + except ValueError: + return None + + +def _file_stamp(path) -> Optional[tuple]: + """``(mtime_ns, size)`` for *path*, or None when it is not there. + + The same cheap identity ``AuthConfig`` and ``discovery/signing.py`` use to + tell "this file changed" from "this file is the one I already read". + """ + try: + st = os.stat(path) + except OSError: + return None + return (st.st_mtime_ns, st.st_size) + + +def normalize_name(name) -> str: + """A name in its stored form, or "" when it is not a usable name. + + Case-folded, because "Jason" and "jason" are one person and a node that + accepted both would have two accounts nobody can tell apart in a table. + """ + text = str(name or "").strip().casefold() + return text if NAME_PATTERN.match(text) else "" + + +def hash_password(password: str, salt: Optional[bytes] = None) -> tuple[str, str]: + """``(hash hex, salt hex)`` for *password*, minting a salt when none is given. + + Per-password salt, so two people who pick the same password do not share a + hash and a precomputed table buys nothing. + """ + if salt is None: + salt = secrets.token_bytes(SALT_BYTES) + digest = hashlib.scrypt(str(password).encode("utf-8"), salt=salt, + n=SCRYPT_N, r=SCRYPT_R, p=SCRYPT_P, dklen=SCRYPT_DKLEN) + return digest.hex(), salt.hex() + + +def verify_hash(password: str, password_hash: str, salt_hex: str) -> bool: + """Constant-time check of *password* against a stored hash and salt.""" + try: + salt = bytes.fromhex(str(salt_hex or "")) + except ValueError: + return False + if not salt or not password_hash: + return False + digest, _ = hash_password(password, salt=salt) + return hmac.compare_digest(digest, str(password_hash)) + + +def hash_token(token: str) -> str: + """The stored form of a session token. + + SHA-256 and not scrypt on purpose: the token is 256 random bits this node + minted, so there is no guessing to slow down, and the check is on the hot + path of every cookie-authenticated request. + """ + return hashlib.sha256(str(token).encode("utf-8")).hexdigest() + + +def public_user(record: dict) -> dict: + """A user record as a caller may see it: never a hash and never a salt.""" + return { + "name": str(record.get("name") or ""), + "role": str(record.get("role") or ROLE_MEMBER), + "created_at": str(record.get("created_at") or ""), + "disabled": bool(record.get("disabled", False)), + } + + +def public_session(record: dict) -> dict: + """A session as a caller may see it: never the token hash. + + The hash is not the token, but it is the only field in the file that a leak + turns into a lookup key, and nothing outside this module needs it. + """ + return { + "id": str(record.get("id") or ""), + "user": str(record.get("user") or ""), + "client": str(record.get("client") or ""), + "agent": str(record.get("agent") or ""), + "created_at": str(record.get("created_at") or ""), + "last_seen": str(record.get("last_seen") or ""), + } + + +def require_admin(request) -> bool: + """May *request* administer this node: users, keys and the auth switch? + + Three callers qualify, and the second and third are why the first can exist + at all: + + * a session whose account is ``role: "admin"``, + * an operator API key, which is how the CLI on the box works and how the + FIRST admin is created on a node that has no accounts yet, + * the fleet key, so a head can replicate its user list to a member. + + Tolerant of a request-like object with no mapping interface (several tests + call handlers with a stub), which reads as "not an admin": the safe + direction. + """ + getter = getattr(request, "get", None) + if not callable(getter): + return False + if str(getter("user_role", "") or "") == ROLE_ADMIN: + return True + if str(getter("api_key_id", "") or "") == FLEET_KEY_ID: + return True + # An operator key authenticates with no account attached; a cookie + # authenticates with one. That is the whole difference between "the machine + # credential" and "a person", and it is why a member session is refused here + # while a key is not. + return bool(getter("authenticated", False)) and not getter("user", "") + + +class UsersStore: + """The accounts and sessions on this node, backed by ``users.json``. + + Constructed once per process and put on the app as ``app["users_store"]``, + beside ``app["auth_config"]``. ``load()`` is a classmethod for the same + reason ``AuthConfig.load()`` is: every caller (the server, the CLI, the + fleet endpoints) wants "the store this node has" and not a path. + + The file is written 0600 through a temp file in the same directory, and + re-read when its stamp changes, so ``ainode auth user add`` on the box is + live on the running server without a restart. Same trade, same reasons, as + ``AuthConfig``. + """ + + def __init__(self, path=None) -> None: + # None means "whatever USERS_FILE says now", so a monkeypatch of the + # module constant reaches a store that already exists. + self._path: Optional[Path] = Path(path) if path is not None else None + self.users: list[dict] = [] + self.sessions: list[dict] = [] + self._stamp: Optional[tuple] = None + + @property + def path(self) -> Path: + return self._path if self._path is not None else USERS_FILE + + # -- persistence --------------------------------------------------------- + + @classmethod + def load(cls, path=None) -> "UsersStore": + """The store on disk, or an empty one when there is no file yet. + + A malformed file raises, exactly as ``AuthConfig.load`` does: a node + whose account list cannot be read must fail loudly at boot rather than + come up with no accounts and an operator who thinks there are some. The + tolerant path is :meth:`reload_if_changed`. + """ + store = cls(path) + if store.path.exists(): + store._adopt(json.loads(store.path.read_text())) + store._stamp = _file_stamp(store.path) + store.tighten_file_mode() + return store + + def _adopt(self, data) -> None: + if not isinstance(data, dict): + raise ValueError(f"{self.path} does not hold a JSON object") + self.users = [dict(u) for u in (data.get("users") or []) if isinstance(u, dict)] + self.sessions = [dict(s) for s in (data.get("sessions") or []) + if isinstance(s, dict)] + + def save(self) -> None: + """Write the store 0600, through a temp file in the same directory. + + 0600 because this file decides who may log in, and the server and the + installer both run as root, where the default umask leaves it + world-readable. Atomic because every other process re-reads it on change + and must never see half a document. + """ + self.path.parent.mkdir(parents=True, exist_ok=True) + tmp = self.path.with_name(self.path.name + ".tmp") + tmp.write_text(json.dumps({"users": self.users, "sessions": self.sessions}, + indent=2)) + os.chmod(tmp, 0o600) + tmp.replace(self.path) + self._stamp = _file_stamp(self.path) + + def tighten_file_mode(self) -> bool: + """chmod the store to 0600 when it is wider. True when it changed.""" + try: + mode = stat.S_IMODE(os.stat(self.path).st_mode) + except OSError: + return False + if not mode & 0o077: + return False + try: + os.chmod(self.path, 0o600) + except OSError as exc: # pragma: no cover - a read-only home + logger.warning("could not chmod %s to 0600: %s", self.path, exc) + return False + logger.info("tightened %s from %s to 0600", self.path, oct(mode)) + return True + + def reload_if_changed(self) -> bool: + """Adopt what ``users.json`` says now. True when this changed something. + + Called per request by the middleware, so ``ainode auth user add`` and a + session revoked from another process take effect on the running server + the way an ``ainode auth key revoke`` does. Deliberately conservative in + both directions: a file that cannot be stat'ed or parsed leaves the state + in memory alone, because a half-written document must never be the thing + that logs everybody out, and must never be the thing that lets somebody + in either. + """ + stamp = _file_stamp(self.path) + if stamp is None or stamp == self._stamp: + return False + try: + data = json.loads(self.path.read_text()) + except (OSError, ValueError): + logger.warning("could not re-read %s; keeping the accounts in memory", + self.path) + return False + if not isinstance(data, dict): + return False + before = (self.users, self.sessions) + try: + self._adopt(data) + except ValueError: + return False + self._stamp = stamp + changed = before != (self.users, self.sessions) + if changed: + logger.info("users.json changed on disk: %d account(s), %d session(s)", + len(self.users), len(self.sessions)) + return changed + + # -- users --------------------------------------------------------------- + + def find_user(self, name) -> Optional[dict]: + """The stored record for *name*, or None. Hashes included: internal.""" + wanted = normalize_name(name) + if not wanted: + return None + for record in self.users: + if str(record.get("name") or "") == wanted: + return record + return None + + def has_users(self) -> bool: + return bool(self.users) + + def admin_count(self) -> int: + """Enabled admins. A disabled admin cannot administer anything.""" + return sum(1 for u in self.users + if str(u.get("role") or "") == ROLE_ADMIN + and not u.get("disabled", False)) + + def is_last_admin(self, name) -> bool: + """Would losing *name* leave this node with nobody who can administer it? + + The one rule behind both the 409 on ``DELETE /api/auth/users/{name}`` and + the 409 on disabling an account: an operator must not be able to lock + every admin out of the node they are standing in front of. A node whose + admins are ALREADY all disabled has nothing left to protect, so this is + False there rather than a refusal nobody can clear. + """ + record = self.find_user(name) + if record is None: + return False + if str(record.get("role") or "") != ROLE_ADMIN or record.get("disabled", False): + return False + return self.admin_count() <= 1 + + def list_users(self) -> list[dict]: + """Every account as a caller may see it, with its live session count.""" + out = [] + for record in self.users: + row = public_user(record) + row["sessions"] = len(self.sessions_for(row["name"])) + out.append(row) + return out + + def add_user(self, name, password, role: str = ROLE_MEMBER) -> dict: + """Add an account and persist. Returns the record without hashes. + + Raises ``ValueError`` with the rule it broke on a bad name, a short + password, an unknown role or a duplicate, so one message can go straight + into a 400 and into the CLI's stderr. + """ + wanted = normalize_name(name) + if not wanted: + raise ValueError(NAME_RULE) + if len(str(password or "")) < MIN_PASSWORD_LENGTH: + raise ValueError(PASSWORD_RULE) + role = str(role or ROLE_MEMBER).strip().casefold() + if role not in ROLES: + raise ValueError(f"Role must be one of: {', '.join(ROLES)}.") + if self.find_user(wanted) is not None: + raise ValueError(f"There is already an account named '{wanted}'.") + password_hash, salt = hash_password(password) + record = { + "name": wanted, + "password_hash": password_hash, + "salt": salt, + "role": role, + "created_at": _now(), + "disabled": False, + } + self.users.append(record) + self.save() + logger.info("account added: %s (%s)", wanted, role) + return public_user(record) + + def remove_user(self, name) -> bool: + """Remove an account and every session it holds. False when unknown. + + Raises ``ValueError`` rather than stranding the node when *name* is the + last enabled admin (see :meth:`is_last_admin`). The rule lives here and + not in the route so the CLI cannot get around it. + """ + record = self.find_user(name) + if record is None: + return False + if self.is_last_admin(record["name"]): + raise ValueError( + "This is the only admin on this node. Add another admin before " + "removing this one.") + wanted = record["name"] + self.users = [u for u in self.users if u is not record] + self.sessions = [s for s in self.sessions + if str(s.get("user") or "") != wanted] + self.save() + logger.info("account removed: %s", wanted) + return True + + def set_password(self, name, password) -> bool: + """Set a new password and revoke every session. False when unknown. + + The revocation is the point: a password is changed because it leaked or + because somebody is being locked out, and a change that leaves the old + sessions logged in does neither. + """ + record = self.find_user(name) + if record is None: + return False + if len(str(password or "")) < MIN_PASSWORD_LENGTH: + raise ValueError(PASSWORD_RULE) + record["password_hash"], record["salt"] = hash_password(password) + self.sessions = [s for s in self.sessions + if str(s.get("user") or "") != record["name"]] + self.save() + logger.info("password changed for %s; its sessions were revoked", + record["name"]) + return True + + def set_disabled(self, name, disabled: bool) -> bool: + """Disable or enable an account. False when unknown. + + Disabling revokes its sessions too: an account that can still act through + a cookie is not disabled. Raises ``ValueError`` when it would leave the + node with no enabled admin. + """ + record = self.find_user(name) + if record is None: + return False + if disabled and self.is_last_admin(record["name"]): + raise ValueError( + "This is the only admin on this node. Add another admin before " + "disabling this one.") + record["disabled"] = bool(disabled) + if disabled: + self.sessions = [s for s in self.sessions + if str(s.get("user") or "") != record["name"]] + self.save() + logger.info("account %s: %s", record["name"], + "disabled" if disabled else "enabled") + return True + + def role_of(self, name) -> str: + """The role of an ENABLED account, or "" for unknown and disabled. + + "" is what the middleware stamps as ``request["user_role"]``, so a + disabled account reads as no role at all rather than as a member. + """ + record = self.find_user(name) + if record is None or record.get("disabled", False): + return "" + return str(record.get("role") or ROLE_MEMBER) + + def verify_password(self, name, password) -> bool: + """Is this the password for this account? False for unknown and disabled. + + Constant-time in two senses: the comparison is ``compare_digest``, and an + unknown or disabled name pays the same scrypt cost as a real one, so + response time does not say which names exist. + """ + record = self.find_user(name) + if record is None or record.get("disabled", False): + hash_password(str(password or ""), salt=_DUMMY_SALT) + return False + return verify_hash(str(password or ""), + str(record.get("password_hash") or ""), + str(record.get("salt") or "")) + + # -- sessions ------------------------------------------------------------ + + def sessions_for(self, name) -> list[dict]: + """This account's sessions, oldest first, without the token hash.""" + wanted = normalize_name(name) + return [public_session(s) for s in self.sessions + if str(s.get("user") or "") == wanted] + + def create_session(self, name, client: str = "dashboard", + agent: str = "") -> tuple[str, dict]: + """Mint a session for *name*. Returns ``(token, session)``. + + The token is returned HERE and nowhere else, ever: only its SHA-256 is + stored, the same shape an API key has. Raises ``ValueError`` for an + unknown or disabled account, so nothing can mint a session for one. + """ + record = self.find_user(name) + if record is None or record.get("disabled", False): + raise ValueError("No such account on this node.") + token = secrets.token_urlsafe(SESSION_TOKEN_BYTES) + now = _now() + session = { + "id": secrets.token_hex(8), + "token_hash": hash_token(token), + "user": record["name"], + "created_at": now, + "last_seen": now, + "client": str(client or "dashboard"), + "agent": str(agent or "")[:80], + } + self.sessions.append(session) + self._evict_over_cap(record["name"]) + self.save() + logger.info("session %s opened for %s (%s)", session["id"], + session["user"], session["client"]) + return token, session + + def _evict_over_cap(self, name: str) -> None: + """Keep at most MAX_SESSIONS_PER_USER for *name*, dropping the oldest. + + Oldest by position, which is creation order: the list is only ever + appended to. A timestamp comparison would tie at one-second resolution, + and twenty logins inside one second is exactly the case a cap is for. + """ + mine = [s for s in self.sessions if str(s.get("user") or "") == name] + if len(mine) <= MAX_SESSIONS_PER_USER: + return + evicted = {id(s) for s in mine[: len(mine) - MAX_SESSIONS_PER_USER]} + self.sessions = [s for s in self.sessions if id(s) not in evicted] + logger.info("%s is over %d sessions: dropped the oldest %d", + name, MAX_SESSIONS_PER_USER, len(evicted)) + + def session_for_token(self, token, touch: bool = True) -> Optional[dict]: + """The live session *token* names, or None. + + None for a token nobody holds, and also for one whose account has been + removed or disabled since it was minted: disabling an account is the + operator's kill switch, and a switch a cookie outlives is not one. + + Touches ``last_seen`` at most once every + ``LAST_SEEN_REFRESH_SECONDS``, because this runs on every + cookie-authenticated request and a rewrite per request would put a 0600 + file write on the hot path. ``touch=False`` is for the one caller that + asks whether a session exists on a request it is about to REFUSE (the + middleware's CSRF probe): a refused request is not activity. + """ + if not token: + return None + wanted = hash_token(token) + for session in self.sessions: + stored = str(session.get("token_hash") or "") + if not stored or not hmac.compare_digest(wanted, stored): + continue + if not self.role_of(session.get("user")): + return None + if touch: + self._touch(session) + return session + return None + + def _touch(self, session: dict) -> None: + seen = _parse_stamp(session.get("last_seen")) + if seen is not None and datetime.now(timezone.utc) - seen < timedelta( + seconds=LAST_SEEN_REFRESH_SECONDS): + return + session["last_seen"] = _now() + try: + self.save() + except OSError as exc: # pragma: no cover - a read-only home + # A node that cannot write must not stop answering requests for the + # person already logged into it. The stamp goes stale; nothing else. + logger.warning("could not refresh last_seen in %s: %s", self.path, exc) + + def revoke_session(self, session_id, user=None) -> bool: + """Revoke one session by id. False when there is nothing to revoke. + + *user* scopes the revocation to one account, which is how a member may + end its own sessions and only its own; None means any session, which is + what an admin gets. + """ + wanted = str(session_id or "") + if not wanted: + return False + owner = normalize_name(user) if user else "" + keep, gone = [], [] + for session in self.sessions: + if str(session.get("id") or "") == wanted and ( + not owner or str(session.get("user") or "") == owner): + gone.append(session) + else: + keep.append(session) + if not gone: + return False + self.sessions = keep + self.save() + logger.info("session %s revoked", wanted) + return True + + def revoke_sessions_for(self, name) -> int: + """Revoke every session an account holds. Returns how many went.""" + wanted = normalize_name(name) + if not wanted: + return 0 + before = len(self.sessions) + self.sessions = [s for s in self.sessions + if str(s.get("user") or "") != wanted] + gone = before - len(self.sessions) + if gone: + self.save() + return gone + + # -- fleet replication --------------------------------------------------- + + def export_users(self) -> list[dict]: + """The accounts as another NODE needs them: hashes and salts included. + + This is the one method that hands out hash material, and it is why the + export route is fleet-key only. A member node receiving this list can + verify a password without the head, which is the whole point: one login + works on every node in the cluster. + + Sorted by name so :meth:`export_stamp` over the same accounts is the + same string on every node. + """ + return [ + { + "name": str(u.get("name") or ""), + "password_hash": str(u.get("password_hash") or ""), + "salt": str(u.get("salt") or ""), + "role": str(u.get("role") or ROLE_MEMBER), + "created_at": str(u.get("created_at") or ""), + "disabled": bool(u.get("disabled", False)), + } + for u in sorted(self.users, key=lambda u: str(u.get("name") or "")) + ] + + def export_stamp(self) -> str: + """A short digest of the exported accounts. + + A content hash rather than the file's mtime: two nodes holding the same + accounts must produce the same stamp, which is what lets a replication + pass decide "nothing to do" without shipping the list. An mtime says + only when a file was written, which differs on every node. + """ + payload = json.dumps(self.export_users(), sort_keys=True, + separators=(",", ":")).encode("utf-8") + return hashlib.sha256(payload).hexdigest()[:16] + + def import_users(self, users) -> bool: + """Replace the account list with *users*. True when anything changed. + + All or nothing: a malformed record raises ``ValueError`` and the store is + untouched, because a partial import is a node whose account list quietly + differs from the rest of the cluster. + + Sessions survive when their account does. A member's cookie is its own + node's, so a replication pass must not be a fleet-wide logout; a session + whose account the head removed goes with it. + """ + if not isinstance(users, list): + raise ValueError("'users' must be a list of account records.") + adopted: list[dict] = [] + seen: set[str] = set() + for entry in users: + if not isinstance(entry, dict): + raise ValueError("Every account record must be a JSON object.") + name = normalize_name(entry.get("name")) + if not name: + raise ValueError(f"{NAME_RULE} Got: {entry.get('name')!r}") + if name in seen: + raise ValueError(f"'{name}' appears twice in the account list.") + password_hash = str(entry.get("password_hash") or "") + salt = str(entry.get("salt") or "") + if not password_hash or not salt: + raise ValueError(f"'{name}' carries no password hash to import.") + role = str(entry.get("role") or ROLE_MEMBER).strip().casefold() + if role not in ROLES: + raise ValueError(f"'{name}' has an unknown role {role!r}.") + seen.add(name) + adopted.append({ + "name": name, + "password_hash": password_hash, + "salt": salt, + "role": role, + "created_at": str(entry.get("created_at") or _now()), + "disabled": bool(entry.get("disabled", False)), + }) + if adopted == self.export_users(): + return False + if not adopted and self.users: + # Legal, and worth a line in the log: the only way to reach it is a + # head that really has no accounts, which on a configured fleet is a + # mistake somebody will want to find afterwards. + logger.warning("replication is emptying the account list on this node") + self.users = adopted + names = {u["name"] for u in adopted} + self.sessions = [s for s in self.sessions + if str(s.get("user") or "") in names] + self.save() + logger.info("accounts replicated: %d on this node now", len(self.users)) + return True diff --git a/ainode/auth/api_routes.py b/ainode/auth/api_routes.py index 61b11563..62d838e8 100644 --- a/ainode/auth/api_routes.py +++ b/ainode/auth/api_routes.py @@ -3,6 +3,12 @@ ``GET /api/auth/status`` is the one route here the middleware leaves open: the dashboard asks it before anything else so it can tell "this node wants a key" apart from "this node is broken". Everything else needs the key once auth is on. + +Every route here except that one is ADMINISTRATION, so since #261 it takes the +same gate the account routes take (``session_routes.admin_refusal``): an admin +session, an operator API key, or the fleet key. A member session is refused, +because a key is the whole access control on this node and "log in as a member" +must not be a way to mint one, revoke everybody else's, or switch the wall off. """ from __future__ import annotations @@ -10,6 +16,7 @@ from aiohttp import web from ainode.auth.middleware import AuthConfig, is_authenticated +from ainode.auth.session_routes import admin_refusal def register_auth_routes(app: web.Application) -> None: @@ -48,6 +55,9 @@ async def handle_auth_enable(request: web.Request) -> web.Response: so an existing key cannot be shown again. The caller is told to use the key it has, or to create a new one. """ + refused = admin_refusal(request) + if refused is not None: + return refused auth_cfg: AuthConfig = request.app["auth_config"] entry = auth_cfg.enable() payload = { @@ -67,6 +77,9 @@ async def handle_auth_enable(request: web.Request) -> web.Response: async def handle_auth_disable(request: web.Request) -> web.Response: """POST /api/auth/disable -- disable auth.""" + refused = admin_refusal(request) + if refused is not None: + return refused auth_cfg: AuthConfig = request.app["auth_config"] auth_cfg.disable() return web.json_response({"enabled": False, "key_count": len(auth_cfg.api_keys)}) @@ -78,6 +91,9 @@ async def handle_list_keys(request: web.Request) -> web.Response: Never a hash and never a plaintext: a key is shown once, at mint time, and after that the only operations are "list" and "revoke". """ + refused = admin_refusal(request) + if refused is not None: + return refused auth_cfg: AuthConfig = request.app["auth_config"] return web.json_response({ "enabled": auth_cfg.enabled, @@ -93,6 +109,9 @@ async def handle_create_key(request: web.Request) -> web.Response: ``GET /api/auth/keys`` and ``ainode auth key list`` can name it later. A request with no body is the old shape and still mints an unnamed key. """ + refused = admin_refusal(request) + if refused is not None: + return refused auth_cfg: AuthConfig = request.app["auth_config"] name = "" try: @@ -113,6 +132,9 @@ async def handle_create_key(request: web.Request) -> web.Response: async def handle_revoke_key(request: web.Request) -> web.Response: """DELETE /api/auth/keys/:key_id -- revoke a key.""" + refused = admin_refusal(request) + if refused is not None: + return refused key_id = request.match_info["key_id"] auth_cfg: AuthConfig = request.app["auth_config"] revoked = auth_cfg.revoke_key(key_id) diff --git a/ainode/auth/middleware.py b/ainode/auth/middleware.py index 4a6ec007..5c32ba83 100644 --- a/ainode/auth/middleware.py +++ b/ainode/auth/middleware.py @@ -1,13 +1,17 @@ -"""API key authentication middleware for aiohttp. +"""Authentication middleware for aiohttp: an API key, a login, or the fleet. The rule, in one place: **when auth is enabled every path under ``/api`` and -``/v1`` needs the key.** The exceptions are the five things a caller with no key -must still be able to reach: +``/v1`` needs a credential.** The exceptions are the seven things a caller with +none must still be able to reach: * the static shell (``/``, ``/static/*``), * ``/api/health`` (liveness, for a probe that has no key), * ``/api/auth/status`` (so the UI can say "this node wants a key" instead of rendering an empty page), +* ``/api/auth/login`` (the door: a caller with no credential is exactly who + posts to it), +* ``/api/auth/me`` (which answers ``{"user": null}`` to a caller with no + session, so the dashboard can draw the login page instead of guessing), * ``/api/cluster/endpoint`` (node names, addresses and ports, nothing else: a client whose configured node is down has to be able to ask a reachable one where the rest of the fleet is, and the key it holds does not help it find an @@ -22,21 +26,46 @@ ``onboarded`` before the server came up, and it never joined a cluster even when reached. -Every request is stamped with ``request["authenticated"]`` -- True only when a -Bearer token matched a stored key hash -- and with ``request["api_key_id"]``, the -id of the key that matched. Handlers read the first through -``is_authenticated()`` to gate the few fields that are dangerous even when auth -is switched off (``trust_remote_code``; see ``TRUST_REMOTE_CODE_RULE``); the rate -limiter reads the second so a keyed caller gets its own budget. - -There is one caller besides the operator: **the fleet**. A peer presenting the -key derived from this node's ``cluster_secret`` is accepted as ``api_key_id == -"fleet"``, which is what lets a cluster run with auth on everywhere (see -``auth/fleet.py`` for the derivation, and why it is a derivation). It is checked -against the LIVE secret on every request, so rotating ``cluster_secret`` rotates -the fleet's access with no restart, and a node whose secret differs from the rest -of the cluster refuses them exactly as it drops their unverifiable discovery -datagrams. +Every request is stamped with ``request["authenticated"]`` -- True when a +credential matched -- and with ``request["api_key_id"]``, who matched. Handlers +read the first through ``is_authenticated()`` to gate the few fields that are +dangerous even when auth is switched off (``trust_remote_code``; see +``TRUST_REMOTE_CODE_RULE``); the rate limiter reads the second so a keyed caller +gets its own budget. + +**Three credentials, in one pass.** + +1. An operator API key, from ``Authorization: Bearer``, stamped as its key id. +2. **The fleet.** A peer presenting the key derived from this node's + ``cluster_secret`` is accepted as ``api_key_id == "fleet"``, which is what + lets a cluster run with auth on everywhere (see ``auth/fleet.py`` for the + derivation, and why it is a derivation). It is checked against the LIVE + secret on every request, so rotating ``cluster_secret`` rotates the fleet's + access with no restart, and a node whose secret differs from the rest of the + cluster refuses them exactly as it drops their unverifiable discovery + datagrams. +3. **A login session**, from the ``ainode_session`` cookie (#261), stamped as + ``api_key_id == "user:"`` with ``request["user"]`` and + ``request["user_role"]`` beside it. ``auth/accounts.py`` owns the accounts + and the sessions; this module only decides whether one is presented. + +A Bearer token is tried FIRST and a cookie only when no key matched. That is +"the key wins" for a caller holding both, without locking out the one case that +really happens: a browser that logged in while a stale key was still sitting in +``localStorage`` would otherwise 401 on every request with a valid session in +the jar. + +**The cookie carries a CSRF rule the other two do not need.** A browser attaches +a cookie to a cross-site request on its own, so a cookie alone would let any page +on the internet POST to a node the operator is logged into. A Bearer token cannot +be attached by anybody but the caller, so it needs nothing. The rule: a +cookie-authenticated request whose method is not GET, HEAD or OPTIONS must carry +``X-AINode-Client: dashboard``. A custom header cannot be set cross-origin +without a CORS preflight this node never approves, so the header IS the proof +that the caller is the dashboard's own code and not somebody else's page. Without +it the cookie is ignored: the request proceeds as unauthenticated, which with +auth on is a 401 whose message names the missing header rather than claiming the +login was bad. """ from __future__ import annotations @@ -54,6 +83,7 @@ from aiohttp import web +from ainode.auth.accounts import USER_KEY_PREFIX, UsersStore from ainode.auth.fleet import FLEET_KEY_ID, cluster_secret_of, is_fleet_key from ainode.core.config import AINODE_HOME @@ -82,12 +112,34 @@ def _file_stamp(path) -> Optional[tuple]: AUTH_FILE = AINODE_HOME / "auth.json" SKIP_PATHS: set[str] = {"/", "/api/health", "/api/auth/status", + "/api/auth/login", "/api/auth/me", "/api/cluster/endpoint", "/api/cluster/join"} SKIP_PREFIXES: tuple[str, ...] = ("/static/",) +#: The cookie a logged-in browser carries. HttpOnly, so no script on the page can +#: read it (``auth/session_routes.py`` writes it and owns the attributes). +SESSION_COOKIE = "ainode_session" + +#: The header a cookie-authenticated write must carry, and its one accepted +#: value. See the module docstring for why a custom header is the CSRF proof. +CLIENT_HEADER = "X-AINode-Client" +DASHBOARD_CLIENT = "dashboard" + +#: Methods a cookie alone is allowed to authenticate. The three that must not +#: change anything, so a cross-site GET buys an attacker nothing it did not +#: already have from the browser's own address bar. +SAFE_METHODS: frozenset[str] = frozenset({"GET", "HEAD", "OPTIONS"}) + #: ``request`` key carrying the outcome of token validation for this request. AUTHENTICATED_KEY = "authenticated" +#: ``request`` keys carrying the logged-in account, or "" when the caller is a +#: key, the fleet, or nobody. Handlers read them through ``request["user"]`` / +#: ``request["user_role"]``; ``accounts.require_admin`` is the one place that +#: turns them into a yes or no. +USER_KEY = "user" +USER_ROLE_KEY = "user_role" + #: ``request`` key carrying the id of the API key this request presented, or "". #: Set by the same validation pass as AUTHENTICATED_KEY, so anything downstream #: can tell WHICH key called without hashing the token again. The rate limiter @@ -105,11 +157,23 @@ def _file_stamp(path) -> Optional[tuple]: "recipe already declares it." ) -#: What a 401 tells the caller. The dashboard turns this into its API access -#: panel; a curl user gets the header to send. +#: What a 401 tells the caller. The dashboard turns this into its login page or +#: its API access panel; a curl user gets the header to send. Both doors are +#: named, because a person who is told only about a Bearer token pastes a machine +#: key into a browser, which is the thing #261 exists to stop. MISSING_KEY_MESSAGE = ( - "This node requires an API key. Send Authorization: Bearer , or paste " - "the key into the dashboard under Config > API access." + "This node requires a login or an API key. Sign in on the dashboard, or send " + "Authorization: Bearer ." +) + +#: What a 401 tells a caller whose session cookie was ignored for want of the +#: client header. Distinct from MISSING_KEY_MESSAGE on purpose: the credential +#: was fine and the request shape was not, and a message blaming the login sends +#: somebody to re-type a password that was never wrong. +CSRF_HEADER_MESSAGE = ( + f"A session cookie only authenticates a write that carries " + f"{CLIENT_HEADER}: {DASHBOARD_CLIENT}. Send that header, or use " + f"Authorization: Bearer ." ) @@ -344,6 +408,67 @@ def identify_caller(app, auth_cfg: "AuthConfig | None", return False, "" +def session_token(request) -> str: + """The session token in this request's cookie jar, or "". + + ``request.cookies`` is tolerant of a request-like stub with none, because the + handlers in this package are called with one in several tests. + """ + cookies = getattr(request, "cookies", None) or {} + try: + return str(cookies.get(SESSION_COOKIE) or "") + except Exception: # pragma: no cover - a stub with a hostile mapping + return "" + + +def csrf_header_ok(request) -> bool: + """Is this request allowed to authenticate on a cookie alone? + + True for a safe method, and for any method carrying + ``X-AINode-Client: dashboard``. See the module docstring: the header is the + proof that the caller is the dashboard's own code, because a cross-origin + page cannot set one without a preflight this node never approves. + """ + method = str(getattr(request, "method", "GET") or "GET").upper() + if method in SAFE_METHODS: + return True + headers = getattr(request, "headers", None) or {} + try: + sent = str(headers.get(CLIENT_HEADER) or "") + except Exception: # pragma: no cover - a stub with a hostile mapping + sent = "" + return sent.strip().casefold() == DASHBOARD_CLIENT + + +def identify_session(request) -> tuple[Optional[dict], bool]: + """``(session, refused for want of the client header)`` for this request. + + The session is None whenever the cookie does not name a live one: no cookie, + a token nobody holds, or an account that has since been removed or disabled + (``UsersStore.session_for_token`` owns that last rule). + + The second value is True only in the one case worth a different message: the + cookie DID name a live session and the method needed the client header, which + was not there. The caller is refused either way; the flag is how the 401 says + which thing to fix. + """ + app = getattr(request, "app", None) + getter = getattr(app, "get", None) + store: Optional[UsersStore] = getter("users_store") if callable(getter) else None + if store is None: + return None, False + token = session_token(request) + if not token: + return None, False + if not csrf_header_ok(request): + # Look the session up anyway, so the message can tell "your cookie is + # fine, the header is missing" from "that cookie is stale". Without + # touching last_seen: this request is about to be refused, and a refused + # request is not activity. + return None, store.session_for_token(token, touch=False) is not None + return store.session_for_token(token), False + + def is_authenticated(request) -> bool: """True when this request presented a token matching a stored key. @@ -375,26 +500,51 @@ async def auth_middleware(request: web.Request, handler): # One stat, so an `ainode auth ...` on this box is live rather than # waiting for a restart nobody was told to do. auth_cfg.reload_if_changed() + users: UsersStore | None = request.app.get("users_store") + if users is not None: + # The same one stat, for the same reason: `ainode auth user add`, a + # password change and a revoked session all land on a running server. + users.reload_if_changed() token = bearer_token(request) # Stamped on every request, enabled or not: handlers gate on it (see # is_authenticated) even when the node is running open. The key id goes on # with it so the rate limiter can count a keyed caller as itself rather than - # as its address (see API_KEY_ID_KEY), and so a peer reads as "fleet". + # as its address (see API_KEY_ID_KEY), so a peer reads as "fleet", and so a + # logged-in person reads as "user:". matched, key_id = identify_caller(request.app, auth_cfg, token) + user_name = "" + user_role = "" + csrf_refused = False + if not matched: + # Only when no key matched: a Bearer token wins, and a stale one in a + # browser's localStorage must not cost a valid session its request. + session, csrf_refused = identify_session(request) + if session is not None and users is not None: + user_name = str(session.get("user") or "") + user_role = users.role_of(user_name) + matched = True + key_id = f"{USER_KEY_PREFIX}{user_name}" request[AUTHENTICATED_KEY] = matched request[API_KEY_ID_KEY] = key_id + request[USER_KEY] = user_name + request[USER_ROLE_KEY] = user_role if auth_cfg is None or not auth_cfg.enabled: return await handler(request) if _should_skip(request): return await handler(request) - if not token: + if matched: + return await handler(request) + if csrf_refused: return web.json_response( - {"error": {"message": MISSING_KEY_MESSAGE, "type": "auth_error"}}, + {"error": {"message": CSRF_HEADER_MESSAGE, "type": "auth_error"}}, status=401, ) - if not request[AUTHENTICATED_KEY]: + if token: return web.json_response( {"error": {"message": "Invalid API key", "type": "auth_error"}}, status=401, ) - return await handler(request) + return web.json_response( + {"error": {"message": MISSING_KEY_MESSAGE, "type": "auth_error"}}, + status=401, + ) diff --git a/ainode/auth/session_routes.py b/ainode/auth/session_routes.py new file mode 100644 index 00000000..3ca8c53d --- /dev/null +++ b/ainode/auth/session_routes.py @@ -0,0 +1,722 @@ +"""The login routes: the door, the session, and who may hold the keys to it. + +``auth/api_routes.py`` manages API keys, which are machine credentials. +This module is the half a PERSON uses (#261): ``POST /api/auth/login`` takes a +name and a password and hands back an ``HttpOnly`` cookie, ``GET /api/auth/me`` +is what the dashboard asks before it decides whether to draw a login page, and +the rest is the account and session management around them. ``auth/accounts.py`` +owns the storage and the rules; this module owns the HTTP. + +Four things here are decisions rather than plumbing: + +* **Login and ``/api/auth/me`` are open with no credential** (they are in + ``middleware.SKIP_PATHS``), and they are safe open for opposite reasons. Login + is the door: the caller with no credential is exactly who knocks. ``me`` + answers ``{"user": null}`` to a caller it does not recognise, and nothing else, + so a stranger learns only what the login page already tells them. Everything + else in this module needs a credential like every other ``/api`` path. +* **A failed login is throttled per source address**, not per name. Ten failures + in five minutes and the address waits, because a name-keyed limiter is a + lockout an attacker can aim at somebody else's account, and an unlimited login + route in front of scrypt is both a guessing oracle and a way to make a GPU node + spend its CPU on hashes. +* **The 401 says the same thing for a wrong password and a name nobody has.** + ``UsersStore.verify_password`` pays the same scrypt cost either way, so neither + the message nor the timing tells a stranger which accounts exist. +* **Administration accepts three callers, and the second and third are why the + first can exist** (``accounts.require_admin``): an admin session, an operator + API key, or the fleet key. A node with no accounts has nobody to authorise the + first admin, so the key the installer already minted is what authorises it, and + the fleet key is what lets a head replicate its accounts to a member. A member + session is refused, which is the whole point of having two roles. + +Replication is two routes, both fleet-key only: ``GET /api/auth/users/export`` +hands out the records WITH their hashes so a member can verify a password without +asking the head, and ``POST /api/auth/users/sync`` takes them. Sessions are never +part of it (see ``accounts.py``). Every mutation calls ``app["users_changed"]`` +when something registered one, which is how a head learns it has replicating to +do without this module knowing anything about the fleet. +""" + +from __future__ import annotations + +import asyncio +import inspect +import logging +import time +from typing import Optional + +from aiohttp import web + +from ainode.auth.accounts import ( + MIN_PASSWORD_LENGTH, + ROLE_MEMBER, + UsersStore, + normalize_name, + public_session, + public_user, + require_admin, +) +from ainode.auth.fleet import FLEET_KEY_ID +from ainode.auth.middleware import ( + CLIENT_HEADER, + DASHBOARD_CLIENT, + SESSION_COOKIE, +) + + +logger = logging.getLogger(__name__) + +#: How long the cookie lives in the browser: 400 days, the longest a browser will +#: honour. The SESSION itself has no expiry at all (see ``accounts.py``), so this +#: number is only about how long a browser keeps offering the token, not about how +#: long the node accepts it. A Max-Age far in the future is the cookie spelling of +#: "you are in until you log out". +COOKIE_MAX_AGE = 34560000 + +#: Failed logins one source address may make per window, and the window. +LOGIN_RATE_LIMIT = 10 +LOGIN_RATE_WINDOW = 300.0 +#: ``app`` key holding the per-address failure log. +LOGIN_RATE_STATE_KEY = "login_failures" +#: Addresses tracked at once. A flood from forged addresses must not grow this +#: dict without bound; past the cap the least recently active source is dropped, +#: which at worst gives an attacker back failures it had already spent. +LOGIN_RATE_MAX_SOURCES = 512 + +#: The one answer a failed login gets, whatever was wrong with it. +LOGIN_REFUSED_MESSAGE = "Wrong name or password" + +#: What a caller is told when there is nobody to log in as. Names the command, +#: because the person reading it is looking at a login page on a node that cannot +#: let anybody in and needs a shell, not a support article. +NO_ACCOUNTS_MESSAGE = ( + "No accounts yet. Create the first admin on this node: " + "ainode auth user add --admin" +) + +#: Clients a session may be opened for. Free text would make the session list +#: unreadable, and these two are the only things that log in. +CLIENTS = ("dashboard", "cli") + + +# ============================================================================= +# Registration +# ============================================================================= + +def register_session_routes(app: web.Application) -> None: + """Register the login, session and account routes. + + The throttle log is seeded HERE and not on first use: an aiohttp Application + is read-only once it has started, so a handler that created the key would be + mutating a started app (the same reason ``api/cluster_join.py`` seeds its + own). + + ``/api/auth/users/export`` and ``/api/auth/users/sync`` are registered BEFORE + ``/api/auth/users/{name}``: aiohttp resolves in registration order, and a + variable segment registered first would swallow a literal one that follows it. + """ + app[LOGIN_RATE_STATE_KEY] = {} + + app.router.add_post("/api/auth/login", handle_login) + app.router.add_post("/api/auth/logout", handle_logout) + app.router.add_get("/api/auth/me", handle_me) + + app.router.add_get("/api/auth/sessions", handle_list_sessions) + app.router.add_delete("/api/auth/sessions/{id}", handle_revoke_session) + app.router.add_post("/api/auth/password", handle_change_own_password) + + app.router.add_get("/api/auth/users/export", handle_export_users) + app.router.add_post("/api/auth/users/sync", handle_sync_users) + app.router.add_get("/api/auth/users", handle_list_users) + app.router.add_post("/api/auth/users", handle_add_user) + app.router.add_delete("/api/auth/users/{name}", handle_remove_user) + app.router.add_post("/api/auth/users/{name}/password", handle_set_user_password) + app.router.add_post("/api/auth/users/{name}/disable", handle_disable_user) + app.router.add_post("/api/auth/users/{name}/enable", handle_enable_user) + + +# ============================================================================= +# Small shared pieces +# ============================================================================= + +def _error(message: str, kind: str, status: int, **kwargs) -> web.Response: + """The error shape every route in this module answers with.""" + return web.json_response({"error": {"message": message, "type": kind}}, + status=status, **kwargs) + + +async def _body(request: web.Request) -> dict: + """The request's JSON object, or ``{}``. Never raises. + + A handler validates the fields it needs and says which one is missing, which + is a better answer than a 400 about JSON for a body that was simply empty. + """ + if not request.can_read_body: + return {} + try: + data = await request.json() + except Exception: + return {} + return data if isinstance(data, dict) else {} + + +def _store(request: web.Request) -> UsersStore: + """This node's account store, creating an empty one only as a fallback. + + ``create_app`` always installs one. The fallback keeps a hand-built test app + (and any embedder that only wired the middleware) from 500ing on an + ``AttributeError`` instead of answering "there are no accounts". + """ + store = request.app.get("users_store") + if store is None: + store = UsersStore() + return store + + +def _auth_enabled(request: web.Request) -> bool: + return bool(getattr(request.app.get("auth_config"), "enabled", False)) + + +def client_address(request: web.Request) -> str: + """The peer address, as the socket reports it. + + Deliberately NOT ``X-Forwarded-For``: that header is caller-supplied, so + keying a throttle on it would let one client mint itself a fresh budget per + attempt. Same rule, same words, as ``api/cluster_join.py``. + """ + peer = request.transport.get_extra_info("peername") if request.transport else None + if isinstance(peer, tuple) and peer: + return str(peer[0]) + return str(request.remote or "unknown") + + +def request_is_https(request: web.Request) -> bool: + """Is the caller on HTTPS, directly or through a terminating proxy? + + Both halves matter for the cookie's ``Secure`` flag. A node with TLS on serves + a second listener itself (``api/server.py::listener_plan``), and a node behind + a reverse proxy sees plain HTTP with ``X-Forwarded-Proto: https``. Marking the + cookie ``Secure`` on a plain-HTTP node would be a cookie the browser then + refuses to send, so this cannot simply always be true. + """ + if bool(getattr(request, "secure", False)): + return True + forwarded = str(request.headers.get("X-Forwarded-Proto", "") or "") + return forwarded.split(",")[0].strip().casefold() == "https" + + +def session_cookie(token: str, secure: bool, + max_age: int = COOKIE_MAX_AGE) -> str: + """The ``Set-Cookie`` value for a session token. + + ``HttpOnly`` so no script on the page can read it, which is what keeps an XSS + bug from turning into a stolen session that outlives the fix. ``SameSite=Lax`` + rather than ``Strict`` because a node is reached from links, bookmarks and the + installer's printed URL, and ``Strict`` drops the cookie on the first + navigation from any of them: the operator lands on the dashboard logged out + and logs in again, every time. ``Lax`` still withholds the cookie from + cross-site POSTs, and the ``X-AINode-Client`` rule in the middleware is what + covers the rest. + """ + parts = [f"{SESSION_COOKIE}={token}", "Path=/", "HttpOnly", "SameSite=Lax", + f"Max-Age={int(max_age)}"] + if secure: + parts.append("Secure") + return "; ".join(parts) + + +def _clearing_cookie(secure: bool) -> str: + return session_cookie("", secure, max_age=0) + + +def _log_hook_failure(task) -> None: + if task.cancelled(): + return + exc = task.exception() + if exc is not None: + logger.warning("the users_changed hook failed: %s", exc) + + +async def notify_users_changed(app) -> None: + """Tell whoever registered ``app["users_changed"]`` that accounts changed. + + Optional by design: nothing in this module knows how a fleet replicates, so + the CLI and fleet half of #261 registers a broadcaster here and this module + stays a no-op until it does. An async hook is scheduled rather than awaited, + because the operator's 201 must not wait on a fan-out to every peer, and a + hook that raises is one warning rather than a failed mutation: the accounts + are already on disk by the time it runs. + """ + getter = getattr(app, "get", None) + hook = getter("users_changed") if callable(getter) else None + if not callable(hook): + return + try: + result = hook() + except Exception as exc: + logger.warning("the users_changed hook raised: %s", exc) + return + if inspect.isawaitable(result): + task = asyncio.ensure_future(result) + task.add_done_callback(_log_hook_failure) + + +# ============================================================================= +# The login throttle: pure functions over a mutable log, so a test drives time +# ============================================================================= + +def login_locked_out(state: dict, source: str, now: float, + limit: int = LOGIN_RATE_LIMIT, + window: float = LOGIN_RATE_WINDOW) -> bool: + """Has *source* spent its failed logins for this window? + + Reads and trims; it never records. A successful login must not count against + anybody, so recording is :func:`record_login_failure`'s job alone. + """ + failures = [t for t in state.get(source, []) if now - t < window] + if failures: + state[source] = failures + else: + state.pop(source, None) + return len(failures) >= limit + + +def record_login_failure(state: dict, source: str, now: float, + window: float = LOGIN_RATE_WINDOW, + max_sources: int = LOGIN_RATE_MAX_SOURCES) -> None: + """Log one failed attempt from *source*, trimming the window as it goes.""" + failures = [t for t in state.get(source, []) if now - t < window] + failures.append(now) + state[source] = failures + if len(state) > max_sources: + ordered = sorted(state.items(), key=lambda kv: max(kv[1] or [0])) + for key, _ in ordered[: len(state) - max_sources]: + if key != source: + state.pop(key, None) + + +def clear_login_failures(state: dict, source: str) -> None: + """Forget *source*'s failures. Called on a success. + + Somebody who mistyped twice and then got it right is not halfway to a + lockout, and leaving the failures on the log would make an honest person's + next bad day start from eight. + """ + state.pop(source, None) + + +# ============================================================================= +# The door +# ============================================================================= + +async def handle_login(request: web.Request) -> web.Response: + """POST /api/auth/login {name, password} -- open a session. + + 200 with the account, the session id and the ``Set-Cookie``. 400 for a body + with no name or password, 401 for anything wrong with them, 409 when this node + has no accounts to log into, 429 when this address has spent its attempts. + """ + store = _store(request) + store.reload_if_changed() + state = request.app.get(LOGIN_RATE_STATE_KEY) + if state is None: # an app built without register_session_routes + state = {} + source = client_address(request) + if login_locked_out(state, source, time.time()): + logger.warning("login refused: %s is over %d failures in %.0fs", + source, LOGIN_RATE_LIMIT, LOGIN_RATE_WINDOW) + return _error( + f"Too many failed logins from this address. {LOGIN_RATE_LIMIT} per " + f"{int(LOGIN_RATE_WINDOW)} seconds.", + "rate_limited", 429, + headers={"Retry-After": str(int(LOGIN_RATE_WINDOW))}, + ) + + if not store.has_users(): + # Not a credential failure, so it does not count against the throttle. + # Answered whether or not auth is enabled: on a node with no accounts + # there is no name for "wrong name or password" to be about, and the + # caller needs the command, not a guess. + return _error(NO_ACCOUNTS_MESSAGE, "no_accounts", 409) + + body = await _body(request) + name = str(body.get("name") or "") + password = str(body.get("password") or "") + if not name or not password: + return _error("Send a name and a password.", "invalid_request", 400) + + if not store.verify_password(name, password): + record_login_failure(state, source, time.time()) + logger.info("login refused for %r from %s", normalize_name(name) or name, + source) + return _error(LOGIN_REFUSED_MESSAGE, "auth_error", 401) + + clear_login_failures(state, source) + token, session = store.create_session( + name, + client=_requested_client(request, body), + agent=request.headers.get("User-Agent", ""), + ) + return web.json_response( + {"user": {"name": session["user"], "role": store.role_of(session["user"])}, + "session_id": session["id"]}, + headers={"Set-Cookie": session_cookie(token, request_is_https(request))}, + ) + + +def _requested_client(request: web.Request, body: dict) -> str: + """Which client this session is for: the body says, or the header, or the UI. + + The browser is the default because it is the client that cannot easily add a + field; ``ainode auth login`` passes ``{"client": "cli"}``. + """ + asked = str(body.get("client") or "").strip().casefold() + if asked in CLIENTS: + return asked + sent = str(request.headers.get(CLIENT_HEADER, "") or "").strip().casefold() + if sent in CLIENTS: + return sent + return DASHBOARD_CLIENT + + +async def handle_logout(request: web.Request) -> web.Response: + """POST /api/auth/logout -- revoke this cookie's session and clear it. + + 200 even when there was no session to revoke, because "log me out" has one + meaningful answer and a browser holding a stale cookie is exactly the caller + that needs it cleared. Not in ``SKIP_PATHS``, so on a node with auth ON a + caller presenting nothing at all is still refused by the middleware first; a + dashboard should read that 401 as "already logged out". + """ + store = _store(request) + cookie = str(request.cookies.get(SESSION_COOKIE) or "") + revoked = False + if cookie: + session = store.session_for_token(cookie, touch=False) + if session is not None: + revoked = store.revoke_session(session.get("id")) + return web.json_response( + {"ok": True, "revoked": revoked}, + headers={"Set-Cookie": _clearing_cookie(request_is_https(request))}, + ) + + +async def handle_me(request: web.Request) -> web.Response: + """GET /api/auth/me -- who this request is, if anybody. + + The dashboard's first question, and the reason it can draw a login page + without guessing: ``auth_enabled`` says whether a credential is required at + all, ``has_users`` says whether logging in is even possible on this node, and + ``user`` is null for a caller with no session (including one holding a + perfectly good API key, which is not a person). + """ + store = _store(request) + payload = { + "user": None, + "auth_enabled": _auth_enabled(request), + "has_users": store.has_users(), + } + name = str(request.get("user", "") or "") + if not name: + return web.json_response(payload) + session = store.session_for_token( + str(request.cookies.get(SESSION_COOKIE) or ""), touch=False) + payload["user"] = { + "name": name, + "role": str(request.get("user_role", "") or ""), + "session": public_session(session) if session else None, + } + return web.json_response(payload) + + +# ============================================================================= +# Sessions +# ============================================================================= + +async def handle_list_sessions(request: web.Request) -> web.Response: + """GET /api/auth/sessions[?user=] -- sessions, mine or an account's. + + Own sessions by default. ``?user=`` names somebody else's, which takes the + same administration check as the account routes; naming yourself is always + allowed. A caller with no session and no query (an API key, say) gets an empty + list rather than everybody's: a machine credential is not a person, and this + route is a person's view of their own logins. + """ + store = _store(request) + me = str(request.get("user", "") or "") + asked = normalize_name(request.query.get("user", "")) + if request.query.get("user") and not asked: + return _error("That is not a usable account name.", "invalid_request", 400) + target = asked or me + if target and target != me and not require_admin(request): + return _error("Only an admin can read another account's sessions.", + "forbidden", 403) + return web.json_response({ + "user": target or None, + "sessions": store.sessions_for(target) if target else [], + }) + + +async def handle_revoke_session(request: web.Request) -> web.Response: + """DELETE /api/auth/sessions/{id} -- end one session. + + Mine, or anybody's for an admin. 404 when there is no such session in the + scope this caller may see, which is deliberately the same answer as "that id + belongs to somebody else": a member must not be able to use this route to + learn which session ids exist. + """ + store = _store(request) + session_id = request.match_info["id"] + me = str(request.get("user", "") or "") + if require_admin(request): + owner = None + elif me: + owner = me + else: + return _error("Sign in to manage sessions.", "forbidden", 403) + if not store.revoke_session(session_id, user=owner): + return _error(f"No session '{session_id}' on this node.", "not_found", 404) + return web.json_response({"revoked": True, "session_id": session_id}) + + +async def handle_change_own_password(request: web.Request) -> web.Response: + """POST /api/auth/password {current, new} -- change my own password. + + 403 when ``current`` is wrong, which is what stops a borrowed browser from + becoming a taken-over account. Every OTHER session of this account is revoked + (``UsersStore.set_password``) and this one is replaced with a fresh cookie, so + a password change logs out the laptop somebody else has and not the browser + doing the changing. + """ + store = _store(request) + me = str(request.get("user", "") or "") + if not me: + return _error( + "Sign in to change your own password. An operator key changes an " + "account's password through POST /api/auth/users//password.", + "forbidden", 403) + body = await _body(request) + current = str(body.get("current") or "") + new = str(body.get("new") or "") + if not current or not new: + return _error("Send the current password and the new one.", + "invalid_request", 400) + if not store.verify_password(me, current): + return _error("That is not the current password.", "forbidden", 403) + if len(new) < MIN_PASSWORD_LENGTH: + return _error(f"A password is at least {MIN_PASSWORD_LENGTH} characters.", + "invalid_request", 400) + try: + store.set_password(me, new) + except ValueError as exc: + return _error(str(exc), "invalid_request", 400) + token, session = store.create_session( + me, + client=_requested_client(request, body), + agent=request.headers.get("User-Agent", ""), + ) + await notify_users_changed(request.app) + return web.json_response( + {"ok": True, "session_id": session["id"]}, + headers={"Set-Cookie": session_cookie(token, request_is_https(request))}, + ) + + +# ============================================================================= +# Accounts (admin, an operator key, or the fleet key) +# ============================================================================= + +def admin_refusal(request) -> Optional[web.Response]: + """403 when *request* must not administer this node, else None. + + The one gate in front of every account route, ``/api/auth/keys`` and + ``/api/auth/enable|disable``. Three callers pass (see + ``accounts.require_admin``), and exactly one is refused: a signed-in account + whose role is not admin. + + A caller with no account and no key reaches here only on a node running with + auth OFF, because the middleware refuses every ``/api`` path on a node with + auth on. Such a node is open to anybody who can reach the port, so inventing + a wall here would protect nothing and would make it impossible to create the + first admin from a fresh install. + """ + if require_admin(request): + return None + getter = getattr(request, "get", None) + who = str((getter("user", "") if callable(getter) else "") or "") + if who: + return _error( + f"'{who}' is not an admin on this node. An admin, an API key or a " + f"peer of this cluster can do this.", "forbidden", 403) + return None + + +def fleet_only_refusal(request) -> Optional[web.Response]: + """403 unless *request* presented the FLEET key. + + The replication pair hands out and takes in password hashes, so it is not + gated on "an admin" but on "a node of this cluster": the credential is derived + from ``cluster_secret`` (``auth/fleet.py``), which is the same thing as being + a member of the cluster. An operator who wants the accounts has the file. + """ + getter = getattr(request, "get", None) + key_id = str((getter("api_key_id", "") if callable(getter) else "") or "") + if key_id == FLEET_KEY_ID: + return None + return _error( + "This route is for nodes of this cluster. It authenticates with the " + "fleet key derived from cluster_secret.", "forbidden", 403) + + +async def handle_list_users(request: web.Request) -> web.Response: + """GET /api/auth/users -- every account, with no hash and no salt.""" + refused = admin_refusal(request) + if refused is not None: + return refused + store = _store(request) + return web.json_response({ + "users": store.list_users(), + "admin_count": store.admin_count(), + }) + + +async def handle_add_user(request: web.Request) -> web.Response: + """POST /api/auth/users {name, password, role} -- create an account. + + 201 with the account. 400 with the rule it broke, which is the same string + the CLI prints, because ``UsersStore.add_user`` raises it once for both. + """ + refused = admin_refusal(request) + if refused is not None: + return refused + store = _store(request) + body = await _body(request) + try: + record = store.add_user(body.get("name"), body.get("password"), + str(body.get("role") or ROLE_MEMBER)) + except ValueError as exc: + return _error(str(exc), "invalid_request", 400) + await notify_users_changed(request.app) + return web.json_response({"user": record}, status=201) + + +async def handle_remove_user(request: web.Request) -> web.Response: + """DELETE /api/auth/users/{name} -- remove an account and its sessions. + + 409 when it is the only admin left: a node nobody can administer is not a + state an API should be able to put an operator in. + """ + refused = admin_refusal(request) + if refused is not None: + return refused + store = _store(request) + name = request.match_info["name"] + try: + removed = store.remove_user(name) + except ValueError as exc: + return _error(str(exc), "conflict", 409) + if not removed: + return _error(f"No account named '{name}' on this node.", "not_found", 404) + await notify_users_changed(request.app) + return web.json_response({"removed": True, "name": normalize_name(name)}) + + +async def handle_set_user_password(request: web.Request) -> web.Response: + """POST /api/auth/users/{name}/password {password} -- reset a password. + + Every session that account holds is revoked, because this is the route an + operator uses when somebody has lost a password or should no longer be logged + in, and both cases mean the open browsers have to go. + """ + refused = admin_refusal(request) + if refused is not None: + return refused + store = _store(request) + name = request.match_info["name"] + body = await _body(request) + try: + changed = store.set_password(name, body.get("password")) + except ValueError as exc: + return _error(str(exc), "invalid_request", 400) + if not changed: + return _error(f"No account named '{name}' on this node.", "not_found", 404) + await notify_users_changed(request.app) + return web.json_response({"ok": True, "name": normalize_name(name)}) + + +async def _set_disabled(request: web.Request, disabled: bool) -> web.Response: + refused = admin_refusal(request) + if refused is not None: + return refused + store = _store(request) + name = request.match_info["name"] + try: + changed = store.set_disabled(name, disabled) + except ValueError as exc: + return _error(str(exc), "conflict", 409) + if not changed: + return _error(f"No account named '{name}' on this node.", "not_found", 404) + record = store.find_user(name) + await notify_users_changed(request.app) + return web.json_response({"user": public_user(record or {})}) + + +async def handle_disable_user(request: web.Request) -> web.Response: + """POST /api/auth/users/{name}/disable -- lock an account out. + + Its sessions go with it: an account that can still act through a cookie it + already holds is not disabled. 409 when it is the only admin left. + """ + return await _set_disabled(request, True) + + +async def handle_enable_user(request: web.Request) -> web.Response: + """POST /api/auth/users/{name}/enable -- let an account back in. + + It has no sessions afterwards, because disabling revoked them: the person logs + in again, which is the right amount of ceremony for being let back in. + """ + return await _set_disabled(request, False) + + +# ============================================================================= +# Fleet replication (the fleet key only) +# ============================================================================= + +async def handle_export_users(request: web.Request) -> web.Response: + """GET /api/auth/users/export -- the accounts, hashes included, for a peer. + + ``stamp`` is a content digest of exactly what ``users`` holds, so a caller can + compare two nodes without shipping either list twice. + """ + refused = fleet_only_refusal(request) + if refused is not None: + return refused + store = _store(request) + store.reload_if_changed() + return web.json_response({"users": store.export_users(), + "stamp": store.export_stamp()}) + + +async def handle_sync_users(request: web.Request) -> web.Response: + """POST /api/auth/users/sync {users} -- adopt a peer's account list. + + All or nothing: a malformed record is a 400 and this node's accounts are + untouched, because a half-adopted list is a node whose logins quietly differ + from the rest of the cluster. ``changed`` is false for a list this node + already has, which is what makes a replication pass cheap to repeat. + """ + refused = fleet_only_refusal(request) + if refused is not None: + return refused + store = _store(request) + store.reload_if_changed() + body = await _body(request) + if "users" not in body: + return _error("Send a 'users' list.", "invalid_request", 400) + try: + changed = store.import_users(body.get("users")) + except ValueError as exc: + return _error(str(exc), "invalid_request", 400) + return web.json_response({"changed": changed, "count": len(store.users)}) diff --git a/tests/conftest.py b/tests/conftest.py index b24d3b9e..86df6e19 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,17 @@ """Shared pytest fixtures. -Five are global, all there to keep the suite from reading or touching the machine -it runs on: netdev isolation, the metrics-store redirect, the boot-reconcile -guard and the Hub size lookup (per test), and the +Six are global, all there to keep the suite from reading or touching the machine +it runs on: netdev isolation, the metrics-store redirect, the users-store +redirect, the boot-reconcile guard and the Hub size lookup (per test), and the engine-container guard (per session, at the bottom of this file). +``isolate_users_store``: ``create_app`` loads the login accounts +(``ainode/auth/accounts.py``) the way it loads ``auth.json``, so without this +every test that starts an application would read, and any test that created an +account would WRITE, the developer's own ``~/.ainode/users.json``. Point the +module constant at a temporary file per test. A test that wants a specific path +passes one to ``UsersStore``, which this does not touch. + ``isolate_netdev``: ``ainode.cluster.netdev`` reads the real host (``ip -o -4 addr show``, ``/sys/class/net``) and caches the answer per process, so without this fixture the suite would give different results @@ -83,6 +90,20 @@ def isolate_metrics_store(monkeypatch, tmp_path_factory): yield +@pytest.fixture(autouse=True) +def isolate_users_store(monkeypatch, tmp_path): + """Keep the login accounts out of the operator's real AINODE_HOME. + + ``UsersStore`` resolves ``accounts.USERS_FILE`` at call time rather than in + ``__init__`` precisely so this one patch reaches a store that already exists, + the same way the ``auth_home`` fixtures redirect ``AUTH_FILE``. + """ + from ainode.auth import accounts + + monkeypatch.setattr(accounts, "USERS_FILE", tmp_path / "users.json") + yield + + @pytest.fixture(autouse=True) def isolate_netdev(monkeypatch, tmp_path_factory): from ainode.cluster import netdev diff --git a/tests/test_accounts.py b/tests/test_accounts.py new file mode 100644 index 00000000..84a14cf4 --- /dev/null +++ b/tests/test_accounts.py @@ -0,0 +1,491 @@ +"""The account and session store: what ``users.json`` is allowed to contain. + +Five things are pinned here, and each of them is a promise the login page makes +on behalf of this file (#261): + +1. **A password never reaches disk.** What lands is scrypt over a per-password + salt, and the check is constant time. An unknown or disabled name pays the same + scrypt cost as a real one, so neither the answer nor the clock says which + accounts exist. +2. **A session token never reaches disk either**, only its SHA-256, and it is + returned exactly once. A stolen ``users.json`` logs nobody in. +3. **A session lives until it is revoked**, which is Jason's rule for this + feature, so the tests here are about REVOCATION being complete: a password + change, a disable and a removal all take the open browsers with them. +4. **The file is 0600**, like ``auth.json``, and re-read when it changes, so the + CLI and the server never disagree about who may log in. +5. **Replication is whole records or nothing.** ``import_users`` either adopts a + valid list or leaves this node exactly as it was, because a half-adopted list + is a node whose logins quietly differ from the rest of the cluster. +""" + +import json +import os +import stat +from datetime import datetime, timedelta, timezone + +import pytest + +from ainode.auth.accounts import ( + LAST_SEEN_REFRESH_SECONDS, + MAX_SESSIONS_PER_USER, + MIN_PASSWORD_LENGTH, + ROLE_ADMIN, + ROLE_MEMBER, + UsersStore, + hash_password, + hash_token, + normalize_name, + public_session, + public_user, + require_admin, + verify_hash, +) +from ainode.auth.fleet import FLEET_KEY_ID + + +GOOD = "hunter2hunter2" +OTHER = "correct horse battery" + + +@pytest.fixture +def users_file(tmp_path, monkeypatch): + """Point the module constant at a temp file, the way AUTH_FILE is redirected.""" + path = tmp_path / "users.json" + monkeypatch.setattr("ainode.auth.accounts.USERS_FILE", path) + return path + + +@pytest.fixture +def store(users_file): + return UsersStore() + + +@pytest.fixture +def admin_store(store): + store.add_user("jason", GOOD, ROLE_ADMIN) + return store + + +# ============================================================================= +# Hashing +# ============================================================================= + +class TestHashing: + def test_a_password_hash_is_hex_and_salted(self): + first_hash, first_salt = hash_password(GOOD) + second_hash, second_salt = hash_password(GOOD) + assert len(first_hash) == 64 and len(first_salt) == 32 + bytes.fromhex(first_hash) + assert first_salt != second_salt, "the salt is not per password" + assert first_hash != second_hash, "two salts produced one hash" + + def test_verify_accepts_the_password_and_nothing_else(self): + password_hash, salt = hash_password(GOOD) + assert verify_hash(GOOD, password_hash, salt) is True + assert verify_hash(OTHER, password_hash, salt) is False + assert verify_hash(GOOD, password_hash, "00" * 16) is False + assert verify_hash(GOOD, "", salt) is False + assert verify_hash(GOOD, password_hash, "not hex") is False + + def test_a_token_is_stored_as_its_sha256(self): + assert len(hash_token("abc")) == 64 + assert hash_token("abc") == hash_token("abc") + assert hash_token("abc") != hash_token("abd") + + def test_names_are_case_folded_and_bounded(self): + assert normalize_name("Jason") == "jason" + assert normalize_name(" JASON ") == "jason" + assert normalize_name("a.b_c-1") == "a.b_c-1" + assert normalize_name("") == "" + assert normalize_name("has space") == "" + assert normalize_name("bang!") == "" + assert normalize_name("a" * 64) == "a" * 64 + assert normalize_name("a" * 65) == "" + + +# ============================================================================= +# Accounts +# ============================================================================= + +class TestAccounts: + def test_an_account_round_trips_through_the_file(self, store, users_file): + store.add_user("jason", GOOD, ROLE_ADMIN) + loaded = UsersStore.load() + assert loaded.has_users() is True + assert loaded.admin_count() == 1 + assert loaded.verify_password("jason", GOOD) is True + + def test_the_file_is_0600(self, store, users_file): + store.add_user("jason", GOOD, ROLE_ADMIN) + mode = stat.S_IMODE(os.stat(users_file).st_mode) + assert mode == 0o600, f"users.json is {oct(mode)}" + + def test_a_wide_file_is_tightened_on_load(self, store, users_file): + store.add_user("jason", GOOD, ROLE_ADMIN) + os.chmod(users_file, 0o644) + UsersStore.load() + assert stat.S_IMODE(os.stat(users_file).st_mode) == 0o600 + + def test_the_file_holds_no_plaintext(self, store, users_file): + store.add_user("jason", GOOD, ROLE_ADMIN) + token, _ = store.create_session("jason") + text = users_file.read_text() + assert GOOD not in text + assert token not in text + assert hash_token(token) in text + + def test_list_users_carries_no_hash_and_no_salt(self, admin_store): + row = admin_store.list_users()[0] + assert row == {"name": "jason", "role": ROLE_ADMIN, + "created_at": row["created_at"], "disabled": False, + "sessions": 0} + + def test_a_name_is_normalised_on_the_way_in(self, store): + store.add_user(" JaSoN ", GOOD) + assert store.find_user("jason")["name"] == "jason" + assert store.verify_password("JASON", GOOD) is True + + @pytest.mark.parametrize("name", ["", "has space", "bang!", "a" * 65]) + def test_a_bad_name_is_refused_with_the_rule(self, store, name): + with pytest.raises(ValueError, match="1 to 64 characters"): + store.add_user(name, GOOD) + + def test_a_short_password_is_refused_with_the_rule(self, store): + with pytest.raises(ValueError, match="at least 8 characters"): + store.add_user("jason", "a" * (MIN_PASSWORD_LENGTH - 1)) + + def test_an_unknown_role_is_refused(self, store): + with pytest.raises(ValueError, match="Role must be one of"): + store.add_user("jason", GOOD, "superuser") + + def test_a_duplicate_name_is_refused(self, admin_store): + with pytest.raises(ValueError, match="already an account"): + admin_store.add_user("JASON", OTHER) + + def test_verify_is_false_for_unknown_and_disabled(self, admin_store): + admin_store.add_user("sam", GOOD) + assert admin_store.verify_password("sam", GOOD) is True + assert admin_store.verify_password("sam", OTHER) is False + assert admin_store.verify_password("nobody", GOOD) is False + admin_store.set_disabled("sam", True) + assert admin_store.verify_password("sam", GOOD) is False + assert admin_store.role_of("sam") == "" + admin_store.set_disabled("sam", False) + assert admin_store.verify_password("sam", GOOD) is True + assert admin_store.role_of("sam") == ROLE_MEMBER + + def test_a_password_change_revokes_every_session(self, admin_store): + admin_store.create_session("jason") + admin_store.create_session("jason") + assert len(admin_store.sessions_for("jason")) == 2 + assert admin_store.set_password("jason", OTHER) is True + assert admin_store.sessions_for("jason") == [] + assert admin_store.verify_password("jason", OTHER) is True + assert admin_store.verify_password("jason", GOOD) is False + + def test_a_password_change_on_an_unknown_account_is_false(self, store): + assert store.set_password("nobody", GOOD) is False + + def test_a_short_new_password_is_refused(self, admin_store): + with pytest.raises(ValueError, match="at least 8 characters"): + admin_store.set_password("jason", "short") + + def test_disabling_revokes_the_sessions_it_leaves_behind(self, admin_store): + admin_store.add_user("sam", GOOD) + token, _ = admin_store.create_session("sam") + assert admin_store.session_for_token(token) is not None + admin_store.set_disabled("sam", True) + assert admin_store.sessions_for("sam") == [] + assert admin_store.session_for_token(token) is None + + def test_removing_an_account_takes_its_sessions(self, admin_store): + admin_store.add_user("sam", GOOD) + token, _ = admin_store.create_session("sam") + assert admin_store.remove_user("sam") is True + assert admin_store.session_for_token(token) is None + assert admin_store.sessions == [] + + def test_removing_an_unknown_account_is_false(self, store): + assert store.remove_user("nobody") is False + + def test_the_last_admin_cannot_be_removed_or_disabled(self, admin_store): + assert admin_store.is_last_admin("jason") is True + with pytest.raises(ValueError, match="only admin"): + admin_store.remove_user("jason") + with pytest.raises(ValueError, match="only admin"): + admin_store.set_disabled("jason", True) + assert admin_store.admin_count() == 1 + + def test_a_second_admin_unlocks_the_first(self, admin_store): + admin_store.add_user("sam", GOOD, ROLE_ADMIN) + assert admin_store.admin_count() == 2 + assert admin_store.is_last_admin("jason") is False + assert admin_store.remove_user("jason") is True + assert admin_store.is_last_admin("sam") is True + + def test_a_member_is_never_the_last_admin(self, admin_store): + admin_store.add_user("sam", GOOD) + assert admin_store.is_last_admin("sam") is False + assert admin_store.remove_user("sam") is True + + def test_a_disabled_admin_is_not_counted(self, admin_store): + admin_store.add_user("sam", GOOD, ROLE_ADMIN) + admin_store.set_disabled("sam", True) + assert admin_store.admin_count() == 1 + assert admin_store.is_last_admin("sam") is False + + +# ============================================================================= +# Sessions +# ============================================================================= + +class TestSessions: + def test_a_session_is_found_by_its_token_and_by_nothing_else(self, admin_store): + token, session = admin_store.create_session("jason", client="cli", + agent="curl/8") + assert session["user"] == "jason" + assert session["client"] == "cli" + assert session["token_hash"] == hash_token(token) + assert admin_store.session_for_token(token)["id"] == session["id"] + assert admin_store.session_for_token(token + "x") is None + assert admin_store.session_for_token("") is None + + def test_the_agent_is_cut_to_80_characters(self, admin_store): + _, session = admin_store.create_session("jason", agent="M" * 300) + assert len(session["agent"]) == 80 + + def test_a_session_cannot_be_minted_for_an_unknown_or_disabled_account( + self, admin_store): + admin_store.add_user("sam", GOOD) + admin_store.set_disabled("sam", True) + with pytest.raises(ValueError, match="No such account"): + admin_store.create_session("nobody") + with pytest.raises(ValueError, match="No such account"): + admin_store.create_session("sam") + + def test_sessions_for_hides_the_token_hash(self, admin_store): + admin_store.create_session("jason") + row = admin_store.sessions_for("jason")[0] + assert "token_hash" not in row + assert set(row) == {"id", "user", "client", "agent", "created_at", + "last_seen"} + + def test_last_seen_is_refreshed_at_most_once_a_minute(self, admin_store): + token, session = admin_store.create_session("jason") + first = session["last_seen"] + admin_store.session_for_token(token) + assert session["last_seen"] == first, "a second lookup rewrote last_seen" + + stale = (datetime.now(timezone.utc) + - timedelta(seconds=LAST_SEEN_REFRESH_SECONDS + 5) + ).strftime("%Y-%m-%dT%H:%M:%SZ") + session["last_seen"] = stale + admin_store.session_for_token(token) + assert session["last_seen"] > stale, "a stale session was never refreshed" + # Back to "about now", which within one test run is the stamp the session + # was minted with; the point is that it moved off the stale value. + assert session["last_seen"] >= first + + def test_a_refused_lookup_does_not_count_as_activity(self, admin_store): + token, session = admin_store.create_session("jason") + session["last_seen"] = "2020-01-01T00:00:00Z" + assert admin_store.session_for_token(token, touch=False) is not None + assert session["last_seen"] == "2020-01-01T00:00:00Z" + + def test_the_cap_evicts_the_oldest(self, admin_store): + ids = [] + for _ in range(MAX_SESSIONS_PER_USER + 3): + _, session = admin_store.create_session("jason") + ids.append(session["id"]) + live = [row["id"] for row in admin_store.sessions_for("jason")] + assert len(live) == MAX_SESSIONS_PER_USER + assert live == ids[3:], "the cap dropped something other than the oldest" + + def test_the_cap_is_per_account(self, admin_store): + admin_store.add_user("sam", GOOD) + for _ in range(MAX_SESSIONS_PER_USER + 2): + admin_store.create_session("jason") + admin_store.create_session("sam") + assert len(admin_store.sessions_for("jason")) == MAX_SESSIONS_PER_USER + assert len(admin_store.sessions_for("sam")) == 1 + + def test_revoking_is_scoped_to_its_owner_when_asked(self, admin_store): + admin_store.add_user("sam", GOOD) + _, mine = admin_store.create_session("jason") + _, theirs = admin_store.create_session("sam") + assert admin_store.revoke_session(theirs["id"], user="jason") is False + assert admin_store.revoke_session(theirs["id"], user="sam") is True + assert admin_store.revoke_session(mine["id"]) is True + assert admin_store.sessions == [] + assert admin_store.revoke_session(mine["id"]) is False + assert admin_store.revoke_session("") is False + + def test_revoking_every_session_of_an_account(self, admin_store): + admin_store.add_user("sam", GOOD) + admin_store.create_session("jason") + admin_store.create_session("jason") + admin_store.create_session("sam") + assert admin_store.revoke_sessions_for("jason") == 2 + assert admin_store.revoke_sessions_for("jason") == 0 + assert admin_store.revoke_sessions_for("nobody") == 0 + assert len(admin_store.sessions_for("sam")) == 1 + + def test_a_session_survives_a_restart(self, admin_store, users_file): + token, session = admin_store.create_session("jason") + reopened = UsersStore.load() + found = reopened.session_for_token(token) + assert found is not None and found["id"] == session["id"] + + def test_public_helpers_never_leak(self): + assert "token_hash" not in public_session({"token_hash": "x", "id": "1"}) + assert set(public_user({"name": "a"})) == {"name", "role", "created_at", + "disabled"} + + +# ============================================================================= +# Live reload +# ============================================================================= + +class TestReload: + def test_a_change_on_disk_is_adopted(self, store, users_file): + store.add_user("jason", GOOD, ROLE_ADMIN) + reader = UsersStore.load() + assert reader.reload_if_changed() is False + + store.add_user("sam", GOOD) + assert reader.reload_if_changed() is True + assert {u["name"] for u in reader.list_users()} == {"jason", "sam"} + assert reader.reload_if_changed() is False + + def test_a_missing_or_broken_file_keeps_the_state_in_memory(self, store, + users_file): + store.add_user("jason", GOOD, ROLE_ADMIN) + reader = UsersStore.load() + + users_file.write_text("{ not json") + assert reader.reload_if_changed() is False + assert reader.has_users() is True + + users_file.write_text('["a list, not an object"]') + assert reader.reload_if_changed() is False + assert reader.has_users() is True + + users_file.unlink() + assert reader.reload_if_changed() is False + assert reader.has_users() is True + + def test_load_raises_on_a_malformed_file(self, users_file): + users_file.write_text("{ not json") + with pytest.raises(ValueError): + UsersStore.load() + + def test_load_of_a_missing_file_is_an_empty_store(self, users_file): + empty = UsersStore.load() + assert empty.has_users() is False + assert empty.admin_count() == 0 + assert empty.list_users() == [] + + +# ============================================================================= +# Fleet replication +# ============================================================================= + +class TestReplication: + def test_export_carries_the_hashes_and_is_sorted(self, admin_store): + admin_store.add_user("sam", GOOD) + rows = admin_store.export_users() + assert [r["name"] for r in rows] == ["jason", "sam"] + assert set(rows[0]) == {"name", "password_hash", "salt", "role", + "created_at", "disabled"} + assert rows[0]["password_hash"] and rows[0]["salt"] + + def test_the_stamp_follows_the_content_and_not_the_file(self, admin_store, + users_file): + stamp = admin_store.export_stamp() + assert UsersStore.load().export_stamp() == stamp + admin_store.add_user("sam", GOOD) + assert admin_store.export_stamp() != stamp + + def test_import_replaces_the_list_and_lets_a_login_work(self, admin_store, + users_file, + tmp_path, monkeypatch): + exported = admin_store.export_users() + + other = tmp_path / "peer-users.json" + monkeypatch.setattr("ainode.auth.accounts.USERS_FILE", other) + peer = UsersStore() + assert peer.import_users(exported) is True + assert peer.verify_password("jason", GOOD) is True, ( + "a replicated hash must verify without asking the head") + assert peer.admin_count() == 1 + + def test_import_of_the_same_list_changes_nothing(self, admin_store): + assert admin_store.import_users(admin_store.export_users()) is False + + def test_import_keeps_the_sessions_whose_account_survived(self, admin_store): + admin_store.add_user("sam", GOOD) + mine, _ = admin_store.create_session("jason") + theirs, _ = admin_store.create_session("sam") + without_sam = [r for r in admin_store.export_users() if r["name"] != "sam"] + assert admin_store.import_users(without_sam) is True + assert admin_store.session_for_token(mine) is not None + assert admin_store.session_for_token(theirs) is None + + @pytest.mark.parametrize("payload, match", [ + ("not a list", "must be a list"), + (["not an object"], "JSON object"), + ([{"name": "has space", "password_hash": "a", "salt": "b"}], "1 to 64"), + ([{"name": "jason", "salt": "b"}], "no password hash"), + ([{"name": "jason", "password_hash": "a"}], "no password hash"), + ([{"name": "jason", "password_hash": "a", "salt": "b", "role": "root"}], + "unknown role"), + ([{"name": "jason", "password_hash": "a", "salt": "b"}, + {"name": "JASON", "password_hash": "c", "salt": "d"}], "twice"), + ]) + def test_a_malformed_import_leaves_the_node_alone(self, admin_store, payload, + match): + before = admin_store.export_users() + with pytest.raises(ValueError, match=match): + admin_store.import_users(payload) + assert admin_store.export_users() == before + + def test_an_import_lands_on_disk(self, admin_store, users_file): + admin_store.import_users([ + {"name": "sam", "password_hash": "a" * 64, "salt": "b" * 32, + "role": ROLE_ADMIN, "created_at": "2026-01-01T00:00:00Z"}, + ]) + data = json.loads(users_file.read_text()) + assert [u["name"] for u in data["users"]] == ["sam"] + + +# ============================================================================= +# require_admin +# ============================================================================= + +class TestRequireAdmin: + def test_an_admin_session_may_administer(self): + assert require_admin({"user": "jason", "user_role": ROLE_ADMIN, + "authenticated": True, + "api_key_id": "user:jason"}) is True + + def test_a_member_session_may_not(self): + assert require_admin({"user": "sam", "user_role": ROLE_MEMBER, + "authenticated": True, + "api_key_id": "user:sam"}) is False + + def test_an_operator_key_may(self): + """How the CLI works, and how the FIRST admin gets created.""" + assert require_admin({"user": "", "user_role": "", + "authenticated": True, "api_key_id": "abc123"}) is True + + def test_the_fleet_key_may(self): + assert require_admin({"user": "", "user_role": "", "authenticated": True, + "api_key_id": FLEET_KEY_ID}) is True + + def test_nobody_may_not(self): + assert require_admin({"user": "", "user_role": "", "authenticated": False, + "api_key_id": ""}) is False + + def test_a_stub_with_no_mapping_reads_as_not_an_admin(self): + assert require_admin(object()) is False diff --git a/tests/test_auth_gate.py b/tests/test_auth_gate.py index a2634389..a00859f3 100644 --- a/tests/test_auth_gate.py +++ b/tests/test_auth_gate.py @@ -1,13 +1,17 @@ """With auth on, every route needs the key except the few that cannot. The rule this file pins (#168): when auth is enabled, every path under ``/api`` -and ``/v1`` requires ``Authorization: Bearer ``, except ``/api/health``, -``/api/auth/status``, ``/api/cluster/endpoint``, ``POST /api/cluster/join`` and -the static shell. It is checked against the REAL route -table (``create_app``), not a list of paths typed out here, because a list is -what drifts: the next mutating route somebody adds is covered by the rule and +and ``/v1`` requires a credential, except ``/api/health``, ``/api/auth/status``, +``/api/auth/login``, ``/api/auth/me``, ``/api/cluster/endpoint``, +``POST /api/cluster/join`` and the static shell. It is checked against the REAL +route table (``create_app``), not a list of paths typed out here, because a list +is what drifts: the next mutating route somebody adds is covered by the rule and by this test on the day it is registered. +"A credential" is three things since #261: an operator API key, the fleet key, or +a login session cookie. This file drives the first; ``tests/test_session_routes.py`` +drives the third, including the CSRF header a cookie-authenticated write needs. + The second half pins the ``trust_remote_code`` rule, which holds even when auth is switched off: setting it needs a key, or a curated catalog entry that already declares it. @@ -97,7 +101,14 @@ async def keyed(app): # key yet, so a single-use expiring join token is the # credential and the handler rate limits per source IP # (api/cluster_join.py). Covered by tests/test_join_flow.py. +# /api/auth/login the door: the caller with no credential is exactly who +# posts to it. It has its own per-address throttle on FAILED +# attempts instead (tests/test_session_routes.py). +# /api/auth/me answers {"user": null} to a caller it does not recognise, +# so the dashboard can draw the login page instead of +# guessing. It tells a stranger nothing else. OPEN_WITH_AUTH_ON = {"/", "/api/health", "/api/auth/status", + "/api/auth/login", "/api/auth/me", "/api/cluster/endpoint", "/api/cluster/join"} # Routes whose path carries a variable. Filled in with something harmless: the diff --git a/tests/test_fleet_auth.py b/tests/test_fleet_auth.py index a745726d..0316f805 100644 --- a/tests/test_fleet_auth.py +++ b/tests/test_fleet_auth.py @@ -1151,6 +1151,15 @@ def test_the_cli_says_the_change_is_live(capsys): # 8. The copy that describes the rule matches the rule # ============================================================================= +#: Keyless paths whose sentence in the dashboard's API access panel lands with the +#: FRONT-END half of #261 (``ainode/web/static/js/app.js``, a different worker's +#: file in the same wave, PR 264). Only these two are excused, and only until that +#: PR lands: once the panel names them, DELETE them from this set, because every +#: entry left here is a keyless path this test has stopped checking. The set is +#: asserted to be a subset of SKIP_PATHS, so it cannot outlive the paths it names. +COPY_PENDING_IN_THE_PANEL = {"/api/auth/login", "/api/auth/me"} + + def test_the_dashboard_panel_lists_every_keyless_path(): """One rule, one wording: the panel used to name three of the six.""" from ainode.auth.middleware import SKIP_PATHS, SKIP_PREFIXES @@ -1160,8 +1169,16 @@ def test_the_dashboard_panel_lists_every_keyless_path(): if "config-section-desc" in line and "Who may call this node" in line] assert len(section) == 1, "the API access panel's description moved" text = section[0] - for path in SKIP_PATHS: - assert f"{path}" in text, f"{path} is keyless and unmentioned" + assert COPY_PENDING_IN_THE_PANEL <= SKIP_PATHS, ( + "COPY_PENDING_IN_THE_PANEL names a path that is no longer keyless: " + f"{COPY_PENDING_IN_THE_PANEL - SKIP_PATHS}. Delete those entries.") + missing = [path for path in sorted(SKIP_PATHS - COPY_PENDING_IN_THE_PANEL) + if f"{path}" not in text] + assert not missing, ( + f"these paths answer with no key and the dashboard's API access panel does " + f"not name them: {', '.join(missing)}. Add each as the path to " + f"the 'Who may call this node' paragraph in " + f"ainode/web/static/js/app.js, with the reason it is open.") for prefix in SKIP_PREFIXES: assert prefix.rstrip("/") in text, f"{prefix} is keyless and unmentioned" diff --git a/tests/test_session_routes.py b/tests/test_session_routes.py new file mode 100644 index 00000000..d63b5b1d --- /dev/null +++ b/tests/test_session_routes.py @@ -0,0 +1,962 @@ +"""The login routes, driven through the real middleware (#261). + +Same shape as ``tests/test_auth.py``: a small aiohttp app carrying the auth +middleware, the real auth and session routes, and a couple of protected routes to +aim a credential at. What it pins: + +* **The door.** A good login sets one cookie with exactly the attributes the + browser needs, a bad one says "Wrong name or password" whether or not the name + exists, a node with no accounts says how to make the first admin, and an address + that keeps guessing is throttled. +* **The cookie is a third credential**, equal to a key on a GET and refused on a + write that does not carry ``X-AINode-Client: dashboard``. That header is the + whole CSRF defence, so it is checked in both directions. +* **A Bearer token still wins.** The dashboard, the bench and the desktop app + keep working exactly as they did. +* **Roles mean something.** A member session is refused by every account route, + by the key routes and by the auth switch; an admin session, an operator key and + the fleet key are not. The last enabled admin cannot be removed or disabled. +* **Replication is the fleet's alone.** Export and sync answer 403 to an admin + and to an operator key, because they move password hashes and the credential for + that is membership of the cluster. +* **The keyless list is exactly two longer.** ``SKIP_PATHS`` is asserted here as a + literal set, because it is the one list in the codebase where a typo is an open + door. +""" + +import asyncio +import time + +import pytest +import pytest_asyncio +from aiohttp import DummyCookieJar, web +from aiohttp.test_utils import TestClient, TestServer + +from ainode.auth.accounts import ROLE_ADMIN, ROLE_MEMBER, UsersStore +from ainode.auth.api_routes import register_auth_routes +from ainode.auth.fleet import fleet_key +from ainode.auth.middleware import ( + CLIENT_HEADER, + MISSING_KEY_MESSAGE, + SESSION_COOKIE, + SKIP_PATHS, + AuthConfig, + auth_middleware, +) +from ainode.auth.session_routes import ( + COOKIE_MAX_AGE, + LOGIN_RATE_LIMIT, + LOGIN_RATE_STATE_KEY, + LOGIN_RATE_WINDOW, + NO_ACCOUNTS_MESSAGE, + clear_login_failures, + login_locked_out, + record_login_failure, + register_session_routes, + session_cookie, +) +from ainode.core.config import NodeConfig + + +SECRET = "0123456789abcdef0123456789abcdef" +ADMIN_PASSWORD = "hunter2hunter2" +MEMBER_PASSWORD = "correct horse battery" +DASH = {CLIENT_HEADER: "dashboard"} + + +# ============================================================================= +# Fixtures +# ============================================================================= + +@pytest.fixture +def stores(tmp_path, monkeypatch): + """A fresh ``auth.json`` and ``users.json`` in a temp home.""" + monkeypatch.setattr("ainode.auth.middleware.AUTH_FILE", tmp_path / "auth.json") + monkeypatch.setattr("ainode.auth.middleware.AINODE_HOME", tmp_path) + monkeypatch.setattr("ainode.auth.accounts.USERS_FILE", tmp_path / "users.json") + users = UsersStore() + users.add_user("jason", ADMIN_PASSWORD, ROLE_ADMIN) + users.add_user("sam", MEMBER_PASSWORD, ROLE_MEMBER) + return AuthConfig(), users + + +async def _whoami(request): + """Echoes what the middleware stamped, which is the contract under test.""" + return web.json_response({ + "authenticated": request["authenticated"], + "api_key_id": request["api_key_id"], + "user": request["user"], + "user_role": request["user_role"], + }) + + +async def _ok(_request): + return web.json_response({"ok": True}) + + +def _make_app(auth_config, users, secret=SECRET): + app = web.Application(middlewares=[auth_middleware]) + app["config"] = NodeConfig(node_id="login-node", node_name="Login", + cluster_secret=secret, onboarded=True) + app["auth_config"] = auth_config + app["users_store"] = users + app.router.add_get("/api/whoami", _whoami) + app.router.add_post("/api/whoami", _whoami) + app.router.add_post("/api/models/unload", _ok) + register_auth_routes(app) + register_session_routes(app) + return app + + +def _client(app): + """A client with NO cookie jar, so every cookie in this file is explicit. + + A jar would re-send whichever session logged in last, which is exactly what a + browser does and exactly what makes a multi-account test lie: "the member + cannot see the admin's sessions" passes for the wrong reason when the jar + quietly upgraded the request to the admin's cookie. Every test here names the + session it is using in a ``Cookie`` header instead. + """ + return TestClient(TestServer(app), cookie_jar=DummyCookieJar()) + + +@pytest_asyncio.fixture +async def open_client(stores): + """Auth off, which is still the default on an installed node before 0.5.30.""" + auth_config, users = stores + async with _client(_make_app(auth_config, users)) as client: + yield client + + +@pytest_asyncio.fixture +async def protected(stores): + """Auth on, with one operator key whose plaintext exists only here.""" + auth_config, users = stores + entry = auth_config.enable("test") + async with _client(_make_app(auth_config, users)) as client: + yield client, entry["key"] + + +async def _login(client, name, password, **kwargs): + return await client.post("/api/auth/login", + json={"name": name, "password": password}, **kwargs) + + +async def _cookie_of(client, name, password): + """Log in and hand back the raw token, for a test that sets Cookie itself.""" + resp = await _login(client, name, password) + assert resp.status == 200, await resp.text() + header = resp.headers["Set-Cookie"] + return header.split(";")[0].split("=", 1)[1] + + +def _jar(token): + return {"Cookie": f"{SESSION_COOKIE}={token}"} + + +# ============================================================================= +# SKIP_PATHS +# ============================================================================= + +def test_the_keyless_list_is_exactly_these_seven(): + """The one list where a typo is an open door, so it is typed out in full.""" + assert SKIP_PATHS == { + "/", + "/api/health", + "/api/auth/status", + "/api/auth/login", + "/api/auth/me", + "/api/cluster/endpoint", + "/api/cluster/join", + }, ("SKIP_PATHS changed. Every path here answers with NO credential on a node " + "with auth on, so a new entry needs a reason in the middleware docstring, " + "a sentence in the dashboard's API access panel " + "(tests/test_fleet_auth.py::test_the_dashboard_panel_lists_every_keyless_path) " + "and a line in tests/test_auth_gate.py::OPEN_WITH_AUTH_ON.") + + +def test_logout_is_not_keyless(): + """Deliberate: logout revokes something, so it is a write like any other. + + A caller with no credential at all has no session to end, and on a node with + auth on it gets the middleware's 401. A dashboard reads that as "already + logged out" and clears its own state. + """ + assert "/api/auth/logout" not in SKIP_PATHS + + +# ============================================================================= +# The door +# ============================================================================= + +class TestLogin: + @pytest.mark.asyncio + async def test_a_good_login_answers_the_account_and_one_cookie(self, protected): + client, _ = protected + resp = await _login(client, "jason", ADMIN_PASSWORD) + assert resp.status == 200 + body = await resp.json() + assert body["user"] == {"name": "jason", "role": ROLE_ADMIN} + assert body["session_id"] + + cookie = resp.headers["Set-Cookie"] + assert cookie.startswith(f"{SESSION_COOKIE}=") + assert "; Path=/" in cookie + assert "; HttpOnly" in cookie + assert "; SameSite=Lax" in cookie + assert f"; Max-Age={COOKIE_MAX_AGE}" in cookie + assert "Secure" not in cookie, "a plain-HTTP node must not set Secure" + token = cookie.split(";")[0].split("=", 1)[1] + assert token not in await resp.text(), "the token is in the body too" + + @pytest.mark.asyncio + async def test_the_name_is_case_folded(self, protected): + client, _ = protected + resp = await _login(client, " JASON ", ADMIN_PASSWORD) + assert resp.status == 200 + assert (await resp.json())["user"]["name"] == "jason" + + @pytest.mark.asyncio + async def test_https_adds_secure(self, protected): + """Behind a terminating proxy the node sees HTTP plus the header.""" + client, _ = protected + resp = await client.post("/api/auth/login", + json={"name": "jason", "password": ADMIN_PASSWORD}, + headers={"X-Forwarded-Proto": "https"}) + assert "; Secure" in resp.headers["Set-Cookie"] + + @pytest.mark.asyncio + async def test_a_wrong_password_and_an_unknown_name_answer_the_same(self, + protected): + client, _ = protected + wrong = await _login(client, "jason", "not the password") + missing = await _login(client, "nobody", ADMIN_PASSWORD) + assert wrong.status == missing.status == 401 + assert (await wrong.json()) == (await missing.json()) + assert (await wrong.json())["error"] == {"message": "Wrong name or password", + "type": "auth_error"} + + @pytest.mark.asyncio + async def test_a_body_with_nothing_in_it_is_a_400(self, protected): + client, _ = protected + resp = await client.post("/api/auth/login", json={}) + assert resp.status == 400 + assert "name and a password" in (await resp.json())["error"]["message"] + + @pytest.mark.asyncio + async def test_a_disabled_account_cannot_log_in(self, protected): + client, _ = protected + client.app["users_store"].set_disabled("sam", True) + resp = await _login(client, "sam", MEMBER_PASSWORD) + assert resp.status == 401 + + @pytest.mark.asyncio + async def test_a_node_with_no_accounts_says_how_to_make_the_first(self, stores): + auth_config, users = stores + users.import_users([]) + auth_config.enable("test") + async with _client(_make_app(auth_config, users)) as client: + resp = await _login(client, "jason", ADMIN_PASSWORD) + assert resp.status == 409 + body = await resp.json() + assert body["error"]["message"] == NO_ACCOUNTS_MESSAGE + assert "ainode auth user add" in body["error"]["message"] + + @pytest.mark.asyncio + async def test_login_is_open_with_no_credential_on_a_protected_node(self, + protected): + """The whole point of the exemption: no key, and the door still opens.""" + client, _ = protected + resp = await client.get("/api/whoami") + assert resp.status == 401 + assert (await _login(client, "jason", ADMIN_PASSWORD)).status == 200 + + @pytest.mark.asyncio + async def test_the_cli_can_name_itself(self, protected): + client, _ = protected + resp = await client.post("/api/auth/login", + json={"name": "jason", "password": ADMIN_PASSWORD, + "client": "cli"}) + assert resp.status == 200 + sessions = client.app["users_store"].sessions_for("jason") + assert [s["client"] for s in sessions] == ["cli"] + + +class TestThrottle: + def test_the_counter_only_counts_failures(self): + state, now = {}, 1000.0 + assert login_locked_out(state, "1.2.3.4", now) is False + for n in range(LOGIN_RATE_LIMIT): + assert login_locked_out(state, "1.2.3.4", now) is False + record_login_failure(state, "1.2.3.4", now) + assert login_locked_out(state, "1.2.3.4", now) is True + assert login_locked_out(state, "5.6.7.8", now) is False, "one address, one bucket" + + def test_the_window_expires(self): + state = {} + for _ in range(LOGIN_RATE_LIMIT): + record_login_failure(state, "1.2.3.4", 1000.0) + assert login_locked_out(state, "1.2.3.4", 1000.0) is True + assert login_locked_out(state, "1.2.3.4", 1000.0 + LOGIN_RATE_WINDOW) is False + assert "1.2.3.4" not in state, "the log did not trim itself" + + def test_a_success_clears_the_slate(self): + state = {} + record_login_failure(state, "1.2.3.4", 1000.0) + clear_login_failures(state, "1.2.3.4") + assert state == {} + + def test_the_log_does_not_grow_without_bound(self): + state = {} + for n in range(40): + record_login_failure(state, f"10.0.0.{n}", 1000.0 + n, max_sources=8) + assert len(state) <= 8 + + @pytest.mark.asyncio + async def test_a_guessing_address_gets_429_with_retry_after(self, protected): + client, _ = protected + resp = await _login(client, "jason", "wrong") + assert resp.status == 401 + state = client.app[LOGIN_RATE_STATE_KEY] + source = next(iter(state)) + assert len(state[source]) == 1, "a failure was not recorded" + + state[source] = [time.time()] * LOGIN_RATE_LIMIT + resp = await _login(client, "jason", ADMIN_PASSWORD) + assert resp.status == 429 + assert resp.headers["Retry-After"] == str(int(LOGIN_RATE_WINDOW)) + assert (await resp.json())["error"]["type"] == "rate_limited" + + @pytest.mark.asyncio + async def test_a_good_login_clears_the_failures(self, protected): + client, _ = protected + await _login(client, "jason", "wrong") + state = client.app[LOGIN_RATE_STATE_KEY] + assert state + assert (await _login(client, "jason", ADMIN_PASSWORD)).status == 200 + assert state == {} + + @pytest.mark.asyncio + async def test_a_node_with_no_accounts_does_not_count_against_the_address( + self, stores): + auth_config, users = stores + users.import_users([]) + async with _client(_make_app(auth_config, users)) as client: + for _ in range(LOGIN_RATE_LIMIT + 2): + assert (await _login(client, "jason", ADMIN_PASSWORD)).status == 409 + assert client.app[LOGIN_RATE_STATE_KEY] == {} + + +# ============================================================================= +# The cookie as a credential +# ============================================================================= + +class TestCookieAuth: + @pytest.mark.asyncio + async def test_a_session_gets_a_get_past_the_wall(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.get("/api/whoami", headers=_jar(token)) + assert resp.status == 200 + assert await resp.json() == {"authenticated": True, + "api_key_id": "user:jason", + "user": "jason", "user_role": ROLE_ADMIN} + + @pytest.mark.asyncio + async def test_a_write_needs_the_client_header(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + + resp = await client.post("/api/whoami", headers=_jar(token)) + assert resp.status == 401 + message = (await resp.json())["error"]["message"] + assert CLIENT_HEADER in message, "the 401 does not name the missing header" + assert "Wrong name or password" not in message + + resp = await client.post("/api/whoami", headers={**_jar(token), **DASH}) + assert resp.status == 200 + assert (await resp.json())["user"] == "jason" + + @pytest.mark.asyncio + async def test_a_wrong_client_header_is_no_header(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.post("/api/whoami", + headers={**_jar(token), CLIENT_HEADER: "someone"}) + assert resp.status == 401 + + @pytest.mark.asyncio + async def test_the_header_is_case_insensitive_in_its_value(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.post("/api/whoami", + headers={**_jar(token), CLIENT_HEADER: "Dashboard"}) + assert resp.status == 200 + + @pytest.mark.asyncio + async def test_a_stale_cookie_is_simply_unauthenticated(self, protected): + client, _ = protected + resp = await client.get("/api/whoami", headers=_jar("not-a-token")) + assert resp.status == 401 + assert (await resp.json())["error"]["message"] == MISSING_KEY_MESSAGE + + @pytest.mark.asyncio + async def test_a_revoked_session_stops_working_at_once(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + assert (await client.get("/api/whoami", headers=_jar(token))).status == 200 + client.app["users_store"].revoke_sessions_for("jason") + assert (await client.get("/api/whoami", headers=_jar(token))).status == 401 + + @pytest.mark.asyncio + async def test_disabling_an_account_ends_its_session(self, protected): + client, _ = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + assert (await client.get("/api/whoami", headers=_jar(token))).status == 200 + client.app["users_store"].set_disabled("sam", True) + assert (await client.get("/api/whoami", headers=_jar(token))).status == 401 + + @pytest.mark.asyncio + async def test_a_bearer_key_wins_over_a_cookie(self, protected): + client, key = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.get("/api/whoami", + headers={**_jar(token), + "Authorization": f"Bearer {key}"}) + body = await resp.json() + assert body["user"] == "", "the cookie overrode the key" + assert body["api_key_id"] not in ("", "user:jason") + + @pytest.mark.asyncio + async def test_a_stale_key_does_not_cost_a_good_session_its_request(self, + protected): + """A browser that logged in with an old key still in localStorage.""" + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.get("/api/whoami", + headers={**_jar(token), + "Authorization": "Bearer stale-key"}) + assert resp.status == 200 + assert (await resp.json())["user"] == "jason" + + @pytest.mark.asyncio + async def test_a_key_still_needs_no_client_header(self, protected): + """Nothing about #261 changes what the bench and the desktop app send.""" + client, key = protected + resp = await client.post("/api/models/unload", + headers={"Authorization": f"Bearer {key}"}) + assert resp.status == 200 + + @pytest.mark.asyncio + async def test_a_member_session_reads_as_a_member(self, protected): + client, _ = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + body = await (await client.get("/api/whoami", headers=_jar(token))).json() + assert body["user_role"] == ROLE_MEMBER + assert body["api_key_id"] == "user:sam" + + @pytest.mark.asyncio + async def test_a_session_authenticates_on_an_open_node_too(self, open_client): + """``is_authenticated`` gates trust_remote_code even with auth off.""" + token = await _cookie_of(open_client, "jason", ADMIN_PASSWORD) + body = await (await open_client.get("/api/whoami", + headers=_jar(token))).json() + assert body["authenticated"] is True + assert body["user"] == "jason" + + def test_the_cookie_builder_is_the_one_spelling(self): + plain = session_cookie("tok", False) + assert plain == (f"{SESSION_COOKIE}=tok; Path=/; HttpOnly; SameSite=Lax; " + f"Max-Age={COOKIE_MAX_AGE}") + assert session_cookie("tok", True).endswith("; Secure") + assert session_cookie("", False, max_age=0).startswith( + f"{SESSION_COOKIE}=; ") + + +# ============================================================================= +# logout and me +# ============================================================================= + +class TestLogoutAndMe: + @pytest.mark.asyncio + async def test_logout_revokes_the_session_and_clears_the_cookie(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.post("/api/auth/logout", + headers={**_jar(token), **DASH}) + assert resp.status == 200 + assert (await resp.json()) == {"ok": True, "revoked": True} + assert "Max-Age=0" in resp.headers["Set-Cookie"] + assert client.app["users_store"].sessions_for("jason") == [] + assert (await client.get("/api/whoami", headers=_jar(token))).status == 401 + + @pytest.mark.asyncio + async def test_logout_with_no_session_is_still_200(self, open_client): + resp = await open_client.post("/api/auth/logout", headers=DASH) + assert resp.status == 200 + assert (await resp.json()) == {"ok": True, "revoked": False} + assert "Max-Age=0" in resp.headers["Set-Cookie"] + + @pytest.mark.asyncio + async def test_logout_only_ends_the_session_that_asked(self, protected): + client, _ = protected + mine = await _cookie_of(client, "jason", ADMIN_PASSWORD) + await _cookie_of(client, "jason", ADMIN_PASSWORD) + assert len(client.app["users_store"].sessions_for("jason")) == 2 + await client.post("/api/auth/logout", headers={**_jar(mine), **DASH}) + assert len(client.app["users_store"].sessions_for("jason")) == 1 + + @pytest.mark.asyncio + async def test_me_answers_a_stranger_with_null_and_nothing_else(self, protected): + client, _ = protected + resp = await client.get("/api/auth/me") + assert resp.status == 200 + assert await resp.json() == {"user": None, "auth_enabled": True, + "has_users": True} + + @pytest.mark.asyncio + async def test_me_answers_a_session_with_its_account_and_session(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + body = await (await client.get("/api/auth/me", headers=_jar(token))).json() + assert body["auth_enabled"] is True and body["has_users"] is True + assert body["user"]["name"] == "jason" + assert body["user"]["role"] == ROLE_ADMIN + assert set(body["user"]["session"]) >= {"id", "created_at", "last_seen"} + assert "token_hash" not in body["user"]["session"] + + @pytest.mark.asyncio + async def test_me_says_an_open_node_with_no_accounts_is_both(self, stores): + auth_config, users = stores + users.import_users([]) + async with _client(_make_app(auth_config, users)) as client: + assert await (await client.get("/api/auth/me")).json() == { + "user": None, "auth_enabled": False, "has_users": False} + + @pytest.mark.asyncio + async def test_auth_status_counts_a_session_as_authenticated(self, protected): + """The dashboard shows "stop requiring a key" only to an authenticated + caller (#262), and a person who has logged in is one.""" + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + body = await (await client.get("/api/auth/status", + headers=_jar(token))).json() + assert body["authenticated"] is True + anonymous = await (await client.get("/api/auth/status")).json() + assert anonymous["authenticated"] is False + + @pytest.mark.asyncio + async def test_an_api_key_is_not_a_person(self, protected): + client, key = protected + body = await (await client.get( + "/api/auth/me", headers={"Authorization": f"Bearer {key}"})).json() + assert body["user"] is None + + +# ============================================================================= +# Sessions and the self-service password change +# ============================================================================= + +class TestSessionRoutes: + @pytest.mark.asyncio + async def test_a_member_sees_its_own_sessions_only(self, protected): + client, _ = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + await _cookie_of(client, "jason", ADMIN_PASSWORD) + + resp = await client.get("/api/auth/sessions", headers=_jar(token)) + body = await resp.json() + assert body["user"] == "sam" + assert [s["user"] for s in body["sessions"]] == ["sam"] + + resp = await client.get("/api/auth/sessions?user=jason", headers=_jar(token)) + assert resp.status == 403 + + @pytest.mark.asyncio + async def test_an_admin_may_read_another_account(self, protected): + client, _ = protected + await _cookie_of(client, "sam", MEMBER_PASSWORD) + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + body = await (await client.get("/api/auth/sessions?user=sam", + headers=_jar(token))).json() + assert [s["user"] for s in body["sessions"]] == ["sam"] + + @pytest.mark.asyncio + async def test_a_bad_name_in_the_query_is_a_400(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + resp = await client.get("/api/auth/sessions?user=not%20a%20name", + headers=_jar(token)) + assert resp.status == 400 + + @pytest.mark.asyncio + async def test_a_key_with_no_query_sees_no_sessions(self, protected): + client, key = protected + await _cookie_of(client, "jason", ADMIN_PASSWORD) + body = await (await client.get( + "/api/auth/sessions", + headers={"Authorization": f"Bearer {key}"})).json() + assert body == {"user": None, "sessions": []} + + @pytest.mark.asyncio + async def test_a_member_revokes_its_own_session_and_not_another(self, protected): + client, _ = protected + member = await _cookie_of(client, "sam", MEMBER_PASSWORD) + await _cookie_of(client, "jason", ADMIN_PASSWORD) + store = client.app["users_store"] + theirs = store.sessions_for("jason")[0]["id"] + mine = store.sessions_for("sam")[0]["id"] + + resp = await client.delete(f"/api/auth/sessions/{theirs}", + headers={**_jar(member), **DASH}) + assert resp.status == 404, "a member learned that somebody else's id exists" + + resp = await client.delete(f"/api/auth/sessions/{mine}", + headers={**_jar(member), **DASH}) + assert resp.status == 200 + assert store.sessions_for("sam") == [] + + @pytest.mark.asyncio + async def test_an_admin_revokes_anybodys_session(self, protected): + client, _ = protected + await _cookie_of(client, "sam", MEMBER_PASSWORD) + admin = await _cookie_of(client, "jason", ADMIN_PASSWORD) + store = client.app["users_store"] + theirs = store.sessions_for("sam")[0]["id"] + resp = await client.delete(f"/api/auth/sessions/{theirs}", + headers={**_jar(admin), **DASH}) + assert resp.status == 200 + assert store.sessions_for("sam") == [] + + @pytest.mark.asyncio + async def test_a_password_change_keeps_this_browser_and_drops_the_others( + self, protected): + client, _ = protected + keeping = await _cookie_of(client, "sam", MEMBER_PASSWORD) + elsewhere = await _cookie_of(client, "sam", MEMBER_PASSWORD) + resp = await client.post("/api/auth/password", + json={"current": MEMBER_PASSWORD, + "new": "a brand new password"}, + headers={**_jar(keeping), **DASH}) + assert resp.status == 200 + fresh = resp.headers["Set-Cookie"].split(";")[0].split("=", 1)[1] + assert fresh != keeping + + store = client.app["users_store"] + assert store.verify_password("sam", "a brand new password") is True + assert len(store.sessions_for("sam")) == 1 + assert (await client.get("/api/whoami", headers=_jar(elsewhere))).status == 401 + assert (await client.get("/api/whoami", headers=_jar(fresh))).status == 200 + + @pytest.mark.asyncio + async def test_a_wrong_current_password_is_a_403(self, protected): + client, _ = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + resp = await client.post("/api/auth/password", + json={"current": "nope", "new": "a new password"}, + headers={**_jar(token), **DASH}) + assert resp.status == 403 + assert client.app["users_store"].verify_password( + "sam", MEMBER_PASSWORD) is True + + @pytest.mark.asyncio + async def test_a_short_new_password_is_refused(self, protected): + client, _ = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + resp = await client.post("/api/auth/password", + json={"current": MEMBER_PASSWORD, "new": "short"}, + headers={**_jar(token), **DASH}) + assert resp.status == 400 + + @pytest.mark.asyncio + async def test_a_key_cannot_change_a_password_through_the_self_route(self, + protected): + client, key = protected + resp = await client.post("/api/auth/password", + json={"current": "x", "new": "a new password"}, + headers={"Authorization": f"Bearer {key}"}) + assert resp.status == 403 + assert "/api/auth/users//password" in ( + await resp.json())["error"]["message"] + + +# ============================================================================= +# Accounts, and who may manage them +# ============================================================================= + +class TestUserRoutes: + @pytest.mark.asyncio + async def test_an_admin_session_manages_accounts(self, protected): + client, _ = protected + token = await _cookie_of(client, "jason", ADMIN_PASSWORD) + headers = {**_jar(token), **DASH} + + resp = await client.post("/api/auth/users", + json={"name": "kim", "password": "a good password", + "role": "member"}, headers=headers) + assert resp.status == 201 + assert (await resp.json())["user"] == { + "name": "kim", "role": ROLE_MEMBER, + "created_at": (await resp.json())["user"]["created_at"], + "disabled": False} + + body = await (await client.get("/api/auth/users", headers=headers)).json() + assert {u["name"] for u in body["users"]} == {"jason", "sam", "kim"} + assert body["admin_count"] == 1 + assert all("password_hash" not in u for u in body["users"]) + + resp = await client.delete("/api/auth/users/kim", headers=headers) + assert resp.status == 200 + assert client.app["users_store"].find_user("kim") is None + + @pytest.mark.asyncio + async def test_a_member_session_is_refused_everywhere(self, protected): + client, _ = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + headers = {**_jar(token), **DASH} + refused = [ + ("GET", "/api/auth/users", None), + ("POST", "/api/auth/users", {"name": "kim", "password": "a password"}), + ("DELETE", "/api/auth/users/jason", None), + ("POST", "/api/auth/users/jason/password", {"password": "a password"}), + ("POST", "/api/auth/users/jason/disable", {}), + ("POST", "/api/auth/users/jason/enable", {}), + ("GET", "/api/auth/keys", None), + ("POST", "/api/auth/keys", {}), + ("DELETE", "/api/auth/keys/whatever", None), + ("POST", "/api/auth/enable", {}), + ("POST", "/api/auth/disable", {}), + ] + for method, path, body in refused: + kwargs = {"json": body} if body is not None else {} + resp = await client.request(method, path, headers=headers, **kwargs) + assert resp.status == 403, f"{method} {path} answered {resp.status}" + assert (await resp.json())["error"]["type"] == "forbidden" + assert client.app["auth_config"].enabled is True, "a member turned auth off" + + @pytest.mark.asyncio + async def test_an_operator_key_manages_accounts(self, protected): + """How the CLI works, and how the first admin is created.""" + client, key = protected + headers = {"Authorization": f"Bearer {key}"} + resp = await client.post("/api/auth/users", + json={"name": "kim", "password": "a good password", + "role": "admin"}, headers=headers) + assert resp.status == 201 + assert client.app["users_store"].admin_count() == 2 + # A browser holding a key and no account still gets the list, not a 403: + # the dashboard renders 403 as "only an admin can manage accounts". + resp = await client.get("/api/auth/users", headers=headers) + assert resp.status == 200 + assert {u["name"] for u in (await resp.json())["users"]} == { + "jason", "sam", "kim"} + + @pytest.mark.asyncio + async def test_the_fleet_key_manages_accounts(self, protected): + client, _ = protected + headers = {"Authorization": f"Bearer {fleet_key(SECRET)}"} + resp = await client.get("/api/auth/users", headers=headers) + assert resp.status == 200 + + @pytest.mark.asyncio + async def test_the_first_admin_can_be_created_on_an_open_node(self, stores): + """A fresh install with auth off and nobody to authorise anything.""" + auth_config, users = stores + users.import_users([]) + async with _client(_make_app(auth_config, users)) as client: + resp = await client.post("/api/auth/users", + json={"name": "jason", "role": "admin", + "password": "a good password"}) + assert resp.status == 201 + assert users.admin_count() == 1 + + @pytest.mark.asyncio + async def test_a_bad_account_is_a_400_carrying_the_rule(self, protected): + client, key = protected + headers = {"Authorization": f"Bearer {key}"} + for body, expect in ( + ({"name": "has space", "password": "a good password"}, "1 to 64"), + ({"name": "kim", "password": "short"}, "at least 8"), + ({"name": "kim", "password": "a good password", "role": "root"}, + "Role must be one of"), + ({"name": "jason", "password": "a good password"}, "already an account"), + ): + resp = await client.post("/api/auth/users", json=body, headers=headers) + assert resp.status == 400, body + assert expect in (await resp.json())["error"]["message"] + + @pytest.mark.asyncio + async def test_the_last_admin_cannot_be_removed_or_disabled(self, protected): + client, key = protected + headers = {"Authorization": f"Bearer {key}"} + for method, path in (("DELETE", "/api/auth/users/jason"), + ("POST", "/api/auth/users/jason/disable")): + resp = await client.request(method, path, headers=headers) + assert resp.status == 409, f"{method} {path} answered {resp.status}" + assert "only admin" in (await resp.json())["error"]["message"] + assert client.app["users_store"].admin_count() == 1 + + @pytest.mark.asyncio + async def test_disable_and_enable_round_trip(self, protected): + client, key = protected + headers = {"Authorization": f"Bearer {key}"} + resp = await client.post("/api/auth/users/sam/disable", headers=headers) + assert resp.status == 200 + assert (await resp.json())["user"]["disabled"] is True + resp = await client.post("/api/auth/users/sam/enable", headers=headers) + assert (await resp.json())["user"]["disabled"] is False + + @pytest.mark.asyncio + async def test_an_admin_resets_a_password_and_logs_that_account_out(self, + protected): + client, key = protected + token = await _cookie_of(client, "sam", MEMBER_PASSWORD) + resp = await client.post("/api/auth/users/sam/password", + json={"password": "a reset password"}, + headers={"Authorization": f"Bearer {key}"}) + assert resp.status == 200 + assert (await client.get("/api/whoami", headers=_jar(token))).status == 401 + assert (await _login(client, "sam", "a reset password")).status == 200 + + @pytest.mark.asyncio + async def test_an_unknown_account_is_a_404(self, protected): + client, key = protected + headers = {"Authorization": f"Bearer {key}"} + for method, path, body in ( + ("DELETE", "/api/auth/users/nobody", None), + ("POST", "/api/auth/users/nobody/password", {"password": "a password"}), + ("POST", "/api/auth/users/nobody/disable", {}), + ("POST", "/api/auth/users/nobody/enable", {}), + ): + kwargs = {"json": body} if body is not None else {} + resp = await client.request(method, path, headers=headers, **kwargs) + assert resp.status == 404, f"{method} {path} answered {resp.status}" + + @pytest.mark.asyncio + async def test_every_mutation_tells_the_broadcaster(self, stores): + """The hook the CLI and fleet half registers, so a head knows to replicate.""" + auth_config, users = stores + entry = auth_config.enable("test") + app = _make_app(auth_config, users) + calls = [] + app["users_changed"] = lambda: calls.append(1) + headers = {"Authorization": f"Bearer {entry['key']}"} + async with _client(app) as client: + await client.post("/api/auth/users", + json={"name": "kim", "password": "a good password"}, + headers=headers) + await client.post("/api/auth/users/kim/password", + json={"password": "another password"}, headers=headers) + await client.post("/api/auth/users/kim/disable", headers=headers) + await client.post("/api/auth/users/kim/enable", headers=headers) + await client.delete("/api/auth/users/kim", headers=headers) + assert len(calls) == 5 + + @pytest.mark.asyncio + async def test_an_async_broadcaster_is_scheduled_and_not_awaited(self, stores): + """A fan-out to every peer must not be inside the operator's 201.""" + auth_config, users = stores + entry = auth_config.enable("test") + app = _make_app(auth_config, users) + ran = [] + + async def broadcast(): + ran.append(1) + + app["users_changed"] = broadcast + async with _client(app) as client: + resp = await client.post( + "/api/auth/users", + json={"name": "kim", "password": "a good password"}, + headers={"Authorization": f"Bearer {entry['key']}"}) + assert resp.status == 201 + await asyncio.sleep(0) + assert ran == [1] + + @pytest.mark.asyncio + async def test_a_broken_broadcaster_does_not_fail_the_mutation(self, stores): + auth_config, users = stores + entry = auth_config.enable("test") + app = _make_app(auth_config, users) + + def boom(): + raise RuntimeError("no peers today") + + app["users_changed"] = boom + async with _client(app) as client: + resp = await client.post( + "/api/auth/users", + json={"name": "kim", "password": "a good password"}, + headers={"Authorization": f"Bearer {entry['key']}"}) + assert resp.status == 201 + + +# ============================================================================= +# Fleet replication +# ============================================================================= + +class TestReplicationRoutes: + @pytest.mark.asyncio + async def test_export_and_sync_are_the_fleet_key_alone(self, protected): + client, key = protected + admin = await _cookie_of(client, "jason", ADMIN_PASSWORD) + for headers in ({"Authorization": f"Bearer {key}"}, + {**_jar(admin), **DASH}): + resp = await client.get("/api/auth/users/export", headers=headers) + assert resp.status == 403, "an operator read the password hashes" + assert "fleet key" in (await resp.json())["error"]["message"] + resp = await client.post("/api/auth/users/sync", json={"users": []}, + headers=headers) + assert resp.status == 403 + + @pytest.mark.asyncio + async def test_the_fleet_key_exports_the_hashes_with_a_stamp(self, protected): + client, _ = protected + headers = {"Authorization": f"Bearer {fleet_key(SECRET)}"} + body = await (await client.get("/api/auth/users/export", + headers=headers)).json() + assert [u["name"] for u in body["users"]] == ["jason", "sam"] + assert body["users"][0]["password_hash"] + assert body["stamp"] == client.app["users_store"].export_stamp() + + @pytest.mark.asyncio + async def test_sync_adopts_a_list_once(self, protected): + client, _ = protected + headers = {"Authorization": f"Bearer {fleet_key(SECRET)}"} + exported = client.app["users_store"].export_users() + incoming = [u for u in exported if u["name"] == "jason"] + + resp = await client.post("/api/auth/users/sync", json={"users": incoming}, + headers=headers) + assert await resp.json() == {"changed": True, "count": 1} + + resp = await client.post("/api/auth/users/sync", json={"users": incoming}, + headers=headers) + assert await resp.json() == {"changed": False, "count": 1} + assert client.app["users_store"].verify_password( + "jason", ADMIN_PASSWORD) is True + + @pytest.mark.asyncio + async def test_a_malformed_sync_leaves_the_node_alone(self, protected): + client, _ = protected + headers = {"Authorization": f"Bearer {fleet_key(SECRET)}"} + before = client.app["users_store"].export_users() + + resp = await client.post("/api/auth/users/sync", json={}, headers=headers) + assert resp.status == 400 + assert "'users' list" in (await resp.json())["error"]["message"] + + resp = await client.post("/api/auth/users/sync", + json={"users": [{"name": "no hash"}]}, + headers=headers) + assert resp.status == 400 + assert client.app["users_store"].export_users() == before + + @pytest.mark.asyncio + async def test_export_is_not_swallowed_by_the_name_route(self, protected): + """``/users/export`` must not resolve as ``/users/{name}``.""" + client, _ = protected + resp = await client.get("/api/auth/users/export", + headers={"Authorization": f"Bearer {fleet_key(SECRET)}"}) + assert resp.status == 200 + assert "users" in await resp.json() From abcca3b12ee45c86022b208cbd6d143b4ad26f61 Mon Sep 17 00:00:00 2001 From: webdevtodayjason Date: Mon, 21 Sep 2026 19:04:53 -0500 Subject: [PATCH 2/2] AGENTS.md: the login half of the auth edit contract The root contract had bullets for the fleet key, the installer's first key and the 0600 stores, and nothing for the credential a person presents. The next person editing ainode/auth/ needs the four rules that are not obvious from the code: users.json is re-read per request for the same reason auth.json is, the crypto is standard library only, a session has no expiry so revocation has to be complete, and the X-AINode-Client header is the whole CSRF defence. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 334f7f37..619dcfa3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,7 @@ State / architecture / decisions / "why": Obsidian Vault, `AINode` (cluster ops: - **Every node-to-node request AINode makes carries the FLEET KEY, and there is one helper that puts it there** (`ainode/auth/fleet.py`: `fleet_headers(app)` off a running app, `fleet_key_headers(secret)` off a config). The key is `HMAC-SHA256(cluster_secret, "ainode-fleet-key-v1")`, so every node holding the secret computes the same one, the join flow already distributes it, rotation follows the secret, and NOTHING new is written to disk. The middleware accepts it as the caller id `fleet` (`auth/middleware.py::identify_caller`), reading the secret LIVE per request, so a node whose secret differs refuses its would-be peers exactly as it drops their datagrams. Auth was enabled-able and unusable on a cluster before this: a node with `auth.enabled` answered 401 to its own fan-outs, so the fleet ran open. Add a peer call and you add the header: an ENGINE port is not a peer call in this sense (the inference proxy, the capability probes and the embeddings route talk to a vLLM container, which never sees this middleware), and `POST /api/cluster/join` is keyless by construction. `tests/test_fleet_auth.py` WALKS THE SOURCE for peer URLs and fails on one whose function does not name the helper, with an exempt list that has to state a reason. - **A fresh install requires a key, and an update never changes an installed node's access control** (`scripts/install.sh`). The installer mints one key on a node with no `config.json`, stores the SHA-256 the way `AuthConfig` does, prints the plaintext ONCE in a box, and the summary reads "API protected, one key"; `AINODE_AUTH=off` keeps the old open behaviour and prints what that choice means. Both gates matter: minting on an existing home would lock out every client the operator already pointed at the node, with a key they never saw. Because of this, `ainode update` verifies the running release on `/api/health` and not `/api/status` (health is the one keyless route, which is why it carries `version`): reading a keyed route there made every update on a protected node pull, pin, restart and then report that it had not applied. - **`auth.json` and `config.json` are 0600, written temp-then-replace, and re-read when they change.** Both carry credentials (`config.json` holds `cluster_secret` and may hold `hf_token`) and both were written under the default umask as root. `AuthConfig.reload_if_changed()` stats the store per request the way `ClusterSecret` stats the config per datagram, which is what makes `ainode auth enable|disable|key create|key revoke` LIVE on a running node (it used to need a restart nobody was told to do, so the CLI said "Auth enabled" about a node that went on answering everybody). A file that cannot be stat'ed or parsed keeps the state in memory: the tolerant direction is never "let everybody in". A credential-shaped config key goes in `api/server.py::SCRUBBED_CONFIG_KEYS` and is masked by `ainode config`, or it leaves the node in a `GET /api/config`. +- **A person logs in, a machine presents a key, and the two stores sit side by side** (`ainode/auth/accounts.py`, `auth/session_routes.py`, #261). `users.json` is 0600, written temp-then-replace and re-read per request exactly as `auth.json` is, because `ainode auth user add`, a password change and a revoked session all have to land on a running node. The ONLY crypto is the standard library's: `hashlib.scrypt` (n=2**14, r=8, p=1) over a per-password salt, and a session token stored as its SHA-256 alone, so the file a backup or a support bundle picks up logs nobody in and a login page is not the reason the image grows an argon2 wheel. **A session does not expire** (in until you log out), which makes REVOCATION the whole control and it has to be complete: a password change, a disable and a removal each take that account's open sessions with them, and `session_for_token` refuses one whose account has since been removed or disabled. **A cookie-authenticated request whose method is not GET, HEAD or OPTIONS must carry `X-AINode-Client: dashboard`**, and that header is the whole CSRF defence: a browser attaches a cookie to a cross-site request on its own, and a custom header cannot be set cross-origin without a preflight this node never approves, so dropping the rule would make every node an operator is logged into writable by any page on the internet. A Bearer token is tried FIRST and the cookie only when no key matched, so automation is untouched and a stale key in a browser does not cost a good session its request. Administration (`/api/auth/users*`, `/api/auth/keys*`, `/api/auth/enable|disable`) goes through `accounts.require_admin`: an admin session, an operator key, or the fleet key, never a member; the two replication routes are the FLEET key alone because they move password hashes; and the last ENABLED admin cannot be removed or disabled, enforced in the store (`UsersStore.is_last_admin`) so the CLI cannot get around it. `/api/auth/login` and `/api/auth/me` are the newest entries in `SKIP_PATHS`, and adding one there means naming it in the middleware docstring, `api/cluster_join.py`'s docstring, the README's keyless paragraph and the dashboard's API access panel, because four tests assert each of those. - **A discovery announcement is SIGNED when `cluster_secret` is set, and a node that has one drops what it cannot verify** (`discovery/signing.py`). The signature is one extra top-level key beside the payload's own fields, never a wrapper around them, so a peer that has never heard of it drops the unknown key in `from_json` and stays in the cluster view. Both directions read the secret per datagram through `ClusterSecret`, so rotating it is a `config.json` edit and not a fleet-wide restart: never capture the value at startup. A node with NO secret behaves exactly as it did and says so once per process, because nothing on the fleet sets one and a mandatory-signing release would partition every existing cluster on upgrade. - **The discovery port has one home: `core/config.py::DEFAULT_DISCOVERY_PORT` (5679).** `NodeConfig`, both discovery classes and the installer read it from there. They each carried their own literal, and the dataclass said 5678 while the installer, the fleet and the docs said 5679, so a source install listened where nobody spoke and vanished from every cluster view with nothing logged (#181). The port, the cluster id and whether the wire is signed are logged together at startup. - **`ainode_version` travels on the announcement and into every view that lists nodes** (`/api/nodes`, `/api/cluster/resources`, `/api/version/check`, `/api/cluster/update-status`). A peer that announces none reports `""` and is counted as unknown: filling it in with the local version is how a split fleet goes on looking like a healthy one (#171).