diff --git a/docs/waves/decisions-2026-08-21.md b/docs/waves/decisions-2026-08-21.md index fcd2624..59d5545 100644 --- a/docs/waves/decisions-2026-08-21.md +++ b/docs/waves/decisions-2026-08-21.md @@ -155,3 +155,56 @@ Two consequences worth knowing: - Group-to-role mapping (e.g. an AD group that confers `project_admin`). Criterion 4 keeps authorization local on purpose. Worth its own item later; logged in `backlog.md`. - Replacing the Let's Encrypt certificate or changing anything on the OpenResty host. + +--- + +## D14 — The CLI authenticates against the domain, and stops creating accounts + +- **Amends:** `D13` criterion 4, which said role granting keeps working *from the Admin + console*. It said nothing about `manage_users.py`, which had no authentication of any + kind. Requiring one is a new requirement, so it gets its own id rather than widening + criterion 4. +- **Surface:** `server/manage_users.py` +- **Task:** `T10.9` + +### The decision + +1. **`create-admin` and `create` are removed.** `D13` provisions accounts on first + successful sign-in, so creating them by hand is redundant. Removing them also closes a + class of problem: every row now originates from a bind, so a username cannot be typed + in wrong and end up orphaned from the directory identity it was meant to match. That + risk now applies only to rows the old CLI already created. +2. **`promote` and `demote` replace them.** The directory supplies identity; this app + supplies authorization, and this is where authorization is assigned from a shell. +3. **Every state-changing command requires a domain bind.** Prompted, via `getpass`. + There is deliberately no `--password` flag: that would put a live domain password into + shell history and into `ps` output for every other user on the box. +4. **`list` needs no credential**, so an outage stays diagnosable. + +### Bootstrapping the first admin, which changed shape + +Two steps, in order: **sign in once** (which provisions the account at `project_user`), +then **`promote `**. Before D14 the first admin was created with a +password; there is no password now, and no account to create. + +### What this is worth, stated plainly + +Anyone with a shell on the api container can still write to the `users` table directly +with `psql` or `sqlite3`. So the bind is **defence in depth and, mostly, +ACCOUNTABILITY** — not a security boundary. Before D14 every role change made from a +shell was invisible in `AuditLog` while the same change through the console was recorded; +now both are recorded and both name a person. Any-domain-user was accepted as sufficient +(no privileged group exists on this estate, and machine access is already restricted to a +few people), which was decided knowing the above. + +### Two deliberate divergences from the API + +- **The bind does NOT apply the login group gate.** If a mistyped required group locks + everyone out of the console, this tool has to still work — otherwise the only route to + fixing the lockout is the thing the lockout prevents. +- **Changing your OWN role is permitted here.** `set_user_role` in `app.py` forbids it to + stop an admin locking themselves out of the console. Here it is the entire bootstrap + path, so it is allowed and recorded with `{"self": true}` in the audit detail. + +The last-admin guard is kept, matching `set_user_role`: an app with no admin cannot be +administered, and there is no password login left to recover through. diff --git a/docs/waves/wave-10.md b/docs/waves/wave-10.md index d6b4e2b..aa14446 100644 --- a/docs/waves/wave-10.md +++ b/docs/waves/wave-10.md @@ -1,6 +1,6 @@ # Wave 10 — Domain authentication over LDAPS -**Items:** `D13` +**Items:** `D13`, `D14` **Depends on:** wave 9 merged (it is — `a8e28bf`) One item, eight tasks. The item is stated in `docs/waves/decisions-2026-08-21.md`; read it @@ -337,3 +337,36 @@ happens when the DC is unreachable, whatever `T10.2` decides. - [ ] the DC-unreachable behaviour is stated explicitly - [ ] `IMPLEMENTATION.md` section 4 lists wave 10 - [ ] no doc still claims passwords are stored as bcrypt hashes + +--- + +### T10.9 — D14: the CLI authenticates, and stops creating accounts + +- **Items:** `D14` +- **Depends on:** T10.3 +- **Blocks:** T10.8 +- **Surface:** `server/` +- **Files:** `server/manage_users.py` + +**Problem:** `manage_users.py` writes to the `users` table with no authentication at all. +It also still offers `create-admin` / `create`, which are redundant now that accounts +provision themselves — and worse than redundant, because a hand-typed username can end up +matching no directory identity. + +**Do:** As stated in `D14`. Remove the two create commands, add `promote` / `demote`, gate +every state-changing command on a prompted domain bind, and write an `AuditLog` row naming +the operator. Write the audit row by hand rather than importing `log_event` from `app.py` — +that would pull FastAPI and the whole application into a CLI startup for one INSERT. + +**Done when:** + +- [x] `create-admin`, `create` and `reset-password` are rejected as invalid choices +- [x] `list` works with no credential and with no LDAP configured +- [x] a state-changing command with LDAP misconfigured refuses instead of proceeding +- [x] there is no `--password` flag on any command +- [x] `promote` raises a role; `demote` returns an account to `project_user` +- [x] promoting YOURSELF is allowed and recorded with `self: true` +- [x] the last active admin cannot be demoted +- [x] an unknown account gives an error that says accounts are made on first sign-in +- [x] every change writes an `AuditLog` row naming the operator +- [ ] verified against a real domain bind rather than a stubbed `authenticate_operator` diff --git a/server/manage_users.py b/server/manage_users.py index e52e2cb..ed18acf 100644 --- a/server/manage_users.py +++ b/server/manage_users.py @@ -1,118 +1,219 @@ """Command-line user management for the Work Package Suite. -Use this to bootstrap the FIRST admin (the /api/auth/users endpoint needs an -existing admin, so one has to be made here) and for occasional account maintenance -from a shell on the server. +Accounts are not created here any more. D13 provisions them on first successful +sign-in, so this tool exists to do the one thing the directory cannot decide: +assign the app's PERMISSIONS role. The directory supplies identity; this supplies +authorization. -NO PASSWORDS. D13 moved authentication to an LDAPS bind against the domain, so -this tool never sets or resets a credential — it only creates accounts and assigns -roles. People sign in with their Windows password; the login page sends anyone who -has forgotten it to Okta. - -Run from the PROJECT ROOT (same place you run uvicorn), so the package imports and -.env resolve the same way the API does: - - python -m server.manage_users create-admin alice --name "Alice Smith" - python -m server.manage_users create bob --role user --name "Bob Jones" python -m server.manage_users list + python -m server.manage_users promote alice # -> admin + python -m server.manage_users promote bob --role project_admin + python -m server.manage_users demote alice # -> project_user python -m server.manage_users disable bob python -m server.manage_users enable bob -`username` must be the person's sAMAccountName, because that is what the bind and -the account match use. Creating an account is optional: anyone who authenticates -against the domain and is not already here is provisioned on first sign-in, at -project_user with no project access. +Run from the PROJECT ROOT (same place you run uvicorn) so the package imports and +.env resolve the way the API does. + +`create-admin` and `create` are GONE (D14). They were redundant once accounts +provision themselves, and removing them closes a whole class of problem: every +row now originates from a successful bind, so a username can no longer be typed +in wrong and end up orphaned from the directory identity it was meant to match. + +BOOTSTRAPPING THE FIRST ADMIN is therefore two steps, in this order: + 1. Sign in to the app once. That provisions your account at project_user. + 2. Run `promote ` here. + +EVERY COMMAND THAT CHANGES ANYTHING REQUIRES A DOMAIN BIND (D14). Shell access +alone is no longer enough to mint an admin. Be clear about what that is and is +not worth: anyone with a shell on this container can still write to the `users` +table directly with psql or sqlite3, so this is defence in depth and — mostly — +ACCOUNTABILITY. Before D14 every role change made from a shell was invisible in +the audit trail while the same change through the console was recorded. Now both +are recorded, and both name a person. + +The bind here deliberately does NOT apply the login group gate. If a mistyped +required group locks everyone out of the console, this tool has to still work — +otherwise the only way to fix the lockout is the only thing the lockout prevents. + +`list` needs no credential, so an outage stays diagnosable. """ import argparse +import getpass import sys import uuid from .db import SessionLocal, Base, engine -from . import models, auth +from . import models, auth, ldap_auth -def _gen_id() -> str: - return f"user_{uuid.uuid4().hex[:12]}" +def _gen_id(prefix: str = "user") -> str: + return f"{prefix}_{uuid.uuid4().hex[:12]}" -def cmd_create(args, role: str | None = None) -> None: - role = role or args.role - # 'user' is the pre-roles spelling of 'project_user' and is still accepted so the - # documented one-liners keep working; anything else has to be a current role. - if role == "user": - role = auth.ROLE_PROJECT_USER - if role not in auth.ROLES: - sys.exit(f"role must be one of {', '.join(auth.ROLES)}") - with SessionLocal() as db: - if auth.find_user(db, args.username): - sys.exit(f"A user named '{args.username}' already exists.") - u = models.User( - id=_gen_id(), - username=args.username.strip(), - full_name=(args.name or "").strip(), - email=(args.email or "").strip(), - role=role, - ) - db.add(u) - db.commit() - print(f"Created {role}: {u.username} (id={u.id})") +def _audit(db, actor: str, action: str, user: "models.User", detail: dict) -> None: + """Append an audit row in the caller's transaction. + + Written by hand rather than via app.py's log_event: importing that would drag + FastAPI and the entire application into a CLI startup for one INSERT. + """ + db.add(models.AuditLog( + id=_gen_id("ev"), actor=actor, action=action, entity_type="user", + entity_id=user.id, summary=user.username, detail=detail, + )) + + +def authenticate_operator() -> str: + """Prompt for a domain credential, bind, and return the operator's sAMAccountName. + + Exits on failure — a command that changes a role must not proceed unauthenticated. + The password is only ever read from a hidden prompt: there is no --password flag, + because that would put a live domain password into shell history and into the + output of `ps` for every other user on the box. + """ + if not ldap_auth.is_configured(): + sys.exit(f"Cannot authenticate: {ldap_auth.describe()}\n" + f"This command needs a domain bind. Fix the LDAP configuration first.") + who = input("Your domain username: ").strip() + if not who: + sys.exit("Cancelled.") + pw = getpass.getpass("Your domain password: ") + if not pw: + sys.exit("Cancelled.") + # required_group="" on purpose: see the module docstring. The login group must + # not be able to lock an operator out of the tool that fixes the login group. + result = ldap_auth.verify(who, pw, required_group="") + if not result.ok: + if result.is_config_problem: + sys.exit(f"Could not reach the domain ({result.reason}): {result.detail}") + sys.exit("Authentication failed.") + print(f"Authenticated as {result.sam}.") + return result.sam + + +def _load(db, username: str) -> "models.User": + u = auth.find_user(db, username) + if not u: + sys.exit(f"No account named '{username}'. Accounts are created on first " + f"sign-in — has this person signed in yet? `list` shows who exists.") + return u def cmd_list(args) -> None: + """Read-only, and deliberately needs no credential: during an outage this is + how you find out what the app thinks the world looks like.""" with SessionLocal() as db: rows = db.query(models.User).order_by(models.User.username).all() if not rows: - print("No users yet. Create one with: create-admin ") + print("No accounts yet. They are created on first successful sign-in.") return - print(f"{'USERNAME':<24}{'ROLE':<20}{'ACTIVE':<8}{'NAME'}") + print(f"{'USERNAME':<24}{'ROLE':<20}{'ACTIVE':<8}{'LAST LOGIN':<22}{'NAME'}") for u in rows: + last = u.last_login_at.strftime("%Y-%m-%d %H:%M") if u.last_login_at else "never" print(f"{u.username:<24}{auth.normalize_role(u.role):<20}" - f"{('yes' if u.is_active else 'no'):<8}{u.full_name}") + f"{('yes' if u.is_active else 'no'):<8}{last:<22}{u.full_name}") + + +def _set_role(username: str, role: str) -> None: + if role not in auth.ROLES: + sys.exit(f"role must be one of {', '.join(auth.ROLES)}") + operator = authenticate_operator() + with SessionLocal() as db: + u = _load(db, username) + old = auth.normalize_role(u.role) + if old == role: + print(f"{u.username} is already {role}. Nothing to do.") + return + # The last-admin guard, matching set_user_role in app.py: an app with no + # admin cannot be administered, and there is no password login left to + # recover through. + if old == auth.ROLE_ADMIN and role != auth.ROLE_ADMIN: + others = db.query(models.User).filter( + models.User.role == auth.ROLE_ADMIN, + models.User.id != u.id, + models.User.is_active.is_(True), + ).count() + if not others: + sys.exit("Refusing: that is the last active admin account. Promote " + "someone else first.") + detail = {"from": old, "to": role, "via": "manage_users"} + u.role = role + # Mirrors set_user_role: an admin already reaches every project, so the + # default-member flag would sit there doing nothing and spring back to life + # on demotion. + if role == auth.ROLE_ADMIN and (u.auto_add_projects or u.auto_add_role): + u.auto_add_projects = False + u.auto_add_role = "" + detail["auto_add_cleared"] = True + # NOTE: unlike the API, this permits changing your OWN role. The endpoint + # forbids it to stop an admin locking themselves out of the console; here it + # is the entire bootstrap path — sign in, then promote yourself. + if u.username.lower() == operator.lower(): + detail["self"] = True + _audit(db, operator, "role_changed", u, detail) + db.commit() + print(f"{u.username}: {old} -> {role}") + + +def cmd_promote(args) -> None: + _set_role(args.username, args.role) + + +def cmd_demote(args) -> None: + _set_role(args.username, auth.ROLE_PROJECT_USER) def _set_active(username: str, active: bool) -> None: + operator = authenticate_operator() with SessionLocal() as db: - u = auth.find_user(db, username) - if not u: - sys.exit(f"No user named '{username}'.") + u = _load(db, username) + if bool(u.is_active) == active: + print(f"{u.username} is already {'enabled' if active else 'disabled'}.") + return u.is_active = active + # Disabling has to take effect on sessions already issued, and role reads go + # through the database on every request — but token_version is what get_current_user + # checks, so bump it to sign them out now rather than at session expiry. + u.token_version = (u.token_version or 0) + 1 + _audit(db, operator, "user_active_changed", u, + {"is_active": active, "via": "manage_users"}) db.commit() print(f"{u.username} is now {'enabled' if active else 'disabled'}.") def main() -> None: - # Ensure the users table exists even on a fresh database. + # Ensure tables exist on a fresh local database (SQLite dev). Production owns + # its schema through alembic. Base.metadata.create_all(bind=engine) - p = argparse.ArgumentParser(prog="manage_users", description="Work Package Suite user management") + p = argparse.ArgumentParser( + prog="manage_users", + description="Work Package Suite user management. Accounts are created on " + "first sign-in (D13); this assigns roles.") sub = p.add_subparsers(dest="cmd", required=True) - def add_create(name, help_): - sp = sub.add_parser(name, help=help_) - sp.add_argument("username", help="the person's sAMAccountName") - sp.add_argument("--name", default="", help="full name") - sp.add_argument("--email", default="") - return sp + sub.add_parser("list", help="list all accounts (no credential needed)") - add_create("create-admin", "create an admin account") - c = add_create("create", "create an account") - c.add_argument("--role", choices=list(auth.ROLES) + ["user"], default=auth.ROLE_PROJECT_USER, - help="permissions role ('user' is the legacy name for project_user)") + pr = sub.add_parser("promote", help="raise an account's permissions role (needs a domain bind)") + pr.add_argument("username", help="the person's sAMAccountName") + pr.add_argument("--role", default=auth.ROLE_ADMIN, choices=list(auth.ROLES), + help="target role (default: admin)") - sub.add_parser("list", help="list all accounts") + dm = sub.add_parser("demote", help=f"set an account back to {auth.ROLE_PROJECT_USER}") + dm.add_argument("username", help="the person's sAMAccountName") - dp = sub.add_parser("disable", help="disable an account (blocks login)") + dp = sub.add_parser("disable", help="disable an account (blocks sign-in)") dp.add_argument("username") ep = sub.add_parser("enable", help="re-enable an account") ep.add_argument("username") args = p.parse_args() - if args.cmd == "create-admin": - cmd_create(args, role="admin") - elif args.cmd == "create": - cmd_create(args) - elif args.cmd == "list": + if args.cmd == "list": cmd_list(args) + elif args.cmd == "promote": + cmd_promote(args) + elif args.cmd == "demote": + cmd_demote(args) elif args.cmd == "disable": _set_active(args.username, False) elif args.cmd == "enable":