rastrillo / idear Public

The invitation page starts the invitee's sign-in; a removed member is restored, not re-invited

An invitation link is a week-long secret that sits in an inbox, gets
forwarded and lands in logs, so following it must prove nothing by
itself. Until now the invitee was told to go and sign in — a page that
said "sign in" leading to the app's general sign-in page, passwords and
passkeys for a person who has neither, with no word about what to do.

HandlerConfig.Begin, when set, makes POST /invitations/{token} from a
visitor with no session hand the invited address to the app, which sends
its own sign-in link (or keymail ceremony) to that address and answers.
Whoever holds the inbox follows it; admission redeems the invitation on
the way, as always. Whoever merely holds the invitation link sees "an
email was sent" to an inbox they cannot read, and the token stays
pending. InvitationPage.Join tells the page to show the one button; the
address stays off the page. Apps without the hook keep the old contract.

Invite now refuses an address that belongs to a deactivated member
(ErrRemovedInvitee, an ErrInvalid): admission refuses such a member
whatever invitation they hold — readmission is Restore, by an admin — so
the invitation could never be redeemed, and the invitee learned that only
at the end of the link, as a bare "not admitted here". The two tests that
invited a removed member to prove that later refusal now invite first and
remove after, which is the case that refusal exists for.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sx1ewVPRSyeSMuRxrmrLLU
James Adam pushed by james@keymail.dev 764e5d83455bf40776d8bdc17f2edc8642315112 parent 19a26b8
5 files changed, +182 −12
  • errors.go +14 −1
  • handlers.go +56 −7
  • handlers_test.go +96 −2
  • roster.go +10 −0
  • roster_test.go +6 −2

CI failed — run details

CI log
=== vet ===
go vet ./...
go: cannot load module /home/james/projects/rastrilloorg/rastrillo-worktrees/auth listed in go.work file: open /home/james/projects/rastrilloorg/rastrillo-worktrees/auth/go.mod: no such file or directory
make: *** [Makefile:18: vet] Error 1

amadan: step vet failed: exit status 2

