idear: close the reconciliation email-match gap, supersede re-invitations, document the subject rebind
Three security findings from the adversarial review. F1: reconciliation guarded its email match with addressOf(subject), which is "" for a password subject, so the block was skipped and a signed-in orphan could redeem a HIGHER-ROLE token issued to somebody else — while the comment and the spec claimed parity with admission. Config.EmailForSubject (optional) lets the app resolve its own user id, and with it set reconciliation applies admission's rule. Left nil the behaviour is unchanged and now stated plainly: the token alone is the credential there. F2: Invite now revokes any live invitation for the same address in the same transaction as it writes the new one. Without that, keymail redeemed the OLDEST pending row, so re-inviting at a corrected role was silently ignored — and the two identity paths disagreed about which of several coexisting invitations was spent. F4: SKILL.md §7 warns that offboarding under keymail MUST deactivate (a recycled address inherits the row and its role) and that an address change is a loss of access; §8 gains break-glass SQL to rebind a subject. The example's tests read that SQL out of SKILL.md, run it against a database built by the example's migrations, and prove it end to end over real HTTP — including an Owner-only action as the new subject.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
9 files changed,
+884
−17
SKILL.md+100 −1docs/superpowers/specs/2026-08-23-idear-design.md+10 −0example/app.go+10 −0example/app_test.go+305 −1example/models.go+33 −0handlers.go+78 −15handlers_test.go+166 −0roster.go+82 −0roster_test.go+100 −0
diff --git a/SKILL.md b/SKILL.md| index fab2fea..a37aac9 100644 |
| --- a/SKILL.md |
| +++ b/SKILL.md |
| @@ -82,6 +82,7 @@ rs, err := idear.New(idear.Config{ |
| // Subject defaults to sessions.Current(r).Subject — correct for both |
| // shipped identity plugins. Override only if the viewer arrives some |
| // other way. |
| + EmailForSubject: emailForSubject(d.G), // SET IT on the password path (§7) |
| }) |
| ``` |
| @@ -180,6 +181,14 @@ leaves a user row with no membership. A retry does not heal it (the retry's |
| `Create` fails on the now-duplicate email). The orphan signs in and 404s on |
| every route until they open their invitation link while signed in. |
| +What that route asks of them depends on whether idear can learn their |
| +address. Under keymail the Subject *is* the address, so the invitation must |
| +be that address's. Under password the Subject is an opaque user id and only |
| +the app can resolve it: set **`Config.EmailForSubject`** and reconciliation |
| +applies the same email match admission does. Leave it nil and **possession |
| +of the token is the whole credential there** — any signed-in orphan who |
| +gets hold of a live token redeems it, at that token's role. See §7. |
| + |
| ## 4. Rendering |
| Two callbacks, following `password.Config.RenderSignin`. **Neither may write |
| @@ -236,7 +245,7 @@ Every mutation is one transaction, with the invariant enforced **inside** it: |
| ```go |
| rs.Claim(ctx, subject, email, name) // first arrival ⇒ Owner; zero ROWS, not zero active |
| -rs.Invite(ctx, actor, email, role) // returns the plaintext token ONCE |
| +rs.Invite(ctx, actor, email, role) // plaintext token ONCE; SUPERSEDES |
| rs.Revoke(ctx, actor, invitationID) |
| rs.Accept(ctx, token, subject, name) // compare-and-swap, never lookup-then-write |
| rs.SetRole(ctx, actor, target, role) |
| @@ -246,6 +255,14 @@ rs.Transfer(ctx, owner, to) // demote + promote in one transact |
| rs.BySubject / ByID / Members / PendingInvitations / IsEmpty |
| ``` |
| +**Re-inviting supersedes.** `Invite` revokes any live invitation for the |
| +same address in the same transaction as it writes the new one, so an |
| +address has at most one redeemable invitation. Without that the two |
| +identity paths disagree — keymail redeems the *oldest* pending row, so a |
| +correction at a higher role would be silently ignored and a correction at a |
| +lower one would leave the stale higher one live — and password spends |
| +whichever of the coexisting links the invitee happens to click. |
| + |
| Refusals are classes, matched with `errors.Is`: `ErrInvalid` (400), |
| `ErrForbidden` (403), `ErrNotFound` (404), plus `ErrNoInvitation` and |
| `ErrOwnerExists` for the flows that produce them. `ErrLastOwner` unwraps to |
| @@ -327,6 +344,34 @@ Each of these cost a review round. |
| human two subjects — a decimal id and an address — and therefore two |
| roster rows, whose roles drift apart from the first role change. Neither |
| row knows about the other and nothing reconciles them. |
| +- **Set `Config.EmailForSubject` on the password path.** It is one lookup — |
| + `users.id` → address — and it is what makes reconciliation |
| + (`POST /invitations/{token}`) require that the invitation was issued to |
| + the signed-in viewer, which is the rule admission applies. Left nil, |
| + **that route trusts possession of the token alone**: any signed-in orphan |
| + — a create-succeeded-but-member-write-failed admission, or a claim-race |
| + loser — redeems a token issued to somebody else, at that token's role. |
| + The population is small, but a token is not hard to come by: with |
| + `Deliver` nil it lands in a browser flash cookie, and a token in a URL |
| + rides into history, referrers and logs. Under keymail the Subject is the |
| + address, so idear answers this itself and the hook is not consulted. |
| +- **Under keymail the address IS the identity, and idear never rebinds it.** |
| + Three consequences, all of them operational rather than theoretical: |
| + - **Offboarding MUST deactivate.** A departed member's row is keyed by |
| + their address, so an address the provider or the admin later **recycles** |
| + resolves that row — at its role, the Owner's included — and the new |
| + holder is signed in as the old member. Removal in idear is |
| + `POST /members/{id}/remove`; do it at the same moment the mailbox is |
| + closed, not at the next audit. |
| + - **An address change is a loss of access.** The person becomes a |
| + non-member and every Subject-keyed row is orphaned. For a non-Owner an |
| + admin can re-invite the new address (the old row stays as a record — |
| + deactivate it). For the **Owner** there is no API path at all: nobody |
| + may act on an Owner and `Transfer` requires the Owner to run it. The |
| + recovery is §8's rebind. |
| + - **Switching identity plugins orphans the whole roster at once**, since |
| + every Subject changes shape. That is a §8 rebind per member, planned |
| + before the switch and not after it. |
| - **`idear.Schema` merges into `BootSchema`, never the app's own `Schema`, |
| and idear's models never go in the app's `Models`.** `migration |
| generate`/`check` diff one app's Schema against that same app's Models as |
| @@ -381,6 +426,58 @@ own table, not idear's: under password, overwrite `users.password_hash` with |
| a fresh `password.Hash(...)` value; under keymail there is nothing to reset, |
| because the address is the credential. |
| +### Rebinding a subject |
| + |
| +The other break-glass, for the hazards §7 lists: `subject` is the join to |
| +the app's identity and **idear never rewrites it**. Under keymail a changed |
| +address makes a member a stranger, and for the Owner there is no API path |
| +back; a plugin switch does that to every row at once. Rebinding is SQL too. |
| +**Stop the instance first**, same as above. |
| + |
| +```sql |
| +-- 1. Look before you write. BOTH rows matter: the one being moved, and any |
| +-- row the NEW subject already has — subject is UNIQUE, so a rebind onto |
| +-- a subject that already has one fails outright. If it does have one, |
| +-- decide which of the two survives BEFORE touching either: the loser's |
| +-- member id may be referenced by the app's own tables. |
| +SELECT id, subject, email, role, deactivated_at FROM idear_members |
| + WHERE subject IN ('OLD-SUBJECT', 'NEW-SUBJECT'); |
| + |
| +-- 2. Rebind. email moves with the subject, because under keymail the |
| +-- subject IS the address and a stale display cache misleads the members |
| +-- page. deactivated_at is cleared for the same reason it is cleared in |
| +-- the transfer above: a rebind onto a deactivated row hands the new |
| +-- subject a membership that 404s on every route, which reads exactly |
| +-- like the rebind not having worked. Drop that clause — deliberately — |
| +-- if the person is meant to stay removed. updated_at is left alone on |
| +-- purpose: it is a GORM timestamp, CURRENT_TIMESTAMP does not write |
| +-- GORM's format, and this schema already has one column whose |
| +-- comparison is a text comparison. |
| +BEGIN; |
| +UPDATE idear_members |
| + SET subject = 'NEW-SUBJECT', email = 'NEW-EMAIL', deactivated_at = NULL |
| + WHERE subject = 'OLD-SUBJECT'; |
| +COMMIT; |
| + |
| +-- 3. Verify before restarting: exactly one row, at the role it had, with |
| +-- deactivated_at NULL. NOTHING here means the UPDATE matched nothing — |
| +-- check the OLD-SUBJECT spelling against step 1 rather than re-running. |
| +SELECT id, subject, email, role, deactivated_at FROM idear_members |
| + WHERE subject = 'NEW-SUBJECT'; |
| +``` |
| + |
| +`NEW-SUBJECT` is spelled the way the identity plugin mints it, and this is |
| +the one place that is easy to get wrong: under **keymail** it is the new |
| +address, lowercased and trimmed; under **password** it is the decimal |
| +`users.id` of the row the person will sign in as — not their address. Get it |
| +wrong and they sign in successfully and 404 on every route, which is §7's |
| +silent trap arriving by a different door. |
| + |
| +`example/app_test.go`'s `TestBreakGlassRebindsASubject` runs this very |
| +block — read out of this file, with only its three placeholders filled in — |
| +against a database built by the example's migrations, then signs in over |
| +real HTTP as the new subject and performs an Owner-only action with it. |
| + |
| ## 9. Testing |
| Drive the mounted app over real HTTP with a cookie jar and a same-origin |
| @@ -419,3 +516,5 @@ Cover at least: |
| rewritten signup page loses silently. |
| 10. `csrf.Protect(origin)` is mounted app-wide, above every group, so it |
| covers idear's routes as well as yours. |
| +11. `EmailForSubject` is set on the password path, or you have decided, |
| + knowingly, that a signed-in orphan may spend any token they hold. |
diff --git a/docs/superpowers/specs/2026-08-23-idear-design.md b/docs/superpowers/specs/2026-08-23-idear-design.md| index 1da7e80..7429105 100644 |
| --- a/docs/superpowers/specs/2026-08-23-idear-design.md |
| +++ b/docs/superpowers/specs/2026-08-23-idear-design.md |
| @@ -303,6 +303,16 @@ public invitation routes are *for*: |
| row exists — so once invited they redeem here rather than through |
| sign-up, which would fail on the duplicate email. |
| +**Amended 2026-08-24 (review finding F1).** "a valid token matching their |
| +session" was only ever enforceable where idear knows the viewer's address, |
| +which is keymail — there the Subject *is* the verified address. Under |
| +password the Subject is an opaque user id, so the implementation matched |
| +nothing and the code comment's claim of parity with admission was false. |
| +`Config.EmailForSubject` is the optional hook that lets the app resolve its |
| +own id, and with it set the match is made; left nil, reconciliation under |
| +password trusts possession of the token alone, which SKILL.md §3 and §7 now |
| +say in as many words. |
| + |
| Both public routes are rate-limited; `GET` is an unauthenticated lookup |
| of a secret and must not be a free oracle. |
diff --git a/example/app.go b/example/app.go| index ff9b4c5..19cc528 100644 |
| --- a/example/app.go |
| +++ b/example/app.go |
| @@ -88,6 +88,16 @@ func newApp(d *db.DB, origin, site string, logger *slog.Logger) (*app, error) { |
| // Subject is left at its default, sessions.Current(r).Subject, |
| // which is correct for the password plugin: password mints the |
| // decimal user id as the Subject (models.go's subjectFor). |
| + // |
| + // EmailForSubject is the other half of that join, read the |
| + // other way: id → address. Reconciliation |
| + // (POST /invitations/{token}) uses it to require that the |
| + // invitation was issued to the SIGNED-IN VIEWER's address — |
| + // the rule admission already applies. Leave it out and, under |
| + // password, a signed-in orphan may spend any live token they |
| + // get hold of, at whatever role it carries; idear has no way |
| + // to resolve a decimal id on its own. See models.go. |
| + EmailForSubject: emailForSubject(d.G), |
| }) |
| if err != nil { |
| return nil, err |
diff --git a/example/app_test.go b/example/app_test.go| index b073a29..7c7d703 100644 |
| --- a/example/app_test.go |
| +++ b/example/app_test.go |
| @@ -2,12 +2,15 @@ package main |
| import ( |
| "context" |
| + "database/sql" |
| + "fmt" |
| "io" |
| "log/slog" |
| "net/http" |
| "net/http/cookiejar" |
| "net/http/httptest" |
| "net/url" |
| + "os" |
| "path/filepath" |
| "regexp" |
| "strconv" |
| @@ -16,6 +19,7 @@ import ( |
| "github.com/carlosframework/rastrillo/db" |
| "github.com/carlosframework/rastrillo/migrate" |
| + "github.com/carlosframework/rastrillo/password" |
| "amadan.net/rastrillo/idear" |
| ) |
| @@ -31,6 +35,7 @@ import ( |
| type testApp struct { |
| t *testing.T |
| app *app |
| + db *db.DB |
| server *httptest.Server |
| origin string |
| } |
| @@ -57,7 +62,7 @@ func newTestApp(t *testing.T) *testApp { |
| srv.Start() |
| t.Cleanup(srv.Close) |
| - return &testApp{t: t, app: a, server: srv, origin: origin} |
| + return &testApp{t: t, app: a, db: d, server: srv, origin: origin} |
| } |
| // client is one browser: a cookie jar, and whatever session it holds. |
| @@ -566,3 +571,302 @@ func TestInvitedSignupSurvivesAValidationFailure(t *testing.T) { |
| t.Fatalf("second attempt admitted at %q, want admin", got.Role) |
| } |
| } |
| + |
| +// --------------------------------------------------------------- |
| +// SKILL.md §8 break-glass: rebinding a subject. |
| +// --------------------------------------------------------------- |
| + |
| +// TestBreakGlassRebindsASubject executes the rebind block from |
| +// SKILL.md §8 — READ OUT OF THE FILE, not restated here — against a |
| +// database built by this app's own migrations, and then proves the |
| +// result end to end: sign in over real HTTP as the new subject and |
| +// perform an Owner-only action with it. |
| +// |
| +// Documented recovery SQL that nobody has run is a guess. §8's other |
| +// block, the ownership transfer, was verified this way and the review |
| +// found `deactivated_at = NULL` in it load-bearing; this one carries |
| +// the same clause for the same reason, plus the unique index on |
| +// subject, which is what turns a careless rebind into a failed UPDATE. |
| +// |
| +// Reading the SQL from the document rather than copying it here is the |
| +// point of the test. A copy can go stale silently; this cannot — edit |
| +// §8's block into something that does not work and this test is what |
| +// says so. |
| +func TestBreakGlassRebindsASubject(t *testing.T) { |
| + ta := newTestApp(t) |
| + ctx := context.Background() |
| + if err := Seed(ctx, ta.app.db, ta.app.roster); err != nil { |
| + t.Fatalf("Seed: %v", err) |
| + } |
| + owner := ta.member(SeedOwner) |
| + if owner.Role != idear.RoleOwner { |
| + t.Fatalf("the seeded owner is %s", owner.Role) |
| + } |
| + successorID := ta.memberID(SeedAdmin) |
| + |
| + // The Owner's identity changed. Under keymail that is a new |
| + // address; under password — this app — it is a new user row, and |
| + // the subject is that row's decimal id. Either way the roster row |
| + // still points at the old one, and the person is now a stranger to |
| + // every guarded route. |
| + const newAddress = "ada.new@example.test" |
| + hash, err := password.Hash(SeedPassword) |
| + if err != nil { |
| + t.Fatalf("password.Hash: %v", err) |
| + } |
| + newID, err := createUser(ta.app.db)(ctx, newAddress, hash) |
| + if err != nil { |
| + t.Fatalf("creating the replacement user: %v", err) |
| + } |
| + |
| + stranger := ta.visitor() |
| + want(t, stranger.signIn(newAddress), http.StatusSeeOther, "sign in as the new identity") |
| + if res := stranger.get("/"); res.Status != http.StatusNotFound { |
| + t.Fatalf("the new identity reached the board before any rebind: status %d", res.Status) |
| + } |
| + |
| + // The break-glass, verbatim: only the three placeholders are |
| + // filled in, exactly as an operator editing the block would. |
| + script := breakGlassRebind(t, map[string]string{ |
| + "OLD-SUBJECT": owner.Subject, |
| + "NEW-SUBJECT": subjectFor(newID), |
| + "NEW-EMAIL": newAddress, |
| + }) |
| + t.Log("\n" + runSQLScript(t, ta, script)) |
| + |
| + // End to end, through the front door: a real sign-in, a real |
| + // session, and the membership gate. |
| + ada := ta.visitor() |
| + want(t, ada.signIn(newAddress), http.StatusSeeOther, "sign in as the rebound owner") |
| + want(t, ada.get("/"), http.StatusOK, "the board, as the rebound owner") |
| + |
| + // The OWNER-ONLY action. RequireRole(RoleOwner) 403s everybody |
| + // else, and Transfer is the only route behind it — so a 303 here |
| + // is the rebind having restored ownership and not merely |
| + // membership. |
| + res := ada.post("/members/transfer", url.Values{ |
| + "member": {strconv.FormatInt(successorID, 10)}, |
| + }) |
| + want(t, res, http.StatusSeeOther, "transfer ownership as the rebound owner") |
| + if got := ta.member(SeedAdmin); got.Role != idear.RoleOwner { |
| + t.Fatalf("after the transfer %s is %s, want owner", SeedAdmin, got.Role) |
| + } |
| + if got := ta.member(newAddress); got.ID != owner.ID || got.Role != idear.RoleAdmin { |
| + t.Fatalf("the rebound row is %+v, want member %d demoted to admin by its own transfer", got, owner.ID) |
| + } |
| + |
| + // And the OLD subject is nobody: the old user row still signs in — |
| + // credentials are the app's table, not idear's — and 404s on every |
| + // guarded route, which is what a rebind means. |
| + old := ta.visitor() |
| + want(t, old.signIn(SeedOwner), http.StatusSeeOther, "sign in as the old identity") |
| + if res := old.get("/"); res.Status != http.StatusNotFound { |
| + t.Fatalf("the old subject still reached the board after the rebind: status %d", res.Status) |
| + } |
| +} |
| + |
| +// TestBreakGlassRebindClearsDeactivation is the OTHER half of the §8 |
| +// rebind, and it is why `deactivated_at = NULL` is in that statement. |
| +// |
| +// The realistic shape: somebody was offboarded (correctly — §7 says |
| +// offboarding under keymail MUST deactivate), then came back under a |
| +// new identity. The row still carries their role and their removal. |
| +// Rebound without clearing it, the new subject resolves to a |
| +// deactivated member and 404s on every guarded route — which is |
| +// indistinguishable, from the operator's side, from the rebind not |
| +// having worked at all. Delete that clause from SKILL.md and the GET |
| +// below turns red. |
| +func TestBreakGlassRebindClearsDeactivation(t *testing.T) { |
| + ta := newTestApp(t) |
| + ctx := context.Background() |
| + if err := Seed(ctx, ta.app.db, ta.app.roster); err != nil { |
| + t.Fatalf("Seed: %v", err) |
| + } |
| + kim := ta.member(SeedAdmin) |
| + |
| + // Removed through the real route, by the real Owner. |
| + ada := ta.visitor() |
| + want(t, ada.signIn(SeedOwner), http.StatusSeeOther, "sign in as the owner") |
| + want(t, ada.post(fmt.Sprintf("/members/%d/remove", kim.ID), url.Values{}), |
| + http.StatusSeeOther, "remove the admin") |
| + if removed := ta.member(SeedAdmin); removed.Active() { |
| + t.Fatal("the admin is still active after remove") |
| + } |
| + |
| + const newAddress = "kim.new@example.test" |
| + hash, err := password.Hash(SeedPassword) |
| + if err != nil { |
| + t.Fatalf("password.Hash: %v", err) |
| + } |
| + newID, err := createUser(ta.app.db)(ctx, newAddress, hash) |
| + if err != nil { |
| + t.Fatalf("creating the replacement user: %v", err) |
| + } |
| + |
| + t.Log("\n" + runSQLScript(t, ta, breakGlassRebind(t, map[string]string{ |
| + "OLD-SUBJECT": kim.Subject, |
| + "NEW-SUBJECT": subjectFor(newID), |
| + "NEW-EMAIL": newAddress, |
| + }))) |
| + |
| + back := ta.visitor() |
| + want(t, back.signIn(newAddress), http.StatusSeeOther, "sign in as the rebound admin") |
| + want(t, back.get("/"), http.StatusOK, "the board, as the rebound admin") |
| + got := ta.member(newAddress) |
| + if got.ID != kim.ID || got.Role != idear.RoleAdmin || !got.Active() { |
| + t.Fatalf("the rebound row is %+v, want member %d, admin, active", got, kim.ID) |
| + } |
| +} |
| + |
| +// breakGlassRebind returns SKILL.md §8's rebind block with its |
| +// placeholders replaced. The block is located by a placeholder rather |
| +// than by a heading or an index, so reordering the document does not |
| +// silently point this at some other SQL. |
| +func breakGlassRebind(t *testing.T, subs map[string]string) string { |
| + t.Helper() |
| + doc, err := os.ReadFile(filepath.Join("..", "SKILL.md")) |
| + if err != nil { |
| + t.Fatalf("reading SKILL.md: %v", err) |
| + } |
| + block := sqlBlock(t, string(doc), "OLD-SUBJECT") |
| + for name, value := range subs { |
| + if !strings.Contains(block, "'"+name+"'") { |
| + t.Fatalf("SKILL.md's rebind block has no '%s' placeholder to fill in:\n%s", name, block) |
| + } |
| + // Only the QUOTED placeholder is filled in — the same edit an |
| + // operator makes — so the prose above each statement is left |
| + // as the document wrote it. |
| + block = strings.ReplaceAll(block, "'"+name+"'", "'"+value+"'") |
| + } |
| + return block |
| +} |
| + |
| +// sqlBlock is the one ```sql fence in doc that mentions needle. |
| +// Exactly one: two would mean the test is guessing. |
| +func sqlBlock(t *testing.T, doc, needle string) string { |
| + t.Helper() |
| + var found []string |
| + var cur *strings.Builder |
| + for _, line := range strings.Split(doc, "\n") { |
| + switch { |
| + case cur == nil && strings.TrimSpace(line) == "```sql": |
| + cur = &strings.Builder{} |
| + case cur != nil && strings.TrimSpace(line) == "```": |
| + if strings.Contains(cur.String(), needle) { |
| + found = append(found, cur.String()) |
| + } |
| + cur = nil |
| + case cur != nil: |
| + cur.WriteString(line + "\n") |
| + } |
| + } |
| + if len(found) != 1 { |
| + t.Fatalf("SKILL.md has %d sql blocks mentioning %q, want exactly 1", len(found), needle) |
| + } |
| + return found[0] |
| +} |
| + |
| +// runSQLScript executes the script one statement at a time on a SINGLE |
| +// writer connection — the sqlite3 session an operator would have — and |
| +// returns a transcript of what each statement did. |
| +// |
| +// One connection is not a detail: the block wraps its UPDATE in |
| +// BEGIN/COMMIT, and a pool that handed those three statements to |
| +// different connections would prove nothing about the transaction the |
| +// document tells an operator to run. |
| +func runSQLScript(t *testing.T, ta *testApp, script string) string { |
| + t.Helper() |
| + ctx := context.Background() |
| + conn, err := ta.db.Writer().Conn(ctx) |
| + if err != nil { |
| + t.Fatalf("taking the writer connection: %v", err) |
| + } |
| + defer conn.Close() |
| + |
| + var out strings.Builder |
| + for _, stmt := range strings.Split(script, ";") { |
| + stmt = strings.TrimSpace(stmt) |
| + if stmt == "" { |
| + continue |
| + } |
| + fmt.Fprintf(&out, "sqlite> %s;\n", stmt) |
| + if strings.HasPrefix(strings.ToUpper(bare(stmt)), "SELECT") { |
| + fmt.Fprint(&out, query(t, ctx, conn, stmt)) |
| + continue |
| + } |
| + res, err := conn.ExecContext(ctx, stmt) |
| + if err != nil { |
| + t.Fatalf("SKILL.md §8 statement failed: %v\n%s", err, stmt) |
| + } |
| + switch verb := strings.ToUpper(bare(stmt)); { |
| + case strings.HasPrefix(verb, "BEGIN"), strings.HasPrefix(verb, "COMMIT"): |
| + // RowsAffected means nothing for these, and the driver |
| + // answers 1 for both — a number in the transcript that |
| + // looked like a result would be worse than no number. |
| + out.WriteString("-- ok\n") |
| + default: |
| + n, err := res.RowsAffected() |
| + if err != nil { |
| + t.Fatalf("reading rows affected: %v", err) |
| + } |
| + fmt.Fprintf(&out, "-- %d row(s)\n", n) |
| + } |
| + } |
| + return out.String() |
| +} |
| + |
| +// bare strips the leading comment lines from a statement, so the |
| +// SELECT/other decision reads the SQL and not the prose above it. |
| +func bare(stmt string) string { |
| + for _, line := range strings.Split(stmt, "\n") { |
| + line = strings.TrimSpace(line) |
| + if line != "" && !strings.HasPrefix(line, "--") { |
| + return line |
| + } |
| + } |
| + return "" |
| +} |
| + |
| +// query runs a SELECT and renders its rows. Values are scanned into |
| +// any, not into a typed column list: this prints whatever the schema |
| +// holds, including the NULL deactivated_at the verification step is |
| +// looking for. |
| +func query(t *testing.T, ctx context.Context, conn *sql.Conn, stmt string) string { |
| + t.Helper() |
| + rows, err := conn.QueryContext(ctx, stmt) |
| + if err != nil { |
| + t.Fatalf("SKILL.md §8 query failed: %v\n%s", err, stmt) |
| + } |
| + defer rows.Close() |
| + cols, err := rows.Columns() |
| + if err != nil { |
| + t.Fatalf("reading columns: %v", err) |
| + } |
| + |
| + var out strings.Builder |
| + fmt.Fprintf(&out, "%s\n", strings.Join(cols, "|")) |
| + n := 0 |
| + for rows.Next() { |
| + cells := make([]any, len(cols)) |
| + into := make([]any, len(cols)) |
| + for i := range cells { |
| + into[i] = &cells[i] |
| + } |
| + if err := rows.Scan(into...); err != nil { |
| + t.Fatalf("scanning a row: %v", err) |
| + } |
| + parts := make([]string, len(cells)) |
| + for i, c := range cells { |
| + parts[i] = fmt.Sprintf("%v", c) |
| + } |
| + fmt.Fprintf(&out, "%s\n", strings.Join(parts, "|")) |
| + n++ |
| + } |
| + if err := rows.Err(); err != nil { |
| + t.Fatalf("reading rows: %v", err) |
| + } |
| + if n == 0 { |
| + out.WriteString("-- no rows\n") |
| + } |
| + return out.String() |
| +} |
diff --git a/example/models.go b/example/models.go| index 2613477..5cd2384 100644 |
| --- a/example/models.go |
| +++ b/example/models.go |
| @@ -116,6 +116,39 @@ func lookupUser(g *gorm.DB) func(context.Context, string) (int64, string, error) |
| } |
| } |
| +// emailForSubject is idear.Config.EmailForSubject: it answers "which |
| +// address is this session subject?" out of THIS APP's user table. |
| +// |
| +// It is one lookup, and it is what makes reconciliation apply the same |
| +// rule admission does. Without it, POST /invitations/{token} under the |
| +// password plugin has no address to match the invitation against, so |
| +// possession of the token is the whole credential and any signed-in |
| +// orphan may spend a token issued to somebody else — at whatever role |
| +// that token carries. idear cannot resolve a decimal user id itself; |
| +// only this table can. |
| +// |
| +// The two "not this app's" answers are ("", nil), not an error: a |
| +// subject that is not a decimal id, and a decimal id with no row |
| +// behind it. idear refuses the redemption for both. An error is |
| +// reserved for a database that failed, which idear answers 500 rather |
| +// than rendering as a policy refusal. |
| +func emailForSubject(g *gorm.DB) func(context.Context, string) (string, error) { |
| + return func(ctx context.Context, subject string) (string, error) { |
| + id, err := strconv.ParseInt(subject, 10, 64) |
| + if err != nil { |
| + return "", nil |
| + } |
| + var u User |
| + switch err := g.WithContext(ctx).Where("id = ?", id).Take(&u).Error; { |
| + case errors.Is(err, gorm.ErrRecordNotFound): |
| + return "", nil |
| + case err != nil: |
| + return "", err |
| + } |
| + return u.Email, nil |
| + } |
| +} |
| + |
| // createUser is the app's half of password.Config.Create: it writes a |
| // User row and nothing else. |
| // |
diff --git a/handlers.go b/handlers.go| index 994f436..89ee895 100644 |
| --- a/handlers.go |
| +++ b/handlers.go |
| @@ -483,8 +483,14 @@ func (h *Handlers) Invitation(w http.ResponseWriter, r *http.Request) { |
| // have. An unclaimed instance's first account is its Owner, and |
| // the only way to be signed in against an empty roster is to be |
| // the orphan whose Claim did not commit. |
| -// 4. otherwise: Accept the token — the CAS in the store, so a Revoke |
| -// racing this never admits. |
| +// 4. otherwise: the token must be REDEEMABLE BY THIS VIEWER — |
| +// issued to their address, wherever idear can learn it (keymail's |
| +// Subject is the address; under password, Config.EmailForSubject |
| +// resolves one) — and then Accept it, through the CAS in the |
| +// store, so a Revoke racing this never admits. Under password |
| +// with no resolver there is no address to match and the token |
| +// alone decides; that gap is Config.EmailForSubject's whole |
| +// subject matter. |
| // |
| // The member row is written by the STORE, from the live session |
| // Subject, and never assembled here: Subject is canonicalised on every |
| @@ -568,21 +574,48 @@ func (h *Handlers) Accept(w http.ResponseWriter, r *http.Request) { |
| } |
| } |
| - // 4: the token. |
| + // 4: the token — plus the email match, WHEREVER IDEAR CAN MAKE |
| + // ONE. |
| // |
| - // When the session Subject IS an address — which is what keymail |
| - // mints — the invitation must be THAT address's. idear knows the |
| - // viewer's address only in that case; under password the Subject |
| - // is an opaque user id and the token alone is the credential, |
| - // which is the same rule admission applies (possession of the |
| - // token, and an email match wherever there is an email to match). |
| - if addr := addressOf(subject); addr != "" { |
| + // It can when the session Subject IS an address, which is what |
| + // keymail mints; and it can under password when the app supplied |
| + // Config.EmailForSubject to resolve its own user id. Either way |
| + // the rule is the one admission applies: possession of the token |
| + // AND an email match. |
| + // |
| + // With NEITHER — the password path, no resolver — the token alone |
| + // is the credential here, and that is not parity with admission, |
| + // which always has a submitted address to match against. A |
| + // signed-in orphan can then spend any live token they get hold |
| + // of, at whatever role it carries. Config.EmailForSubject states |
| + // that asymmetry in full; it is written down rather than papered |
| + // over, and setting the hook closes it. |
| + addr, known, err := rs.addressFor(ctx, subject) |
| + if err != nil { |
| + h.log().Error("idear: reconciliation could not resolve the viewer's address; refusing", |
| + "subject", subject, "err", err) |
| + h.failInvitation(w, r, token) |
| + return |
| + } |
| + if known { |
| inv, err := rs.pendingInvitation(ctx, token) |
| - if err != nil || normalizeEmail(inv.Email) != normalizeEmail(addr) { |
| - if err == nil { |
| - h.log().Warn("idear: reconciliation presented an invitation issued to another address", |
| - "subject", subject) |
| - } |
| + switch { |
| + case err != nil: |
| + // An unusable token, or a lookup that failed: one answer |
| + // for all of them, as everywhere else on this route. |
| + h.deadInvitation(w, r, ErrNoInvitation, true) |
| + return |
| + case addr == "": |
| + // The resolver ran and placed this subject nowhere. Fail |
| + // closed: a viewer idear cannot identify must not redeem |
| + // an invitation issued to one it can. |
| + h.log().Warn("idear: reconciliation could not place the viewer's subject in the app's own records; refusing", |
| + "subject", subject) |
| + h.deadInvitation(w, r, ErrNoInvitation, true) |
| + return |
| + case normalizeEmail(inv.Email) != addr: |
| + h.log().Warn("idear: reconciliation presented an invitation issued to another address", |
| + "subject", subject) |
| h.deadInvitation(w, r, ErrNoInvitation, true) |
| return |
| } |
| @@ -863,3 +896,33 @@ func addressOf(subject string) string { |
| } |
| return "" |
| } |
| + |
| +// addressFor answers what address a session Subject belongs to, and — |
| +// the part that matters — whether idear CAN know at all. |
| +// |
| +// known false is the honest "no idea": a subject that is not an |
| +// address, on an app that supplied no Config.EmailForSubject. The |
| +// caller must not read that as "no match"; it is the absence of a |
| +// question, and reconciliation's behaviour in that case is the |
| +// documented permissive path. |
| +// |
| +// known true with an empty address is a different answer: the app's |
| +// own resolver ran and placed the subject nowhere. That is a refusal, |
| +// not an unknown. |
| +// |
| +// The address is normalised on the way out so callers compare like |
| +// with like, and so an app may return whatever spelling its user table |
| +// happens to hold. |
| +func (rs *Roster) addressFor(ctx context.Context, subject string) (string, bool, error) { |
| + if addr := addressOf(subject); addr != "" { |
| + return normalizeEmail(addr), true, nil |
| + } |
| + if rs.cfg.EmailForSubject == nil { |
| + return "", false, nil |
| + } |
| + addr, err := rs.cfg.EmailForSubject(ctx, subject) |
| + if err != nil { |
| + return "", true, err |
| + } |
| + return normalizeEmail(addr), true, nil |
| +} |
diff --git a/handlers_test.go b/handlers_test.go| index b7e43d3..983344b 100644 |
| --- a/handlers_test.go |
| +++ b/handlers_test.go |
| @@ -1,6 +1,8 @@ |
| package idear_test |
| import ( |
| + "context" |
| + "errors" |
| "fmt" |
| "net/http" |
| "net/url" |
| @@ -963,6 +965,170 @@ func TestReconciliationRefusesADeactivatedMember(t *testing.T) { |
| } |
| } |
| +// --------------------------------------------------------------- |
| +// Reconciliation under the PASSWORD plugin: the token, and whether |
| +// anything else is asked of the viewer. |
| +// |
| +// Under keymail the session Subject is the verified address, so idear |
| +// matches it against the invitation itself (the test above). Under |
| +// password the Subject is an opaque decimal user id and idear can only |
| +// resolve it through Config.EmailForSubject. These two tests are the |
| +// two configurations, and the difference between them is the whole |
| +// point: one refuses a token issued to somebody else, the other — the |
| +// documented permissive path — does not. |
| +// --------------------------------------------------------------- |
| + |
| +// directory is an app's own user table, as idear sees it through |
| +// Config.EmailForSubject: subject → address, and an error the app |
| +// cannot answer through. |
| +type directory struct { |
| + byID map[string]string |
| + err error |
| +} |
| + |
| +func (d *directory) emailForSubject(ctx context.Context, subject string) (string, error) { |
| + if d.err != nil { |
| + return "", d.err |
| + } |
| + // The app's own miss: not one of this app's subjects, or an id |
| + // with no row behind it. "" with a NIL error, which idear must |
| + // read as a refusal and not as a storage failure. |
| + return d.byID[subject], nil |
| +} |
| + |
| +// passwordApp is an instance whose subjects are password's — decimal |
| +// user ids — with dir standing in for the app's user table. |
| +func passwordApp(t *testing.T, dir *directory) (*ideartest.App, *collector) { |
| + t.Helper() |
| + col := &collector{} |
| + cfg := idear.Config{} |
| + if dir != nil { |
| + cfg.EmailForSubject = dir.emailForSubject |
| + } |
| + return ideartest.NewAppWith(t, cfg, idear.HandlerConfig{Deliver: col.deliver}), col |
| +} |
| + |
| +// TestReconciliationWithEmailForSubjectRefusesAnotherAddressesToken is |
| +// the F1 regression test. |
| +// |
| +// The orphan is real and signed in: their app user row exists and |
| +// their member row does not, which is what a create-succeeded-then- |
| +// member-write-failed admission leaves behind, and what the loser of a |
| +// first-signup claim race is. They are not entitled to a token issued |
| +// to somebody else — tokens leak into browser flash cookies (with |
| +// Deliver nil), into URL history, referrers and logs — and with the |
| +// resolver wired, idear applies the rule admission applies: possession |
| +// of the token AND an email match. |
| +func TestReconciliationWithEmailForSubjectRefusesAnotherAddressesToken(t *testing.T) { |
| + dir := &directory{byID: map[string]string{ |
| + "7": "orphan@example.test", |
| + // The intended recipient's spelling in the app's table is not |
| + // the invitation's: the comparison is normalised on both |
| + // sides, or a correctly-invited person is locked out. |
| + "9": "Intended@Example.Test", |
| + }} |
| + app, col := passwordApp(t, dir) |
| + oc := app.As(app.H.Owner()) |
| + token := invite(t, app, col, oc, "intended@example.test", idear.RoleAdmin) |
| + |
| + orphan := app.SignIn("7") |
| + res := orphan.Post("/invitations/"+token, url.Values{}) |
| + if res.Status != http.StatusNotFound { |
| + t.Fatalf("an orphan redeeming another address's token → %d, want 404; body %q", res.Status, res.Body) |
| + } |
| + if m := memberBySubject(t, app, "7"); m != nil { |
| + t.Fatalf("another address's invitation admitted %+v at %s", m, m.Role) |
| + } |
| + |
| + // The refusal did not spend it: the person it was issued to still |
| + // has it, and lands at the invited role. |
| + intended := app.SignIn("9") |
| + if res := intended.Post("/invitations/"+token, url.Values{}); res.Status != http.StatusSeeOther { |
| + t.Fatalf("the intended recipient → %d, want 303; body %q", res.Status, res.Body) |
| + } |
| + m := memberBySubject(t, app, "9") |
| + if m == nil { |
| + t.Fatal("the intended recipient redeemed the invitation but no member row was written") |
| + } |
| + if m.Role != idear.RoleAdmin { |
| + t.Errorf("the healed orphan is %s, want the invitation's admin", m.Role) |
| + } |
| +} |
| + |
| +// TestReconciliationWithoutEmailForSubjectTrustsTheTokenAlone is the |
| +// OTHER half of F1, and it is deliberately an assertion of the |
| +// permissive behaviour rather than a gap left untested. |
| +// |
| +// With no Config.EmailForSubject, idear has no way to turn a decimal |
| +// user id into an address, so reconciliation asks only for a live |
| +// token. A signed-in orphan holding one issued to somebody else |
| +// redeems it, at that token's role. That is what SKILL.md §7 says |
| +// happens, and this test is what keeps the two of them honest: change |
| +// the behaviour and this goes red, which is the moment to change the |
| +// documentation with it. |
| +func TestReconciliationWithoutEmailForSubjectTrustsTheTokenAlone(t *testing.T) { |
| + app, col := passwordApp(t, nil) |
| + oc := app.As(app.H.Owner()) |
| + token := invite(t, app, col, oc, "intended@example.test", idear.RoleAdmin) |
| + |
| + orphan := app.SignIn("7") |
| + res := orphan.Post("/invitations/"+token, url.Values{}) |
| + if res.Status != http.StatusSeeOther { |
| + t.Fatalf("the documented permissive path → %d, want 303; body %q", res.Status, res.Body) |
| + } |
| + m := memberBySubject(t, app, "7") |
| + if m == nil { |
| + t.Fatal("the permissive path admitted nobody; SKILL.md §7 says the token alone is enough here") |
| + } |
| + if m.Role != idear.RoleAdmin { |
| + t.Errorf("the orphan landed at %s, want the token's own admin — the risk being documented is exactly that it is the TOKEN's role", m.Role) |
| + } |
| +} |
| + |
| +// TestReconciliationRefusesAViewerTheResolverCannotPlace covers the |
| +// resolver's two non-answers, which must not be confused with each |
| +// other. |
| +// |
| +// ("", nil) is the app saying "that subject is nobody I know" — a |
| +// REFUSAL, because a viewer idear cannot identify must not spend an |
| +// invitation issued to one it can. A non-nil error is the app's |
| +// database failing, which is a 500 and must never read as policy: the |
| +// invitation stays live and the person can try again. |
| +func TestReconciliationRefusesAViewerTheResolverCannotPlace(t *testing.T) { |
| + t.Run("unknown subject", func(t *testing.T) { |
| + app, col := passwordApp(t, &directory{byID: map[string]string{}}) |
| + oc := app.As(app.H.Owner()) |
| + token := invite(t, app, col, oc, "intended@example.test", idear.RoleMember) |
| + |
| + res := app.SignIn("7").Post("/invitations/"+token, url.Values{}) |
| + if res.Status != http.StatusNotFound { |
| + t.Fatalf("a subject the app cannot place → %d, want 404; body %q", res.Status, res.Body) |
| + } |
| + if m := memberBySubject(t, app, "7"); m != nil { |
| + t.Fatalf("an unplaceable subject was admitted: %+v", m) |
| + } |
| + }) |
| + |
| + t.Run("the resolver fails", func(t *testing.T) { |
| + app, col := passwordApp(t, &directory{err: errors.New("the app's user table is unreadable")}) |
| + oc := app.As(app.H.Owner()) |
| + token := invite(t, app, col, oc, "intended@example.test", idear.RoleMember) |
| + |
| + res := app.SignIn("7").Post("/invitations/"+token, url.Values{}) |
| + if res.Status != http.StatusInternalServerError { |
| + t.Fatalf("a failing resolver → %d, want 500; a storage failure must not render as a policy refusal. Body %q", res.Status, res.Body) |
| + } |
| + if m := memberBySubject(t, app, "7"); m != nil { |
| + t.Fatalf("a failing resolver admitted %+v", m) |
| + } |
| + // And it cost nobody their invitation. |
| + invs, err := app.H.Roster.PendingInvitations(app.H.Ctx()) |
| + if err != nil || len(invs) != 1 { |
| + t.Fatalf("pending invitations = %v, %v; a failed lookup consumed the invitation", invs, err) |
| + } |
| + }) |
| +} |
| + |
| // --------------------------------------------------------------- |
| // §7.10 — Transfer racing Deactivate never yields a deactivated Owner. |
| // --------------------------------------------------------------- |
diff --git a/roster.go b/roster.go| index cd18430..79efe21 100644 |
| --- a/roster.go |
| +++ b/roster.go |
| @@ -50,6 +50,53 @@ type Config struct { |
| // middleware must be mounted inside the app's session guard. |
| Subject func(*http.Request) (string, bool) |
| + // EmailForSubject resolves a session Subject to the address the |
| + // APP knows that subject by. It is OPTIONAL, and it buys exactly |
| + // one thing: reconciliation (POST /invitations/{token}) can then |
| + // require the invitation to have been issued to this viewer's |
| + // address — the same rule admission applies. |
| + // |
| + // It is not consulted under keymail, where the Subject IS the |
| + // verified address and idear answers the question itself. It is |
| + // for the PASSWORD path, where the Subject is an opaque decimal |
| + // user id that only the app's own user table can resolve: |
| + // |
| + // EmailForSubject: func(ctx context.Context, subject string) (string, error) { |
| + // id, err := strconv.ParseInt(subject, 10, 64) |
| + // if err != nil { |
| + // return "", nil |
| + // } |
| + // var u User |
| + // switch err := g.WithContext(ctx).Where("id = ?", id).Take(&u).Error; { |
| + // case errors.Is(err, gorm.ErrRecordNotFound): |
| + // return "", nil |
| + // case err != nil: |
| + // return "", err |
| + // } |
| + // return u.Email, nil |
| + // } |
| + // |
| + // LEFT NIL, PASSWORD RECONCILIATION TRUSTS POSSESSION OF THE |
| + // TOKEN ALONE. idear cannot then tell whose address an invitation |
| + // was issued to relative to the viewer, so any signed-in ORPHAN — |
| + // someone with an app user row and no member row, which is the |
| + // create-succeeded-then-member-write-failed case and the |
| + // claim-race loser — may spend ANY live token they get hold of, |
| + // at whatever role it carries. That population is small and not |
| + // freely manufacturable, but a token is not hard to come by: with |
| + // HandlerConfig.Deliver nil it goes into a browser flash cookie, |
| + // and a token in a URL rides into history, referrers and logs. |
| + // Set this hook and that redemption is refused. |
| + // |
| + // THE CONTRACT. Return the address, or "" WITH A NIL ERROR when |
| + // the subject resolves to nobody — idear refuses the redemption |
| + // in that case, because a subject it cannot place must not be |
| + // admitted on a token alone. A non-nil error is a STORAGE |
| + // FAILURE, answered 500 and never rendered as a policy refusal. |
| + // The returned address is normalised before it is compared, so |
| + // the app may hand back whatever spelling its own table holds. |
| + EmailForSubject func(ctx context.Context, subject string) (string, error) |
| + |
| // NotFound answers a request from someone who is not an active |
| // member. It MUST be the same renderer the app gives chi's own |
| // NotFound: an app with a custom 404 page and idear's default |
| @@ -326,6 +373,26 @@ func (rs *Roster) Claim(ctx context.Context, subject, email, name string) (*Memb |
| // rank strictly below actor's; RoleOwner is refused for everyone. See |
| // checkInviteRole. The actor is re-read inside the transaction, so an |
| // admin deactivated a moment ago cannot still hand out invitations. |
| +// |
| +// RE-INVITING SUPERSEDES. Any invitation for the same address that is |
| +// still unaccepted and unrevoked is REVOKED in this same transaction, |
| +// so an address has at most one live invitation at a time. That is not |
| +// tidiness — it is the only semantics an admin would predict, and |
| +// without it the two identity paths disagree about which of several |
| +// coexisting invitations is spent: |
| +// |
| +// - keymail redeems by address, and acceptByAddress takes the OLDEST |
| +// redeemable row. Re-inviting Alice at the corrected higher role |
| +// would be silently ignored, and re-inviting her at a corrected |
| +// LOWER role would leave the stale higher one live for her to |
| +// escalate past the admin's intent. |
| +// - password redeems by token, so the invitee lands at whichever of |
| +// the several links they happen to click. |
| +// |
| +// The revocation uses the same conditions Revoke does — unaccepted and |
| +// unrevoked, expiry not consulted, since an expired row is already |
| +// dead and revoking it costs nothing — so a spent invitation is never |
| +// rewritten and the record of what was accepted stays true. |
| func (rs *Roster) Invite(ctx context.Context, actor *Member, email string, role Role) (*Invitation, string, error) { |
| if actor == nil { |
| return nil, "", forbidden("no actor") |
| @@ -367,6 +434,15 @@ func (rs *Roster) Invite(ctx context.Context, actor *Member, email string, role |
| if err := checkInviteRole(cur, role); err != nil { |
| 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 |
| + // leave the invitee holding nothing. |
| + if err := tx.Model(&Invitation{}). |
| + Where("email = ? AND accepted_at IS NULL AND revoked_at IS NULL", email). |
| + Update("revoked_at", now).Error; err != nil { |
| + return err |
| + } |
| inv.InvitedBy = cur.ID |
| return tx.Create(inv).Error |
| }) |
| @@ -507,6 +583,12 @@ func memberFromInvitation(tx *gorm.DB, inv *Invitation, subject, name string) (* |
| // ADDRESS and no token: it redeems the oldest still-redeemable |
| // invitation for email and writes the Member it buys. |
| // |
| +// Invite revokes any live invitation for an address before writing a |
| +// new one, so "the oldest" is normally "the only one". The Order("id") |
| +// stays because this must still be deterministic against rows written |
| +// by a seed, a migration or a hand-repaired database — and because a |
| +// SELECT with no ORDER BY is a coin toss, not a rule. |
| +// |
| // It exists rather than reusing Accept because only the plaintext |
| // token can address a row by token_hash, and keymail never sees one — |
| // auth's whole flow is "we mailed this address a link and it came |
diff --git a/roster_test.go b/roster_test.go| index 80a6ed1..81acd65 100644 |
| --- a/roster_test.go |
| +++ b/roster_test.go |
| @@ -348,6 +348,106 @@ func TestRevoke_KillsAPendingInvitation(t *testing.T) { |
| } |
| } |
| +// TestInvite_SupersedesAPendingInvitationForTheSameAddress pins the |
| +// only semantics an admin would predict: re-inviting REPLACES. |
| +// |
| +// Without it two live invitations for one address coexist and the two |
| +// identity paths disagree about which one is spent. keymail redeems |
| +// the OLDEST redeemable row, so re-inviting Alice at a corrected |
| +// higher role is silently ignored — the admin's members page says |
| +// Admin and Alice arrives as Member — and a corrected LOWER role |
| +// leaves the stale higher one live for her to escalate past the |
| +// correction. password spends whichever of the several links the |
| +// invitee happens to click. |
| +// |
| +// Both redemption paths are driven, in two instances, because "both |
| +// land at the new role" is the claim and one path proving it is half |
| +// an answer. |
| +func TestInvite_SupersedesAPendingInvitationForTheSameAddress(t *testing.T) { |
| + // The mistake being corrected: Member, then Admin. The second |
| + // invite spells the address differently on purpose — invitations |
| + // are stored normalised, so the supersede must match on the |
| + // normalised form or it silently does nothing. |
| + const address = "alice@example.test" |
| + const retyped = " Alice@Example.Test " |
| + |
| + t.Run("password redeems the new token", func(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + stale, staleToken, err := h.Roster.Invite(ctx, owner, address, idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("first Invite: %v", err) |
| + } |
| + fresh, freshToken, err := h.Roster.Invite(ctx, owner, retyped, idear.RoleAdmin) |
| + if err != nil { |
| + t.Fatalf("second Invite: %v", err) |
| + } |
| + |
| + // Exactly one invitation is live, and it is the new one. |
| + pending, err := h.Roster.PendingInvitations(ctx) |
| + if err != nil { |
| + t.Fatalf("PendingInvitations: %v", err) |
| + } |
| + if len(pending) != 1 || pending[0].ID != fresh.ID { |
| + t.Fatalf("pending invitations = %+v, want only the re-invitation %d", pending, fresh.ID) |
| + } |
| + // The stale one is REVOKED — not accepted, not deleted: the |
| + // record of what was offered and withdrawn stays true. |
| + switch old := h.Invitation(stale.ID); { |
| + case old.RevokedAt == nil: |
| + t.Error("re-inviting left the earlier invitation live; the invitee can still land at the superseded role") |
| + case old.AcceptedAt != nil: |
| + t.Error("the superseded invitation was marked accepted; nobody accepted it") |
| + } |
| + |
| + // And the stale token is spent as far as anyone holding it is |
| + // concerned. |
| + if _, err := h.Roster.Accept(ctx, staleToken, "alice-subject", "Alice"); !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Fatalf("the superseded token was redeemable: %v", err) |
| + } |
| + m, err := h.Roster.Accept(ctx, freshToken, "alice-subject", "Alice") |
| + if err != nil { |
| + t.Fatalf("Accept of the re-invitation: %v", err) |
| + } |
| + if m.Role != idear.RoleAdmin { |
| + t.Errorf("password admitted %s at %s, want the corrected admin", address, m.Role) |
| + } |
| + }) |
| + |
| + t.Run("keymail redeems by address", func(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + if _, _, err := h.Roster.Invite(ctx, owner, address, idear.RoleMember); err != nil { |
| + t.Fatalf("first Invite: %v", err) |
| + } |
| + if _, _, err := h.Roster.Invite(ctx, owner, retyped, idear.RoleAdmin); err != nil { |
| + t.Fatalf("second Invite: %v", err) |
| + } |
| + |
| + // Authorize is keymail's whole admission: a verified address, |
| + // no token, and acceptByAddress underneath it taking the |
| + // oldest redeemable row. Before the supersede that row was the |
| + // stale Member one. |
| + if !h.Roster.Authorize(address) { |
| + t.Fatal("Authorize refused an address holding a live invitation") |
| + } |
| + var admitted []idear.Member |
| + if err := h.DB.G.Where("subject = ?", address).Find(&admitted).Error; err != nil { |
| + t.Fatalf("looking up the admitted member: %v", err) |
| + } |
| + if len(admitted) != 1 { |
| + t.Fatalf("roster holds %d rows for %q, want 1", len(admitted), address) |
| + } |
| + if admitted[0].Role != idear.RoleAdmin { |
| + t.Errorf("keymail admitted %s at %s, want the corrected admin", address, admitted[0].Role) |
| + } |
| + }) |
| +} |
| + |
| func TestRevoke_Refusals(t *testing.T) { |
| h := ideartest.New(t) |
| ctx := h.Ctx() |