diff --git a/README.md b/README.md index c2d7860..2e2ca6c 100644 --- a/README.md +++ b/README.md @@ -267,11 +267,11 @@ deploy quietly serving an unauthenticated download client to the internet. Two rules, checked in order: -1. **`MUSE_ALLOWED_USERS`** — the bootstrap allowlist. Always admitted. This is - how the first admin gets in when the database is empty, and the recovery - path if someone locks themselves out. -2. **A `users` row that isn't disabled** — how everyone invited later gets in, - with no env change and no redeploy. +1. **`MUSE_ALLOWED_USERS`** — the bootstrap allowlist. Always admitted, on any + enabled provider. This is how the first admin gets in when the database is + empty, and the recovery path if someone locks themselves out. +2. **A `users` row that isn't disabled, on the provider it was invited for** — + how everyone invited later gets in, with no env change and no redeploy. Anyone matching neither is rejected and the attempt is logged with their username. The check runs on **every request**, not just at login: an @@ -280,6 +280,22 @@ authorization check that only runs at sign-in isn't revocation, it's a delay. The first account to ever sign in becomes **admin**. No bootstrap ceremony, no default password. +#### Invites are bound to a provider + +Invite `friend@gmail.com` for Google and Google is the only way in for that +identity — the same address arriving over a different provider is refused. So +"the owner signs in with GitHub, everyone else with Google" is an enforced +boundary rather than a convention, and there is never a second, weaker door +onto an existing account. + +For the same reason a successful login no longer refreshes the stored +`provider`: it is part of the identity an admin issued, so letting a sign-in +rewrite it would let the login quietly rewrite the rule that permitted it. + +A Google invite must be the full email address, because a Google identity *is* +one — inviting a bare username would authorise something Google can never +return, and the person would just see "not authorised" with no explanation. + ### Setting it up 1. → **New OAuth App** @@ -313,9 +329,20 @@ they control and the allowlist is keyed on exactly that string. Roles are **admin**, **user** and **guest**. Admins get an extra tab. -**Inviting is just adding a row** — no email, no token, no expiry to manage. -Add someone's GitHub username or Google address in the admin panel and they can -sign in. Remove them and they can't. +**The row is the authorisation** — add someone's Google address in the admin +panel and they can sign in. Remove them and they can't. + +**The email is a notification, not a credential.** If SMTP is configured they +also get a message telling them they have access and which account to use. That +link carries **no token**, deliberately: the row already names the exact +identity permitted, and Google will not hand muse an unverified address — so +possession of the email is not what lets anyone in. Which means a forwarded or +leaked invite grants nothing, nothing expires, and **resend is always safe**. +The admin panel offers resend on anyone who hasn't signed in yet. + +The alternative — a signed, expiring token — buys you the ability to withdraw +an invitation before it's used. Deleting the row does that already, so the +token would be machinery guarding something already guarded. ### Quotas @@ -511,7 +538,8 @@ package page → Package settings → Change visibility. | `PATCH /api/me` | your own email address and notification toggle | | `GET /api/health` | slskd reachability, mount writability | | `GET /api/admin/users` | list users with usage | -| `POST /api/admin/users` | invite | +| `POST /api/admin/users` | invite, and email them if SMTP is set up | +| `POST /api/admin/users/{u}/resend` | send the invitation again | | `PATCH /api/admin/users/{u}` | role, quotas, disable | | `DELETE /api/admin/users/{u}` | remove (jobs retained) | | `GET /api/admin/system` | CPU, memory, disk, slskd, library totals | diff --git a/app.py b/app.py index c8f6415..846f2fc 100644 --- a/app.py +++ b/app.py @@ -251,17 +251,26 @@ def new_session(username: str) -> str: return token -def may_login(username: str, row: sqlite3.Row | None) -> bool: +def may_login(username: str, row: sqlite3.Row | None, + provider: str | None = None) -> bool: """The single authorization rule, used at login AND on every request. Bootstrap allowlist first, then a live, non-disabled user row. Deliberately one function: an authorization check that runs only at login is not a revocation mechanism, it is a delay — disabling someone would leave their session valid for up to MUSE_SESSION_DAYS. + + An invite is bound to the provider it was issued for. Invite + friend@gmail.com for Google and Google is the only way in for that + identity; there is no second, weaker door onto the same account. `provider` + is None on the per-request re-check, where the question is only whether the + account is still live. """ if username.lower() in ALLOWED_USERS: return True - return bool(row) and not row["disabled"] + if not row or row["disabled"]: + return False + return provider is None or row["provider"] == provider def current_user(request: Request) -> sqlite3.Row | None: @@ -447,7 +456,11 @@ def upsert_user(username: str, avatar: str = "", display: str = "", "INSERT INTO users (username,provider,role,display,avatar,last_seen) " "VALUES (?,?,?,?,?,datetime('now')) " "ON CONFLICT(username) DO UPDATE SET " - " last_seen=datetime('now'), provider=excluded.provider, " + " last_seen=datetime('now'), " + # provider is NOT refreshed from the login. It is part of the + # identity an admin invited, and may_login now enforces it — so + # overwriting it here would let a successful login silently + # rewrite the very rule that permitted it. " avatar=COALESCE(NULLIF(excluded.avatar,''), users.avatar), " " display=COALESCE(NULLIF(excluded.display,''), users.display)", (username, provider, "admin" if first else "user", display, avatar)) @@ -736,6 +749,28 @@ def recipient(username: str | None) -> str | None: return username if r["provider"] == "google" and "@" in username else None +async def notify_invited(row: sqlite3.Row, by: str, to: str): + """Tell someone they've been invited. + + There is no token in this link and none is needed. The row already names + the exact identity that may sign in, and Google will not hand us an + unverified address — so possession of the email is not what authorises + anyone, and a leaked link grants nothing. It is a courtesy, not a + credential, which is also why it can be resent freely. + """ + label = PROVIDERS.get(row["provider"], {}).get("label", row["provider"]) + where = BASE_URL or "your muse instance" + await send_mail( + to, f"{by} invited you to muse", + f"You've been given access to muse — request a song, and it turns up\n" + f"in the music library, correctly tagged.\n\n" + f" {where}\n\n" + f"Sign in with {label}, using the account for {row['username']}.\n" + f"That address is what authorises you, so it has to be that one.\n\n" + f"Playback lives in Navidrome, not here — muse only decides what gets\n" + f"added to the library.\n") + + async def notify_imported(job, dest): to = recipient(job["requested_by"]) if not to: @@ -1176,10 +1211,15 @@ async def auth_callback(request: Request, code: str = "", state: str = ""): with db() as c: existing = c.execute("SELECT * FROM users WHERE username=?", (username,)).fetchone() - if not may_login(username, existing): + if not may_login(username, existing, provider): # Log the attempt: on an internet-facing box this is the signal that # someone found the login page. - why = "disabled" if existing else "not invited" + if not existing: + why = "not invited" + elif existing["disabled"]: + why = "disabled" + else: + why = f"invited for {existing['provider']}, tried {provider}" log.warning("auth: DENIED %s user %r (%s)", provider, username, why) return JSONResponse({"error": "not authorised"}, status_code=403) @@ -1963,6 +2003,10 @@ class NewUser(BaseModel): role: str = "user" quota: int = 0 storage_quota_gb: float = 0 + # Where to send the invitation. Omit for a Google invite — the username + # already is the address. Only needed for GitHub, where a login is not one. + email: str | None = None + notify: bool = True class PatchUser(BaseModel): @@ -1970,6 +2014,9 @@ class PatchUser(BaseModel): quota: int | None = None storage_quota_gb: float | None = None disabled: bool | None = None + # So a typo in an invited address is fixable without deleting the account + # and losing its request history. + email: str | None = None def user_dict(r: sqlite3.Row) -> dict: @@ -1997,8 +2044,12 @@ async def admin_users(request: Request): @app.post("/api/admin/users") async def admin_add_user(body: NewUser, request: Request): - """Invite someone. There is no email or token — the row IS the invitation: - once it exists, that identity can sign in with the matching provider.""" + """Invite someone. + + The row IS the authorisation: once it exists, that identity can sign in + with the provider it was invited for, and only that one. The email is a + notification on top, sent when there is an address and SMTP is configured. + """ who = require_admin(request) name = body.username.strip() if not name: @@ -2007,17 +2058,57 @@ async def admin_add_user(body: NewUser, request: Request): raise HTTPException(400, f"role must be one of {', '.join(ROLES)}") if body.provider not in PROVIDERS: raise HTTPException(400, f"provider must be one of {', '.join(PROVIDERS)}") + if body.provider not in ENABLED_PROVIDERS: + raise HTTPException( + 400, f"{body.provider} is not enabled on this instance " + f"(MUSE_AUTH={AUTH_MODE})") + + # A Google identity is its own address, so require it to look like one -- + # otherwise the invite silently authorises a username Google can never + # return, and the person just gets "not authorised" with no explanation. + if body.provider == "google" and not EMAIL_RE.match(name): + raise HTTPException(400, "a Google invite must be the full email address") + + email = (body.email or "").strip() or (name if body.provider == "google" else "") + if email and not EMAIL_RE.match(email): + raise HTTPException(400, "that does not look like an email address") + with db() as c: if c.execute("SELECT 1 FROM users WHERE username=?", (name,)).fetchone(): raise HTTPException(409, "that user already exists") c.execute( - "INSERT INTO users (username,provider,role,quota,storage_quota,invited_by) " - "VALUES (?,?,?,?,?,?)", + "INSERT INTO users (username,provider,role,quota,storage_quota," + "invited_by,email,notify) VALUES (?,?,?,?,?,?,?,?)", (name, body.provider, body.role, max(0, body.quota), - int(max(0.0, body.storage_quota_gb) * 2**30), who)) + int(max(0.0, body.storage_quota_gb) * 2**30), who, + email or None, 1 if body.notify else 0)) r = c.execute("SELECT * FROM users WHERE username=?", (name,)).fetchone() - log.info("admin: %s invited %s as %s", who, name, body.role) - return user_dict(r) + + log.info("admin: %s invited %s as %s (%s)", who, name, body.role, body.provider) + sent = bool(MAIL_ON and email) + if sent: + await notify_invited(r, who, email) + return {**user_dict(r), "invite_sent": sent, + "invite_why": None if sent else + ("no SMTP configured on this instance" if not MAIL_ON + else "no email address for this account")} + + +@app.post("/api/admin/users/{username}/resend") +async def admin_resend_invite(username: str, request: Request): + """Send the invitation again. Safe to call repeatedly — the link carries + no token, so resending neither extends nor grants anything.""" + who = require_admin(request) + with db() as c: + r = c.execute("SELECT * FROM users WHERE username=?", (username,)).fetchone() + if not r: + raise HTTPException(404, "no such user") + if not MAIL_ON: + raise HTTPException(409, "no SMTP configured on this instance") + if not r["email"]: + raise HTTPException(409, "no email address on file for that account") + await notify_invited(r, who, r["email"]) + return {"ok": True, "sent_to": r["email"]} @app.patch("/api/admin/users/{username}") @@ -2054,6 +2145,11 @@ async def admin_patch_user(username: str, body: PatchUser, request: Request): vals.append(int(max(0.0, body.storage_quota_gb) * 2**30)) if body.disabled is not None: sets.append("disabled=?"); vals.append(1 if body.disabled else 0) + if body.email is not None: + e = body.email.strip() + if e and not EMAIL_RE.match(e): + raise HTTPException(400, "that does not look like an email address") + sets.append("email=?"); vals.append(e or None) if not sets: raise HTTPException(400, "nothing to change") diff --git a/tests/test_invites.py b/tests/test_invites.py new file mode 100644 index 0000000..2786285 --- /dev/null +++ b/tests/test_invites.py @@ -0,0 +1,226 @@ +"""Invites and provider binding, against a live server with a fake SMTP sink. + +The rule under test: an invite is bound to the provider it was issued for, so +a Google invite cannot be redeemed over GitHub. That is what makes "the owner +on GitHub, everyone else on Google" an actual boundary rather than a habit. +""" +import asyncio, email, hashlib, os, secrets, signal, sqlite3, subprocess, sys, time +from datetime import datetime, timedelta, timezone +import httpx + +HERE = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.dirname(HERE) +DB = os.path.join(ROOT, "dev", "invites.db") +BASE = "http://127.0.0.1:5064" +SMTP_PORT = 5065 +ok = fail = 0 +INBOX: list[tuple[str, str, str]] = [] # (to, subject, body) + + +def check(name, cond, detail=""): + global ok, fail + if cond: + ok += 1; print(f" PASS {name}") + else: + fail += 1; print(f" FAIL {name} {detail}") + + +# ── a minimal SMTP server, so the test proves a real conversation ───────── +async def smtp_sink(): + async def handle(reader, writer): + async def send(s): + writer.write(s.encode() + b"\r\n"); await writer.drain() + + await send("220 sink") + data, collecting = [], False + while True: + line = await reader.readline() + if not line: + break + txt = line.decode(errors="replace").rstrip("\r\n") + if collecting: + if txt == ".": + raw = "\r\n".join(data) + msg = email.message_from_string(raw) + INBOX.append((msg["To"], msg["Subject"], + msg.get_payload(decode=True).decode())) + data, collecting = [], False + await send("250 ok") + else: + data.append(txt) + continue + up = txt.upper() + if up.startswith(("HELO", "EHLO")): + await send("250-sink"); await send("250 STARTTLS" if False else "250 OK") + elif up.startswith(("MAIL", "RCPT")): + await send("250 ok") + elif up.startswith("DATA"): + collecting = True; await send("354 go") + elif up.startswith("QUIT"): + await send("221 bye"); break + else: + await send("250 ok") + writer.close() + + s = await asyncio.start_server(handle, "127.0.0.1", SMTP_PORT) + async with s: + await s.serve_forever() + + +def main() -> int: + for f in (DB, DB + "-wal", DB + "-shm"): + if os.path.exists(f): + os.remove(f) + + env = {**os.environ, + "MUSE_AUTH": "github,google", "MUSE_DB": DB, + "MUSE_BASE_URL": "https://muse.aryan-singh.dev", + "MUSE_GITHUB_CLIENT_ID": "x", "MUSE_GITHUB_CLIENT_SECRET": "y", + "MUSE_GOOGLE_CLIENT_ID": "x", "MUSE_GOOGLE_CLIENT_SECRET": "y", + "MUSE_ALLOWED_USERS": "geneticglitch1", + "MUSE_SMTP_HOST": "127.0.0.1", "MUSE_SMTP_PORT": str(SMTP_PORT), + "MUSE_SMTP_FROM": "muse@aryan-singh.dev", "MUSE_SMTP_TLS": "none", + "MUSE_LIBRARY_DIR": os.path.join(ROOT, "dev", "library"), + "MUSE_DOWNLOAD_DIR": os.path.join(ROOT, "dev", "downloads"), + "MUSE_COOKIE_SECURE": "false", "SLSKD_URL": "http://127.0.0.1:1"} + srv = subprocess.Popen([os.path.join(ROOT, ".venv/bin/uvicorn"), "app:app", + "--host", "127.0.0.1", "--port", "5064"], + cwd=ROOT, env=env, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + try: + for _ in range(60): + try: + if httpx.get(BASE + "/healthz", timeout=1).status_code == 200: + break + except Exception: + time.sleep(0.3) + else: + print("server never came up"); return 1 + + def seed(username, role, provider="github"): + tok = secrets.token_urlsafe(32) + d = sqlite3.connect(DB) + d.execute("INSERT OR REPLACE INTO users (username,provider,role) " + "VALUES (?,?,?)", (username, provider, role)) + d.execute("INSERT INTO sessions (token_hash,username,expires_at) " + "VALUES (?,?,?)", + (hashlib.sha256(tok.encode()).hexdigest(), username, + (datetime.now(timezone.utc) + timedelta(days=1)).isoformat())) + d.commit(); d.close() + return httpx.Client(base_url=BASE, timeout=20, + cookies={"muse_session": tok}) + + admin = seed("geneticglitch1", "admin") + + print("\n=== inviting sends mail ===") + INBOX.clear() + r = admin.post("/api/admin/users", + json={"username": "friend@gmail.com", "provider": "google"}) + check("google invite accepted", r.status_code == 200, r.text[:120]) + check("reports the mail went", r.json().get("invite_sent") is True, r.text[:160]) + time.sleep(1.2) + check("one message delivered", len(INBOX) == 1, str(INBOX)) + if INBOX: + to, subj, body = INBOX[0] + check("addressed to the invitee", to == "friend@gmail.com", to) + check("names the inviter", "geneticglitch1" in subj, subj) + check("carries the instance URL", + "https://muse.aryan-singh.dev" in body, body[:120]) + check("says which provider", "Google" in body, body[:200]) + check("says which account", "friend@gmail.com" in body, body[:200]) + check("carries NO token", "token" not in body and "?t=" not in body) + + print("\n=== validation ===") + check("google invite must be an address", + admin.post("/api/admin/users", + json={"username": "notanemail", + "provider": "google"}).status_code == 400) + check("bad email rejected", + admin.post("/api/admin/users", + json={"username": "x", "provider": "github", + "email": "nope"}).status_code == 400) + check("duplicate rejected", + admin.post("/api/admin/users", + json={"username": "friend@gmail.com", + "provider": "google"}).status_code == 409) + + print("\n=== github invite without an address ===") + INBOX.clear() + r = admin.post("/api/admin/users", + json={"username": "octocat", "provider": "github"}) + check("accepted", r.status_code == 200, r.text[:120]) + check("says no mail was sent", r.json().get("invite_sent") is False) + check("explains why", "no email address" in (r.json().get("invite_why") or ""), + str(r.json().get("invite_why"))) + time.sleep(0.8) + check("nothing delivered", len(INBOX) == 0) + + print("\n=== resend ===") + INBOX.clear() + check("resend to a google invitee", + admin.post("/api/admin/users/friend@gmail.com/resend").status_code == 200) + time.sleep(1.2) + check("delivered again", len(INBOX) == 1, str(len(INBOX))) + check("resend without an address -> 409", + admin.post("/api/admin/users/octocat/resend").status_code == 409) + check("resend unknown user -> 404", + admin.post("/api/admin/users/nobody/resend").status_code == 404) + + print("\n=== provider binding ===") + # may_login is the single rule the callback consults, so exercising it + # directly tests the boundary without standing up real OAuth. + sys.path.insert(0, ROOT) + os.environ["MUSE_ALLOWED_USERS"] = "geneticglitch1" + import importlib + import app as muse + importlib.reload(muse) + + d = sqlite3.connect(DB); d.row_factory = sqlite3.Row + goog = d.execute("SELECT * FROM users WHERE username='friend@gmail.com'").fetchone() + gh = d.execute("SELECT * FROM users WHERE username='octocat'").fetchone() + d.close() + + check("google invitee may use google", + muse.may_login("friend@gmail.com", goog, "google") is True) + check("google invitee may NOT use github", + muse.may_login("friend@gmail.com", goog, "github") is False) + check("github invitee may use github", + muse.may_login("octocat", gh, "github") is True) + check("github invitee may NOT use google", + muse.may_login("octocat", gh, "google") is False) + check("owner on the allowlist may use github", + muse.may_login("geneticglitch1", None, "github") is True) + check("owner on the allowlist may use google too", + muse.may_login("geneticglitch1", None, "google") is True) + check("a stranger is refused on either", + not muse.may_login("stranger", None, "google") + and not muse.may_login("stranger", None, "github")) + + class Row(dict): + def __getitem__(self, k): return dict.__getitem__(self, k) + disabled = Row(goog); disabled["disabled"] = 1 + check("a disabled invitee is refused on their own provider", + muse.may_login("friend@gmail.com", disabled, "google") is False) + check("per-request re-check ignores provider", + muse.may_login("friend@gmail.com", goog, None) is True) + finally: + srv.send_signal(signal.SIGINT) + try: + srv.wait(timeout=10) + except Exception: + srv.kill() + return 1 if fail else 0 + + +if __name__ == "__main__": + t = None + import threading + loop = asyncio.new_event_loop() + t = threading.Thread(target=lambda: (asyncio.set_event_loop(loop), + loop.run_until_complete(smtp_sink())), + daemon=True) + t.start() + time.sleep(0.5) + code = main() + print(f"\n{ok} passed, {fail} failed") + sys.exit(code) diff --git a/web/components/admin/users-table.tsx b/web/components/admin/users-table.tsx index 8689238..d661162 100644 --- a/web/components/admin/users-table.tsx +++ b/web/components/admin/users-table.tsx @@ -2,7 +2,7 @@ import * as React from "react"; import { AnimatePresence, motion } from "framer-motion"; -import { Ban, Check, Plus, ShieldCheck, Trash2, Undo2, X } from "lucide-react"; +import { Ban, Check, Plus, Send, ShieldCheck, Trash2, Undo2, X } from "lucide-react"; import { api, bytes, type AdminUser } from "@/lib/api"; import { useToast } from "@/components/toast"; import { Button } from "@/components/ui/button"; @@ -28,9 +28,11 @@ export function UsersTable({

Users

-

- Adding someone here is the invitation — no email, no token. They sign in - with that provider and they’re in. +

+ Adding someone here is the authorisation, and it binds them to that + one provider. If SMTP is set up they also get an email; the link in it + carries no token, because possession of the mail isn’t what lets + anyone in — the row is.

+ )} +