amadan: job failed at step vet — its output is above, not at the tail of this log
diff --git a/errors.go b/errors.go
index eb1c3c4..62cc413 100644
--- a/errors.go
+++ b/errors.go
@@ -1,6 +1,9 @@
package idear
-import "errors"
+import (
+ "errors"
+ "fmt"
+)
// The store's sentinels, and the two CLASSES a handler switches on.
//
@@ -51,6 +54,16 @@ var (
// name the field that was wrong.
ErrInvalid = errors.New("idear: invalid argument")
+ // ErrRemovedInvitee refuses an invitation to an address that
+ // belongs to a DEACTIVATED member. Admission refuses such a
+ // member whatever invitation they hold — readmission is Restore,
+ // by an admin, so that being removed cannot be undone by the
+ // removed person from their inbox — and an invitation that can
+ // never be redeemed is a dead end for both people. It is a
+ // member of the ErrInvalid class: the address was the wrong
+ // argument, and the right one is the Restore button.
+ ErrRemovedInvitee = fmt.Errorf("%w: that address belongs to a member who was removed; restore them instead", ErrInvalid)
+
// ErrLastOwner refuses any change that would leave the instance
// without exactly one active Owner: deactivating the Owner, or
// changing the Owner's role by any route other than Transfer.
diff --git a/handlers.go b/handlers.go
index 6654f99..f4e990b 100644
--- a/handlers.go
+++ b/handlers.go
@@ -16,13 +16,14 @@ import (
// message, and a page that echoed it would publish them to whoever
// provoked it. The log line gets the detail; the page gets the class.
const (
- invalidCopy = "That request was not valid."
- forbiddenCopy = "You may not do that."
- lastOwnerCopy = "The owner cannot be removed. Transfer ownership first."
- noInvitationCopy = "That invitation is no longer available."
- failedCopy = "Something went wrong. Please try again."
- signInFirstCopy = "Sign in first, then open this invitation link again."
- rateLimitedCopy = "Too many requests. Please wait a moment and try again."
+ invalidCopy = "That request was not valid."
+ forbiddenCopy = "You may not do that."
+ lastOwnerCopy = "The owner cannot be removed. Transfer ownership first."
+ noInvitationCopy = "That invitation is no longer available."
+ failedCopy = "Something went wrong. Please try again."
+ signInFirstCopy = "Sign in first, then open this invitation link again."
+ removedInviteeCopy = "That address belongs to somebody who was removed from the team. Restore them instead of inviting them again."
+ rateLimitedCopy = "Too many requests. Please wait a moment and try again."
)
// The notices a successful mutation flashes. They are one-shot display
@@ -85,6 +86,14 @@ type InvitationPage struct {
Error string
SignedIn bool
Reconcile bool
+ // Join says the app can start the invitee's sign-in from here
+ // (HandlerConfig.Begin is set and the viewer holds no session): the
+ // page shows one button, and pressing it sends the sign-in link to
+ // the invited address. The address itself is deliberately NOT here
+ // — a token in the wrong hands must not name its owner
+ // (TestInvitationPageDoesNotDiscloseTheAddress); the person pressing
+ // the button knows which inbox they opened the link in.
+ Join bool
}
// HandlerConfig wires the flows to the app's own shell. Roster and
@@ -145,6 +154,27 @@ type HandlerConfig struct {
// entirely; NewHandlers logs a warning when it is nil.
Deliver func(r *http.Request, inv *Invitation, link string) error
+ // Begin, when set, lets the invitee start their first sign-in from
+ // the invitation page instead of being told to go and find one.
+ //
+ // An invitation link is a week-long secret that sits in an inbox,
+ // gets forwarded and lands in logs, so following it must prove
+ // nothing by itself. What it CAN do is start the app's own sign-in
+ // for the invited address: POST /invitations/{token} from a visitor
+ // with no session looks up the pending invitation and hands its
+ // address to Begin, which sends the app's sign-in link (or keymail
+ // ceremony) to that address and answers the request. Whoever holds
+ // the inbox follows the link; admission on the way, through
+ // Roster.Authorize, redeems the invitation. Whoever merely holds
+ // the invitation link gets a page saying an email was sent — to an
+ // inbox they cannot read — and the token stays pending, untouched.
+ //
+ // The hook owns the response: redirect, or render. Return an error
+ // only when nothing was written. The app's own budget on sign-in
+ // links bounds a visitor pressing the button over and over, on
+ // top of RateLimit here.
+ Begin func(w http.ResponseWriter, r *http.Request, address string) error
+
// RateLimit bounds the two public routes. See RateLimit — the
// zero value is the documented default, and there is no way to
// turn the limiter off.
@@ -455,6 +485,7 @@ func (h *Handlers) Invitation(w http.ResponseWriter, r *http.Request) {
Token: token,
SignedIn: signedIn,
Reconcile: reconcile,
+ Join: h.cfg.Begin != nil && !signedIn,
})
}
@@ -517,6 +548,22 @@ func (h *Handlers) Accept(w http.ResponseWriter, r *http.Request) {
subject, ok := rs.cfg.Subject(r)
subject = strings.TrimSpace(subject)
if !ok || subject == "" {
+ if h.cfg.Begin != nil {
+ // Not signed in, and the app can start that from here: the
+ // hook sends its sign-in link to the invited address and
+ // answers. The token is not touched — admission redeems it
+ // when the link is followed, by whoever holds the inbox.
+ inv, err := rs.pendingInvitation(ctx, token)
+ if err != nil {
+ h.deadInvitation(w, r, err, false)
+ return
+ }
+ if err := h.cfg.Begin(w, r, inv.Email); err != nil {
+ h.log().Error("idear: the app could not begin the invitee's sign-in", "err", err)
+ h.failInvitation(w, r, token)
+ }
+ return
+ }
// Not signed in. Reconciliation writes a member for the
// session in hand; there is no session in hand. The page says
// so rather than pretending the token is bad — and it says it
@@ -708,6 +755,8 @@ func (h *Handlers) refuse(w http.ResponseWriter, r *http.Request, viewer *Member
// the reason is true for every actor at every rank and the
// generic copy would imply somebody senior could do it.
h.page(w, r, viewer, http.StatusForbidden, lastOwnerCopy)
+ case errors.Is(err, ErrRemovedInvitee):
+ h.page(w, r, viewer, http.StatusBadRequest, removedInviteeCopy)
case errors.Is(err, ErrInvalid):
h.page(w, r, viewer, http.StatusBadRequest, invalidCopy)
case errors.Is(err, ErrForbidden):
diff --git a/handlers_test.go b/handlers_test.go
index 96cae25..22fac2b 100644
--- a/handlers_test.go
+++ b/handlers_test.go
@@ -952,9 +952,15 @@ func TestReconciliationRefusesASignedOutVisitor(t *testing.T) {
func TestReconciliationRefusesADeactivatedMember(t *testing.T) {
app, col := newApp(t)
- oc := app.As(app.H.Owner())
- gone := app.H.Deactivated(idear.RoleAdmin)
+ owner := app.H.Owner()
+ oc := app.As(owner)
+ // Invited while still a member (Invite refuses a removed one),
+ // then removed: the invitation outlives the membership.
+ gone := app.H.Member(idear.RoleAdmin)
token := invite(t, app, col, oc, gone.Email, idear.RoleMember)
+ if err := app.H.Roster.Deactivate(app.H.Ctx(), owner, gone); err != nil {
+ t.Fatalf("Deactivate: %v", err)
+ }
c := app.As(gone)
if res := c.Post("/invitations/"+token, url.Values{}); res.Status != http.StatusNotFound {
@@ -1755,3 +1761,91 @@ func TestEveryHandlerIsMounted(t *testing.T) {
t.Fatalf("*Handlers has %d handler methods (%v) but Routes() mounts %d; an unmounted handler is invisible to every derived test in this file", handlers, names, got)
}
}
+
+// ---------------------------------------------------------------
+// The invitation page starts the invitee's sign-in.
+// ---------------------------------------------------------------
+
+// TestInvitationBeginsTheInviteesSignIn: with HandlerConfig.Begin set,
+// a visitor with no session who presses the button on a live
+// invitation has the app's sign-in link sent to the INVITED address —
+// not signed in, and the token untouched, so holding the invitation
+// link alone still opens nothing.
+func TestInvitationBeginsTheInviteesSignIn(t *testing.T) {
+ col := &collector{}
+ var began []string
+ app := ideartest.NewAppWith(t, idear.Config{}, idear.HandlerConfig{
+ Deliver: col.deliver,
+ Begin: func(w http.ResponseWriter, r *http.Request, address string) error {
+ began = append(began, address)
+ http.Redirect(w, r, "/invitations/sent", http.StatusSeeOther)
+ return nil
+ },
+ })
+ owner := app.H.Owner()
+ const invitee = "new@example.test"
+ token := invite(t, app, col, app.As(owner), invitee, idear.RoleMember)
+
+ visitor := app.Visitor()
+ page := visitor.Get("/invitations/" + token)
+ if page.Status != http.StatusOK || !strings.Contains(page.Body, "join true") {
+ t.Fatalf("the invitation page does not offer to begin: %d %q", page.Status, page.Body)
+ }
+ if strings.Contains(page.Body, invitee) {
+ t.Fatalf("the invitation page names the invitee to whoever holds the token: %q", page.Body)
+ }
+
+ res := visitor.Post("/invitations/"+token, url.Values{})
+ if res.Status != http.StatusSeeOther || res.Location != "/invitations/sent" {
+ t.Fatalf("Accept: %d -> %q, want the app's answer", res.Status, res.Location)
+ }
+ if len(began) != 1 || began[0] != invitee {
+ t.Fatalf("the app was asked to begin for %v, want [%s]", began, invitee)
+ }
+ if memberBySubject(t, app, invitee) != nil {
+ t.Fatal("pressing the button admitted the invitee; only the link they follow may")
+ }
+ // The token is untouched: the link can be asked for again, and the
+ // invitation is redeemed when the invitee signs in, as always.
+ if again := app.Visitor().Post("/invitations/"+token, url.Values{}); again.Status != http.StatusSeeOther {
+ t.Fatalf("a second press answered %d", again.Status)
+ }
+ if !app.H.Roster.Authorize(invitee) {
+ t.Fatal("the invitation could not be redeemed by the invitee's sign-in")
+ }
+ if memberBySubject(t, app, invitee) == nil {
+ t.Fatal("admission did not write the member")
+ }
+}
+
+// TestInvitationWithoutBeginHookStillAsksToSignInFirst pins the older
+// contract for apps that have not set the hook.
+func TestInvitationWithoutBeginHookStillAsksToSignInFirst(t *testing.T) {
+ app, col := newApp(t)
+ token := invite(t, app, col, app.As(app.H.Owner()), "new@example.test", idear.RoleMember)
+ page := app.Visitor().Get("/invitations/" + token)
+ if !strings.Contains(page.Body, "join false") {
+ t.Fatalf("the page offers the button with no hook: %q", page.Body)
+ }
+ if res := app.Visitor().Post("/invitations/"+token, url.Values{}); res.Status != http.StatusForbidden {
+ t.Fatalf("Accept with no hook: %d, want 403", res.Status)
+ }
+}
+
+// TestInvitingARemovedMemberIsRefused: admission refuses a deactivated
+// member whatever invitation they hold, so an invitation to one is a
+// dead end; Invite refuses it up front and points at Restore.
+func TestInvitingARemovedMemberIsRefused(t *testing.T) {
+ app, _ := newApp(t)
+ gone := app.H.Deactivated(idear.RoleMember)
+ oc := app.As(app.H.Owner())
+ before, _ := app.H.Roster.PendingInvitations(app.H.Ctx())
+ res := oc.Post("/members/invitations", url.Values{"email": {gone.Email}, "role": {string(idear.RoleMember)}})
+ if res.Status != http.StatusBadRequest || !strings.Contains(res.Body, "removed from the team") {
+ t.Fatalf("inviting a removed member: %d %q", res.Status, res.Body)
+ }
+ after, _ := app.H.Roster.PendingInvitations(app.H.Ctx())
+ if len(after) != len(before) {
+ t.Fatal("an invitation was created for a removed member")
+ }
+}
diff --git a/roster.go b/roster.go
index 6bbb822..a8b1671 100644
--- a/roster.go
+++ b/roster.go
@@ -434,6 +434,16 @@ func (rs *Roster) Invite(ctx context.Context, actor *Member, email string, role
if err := checkInviteRole(cur, role); err != nil {
return err
}
+ // A removed member is restored, not re-invited: admission
+ // would refuse the invitation anyway, and the invitee would
+ // learn that only at the end of the link.
+ var gone Member
+ switch err := tx.Where("email = ?", email).First(&gone).Error; {
+ case err == nil && !gone.Active():
+ return ErrRemovedInvitee
+ case err != nil && !errors.Is(err, gorm.ErrRecordNotFound):
+ return err
+ }
// Supersede, in the SAME transaction as the create: a reader
// must never see two live invitations for one address, and a
// revocation that committed without its replacement would
diff --git a/roster_test.go b/roster_test.go
index 81acd65..db5274e 100644
--- a/roster_test.go
+++ b/roster_test.go
@@ -605,12 +605,16 @@ func TestAccept_ReturningMemberCollidesAndRollsBack(t *testing.T) {
h := ideartest.New(t)
ctx := h.Ctx()
owner := h.Owner()
- gone := h.Deactivated(idear.RoleMember)
-
+ // Invited while still a member (Invite refuses a removed one),
+ // then removed: the invitation outlives the membership.
+ gone := h.Member(idear.RoleMember)
inv, token, err := h.Roster.Invite(ctx, owner, gone.Email, idear.RoleMember)
if err != nil {
t.Fatalf("Invite: %v", err)
}
+ if err := h.Roster.Deactivate(ctx, owner, gone); err != nil {
+ t.Fatalf("Deactivate: %v", err)
+ }
if _, err := h.Roster.Accept(ctx, token, gone.Subject, gone.Name); err == nil {
t.Fatal("Accept re-admitted a deactivated subject; the unique index should have refused it")
}