amadan
Tenant isolation leftovers: purge credentials and ledgers, members-only app-token revoke, POST invite accept
Discussion #219. Each fix stops something outliving the app or membership
it belonged to.
1. PurgeAppRow now deletes the app's cli_tokens (kind app and service) and
revokes its door keys, bumping the revision so the empty record
outranks what boxes already admitted. Purging through the console
publishes the record right away. MoveApp already refused stale
destination tokens. It now also revokes the name's door keys on both
sides: keys name their account's sqid and cannot follow the app.
2. release.PurgeApp also sweeps ledgers/<acct>/<app>/ before the marker.
release.LedgerAppPrefix is now the single source for that prefix
(internal/ledger and the console's ledger listing build on it). The
edge status cache drops a ledger object that is gone instead of
serving its stale copy forever. A bucket blip still serves stale.
3. The /settings revoke of a legacy app token requires the minter to
still be a member of the token's account. It is refused before the
passkey step.
4. GET /invite/{token} only shows a confirm card. Joining is a
CSRF-checked POST. Existing links land on the card.
5. Fleet names: no change. They are keys in the deployment bucket
(fleets/<fleet>/), and the 409 already says only "taken", never by
whom.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
18 files changed,
+548
−31
internal/console/apidoorkeys.go+14 −0internal/console/apidoorkeys_test.go+133 −0internal/console/apiledger.go+1 −1internal/console/appsettings.go+3 −0internal/console/authcli.go+27 −5internal/console/deletion_test.go+35 −0internal/console/doorkeys_store.go+18 −0internal/console/ownership.go+2 −0internal/console/server.go+64 −8internal/console/server_account_test.go+26 −4internal/console/store.go+49 −0internal/console/svcpage_test.go+46 −0internal/edge/status.go+17 −1internal/edge/status_test.go+46 −6internal/ledger/ledger.go+4 −2internal/release/release.go+9 −1internal/release/trash.go+16 −3internal/release/trash_test.go+38 −0
CI passed — 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 bbd5d466b..16b643436 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -1460,11 +1460,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: |
| @@ -1482,6 +1488,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 072bb3a45..b9923ce11 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1292,7 +1292,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 |
| @@ -2176,21 +2179,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 964b0884c..ac2d37d40 100644 |
| --- a/internal/console/store.go |
| +++ b/internal/console/store.go |
| @@ -1198,6 +1198,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 |
| @@ -1507,6 +1527,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() |
| } |
| @@ -1714,6 +1749,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 27b332bb3..51e0116d4 100644 |
| --- a/internal/release/release.go |
| +++ b/internal/release/release.go |
| @@ -208,9 +208,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. |