amadan
console: invites deliver by keymail federation, then email, then by hand
handleInvite used to send through the inviter's own Keymail app grant, which only authorizes delivery to the grant's own user — every invite to someone else silently failed. It now tries Keymail federation (no grant needed), falls back to the console's own email, and finally leaves the link for the owner to pass on by hand, each within a shared 20s budget that always leaves the email leg a share of the time even if federation hangs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+236
−32
internal/console/invite_delivery_test.go+171 −0internal/console/keymail_test.go+8 −4internal/console/server.go+55 −26internal/console/server_account_test.go+2 −2
diff --git a/internal/console/invite_delivery_test.go b/internal/console/invite_delivery_test.go| new file mode 100644 |
| index 000000000..698ca94da |
| --- /dev/null |
| +++ b/internal/console/invite_delivery_test.go |
| @@ -0,0 +1,171 @@ |
| +package console |
| + |
| +import ( |
| + "context" |
| + "net/http" |
| + "net/url" |
| + "strings" |
| + "sync" |
| + "testing" |
| + "time" |
| +) |
| + |
| +type fakeConsoleMail struct { |
| + mu sync.Mutex |
| + err error |
| + sent []string // to|body |
| +} |
| + |
| +func (f *fakeConsoleMail) Send(_ context.Context, to, _, body string) error { |
| + f.mu.Lock() |
| + defer f.mu.Unlock() |
| + if f.err != nil { |
| + return f.err |
| + } |
| + f.sent = append(f.sent, to+"|"+body) |
| + return nil |
| +} |
| + |
| +// flashAfter reads the flash cookie the last redirect set. |
| +func flashAfter(t *testing.T, c *http.Client, base string) (kind, msg string) { |
| + t.Helper() |
| + u, _ := url.Parse(base) |
| + for _, ck := range c.Jar.Cookies(u) { |
| + if ck.Name == flashCookie { |
| + v, _ := url.QueryUnescape(ck.Value) |
| + kind, msg, _ = strings.Cut(v, "|") |
| + return kind, msg |
| + } |
| + } |
| + return "", "" |
| +} |
| + |
| +func TestInviteDelivery(t *testing.T) { |
| + cases := []struct { |
| + name string |
| + to string |
| + keymail func(fx *consoleFixture) |
| + mail *fakeConsoleMail // nil: no email fallback configured |
| + wantKind string |
| + wantInMsg string // a fragment that identifies the outcome |
| + wantFed bool |
| + wantMail bool |
| + }{ |
| + {name: "keymail delivers", to: "sam@keymail.dev", mail: &fakeConsoleMail{}, |
| + wantKind: "ok", wantInMsg: "Keymail", wantFed: true}, |
| + {name: "no mailbox, email delivers", to: "sam@gmail.com", mail: &fakeConsoleMail{}, |
| + wantKind: "ok", wantInMsg: "emailed", wantMail: true}, |
| + {name: "keymail refuses, email delivers", to: "sam@keymail.dev", mail: &fakeConsoleMail{}, |
| + keymail: func(fx *consoleFixture) { fx.fk.blocked["console@"+OriginHost(fx.ts.URL)] = true }, |
| + wantKind: "ok", wantInMsg: "emailed", wantMail: true}, |
| + {name: "both fail", to: "sam@gmail.com", mail: &fakeConsoleMail{err: ErrConsoleMailNotReady}, |
| + wantKind: "err", wantInMsg: "/invite/"}, |
| + {name: "no email configured", to: "sam@gmail.com", |
| + wantKind: "err", wantInMsg: "/invite/"}, |
| + } |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + if c.keymail != nil { |
| + c.keymail(fx) |
| + } |
| + if c.mail != nil { |
| + fx.srv.consoleMail = c.mail |
| + } |
| + owner := newBrowser(t) |
| + signUp(t, fx, owner, "code-owner") |
| + csrf := sessionToken(t, fx, owner) |
| + st, _, _ := postForm(t, owner, fx.u("/invite"), url.Values{"csrf": {csrf}, "address": {c.to}}) |
| + if st != http.StatusSeeOther { |
| + t.Fatalf("invite = %d", st) |
| + } |
| + kind, msg := flashAfter(t, owner, fx.ts.URL) |
| + if kind != c.wantKind || !strings.Contains(msg, c.wantInMsg) { |
| + t.Fatalf("flash = %q %q, want %q containing %q", kind, msg, c.wantKind, c.wantInMsg) |
| + } |
| + pending, _ := fx.st.PendingInvites(OpsAccountID) |
| + if len(pending) != 1 { |
| + t.Fatalf("invite row missing: %+v", pending) |
| + } |
| + link := "/invite/" + pending[0].Token |
| + if got := strings.Contains(strings.Join(fx.fk.fedBodies(), "\n"), link); got != c.wantFed { |
| + t.Errorf("delivered by keymail = %v, want %v", got, c.wantFed) |
| + } |
| + mailed := c.mail != nil && strings.Contains(strings.Join(c.mail.sent, "\n"), c.to+"|") && |
| + strings.Contains(strings.Join(c.mail.sent, "\n"), link) |
| + if mailed != c.wantMail { |
| + t.Errorf("emailed = %v, want %v", mailed, c.wantMail) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// The §1 second fault: an owner with no stored grant used to get a false |
| +// success. Delivery no longer reads the inviter's grant at all. |
| +func TestInviteDeliversWithoutTheInvitersGrant(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + owner := newBrowser(t) |
| + signUp(t, fx, owner, "code-owner") |
| + if err := fx.st.SetGrantToken("paul@keymail.dev", "", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + csrf := sessionToken(t, fx, owner) |
| + postForm(t, owner, fx.u("/invite"), url.Values{"csrf": {csrf}, "address": {"sam@keymail.dev"}}) |
| + if len(fx.fk.fedBodies()) != 1 { |
| + t.Fatalf("fedBodies = %v, want one delivery", fx.fk.fedBodies()) |
| + } |
| +} |
| + |
| +// Review Focus 3: a Keymail that never answers must not swallow the email leg. |
| +func TestInviteFallsBackWhenKeymailHangs(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + mailer := &fakeConsoleMail{} |
| + fx.srv.consoleMail = mailer |
| + fx.srv.inviteTimeout = 300 * time.Millisecond |
| + fx.srv.inviteKeymailShare = 150 * time.Millisecond |
| + fx.srv.sendFederated = func(ctx context.Context, _, _, _ string) (string, error) { |
| + <-ctx.Done() |
| + return "", ctx.Err() |
| + } |
| + owner := newBrowser(t) |
| + signUp(t, fx, owner, "code-owner") |
| + csrf := sessionToken(t, fx, owner) |
| + start := time.Now() |
| + postForm(t, owner, fx.u("/invite"), url.Values{"csrf": {csrf}, "address": {"sam@keymail.dev"}}) |
| + if d := time.Since(start); d > 2*time.Second { |
| + t.Fatalf("invite took %v", d) |
| + } |
| + if len(mailer.sent) != 1 { |
| + t.Fatalf("email leg not tried after a hung keymail: %v", mailer.sent) |
| + } |
| + if kind, _ := flashAfter(t, owner, fx.ts.URL); kind != "ok" { |
| + t.Fatalf("flash kind = %q, want ok", kind) |
| + } |
| +} |
| + |
| +// Pins the approved copy (Paul's exact strings) so a future edit is visible |
| +// in a diff rather than discovered by a user. |
| +func TestInviteCopyHelpers(t *testing.T) { |
| + if got, want := inviteFlashKeymail("sam@keymail.dev"), "Invited sam@keymail.dev. The link is in their Keymail inbox."; got != want { |
| + t.Errorf("inviteFlashKeymail = %q, want %q", got, want) |
| + } |
| + if got, want := inviteFlashEmail("sam@keymail.dev"), "Invited sam@keymail.dev. We emailed them the link."; got != want { |
| + t.Errorf("inviteFlashEmail = %q, want %q", got, want) |
| + } |
| + if got, want := inviteFlashFailed("sam@keymail.dev", "https://console.example/invite/tok"), |
| + "Invited sam@keymail.dev, but we couldn't deliver the link. Send it to them yourself: https://console.example/invite/tok"; got != want { |
| + t.Errorf("inviteFlashFailed = %q, want %q", got, want) |
| + } |
| + wantSubject := "You've been invited to an account on the CARLOS platform" |
| + wantBody := "paul@keymail.dev invited you to the account \"ops\" on the CARLOS platform.\n\n" + |
| + "Open this link while signed in as sam@keymail.dev to accept. It works once and expires in 7 days:\n\n" + |
| + "https://console.example/invite/tok\n\n" + |
| + "If you weren't expecting this, ignore it. Nothing happens without the link." |
| + gotSubject, gotBody := inviteMessage("paul@keymail.dev", "ops", "sam@keymail.dev", "https://console.example/invite/tok") |
| + if gotSubject != wantSubject { |
| + t.Errorf("inviteMessage subject = %q, want %q", gotSubject, wantSubject) |
| + } |
| + if gotBody != wantBody { |
| + t.Errorf("inviteMessage body = %q, want %q", gotBody, wantBody) |
| + } |
| +} |
diff --git a/internal/console/keymail_test.go b/internal/console/keymail_test.go| index afcaae90b..279e34bce 100644 |
| --- a/internal/console/keymail_test.go |
| +++ b/internal/console/keymail_test.go |
| @@ -387,14 +387,18 @@ func (f *fakeKeymail) handleSend(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| e := req.Envelope |
| - // Any recipient is accepted: a real keymail delivers to whoever the |
| - // envelope names (that is how an invite reaches a teammate). The fake has |
| - // one box key, so everything seals to it — the recipient is recorded, not |
| - // gated. From/signature/blob checks below are the ones that matter. |
| if e.To == "" { |
| jsonErr(w, http.StatusForbidden, "envelope needs a recipient") |
| return |
| } |
| + // The grant authorizes exactly one recipient: the user who approved it |
| + // (keymail appapi.go handleAppSend). This fake used to accept anyone, |
| + // which is how invites shipped broken. |
| + grantUser := strings.TrimPrefix(f.token, "user.tok-") |
| + if e.To != grantUser { |
| + jsonErr(w, http.StatusForbidden, "envelope to must be "+grantUser) |
| + return |
| + } |
| if host := OriginHost(f.origin); !strings.HasSuffix(e.From, "@"+host) { |
| jsonErr(w, http.StatusForbidden, "envelope from must be an address on "+host) |
| return |
diff --git a/internal/console/server.go b/internal/console/server.go| index 6d61f0638..959a6dbe2 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -820,6 +820,12 @@ type Server struct { |
| // km.SendApp in production, a capture in tests. A field rather than a |
| // direct call so an alertPass assertion needs no keymail server. |
| sendApp func(ctx context.Context, token, to, subject, body string) (string, error) |
| + // Invite delivery (handleInvite): Keymail federation first, then the |
| + // console's own email. Fields so tests can substitute and shrink them. |
| + sendFederated func(ctx context.Context, to, subject, body string) (string, error) |
| + consoleMail ConsoleMailer |
| + inviteTimeout time.Duration |
| + inviteKeymailShare time.Duration |
| // metricsCache holds the result of LoadMetrics's last successful bucket |
| // sweep (metricsview.go). Concurrent callers within metricsCacheTTL share |
| // it outright; callers arriving later still get it immediately (stale- |
| @@ -1163,12 +1169,15 @@ func NewServer(cfg Config, st *Store) (*Server, error) { |
| // human running `carlos ship` by hand, tight enough that a fleet of |
| // slow-loris uploads against a stolen or shared token can't pile up |
| // goroutines and buffers without bound. |
| - shipSem: make(chan struct{}, 2), |
| - delegationZone: normalizeDelegationZone(cfg.DelegationZone), |
| - steeringHost: normalizeSteeringHost(cfg.SteeringHost), |
| - lookupCNAME: net.DefaultResolver.LookupCNAME, |
| - lookupTXT: cfg.LookupTXT, |
| - storeWalks: newStoreWalkLocks(), |
| + shipSem: make(chan struct{}, 2), |
| + delegationZone: normalizeDelegationZone(cfg.DelegationZone), |
| + steeringHost: normalizeSteeringHost(cfg.SteeringHost), |
| + lookupCNAME: net.DefaultResolver.LookupCNAME, |
| + lookupTXT: cfg.LookupTXT, |
| + storeWalks: newStoreWalkLocks(), |
| + consoleMail: cfg.ConsoleMail, |
| + inviteTimeout: 20 * time.Second, |
| + inviteKeymailShare: 12 * time.Second, |
| // This process's identity in the relay writer lease. Random rather |
| // than derived from the host: two consoles on one box during a |
| // deploy overlap are two writers, and they must be able to tell |
| @@ -1180,6 +1189,7 @@ func NewServer(cfg Config, st *Store) (*Server, error) { |
| } |
| if s.km != nil { |
| s.sendApp = s.km.SendApp |
| + s.sendFederated = s.km.SendFederated |
| } |
| // No goroutine is started here: StartAlertWatcher is serve()'s job, so a |
| // test that constructs a Server gets a Server and nothing else. |
| @@ -2160,10 +2170,11 @@ func (s *Server) handleNewAccount(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, "/a/"+url.PathEscape(a.Sqid)+"/", http.StatusSeeOther) |
| } |
| -// handleInvite sends a single-use invitation into the addressee's Keymail |
| -// Apps inbox — the same sealed-envelope path sign-in confirmations use. The |
| -// platform never emails anyone: the invite arrives in the mailbox the |
| -// identity itself is anchored to. |
| +// handleInvite creates a single-use invitation and delivers its link: |
| +// Keymail federation first (into the invitee's main inbox, as the console |
| +// itself, no grant involved), then the console's own email, then the link |
| +// shown to the owner to pass on. The invite row exists whichever way it |
| +// went, so a failed delivery is a flash, not a rollback. |
| func (s *Server) handleInvite(w http.ResponseWriter, r *http.Request) { |
| addr, raw, acct, ok := s.owner(w, r) |
| if !ok { |
| @@ -2196,25 +2207,43 @@ func (s *Server) handleInvite(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| link := s.cfg.Origin + "/invite/" + url.PathEscape(tok) |
| - // The console's own grant is the instrument: it can only send from the |
| - // INVITER's stored token, so an owner who has never completed the OAuth |
| - // ceremony cannot invite. The invite row still exists, and the owner can |
| - // pass the link on by hand — which is why the failure is a flash, not a |
| - // rollback. |
| - if gt, _ := s.store.GrantToken(addr); gt != "" { |
| - ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) |
| - defer cancel() |
| - if _, err := s.km.SendApp(ctx, gt, to, |
| - "You've been invited to an account on the CARLOS platform", |
| - addr+" invited you to the account \""+acct.Name+"\" on the CARLOS platform.\n\n"+ |
| - "Open this link while signed in as "+to+" to accept. It works once and expires in 7 days:\n\n"+ |
| - link+"\n\nIf you weren't expecting this, ignore it — nothing happens without the link."); err != nil { |
| - log.Printf("console: invite send to %s: %v", to, err) |
| - back("err", "Invited "+to+", but delivering the link failed — you can send it on yourself: "+link) |
| + subject, body := inviteMessage(addr, acct.Name, to, link) |
| + ctx, cancel := context.WithTimeout(r.Context(), s.inviteTimeout) |
| + defer cancel() |
| + kctx, kcancel := context.WithTimeout(ctx, s.inviteKeymailShare) |
| + _, err = s.sendFederated(kctx, to, subject, body) |
| + kcancel() |
| + if err == nil { |
| + back("ok", inviteFlashKeymail(to)) |
| + return |
| + } |
| + log.Printf("console: invite to %s by keymail: %v", to, err) |
| + if s.consoleMail != nil { |
| + merr := s.consoleMail.Send(ctx, to, subject, body) |
| + if merr == nil { |
| + back("ok", inviteFlashEmail(to)) |
| return |
| } |
| + log.Printf("console: invite to %s by email: %v", to, merr) |
| } |
| - back("ok", "Invited "+to+" — the link is in their Keymail Apps inbox.") |
| + back("err", inviteFlashFailed(to, link)) |
| +} |
| + |
| +// inviteMessage is the subject and body for an invitation, whichever channel |
| +// carries it (Keymail federation or the console's own email). |
| +func inviteMessage(inviter, account, to, link string) (subject, body string) { |
| + return "You've been invited to an account on the CARLOS platform", |
| + inviter + " invited you to the account \"" + account + "\" on the CARLOS platform.\n\n" + |
| + "Open this link while signed in as " + to + " to accept. It works once and expires in 7 days:\n\n" + |
| + link + "\n\nIf you weren't expecting this, ignore it. Nothing happens without the link." |
| +} |
| + |
| +func inviteFlashKeymail(to string) string { |
| + return "Invited " + to + ". The link is in their Keymail inbox." |
| +} |
| +func inviteFlashEmail(to string) string { return "Invited " + to + ". We emailed them the link." } |
| +func inviteFlashFailed(to, link string) string { |
| + return "Invited " + to + ", but we couldn't deliver the link. Send it to them yourself: " + link |
| } |
| func (s *Server) handleRevokeInvite(w http.ResponseWriter, r *http.Request) { |
diff --git a/internal/console/server_account_test.go b/internal/console/server_account_test.go| index 59bdf4501..bdca9efb9 100644 |
| --- a/internal/console/server_account_test.go |
| +++ b/internal/console/server_account_test.go |
| @@ -321,8 +321,8 @@ func TestInviteAcceptCreatesMembership(t *testing.T) { |
| if len(pending) != 1 || pending[0].Address != "sam@keymail.dev" { |
| t.Fatalf("PendingInvites = %+v", pending) |
| } |
| - // The link went into sam's Keymail inbox, not an email. |
| - sent := fx.fk.sentBodies() |
| + // The link went into sam's Keymail inbox by federation. |
| + sent := fx.fk.fedBodies() |
| if len(sent) == 0 || !strings.Contains(strings.Join(sent, "\n"), "/invite/"+pending[0].Token) { |
| t.Fatalf("invite link not delivered through keymail: %v", sent) |
| } |