amadan

carlos / platform Public

Merge branch 'tenant-iso-leftovers' into main

Purge and membership leftovers: credentials, door keys and ledgers end with their app; invite accept by POST (#219)
claude f36ad36ab252f9c090a766dc063f5efe9c3f661e parent b94ad23parent 7297ab4
18 files changed, +548 −31
  • internal/console/apidoorkeys.go +14 −0
  • internal/console/apidoorkeys_test.go +133 −0
  • internal/console/apiledger.go +1 −1
  • internal/console/appsettings.go +3 −0
  • internal/console/authcli.go +27 −5
  • internal/console/deletion_test.go +35 −0
  • internal/console/doorkeys_store.go +18 −0
  • internal/console/ownership.go +2 −0
  • internal/console/server.go +64 −8
  • internal/console/server_account_test.go +26 −4
  • internal/console/store.go +49 −0
  • internal/console/svcpage_test.go +46 −0
  • internal/edge/status.go +17 −1
  • internal/edge/status_test.go +46 −6
  • internal/ledger/ledger.go +4 −2
  • internal/release/release.go +9 −1
  • internal/release/trash.go +16 −3
  • internal/release/trash_test.go +38 −0

CI failed — run details

diff --git a/internal/console/apidoorkeys.go b/internal/console/apidoorkeys.go
index 12f9a706a..814ea3f55 100644
--- a/internal/console/apidoorkeys.go
+++ b/internal/console/apidoorkeys.go
@@ -155,6 +155,20 @@ func (s *Server) publishDoorKeysDetached(r *http.Request, acct *Account, app str
return s.publishDoorKeys(ctx, acct, app)
}
+// republishDoorKeys is publishDoorKeysDetached for a change made elsewhere
+// that revoked an app name's keys in passing (purge, move): best-effort,
+// because that change has already landed and is not undone by a publish
+// that fails. The re-sign pass publishes within 12 hours regardless. A
+// console with nothing to sign with or nowhere to write skips it.
+func (s *Server) republishDoorKeys(r *http.Request, acct *Account, app string) {
+ if len(s.cfg.Key) != ed25519.PrivateKeySize || s.cfg.Releases == nil {
+ return
+ }
+ if err := s.publishDoorKeysDetached(r, acct, app); err != nil {
+ log.Printf("console: door keys %s/%s: publish after revoking (the re-sign pass retries): %v", acct.Sqid, app, err)
+ }
+}
+
// doorKeyWritable answers the 503 a console that cannot publish gives, before
// anything is written.
func (s *Server) doorKeyWritable(w http.ResponseWriter) bool {
diff --git a/internal/console/apidoorkeys_test.go b/internal/console/apidoorkeys_test.go
index fa8acca4f..0e224b2fd 100644
--- a/internal/console/apidoorkeys_test.go
+++ b/internal/console/apidoorkeys_test.go
@@ -435,3 +435,136 @@ func TestDoorKeyResignPassCoversEveryApp(t *testing.T) {
t.Fatal("the re-sign pass did not renew the record")
}
}
+
+// mintTestDoorKey stores one live door key for (account, app) directly.
+func mintTestDoorKey(t *testing.T, s *Store, accountID int64, app, id string) {
+ t.Helper()
+ if _, err := s.DoorKeyInsert(accountID, app, doorkey.Entry{ID: id, SHA256: strings.Repeat("a", 64)}); err != nil {
+ t.Fatalf("DoorKeyInsert(%d, %s): %v", accountID, app, err)
+ }
+}
+
+// TestPurgeAppRowLeavesNoCredentialsForTheName (#219 item 1): purging frees
+// the name, so a later app of that name must not inherit the purged app's
+// tokens (either kind) or door keys. Before the fix all three survived the
+// purge: the tokens still authenticated, and the door keys stayed live and
+// were re-signed every 12 hours.
+func TestPurgeAppRowLeavesNoCredentialsForTheName(t *testing.T) {
+ s := openTestStore(t)
+ now := time.Now()
+ acct := mustAccount(t, s, "amadan")
+ if _, err := s.CreateAppIn(acct.ID, "titogo", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ appTok, err := s.NewAppToken(acct.ID, "titogo", "paul@keymail.dev", "router", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ svcTok, err := s.NewServicePrincipal(acct.ID, "hub", "titogo", svcrole.Publish, "paul@keymail.dev", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ mintTestDoorKey(t, s, acct.ID, "titogo", "0123456789abcdef")
+ _, revBefore, err := s.DoorKeysLive(acct.ID, "titogo")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if err := s.TrashAppRow(acct.ID, "titogo", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := s.PurgeAppRow(acct.ID, "titogo"); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, ok := s.CLIPrincipal(appTok, now); ok {
+ t.Error("the purged app's app token still authenticates")
+ }
+ if _, ok := s.CLIPrincipal(svcTok, now); ok {
+ t.Error("the purged app's service credential still authenticates")
+ }
+ keys, rev, err := s.DoorKeysLive(acct.ID, "titogo")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(keys) != 0 {
+ t.Errorf("door keys after purge = %+v, want none live", keys)
+ }
+ // The revision climbs, so the empty record outranks what boxes admitted.
+ if rev <= revBefore {
+ t.Errorf("door-key revision after purge = %d, want above %d", rev, revBefore)
+ }
+
+ // The name is clean for its next owner: a new app of the same name, and
+ // a rename onto it, are not refused over leftovers.
+ if _, err := s.CreateAppIn(acct.ID, "titogo", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := s.CreateAppIn(acct.ID, "other", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := s.TrashAppRow(acct.ID, "titogo", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := s.PurgeAppRow(acct.ID, "titogo"); err != nil {
+ t.Fatal(err)
+ }
+ if err := s.RenameAppIn(acct.ID, "other", "titogo"); err != nil {
+ t.Errorf("RenameAppIn onto a purged name = %v, want it allowed", err)
+ }
+}
+
+// TestPurgeAppRowKeepsALiveAppsCredentials: the credential sweep rides on the
+// row delete, so a live (untrashed) app, which PurgeAppRow refuses, keeps
+// everything.
+func TestPurgeAppRowKeepsALiveAppsCredentials(t *testing.T) {
+ s := openTestStore(t)
+ now := time.Now()
+ acct := mustAccount(t, s, "amadan")
+ if _, err := s.CreateAppIn(acct.ID, "titogo", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ appTok, err := s.NewAppToken(acct.ID, "titogo", "paul@keymail.dev", "router", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ mintTestDoorKey(t, s, acct.ID, "titogo", "0123456789abcdef")
+ if err := s.PurgeAppRow(acct.ID, "titogo"); err != nil {
+ t.Fatal(err)
+ }
+ if _, ok := s.CLIPrincipal(appTok, now); !ok {
+ t.Error("a refused purge of a live app revoked its token")
+ }
+ if keys, _, _ := s.DoorKeysLive(acct.ID, "titogo"); len(keys) != 1 {
+ t.Errorf("a refused purge of a live app touched its door keys: %+v", keys)
+ }
+}
+
+// TestMoveAppLeavesNoLiveDoorKeysForTheName (#219 item 1): door keys name
+// their account, so they cannot follow a moved app. The source's must not
+// stay live for a later app of that name, and a purged predecessor's keys at
+// the destination must not answer for the app that moved in.
+func TestMoveAppLeavesNoLiveDoorKeysForTheName(t *testing.T) {
+ s := openTestStore(t)
+ now := time.Now()
+ from := mustAccount(t, s, "amadan")
+ to := mustAccount(t, s, "other")
+ if _, err := s.CreateAppIn(from.ID, "titogo", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ mintTestDoorKey(t, s, from.ID, "titogo", "0123456789abcdef")
+ // A leftover at the destination, the shape a purge before this fix left.
+ mintTestDoorKey(t, s, to.ID, "titogo", "fedcba9876543210")
+ if err := s.MoveApp("titogo", from.ID, to.ID); err != nil {
+ t.Fatalf("MoveApp: %v", err)
+ }
+ for _, side := range []*Account{from, to} {
+ keys, _, err := s.DoorKeysLive(side.ID, "titogo")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(keys) != 0 {
+ t.Errorf("%s: live door keys for titogo after the move = %+v, want none", side.Sqid, keys)
+ }
+ }
+}
diff --git a/internal/console/apiledger.go b/internal/console/apiledger.go
index 1b3c683fb..1173ec665 100644
--- a/internal/console/apiledger.go
+++ b/internal/console/apiledger.go
@@ -222,7 +222,7 @@ func (s *Server) handleAPILedgerList(w http.ResponseWriter, r *http.Request) {
// app just to learn a handful of names. Blobs live INSIDE a chain's
// directory, so the children are exactly the chain names; ledgers.json
// is a file at this level, and ListDirs never returns files.
- prefix := "ledgers/" + acct.Sqid + "/" + app + "/"
+ prefix := release.LedgerAppPrefix(acct.Sqid, app)
dirs, err := s.cfg.Releases.ListDirs(ctx, prefix)
if err != nil {
s.ledgerStoreError(w, fmt.Sprintf("ledger list %s/%s", acct.Sqid, app), err)
diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go
index 4d6798069..bed6ff51f 100644
--- a/internal/console/appsettings.go
+++ b/internal/console/appsettings.go
@@ -1353,6 +1353,9 @@ func (s *Server) handleAppMoveAccount(w http.ResponseWriter, r *http.Request) {
return
}
s.audit(acct, app, addr, "move", "moved "+app+" to account "+dest.Name+" ("+target+")")
+ // MoveApp revoked the name's door keys on both sides; tell the boxes now.
+ s.republishDoorKeys(r, acct, app)
+ s.republishDoorKeys(r, dest, app)
if err := s.store.RecordEvent(dest.ID, app, addr, "move", "arrived from account "+acct.Name+" ("+acct.Sqid+")", time.Now()); err != nil {
log.Printf("console: move event %s: %v", app, err)
}
diff --git a/internal/console/authcli.go b/internal/console/authcli.go
index ac070342f..2f35a07dc 100644
--- a/internal/console/authcli.go
+++ b/internal/console/authcli.go
@@ -1463,11 +1463,17 @@ func (s *Server) handleCLITokenRevoke(w http.ResponseWriter, r *http.Request) {
// door, so asking for one would waste it.
//
// SCOPED TO SERVICE ROWS ONLY. A legacy kind='app' row keeps working here
- // exactly as it always has (TestPersonalSettingsStillRevokesALegacyAppToken
- // pins it) — its minter is not necessarily an account owner, so this may
- // be the only door they have. The same hard-DELETE-and-no-audit
- // inconsistency is true of it and predates this branch; widening the
- // refusal would take away a working control to fix nothing here.
+ // (TestPersonalSettingsStillRevokesALegacyAppToken pins it) — its minter
+ // is not necessarily an account owner, so this may be the only door they
+ // have. The same hard-DELETE-and-no-audit inconsistency is true of it and
+ // predates this branch; widening the refusal would take away a working
+ // control to fix nothing here.
+ //
+ // But only while the minter is still a MEMBER of the token's account
+ // (discussion #219, item 3). The app token speaks for that account's app,
+ // so revoking it stops that app's service; somebody removed from the
+ // account has no authority over it any more, and having minted it once
+ // does not keep any.
svc, isService, err := s.serviceRowOf(addr, r.FormValue("id"))
switch {
case err != nil:
@@ -1485,6 +1491,22 @@ func (s *Server) handleCLITokenRevoke(w http.ResponseWriter, r *http.Request) {
"recorded in that account's activity. Nothing was revoked.")
http.Redirect(w, r, "/settings", http.StatusSeeOther)
return
+ case svc.IsApp():
+ // Fails closed on a read error, the same direction as the classify
+ // above, and is refused before the passkey for the same reason.
+ role, rerr := s.store.Role(svc.AccountID, addr)
+ if rerr != nil {
+ log.Printf("console: cli token revoke: membership of %s in %d: %v", addr, svc.AccountID, rerr)
+ s.setFlash(w, "err", "Could not check your membership, so nothing was revoked. Try again.")
+ http.Redirect(w, r, "/settings", http.StatusSeeOther)
+ return
+ }
+ if role == "" {
+ s.setFlash(w, "err", "That credential belongs to an account you are no longer a member of, "+
+ "so it is not yours to revoke. Nothing was revoked.")
+ http.Redirect(w, r, "/settings", http.StatusSeeOther)
+ return
+ }
}
// Step-up (spec §7a): killing a terminal is a security action, and the
// person doing it should be the person whose terminals they are. An
diff --git a/internal/console/deletion_test.go b/internal/console/deletion_test.go
index a6ff58b4d..45223173e 100644
--- a/internal/console/deletion_test.go
+++ b/internal/console/deletion_test.go
@@ -14,6 +14,7 @@ import (
"github.com/carlosframework/platform/internal/appconfig"
"github.com/carlosframework/platform/internal/dnsrec"
"github.com/carlosframework/platform/internal/domains"
+ "github.com/carlosframework/platform/internal/doorkey"
"github.com/carlosframework/platform/internal/mail"
"github.com/carlosframework/platform/internal/objstore"
"github.com/carlosframework/platform/internal/release"
@@ -414,6 +415,40 @@ func TestDeleteNowDestroysEverythingAndFreesTheName(t *testing.T) {
}
}
+// Delete now takes the app's ledgers and door keys with it (#219 items 1
+// and 2): both are keyed by the name the purge frees, outside apps/. The
+// door-key record is republished at once, empty, rather than at the next
+// 12-hourly re-sign.
+func TestDeleteNowSweepsLedgersAndDoorKeys(t *testing.T) {
+ fx, c, csrf := trashPageFixture(t)
+ if _, err := fx.st.DoorKeyInsert(OpsAccountID, "kass", doorkey.Entry{ID: "0123456789abcdef", SHA256: strings.Repeat("a", 64)}); err != nil {
+ t.Fatal(err)
+ }
+ ledgerKey := release.LedgerAppPrefix(fx.sqid, "kass") + "ledgers.json"
+ if _, err := fx.rel.Put(context.Background(), ledgerKey, []byte(`{"schema":1,"public":true}`), ""); err != nil {
+ t.Fatal(err)
+ }
+ status, _, body := postGated(t, fx, c, fx.u("/trash/delete"),
+ url.Values{"csrf": {csrf}, "app": {"kass"}, "confirm": {"kass"}})
+ if status != http.StatusSeeOther {
+ t.Fatalf("delete = %d:\n%s", status, body)
+ }
+ if keys, err := fx.rel.List(context.Background(), release.LedgerAppPrefix(fx.sqid, "kass")); err != nil || len(keys) != 0 {
+ t.Fatalf("ledgers after purge = %v %v, want none", keys, err)
+ }
+ wire, _, err := fx.rel.Get(context.Background(), doorkey.RecordKey(fx.sqid, "kass"))
+ if err != nil {
+ t.Fatalf("door-key record after purge: %v", err)
+ }
+ rec, err := doorkey.Verify(fx.pub, wire, time.Now())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rec.Keys) != 0 {
+ t.Fatalf("published door keys after purge = %+v, want none", rec.Keys)
+ }
+}
+
// The production flag is the deletion protection, all the way to the last
// door: a trashed app that is flagged production is not destroyed, however
// much ceremony is performed at it.
diff --git a/internal/console/doorkeys_store.go b/internal/console/doorkeys_store.go
index 9b00ccee8..c826395ff 100644
--- a/internal/console/doorkeys_store.go
+++ b/internal/console/doorkeys_store.go
@@ -123,6 +123,24 @@ WHERE account_id = ? AND app = ? AND id = ? AND revoked_at = 0`, time.Now().Unix
return rev, tx.Commit()
}
+// revokeAllDoorKeys revokes every live key the account holds under app inside
+// the caller's transaction, bumping the revision when it revoked any. It is
+// for an app name that stops belonging to an app (PurgeAppRow, MoveApp): a
+// key left live there would let a later app of the same name be called with
+// a key nobody minted for it.
+func revokeAllDoorKeys(tx *sql.Tx, accountID int64, app string) error {
+ res, err := tx.Exec(`UPDATE door_keys SET revoked_at = ?
+WHERE account_id = ? AND app = ? AND revoked_at = 0`, time.Now().UnixNano(), accountID, app)
+ if err != nil {
+ return err
+ }
+ if n, err := res.RowsAffected(); err != nil || n == 0 {
+ return err
+ }
+ _, err = bumpDoorKeyRevision(tx, accountID, app)
+ return err
+}
+
// DoorKeysLive reads the app's live keys and its revision in one
// transaction, so the two always describe the same key set. An app with no
// revision row answers no keys at revision 0.
diff --git a/internal/console/ownership.go b/internal/console/ownership.go
index b0b355d81..28d0d6220 100644
--- a/internal/console/ownership.go
+++ b/internal/console/ownership.go
@@ -475,6 +475,8 @@ func (s *Server) handlePurgeApp(w http.ResponseWriter, r *http.Request) {
"did not update — it will catch up on the next restart.")
return
}
+ // PurgeAppRow revoked the app's door keys; tell the boxes now.
+ s.republishDoorKeys(r, acct, app)
s.audit(acct, app, addr, "purge", "permanently deleted "+app)
s.setFlash(w, "ok", "Deleted “"+app+"” permanently. The name is free to claim again.")
http.Redirect(w, r, "/a/"+url.PathEscape(acct.Sqid)+"/trash", http.StatusSeeOther)
diff --git a/internal/console/server.go b/internal/console/server.go
index ddb6ce18f..01c315e6e 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -1311,7 +1311,10 @@ func NewServer(cfg Config, st *Store) (*Server, error) {
s.mux.HandleFunc("GET /a/{sqid}/trash", s.handleTrash)
s.mux.HandleFunc("POST /a/{sqid}/trash/restore", s.handleRestoreApp)
s.mux.HandleFunc("POST /a/{sqid}/trash/delete", s.handlePurgeApp)
- s.mux.HandleFunc("GET /invite/{token}", s.handleAcceptInvite)
+ // GET only shows the invitation; joining is the POST its button sends
+ // (discussion #219, item 4).
+ s.mux.HandleFunc("GET /invite/{token}", s.handleInvitePage)
+ s.mux.HandleFunc("POST /invite/{token}", s.handleAcceptInvite)
s.mux.HandleFunc("GET /accounts/new", s.handleNewAccountForm)
s.mux.HandleFunc("POST /accounts/new", s.handleNewAccount)
// The pre-accounts URLs, kept as redirects so old links and bookmarks
@@ -2203,21 +2206,74 @@ func (s *Server) handleRevokeInvite(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/a/"+url.PathEscape(acct.Sqid)+"/team", http.StatusSeeOther)
}
-// handleAcceptInvite consumes an invite link. It requires a session as the
-// invited address — the link lands in one Keymail inbox and is worthless to
-// anyone else. An address that has never signed in does the OAuth ceremony
-// first and comes back to the link.
+// invitationGone is every refusal of an invite link, on the page and on the
+// accept alike.
+func (s *Server) invitationGone(w http.ResponseWriter, r *http.Request) {
+ s.renderMessage(w, r, http.StatusGone, "Invitation unavailable",
+ "That invitation has expired, was already used, or was sent to a different address. Ask for a fresh one.")
+}
+
+// handleInvitePage is where an invite link lands: it says which account the
+// invitation is for and offers a button that joins it. It changes nothing.
+// Joining used to happen right here, on the GET, which made any page able to
+// add a signed-in person to the inviter's account and switch them into it
+// with a link or a redirect: SameSite=Lax sends the session cookie on a
+// top-level navigation (discussion #219, item 4). Links already sent keep
+// working; they land on this page.
+//
+// It requires a session as the invited address — the link lands in one
+// Keymail inbox and is worthless to anyone else. An address that has never
+// signed in does the OAuth ceremony first and comes back to the link.
+func (s *Server) handleInvitePage(w http.ResponseWriter, r *http.Request) {
+ addr, raw, ok := s.session(r)
+ if !ok {
+ http.Redirect(w, r, "/", http.StatusFound)
+ return
+ }
+ token := r.PathValue("token")
+ inv, err := s.store.PendingInviteFor(token, addr, time.Now())
+ if err != nil {
+ s.invitationGone(w, r)
+ return
+ }
+ a, err := s.store.AccountByID(inv.AccountID)
+ if err != nil || a == nil {
+ s.renderMessage(w, r, http.StatusInternalServerError, "Error", "Could not read the invitation. Try again.")
+ return
+ }
+ p := s.basePage(r)
+ p.Title = "Join " + a.Name + "?"
+ p.AppName = a.Name
+ p.Confirm = &ConfirmView{
+ Title: "Join " + a.Name + "?",
+ Lines: []string{inv.InvitedBy + " invited you to join " + a.Name + " as a member."},
+ Action: "/invite/" + url.PathEscape(token),
+ Fields: [][2]string{{"csrf", raw}},
+ Button: "Join " + a.Name,
+ CancelURL: "/",
+ }
+ s.render(w, http.StatusOK, "confirm_action", p)
+}
+
+// handleAcceptInvite is the confirm page's button: it consumes the invite and
+// makes the account the person's current one. Session, then CSRF, then the
+// invite's own gates (AcceptInvite: unused, addressed to this session,
+// unexpired), which play the owner check's part. No passkey step: joining
+// gives the joiner nothing over anybody else, and it is their own choice.
func (s *Server) handleAcceptInvite(w http.ResponseWriter, r *http.Request) {
- addr, _, ok := s.session(r)
+ addr, raw, ok := s.session(r)
if !ok {
http.Redirect(w, r, "/", http.StatusFound)
return
}
+ if subtle.ConstantTimeCompare([]byte(r.FormValue("csrf")), []byte(raw)) != 1 {
+ s.renderMessage(w, r, http.StatusForbidden, "Refused", "Cross-site request blocked.")
+ return
+ }
now := time.Now()
accountID, err := s.store.AcceptInvite(r.PathValue("token"), addr, now)
if err != nil {
- s.renderMessage(w, r, http.StatusGone, "Invitation unavailable",
- "That invitation has expired, was already used, or was sent to a different address. Ask for a fresh one.")
+ s.invitationGone(w, r)
return
}
a, err := s.store.AccountByID(accountID)
diff --git a/internal/console/server_account_test.go b/internal/console/server_account_test.go
index cac0c3935..59bdf4501 100644
--- a/internal/console/server_account_test.go
+++ b/internal/console/server_account_test.go
@@ -327,11 +327,30 @@ func TestInviteAcceptCreatesMembership(t *testing.T) {
t.Fatalf("invite link not delivered through keymail: %v", sent)
}
- // Sam signs in and accepts.
+ // Sam signs in and opens the link: a page that asks, and changes nothing
+ // (#219 item 4 — a GET that joined let any page add a signed-in person to
+ // the inviter's account and switch them into it).
fx.setUser("sam@keymail.dev")
sam := newBrowser(t)
signUp(t, fx, sam, "code-sam")
- st, loc, _ = get(t, sam, fx.ts.URL+"/invite/"+pending[0].Token)
+ inviteURL := fx.ts.URL + "/invite/" + pending[0].Token
+ st, _, body := get(t, sam, inviteURL)
+ if st != http.StatusOK || !strings.Contains(body, `action="/invite/`+pending[0].Token+`"`) {
+ t.Fatalf("invite link = %d, want 200 and a confirm form:\n%s", st, body)
+ }
+ if role, _ := fx.st.Role(OpsAccountID, "sam@keymail.dev"); role != "" {
+ t.Fatalf("opening the invite link joined the account (role %q)", role)
+ }
+ // A cross-site POST, without the session's CSRF token, is refused.
+ if st, _, _ := postForm(t, sam, inviteURL, url.Values{}); st != http.StatusForbidden {
+ t.Fatalf("accept without csrf = %d, want 403", st)
+ }
+ if role, _ := fx.st.Role(OpsAccountID, "sam@keymail.dev"); role != "" {
+ t.Fatalf("a POST without csrf joined the account (role %q)", role)
+ }
+ // The button.
+ samCSRF := sessionToken(t, fx, sam)
+ st, loc, _ = postForm(t, sam, inviteURL, url.Values{"csrf": {samCSRF}})
if st != http.StatusSeeOther || loc != fx.path("/") {
t.Fatalf("accept = %d -> %q, want 303 -> the account", st, loc)
}
@@ -341,8 +360,11 @@ func TestInviteAcceptCreatesMembership(t *testing.T) {
if st, _, _ := get(t, sam, fx.u("/")); st != http.StatusOK {
t.Errorf("accepted member cannot see the account: %d", st)
}
- // Single use.
- if st, _, _ := get(t, sam, fx.ts.URL+"/invite/"+pending[0].Token); st == http.StatusSeeOther {
+ // Single use, on both the page and the button.
+ if st, _, _ := get(t, sam, inviteURL); st != http.StatusGone {
+ t.Errorf("used invite page = %d, want 410", st)
+ }
+ if st, _, _ := postForm(t, sam, inviteURL, url.Values{"csrf": {samCSRF}}); st == http.StatusSeeOther {
t.Error("invite link worked twice")
}
}
diff --git a/internal/console/store.go b/internal/console/store.go
index 9e94c52da..ef7fec905 100644
--- a/internal/console/store.go
+++ b/internal/console/store.go
@@ -1209,6 +1209,26 @@ VALUES (?, ?, ?, ?, ?, ?)`,
return tok, nil
}
+// PendingInviteFor reads an invite without consuming it, behind the same
+// three gates AcceptInvite applies: unaccepted, addressed to address, and
+// unexpired. Any failure is one error, so the confirm page that shows it can
+// answer every refusal the same way.
+func (s *Store) PendingInviteFor(token, address string, now time.Time) (*Invite, error) {
+ var i Invite
+ var created, expires, accepted int64
+ err := s.db.QueryRow(`SELECT token, account_id, address, invited_by, created_at, expires_at, accepted_at
+FROM invites WHERE token = ?`, token).Scan(&i.Token, &i.AccountID, &i.Address, &i.InvitedBy, &created, &expires, &accepted)
+ if err != nil {
+ return nil, err
+ }
+ if accepted != 0 || i.Address != address || now.Unix() > expires {
+ return nil, fmt.Errorf("console: that invite cannot be used by %s", address)
+ }
+ i.CreatedAt = time.Unix(created, 0).UTC()
+ i.ExpiresAt = time.Unix(expires, 0).UTC()
+ return &i, nil
+}
+
// AcceptInvite consumes an invite and creates the membership, in one
// transaction. Three gates, all inside it: the token must be unaccepted, the
// address must be the one it was addressed to (an invite is not a bearer
@@ -1564,6 +1584,21 @@ func (s *Store) PurgeAppRow(accountID int64, app string) error {
} else if err != sql.ErrNoRows {
return err
}
+ // The same for the app's credentials (discussion #219, item 1). App
+ // tokens of both kinds are bound (account, name), so a later app of
+ // this name would inherit them unminted; they are DELETED rather than
+ // revoked, because RenameAppIn and MoveApp refuse a name that still
+ // carries any such row, revoked or not. Door keys are revoked rather
+ // than deleted: the revision has to keep climbing past what boxes
+ // already admitted, and the re-sign pass publishes the now-empty
+ // record (apidoorkeys.go).
+ if _, err := tx.ExecContext(ctx, `DELETE FROM cli_tokens WHERE kind IN (?, ?) AND account_id = ? AND app = ?`,
+ cliKindApp, cliKindService, accountID, app); err != nil {
+ return err
+ }
+ if err := revokeAllDoorKeys(tx, accountID, app); err != nil {
+ return err
+ }
}
return tx.Commit()
}
@@ -1771,6 +1806,20 @@ LIMIT 1`, cliKindService, to, cliKindService, from, app).Scan(&collidingName)
to, cliKindApp, cliKindService, from, app); err != nil {
return err
}
+ // Door keys cannot follow the app: each key names its account's sqid
+ // (doorkey.Token), and the edge admits it only for routes of that account,
+ // so the moved app's processes need new keys either way. What the move
+ // must not leave behind is a LIVE key on either side (discussion #219,
+ // item 1): the source's would be inherited by a later app of this name
+ // there, and any at the destination are a purged predecessor's, which the
+ // moved-in app would otherwise answer to. The destination has no row for
+ // this name (checked above), so nothing live owns those keys.
+ if err := revokeAllDoorKeys(tx, from, app); err != nil {
+ return err
+ }
+ if err := revokeAllDoorKeys(tx, to, app); err != nil {
+ return err
+ }
// Email history is keyed by the account's sqid, not its id, so it is
// carried by sqid (mailevents.go).
var fromSqid, toSqid string
diff --git a/internal/console/svcpage_test.go b/internal/console/svcpage_test.go
index bc334849d..407d6b093 100644
--- a/internal/console/svcpage_test.go
+++ b/internal/console/svcpage_test.go
@@ -553,6 +553,52 @@ func TestPersonalSettingsCannotHandPostAServiceRevoke(t *testing.T) {
}
}
+// TestPersonalSettingsRefusesAnAppTokenRevokeAfterRemoval (#219 item 3): the
+// legacy app-token revoke on /settings is scoped to the minter's address, so
+// it outlived their membership. Somebody removed from the account must not be
+// able to stop that account's app by revoking a credential they once minted.
+// Refused before the passkey: no gesture would change the answer.
+func TestPersonalSettingsRefusesAnAppTokenRevokeAfterRemoval(t *testing.T) {
+ fx := newConsoleFixture(t)
+ otherID := testAccountID()
+ if err := fx.st.CreateAccount(otherID, accountSqid(otherID), "Sam's", "sam@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.AddMember(otherID, "paul@keymail.dev", RoleMember, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ secret, err := fx.st.NewAppToken(otherID, "kass", "paul@keymail.dev", "router", time.Now())
+ if err != nil {
+ t.Fatal(err)
+ }
+ toks, err := fx.st.CLITokens("paul@keymail.dev")
+ if err != nil || len(toks) != 1 {
+ t.Fatalf("CLITokens = %d rows %v, want the app token", len(toks), err)
+ }
+ id := toks[0].ID
+ if err := fx.st.RemoveMember(otherID, "paul@keymail.dev"); err != nil {
+ t.Fatal(err)
+ }
+
+ c := signInPaul(t, fx)
+ csrf := sessionToken(t, fx, c)
+ status, loc, body := postForm(t, c, fx.ts.URL+"/auth/cli/tokens/revoke",
+ url.Values{"csrf": {csrf}, "id": {id}})
+ if strings.HasPrefix(loc, fx.fk.srv.URL) {
+ t.Fatalf("the refusal cost a passkey trip (-> %q)", loc)
+ }
+ if status != http.StatusSeeOther || loc != "/settings" {
+ t.Fatalf("revoke after removal = %d -> %q, want 303 back to /settings:\n%s", status, loc, body)
+ }
+ if _, ok := fx.st.CLIPrincipal(secret, time.Now()); !ok {
+ t.Fatal("a removed member revoked the account's app token")
+ }
+ _, _, page := get(t, c, fx.ts.URL+"/settings")
+ if !strings.Contains(page, "no longer a member") {
+ t.Fatalf("/settings does not say why the revoke was refused:\n%s", page)
+ }
+}
+
// TestPaneMintRefusalsCostNoPasskey — a bad name or an unknown role is the
// caller's slip, and no gesture could change the answer, so it is refused
// BEFORE the passkey. Same reasoning handleStoreDestroy's "no backend"
diff --git a/internal/edge/status.go b/internal/edge/status.go
index f7ab59944..2975dbe8f 100644
--- a/internal/edge/status.go
+++ b/internal/edge/status.go
@@ -80,6 +80,13 @@ type statusCached struct {
// - outside the TTL with a store error and NO cached copy (nothing has
// ever been successfully fetched): 503. There is nothing honest left to
// serve.
+// - one exception, for ledger objects only: outside the TTL with the
+// object GONE (os.ErrNotExist), drop the cached copy and fail. For a
+// ledger a deletion is the bucket's answer, not a blip: it is how an app
+// purge reaches this cache, and without it the purged app's public
+// ledger kept serving from every edge that had read it, to the end of
+// the process (discussion #219, item 2). The status page's own files
+// keep the stale-on-anything rule above; nothing deletes them on purpose.
type statusHandler struct {
store release.Store
// ttl and now are both overridable by tests (a tiny ttl plus a fake
@@ -364,7 +371,8 @@ func ledgerShape(rest []string) (ledgerObject, bool) {
// get implements the cache contract documented on statusHandler: fresh cache
// hit wins outright (no store touch at all — that's what bounds read
// volume); otherwise it tries the store, falling back to a stale cached copy
-// on error, and only fails when there is nothing cached to fall back to.
+// on error, and only fails when there is nothing cached to fall back to or
+// a ledger object is gone.
func (h *statusHandler) get(ctx context.Context, key, ctype string) ([]byte, string, error) {
h.mu.Lock()
c, cached := h.cache[key]
@@ -376,6 +384,14 @@ func (h *statusHandler) get(ctx context.Context, key, ctype string) ([]byte, str
body, _, err := h.store.Get(ctx, key)
if err != nil {
+ if errors.Is(err, os.ErrNotExist) && strings.HasPrefix(key, "ledgers/") {
+ if cached {
+ h.mu.Lock()
+ delete(h.cache, key)
+ h.mu.Unlock()
+ }
+ return nil, "", err
+ }
if cached {
log.Printf("edge: status %s: store read failed outside TTL (%v) — serving stale copy fetched at %s",
key, err, c.fetched.Format(time.RFC3339))
diff --git a/internal/edge/status_test.go b/internal/edge/status_test.go
index 76cd05c80..87ac694e5 100644
--- a/internal/edge/status_test.go
+++ b/internal/edge/status_test.go
@@ -552,10 +552,14 @@ func TestLedgerMissCacheIsBounded(t *testing.T) {
// bucket blipped — is the same dishonesty the status page's cache exists to
// avoid). An object that was never cached still 404s: stale caching, not
// fabrication.
+//
+// The outage is a flakyStore error, not a deleted directory: a deletion
+// answers os.ErrNotExist, which is the bucket saying the object is gone
+// (TestLedgerServingStopsAfterPurge), not a blip.
func TestLedgerServingStaleManifestHonored(t *testing.T) {
- dir := t.TempDir()
- st := release.NewDirStore(dir)
- entryKey, _ := seedLedger(t, st, true)
+ base := release.NewDirStore(t.TempDir())
+ st := &flakyStore{Store: base}
+ entryKey, _ := seedLedger(t, base, true)
h := newStatusHandler(st)
h.ttl = 20 * time.Millisecond
fakeNow := time.Now()
@@ -567,9 +571,7 @@ func TestLedgerServingStaleManifestHonored(t *testing.T) {
}
// Break the store entirely and age both cached objects past the TTL.
- if err := os.RemoveAll(dir); err != nil {
- t.Fatalf("break store: %v", err)
- }
+ st.breakIt(true)
fakeNow = fakeNow.Add(21 * time.Millisecond)
stale := getStatus(t, h, "/ledgers/bab/platform/carbon/HEAD.json")
@@ -584,6 +586,44 @@ func TestLedgerServingStaleManifestHonored(t *testing.T) {
}
}
+// TestLedgerServingStopsAfterPurge (#219 item 2): purging an app deletes
+// its ledgers from the bucket, and an edge that had cached the public
+// manifest must stop serving them once its TTL runs out, not fall back to
+// the stale copy as it would for a bucket blip. Otherwise a later app of the
+// same name would find the purged app's history public on that edge.
+func TestLedgerServingStopsAfterPurge(t *testing.T) {
+ st := release.NewDirStore(t.TempDir())
+ seedLedger(t, st, true)
+ h := newStatusHandler(st)
+ h.ttl = 20 * time.Millisecond
+ fakeNow := time.Now()
+ h.now = func() time.Time { return fakeNow }
+
+ for _, p := range []string{"/ledgers/bab/platform/ledgers.json", "/ledgers/bab/platform/carbon/HEAD.json"} {
+ if rr := getStatus(t, h, p); rr.Code != http.StatusOK {
+ t.Fatalf("before the purge %s = %d, want 200", p, rr.Code)
+ }
+ }
+ keys, err := st.List(context.Background(), release.LedgerAppPrefix("bab", "platform"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, k := range keys {
+ if err := st.Delete(context.Background(), k); err != nil {
+ t.Fatal(err)
+ }
+ }
+ fakeNow = fakeNow.Add(21 * time.Millisecond)
+ for _, p := range []string{"/ledgers/bab/platform/ledgers.json", "/ledgers/bab/platform/carbon/HEAD.json"} {
+ if rr := getStatus(t, h, p); rr.Code != http.StatusNotFound {
+ t.Errorf("after the purge %s = %d, want 404", p, rr.Code)
+ }
+ }
+ if keys := cacheKeys(h); keys["ledgers/bab/platform/ledgers.json"] {
+ t.Errorf("the purged manifest is still cached: %v", keys)
+ }
+}
+
// statusEdgeWithCache wires a full Edge (Host dispatch, shared response
// cache) in front of one status route — testEdgeWithStore's shape, but
// returning the *Edge so a test can look inside e.cache.
diff --git a/internal/ledger/ledger.go b/internal/ledger/ledger.go
index 47d1f23ad..74a4ec918 100644
--- a/internal/ledger/ledger.go
+++ b/internal/ledger/ledger.go
@@ -13,6 +13,8 @@ import (
"encoding/json"
"regexp"
"strings"
+
+ "github.com/carlosframework/platform/internal/release"
)
const (
@@ -53,7 +55,7 @@ func ValidName(s string) bool {
}
func Prefix(sqid, app, name string) string {
- return "ledgers/" + sqid + "/" + app + "/" + name + "/"
+ return release.LedgerAppPrefix(sqid, app) + name + "/"
}
func HeadKey(sqid, app, name string) string {
@@ -61,7 +63,7 @@ func HeadKey(sqid, app, name string) string {
}
func ManifestKey(sqid, app string) string {
- return "ledgers/" + sqid + "/" + app + "/ledgers.json"
+ return release.LedgerAppPrefix(sqid, app) + "ledgers.json"
}
func BlobKey(sqid, app, name, sha string) string {
diff --git a/internal/release/release.go b/internal/release/release.go
index b9bfcb531..e7d8111cb 100644
--- a/internal/release/release.go
+++ b/internal/release/release.go
@@ -220,9 +220,17 @@ func ChannelKey(acct, app, channel string) string {
func ChannelsPrefix(acct, app string) string { return appPrefix(acct, app) + "channels/" }
func ReleasesPrefix(acct, app string) string { return appPrefix(acct, app) + "releases/" }
-// AppPrefix is everything an app owns in the bucket, under its account.
+// AppPrefix is everything an app owns in the bucket, under its account,
+// except its open ledgers (LedgerAppPrefix) and its door-key record
+// (internal/doorkey), which live outside it.
func AppPrefix(acct, app string) string { return appPrefix(acct, app) }
+// LedgerAppPrefix is where an app's open ledgers live. internal/ledger builds
+// every ledger key under it, and the edge serves public ones from it
+// (internal/edge/status.go). It is defined here, not in internal/ledger,
+// because PurgeApp sweeps it and ledger imports release.
+func LedgerAppPrefix(acct, app string) string { return "ledgers/" + acct + "/" + app + "/" }
+
func appPrefix(acct, app string) string { return "apps/" + acct + "/" + app + "/" }
// The legacy, account-less key shapes. Since the read-fallback was retired
diff --git a/internal/release/trash.go b/internal/release/trash.go
index bf4739902..e9681efab 100644
--- a/internal/release/trash.go
+++ b/internal/release/trash.go
@@ -161,9 +161,9 @@ func PurgeReady(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app s
}
// PurgeApp irreversibly destroys a trashed app's bucket objects: every key
-// under apps/<acct>/<app>/, the trash marker LAST, so a purge that dies
-// half-way leaves a trashed app with fewer objects rather than a live app
-// with holes — the next pass finishes the job.
+// under apps/<acct>/<app>/ and ledgers/<acct>/<app>/, the trash marker LAST,
+// so a purge that dies half-way leaves a trashed app with fewer objects
+// rather than a live app with holes — the next pass finishes the job.
//
// This is the platform's first destructive automation surface, so every
// guard fails toward keeping data:
@@ -254,6 +254,19 @@ func PurgeApp(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app str
if err != nil {
return fmt.Errorf("release: list %s: %w", prefix, err)
}
+ // The app's open ledgers too (discussion #219, item 2). They live
+ // outside the app's prefix, keyed by the same (account, app), so a sweep
+ // of apps/ alone left them for the next app of this name: its ledger
+ // writes would chain onto the purged app's history, and the edge would
+ // keep serving that history if the old manifest said public. Swept
+ // before the marker, so a purge that dies here is retried by the next
+ // pass. The ledger spec's "publication is forever" holds as far as it
+ // ever did: the bucket is versioned, and that is where it is kept.
+ ledgerKeys, err := s.List(ctx, LedgerAppPrefix(acct, app))
+ if err != nil {
+ return fmt.Errorf("release: list %s: %w", LedgerAppPrefix(acct, app), err)
+ }
+ keys = append(keys, ledgerKeys...)
for _, key := range keys {
if key == marker {
continue // last, below
diff --git a/internal/release/trash_test.go b/internal/release/trash_test.go
index 43794d7b3..bda14bd17 100644
--- a/internal/release/trash_test.go
+++ b/internal/release/trash_test.go
@@ -322,6 +322,44 @@ func TestPurgeAppDestroysEverythingMarkerLast(t *testing.T) {
}
}
+// Contract 4b (discussion #219, item 2): the app's open ledgers go too. They
+// live under ledgers/<acct>/<app>/, outside the app's own prefix, and a purge
+// that left them handed them to the next app of the same name. Swept before
+// the marker, and a neighbouring app's ledgers stay.
+func TestPurgeAppSweepsTheAppsLedgers(t *testing.T) {
+ base := NewDirStore(t.TempDir())
+ var deletes []string
+ s := recordingStore{Store: base, deletes: &deletes}
+ _, pub := testKeys(t)
+ seedApp(t, s, "bab", "derp")
+ for _, key := range []string{
+ "ledgers/bab/derp/ledgers.json",
+ "ledgers/bab/derp/carbon/HEAD.json",
+ "ledgers/bab/derp/carbon/entries/20261003T000000Z-0123456789ab.json",
+ "ledgers/bab/derp/carbon/blobs/" + strings.Repeat("a", 64),
+ "ledgers/bab/derpy/ledgers.json",
+ } {
+ if _, err := s.Put(ctx, key, []byte(`{}`), ""); err != nil {
+ t.Fatal(err)
+ }
+ }
+ if err := TrashApp(ctx, s, "bab", "derp", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := PurgeApp(ctx, s, pub, "bab", "derp", time.Now()); err != nil {
+ t.Fatalf("PurgeApp: %v", err)
+ }
+ if got := listUnder(t, s, LedgerAppPrefix("bab", "derp")); len(got) != 0 {
+ t.Fatalf("purge left the app's ledgers behind: %v", got)
+ }
+ if got := listUnder(t, s, LedgerAppPrefix("bab", "derpy")); len(got) != 1 {
+ t.Fatalf("purge reached the neighbouring app's ledgers: %v", got)
+ }
+ if marker := AppTombstoneKey("bab", "derp"); deletes[len(deletes)-1] != marker {
+ t.Fatalf("delete order %v — the marker must go LAST", deletes)
+ }
+}
+
// Contract 5: idempotent. A half-purged app converges on a second pass, and
// an already-purged one reports ErrNotTrashed — which the caller reads as
// "done", not as a failure.