From da8bf670932560f5ba7321318928bcd3476b46ed Mon Sep 17 00:00:00 2001 From: Aryan Singh Date: Wed, 5 Aug 2026 12:05:44 -0500 Subject: [PATCH] Bind invites to a provider, and email them Wanted shape: the owner signs in with GitHub, everyone invited later signs in with Google. That was previously a convention, not a boundary -- may_login checked only that a live row existed, so any enabled provider could satisfy any invite. Now an invite is bound to the provider it was issued for. friend@gmail.com invited for Google cannot be redeemed over GitHub, and there is never a second, weaker door onto an existing account. The per-request re-check still passes provider=None, because there the question is only whether the account is still live. Two related fixes fell out: - upsert_user no longer refreshes `provider` from the login. It is part of the identity an admin issued and may_login now enforces it, so overwriting it on sign-in would let a successful login silently rewrite the very rule that permitted it. - a Google invite must be a full email address. A bare username would authorise something Google can never return, and the invitee would see only "not authorised" with no way to work out why. INVITE EMAILS. Adding a user now mails them, when SMTP is configured and there is an address -- for Google there always is, since the identity IS one. The response reports whether the mail actually went and why not, because "Invited" alone reads as success while the person waits for an email nobody sent. The link carries NO token, deliberately. The row already names the exact identity permitted and Google will not hand us an unverified address, so possession of the mail is not what authorises anyone: a forwarded invite grants nothing, nothing expires, and resend is always safe. A signed expiring token would only buy the ability to withdraw an unused invitation, which deleting the row already does. Resend is offered in the panel on anyone who has an address and has never signed in -- the likeliest reason being that they missed it. tests/test_invites.py: 29 assertions driving a real SMTP conversation against a sink server, plus the full provider-binding matrix. 119 tests pass across four suites. Co-Authored-By: Claude Opus 5 --- README.md | 46 ++++-- app.py | 120 ++++++++++++-- tests/test_invites.py | 226 +++++++++++++++++++++++++++ web/components/admin/users-table.tsx | 61 +++++++- web/lib/api.ts | 11 +- 5 files changed, 434 insertions(+), 30 deletions(-) create mode 100644 tests/test_invites.py 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.

+ )} +