| index 0124023..4e0d6fc 100644 |
| --- a/admit_test.go |
| +++ b/admit_test.go |
| @@ -15,6 +15,7 @@ import ( |
| |
| "github.com/carlosframework/rastrillo/password" |
| "github.com/carlosframework/rastrillo/sessions" |
| + "gorm.io/gorm" |
| |
| "amadan.net/rastrillo/idear" |
| "amadan.net/rastrillo/idear/internal/ideartest" |
| @@ -73,8 +74,29 @@ func (u *users) created() []string { |
| // |
| // The email is lowercased and trimmed first, exactly as |
| // password.Signup does before calling Create, so these tests exercise |
| -// the string admission will really see. |
| +// the string admission will really see in a correctly-wired app. |
| func attempt(t *testing.T, rs *idear.Roster, u *users, form url.Values, carry bool) (int64, error) { |
| + t.Helper() |
| + return runSignup(t, rs, u, form, carry, func(typed string) string { |
| + return strings.ToLower(strings.TrimSpace(typed)) |
| + }) |
| +} |
| + |
| +// attemptAsTyped is attempt with password's own normalisation REMOVED, |
| +// handing admission the address exactly as it came off the form. |
| +// |
| +// It exists because admission must not depend on its caller for that. |
| +// password/handlers.go's normalizeEmail happens to do the same folding |
| +// today, so nothing is broken — but Admitting's contract says it |
| +// normalises the submitted address, and a contract nothing exercises |
| +// is a comment. The next Create wrapper, or a password release that |
| +// stops folding, would find out in production. |
| +func attemptAsTyped(t *testing.T, rs *idear.Roster, u *users, form url.Values, carry bool) (int64, error) { |
| + t.Helper() |
| + return runSignup(t, rs, u, form, carry, func(typed string) string { return typed }) |
| +} |
| + |
| +func runSignup(t *testing.T, rs *idear.Roster, u *users, form url.Values, carry bool, prepare func(string) string) (int64, error) { |
| t.Helper() |
| admitted := rs.Admitting(u.create) |
| |
| @@ -86,8 +108,7 @@ func attempt(t *testing.T, rs *idear.Roster, u *users, form url.Values, carry bo |
| if perr := r.ParseForm(); perr != nil { |
| t.Fatalf("parsing the posted form: %v", perr) |
| } |
| - email := strings.ToLower(strings.TrimSpace(r.FormValue("email"))) |
| - id, err = admitted(r.Context(), email, "a-password-hash") |
| + id, err = admitted(r.Context(), prepare(r.FormValue("email")), "a-password-hash") |
| }) |
| |
| var h http.Handler = inner |
| @@ -701,3 +722,347 @@ func TestAdmittingOpenSignUpStillHonoursAMatchingToken(t *testing.T) { |
| t.Error("the invitation was left pending by a signup that redeemed it") |
| } |
| } |
| + |
| +// ------------------------------------------ the keymail round trip |
| + |
| +// TestAuthorizeRoundTripsTheSubjectAuthMints is the keymail twin of |
| +// TestAdmittingSubjectMatchesThePasswordSession, and it pins the |
| +// failure that broke this task's first round. |
| +// |
| +// rastrillo/auth mints the session as |
| +// sessions.Session{Subject: id.Address} (auth/handlers.go's admit), |
| +// and Identity.Address is the address THE VISITOR TYPED: |
| +// keymaildev/signin's SplitAddress lowercases only the domain and |
| +// deliberately keeps the local part's case, and flow.go stores the raw |
| +// typed string. An iOS keyboard capitalises the first letter of an |
| +// email field by default, so "Alice@Corp.Test" is an ordinary thing to |
| +// receive, not an edge case. |
| +// |
| +// If Authorize writes a Subject that the session auth then mints |
| +// cannot resolve, the first arrival claims the instance, signs in |
| +// successfully forever, and 404s on every guarded route forever — |
| +// /members included, so they can never invite anyone. The claim is |
| +// spent, everyone else is refused, and the instance is dead. |
| +// |
| +// TestAuthorizeNormalisesTheAddress does NOT cover this: it only asks |
| +// whether an unnormalised address finds an ALREADY-SEEDED lowercase |
| +// row. This asks whether the row Authorize ITSELF writes can be found |
| +// by the subject auth itself would mint. |
| +func TestAuthorizeRoundTripsTheSubjectAuthMints(t *testing.T) { |
| + const typed = "Alice@Corp.Test" // exactly what auth passes on |
| + |
| + h := guardedHarness(t) |
| + if !h.Roster.Authorize(typed) { |
| + t.Fatal("Authorize refused the first arrival into an empty roster") |
| + } |
| + |
| + // The session auth mints carries the typed address verbatim. |
| + var s spy |
| + w := as(typed, h.Roster.Require(s.handler())) |
| + if w.Code != http.StatusOK { |
| + t.Fatalf("Require answered %d for the very session auth would mint after this Authorize; the instance is bricked", w.Code) |
| + } |
| + if s.member == nil || s.member.Role != idear.RoleOwner { |
| + t.Fatalf("From = %+v, want the owner", s.member) |
| + } |
| + |
| + // And the same human typing it differently later is the SAME row, |
| + // not a second one — which is why both sides are folded rather |
| + // than the raw string being stored. |
| + var s2 spy |
| + if got := as("alice@corp.test", h.Roster.Require(s2.handler())); got.Code != http.StatusOK { |
| + t.Errorf("the same address in lower case answered %d, want 200", got.Code) |
| + } |
| + if s2.member == nil || s.member.ID != s2.member.ID { |
| + t.Error("two spellings of one address resolved to different member rows") |
| + } |
| + if n := h.CountMembers(); n != 1 { |
| + t.Errorf("roster has %d rows for one person, want 1", n) |
| + } |
| +} |
| + |
| +// TestAuthorizeRoundTripsAnInvitedSubject is the same round trip on |
| +// the invitation path, where the Member is written by |
| +// acceptByAddress rather than by Claim. |
| +func TestAuthorizeRoundTripsAnInvitedSubject(t *testing.T) { |
| + h := guardedHarness(t) |
| + invited(t, h, "Bob@Corp.Test", idear.RoleAdmin) |
| + |
| + const typed = "Bob@Corp.Test" |
| + if !h.Roster.Authorize(typed) { |
| + t.Fatal("Authorize refused an invited address") |
| + } |
| + |
| + var s spy |
| + w := as(typed, h.Roster.Require(s.handler())) |
| + if w.Code != http.StatusOK { |
| + t.Fatalf("Require answered %d for the session auth would mint; the invited member is locked out", w.Code) |
| + } |
| + if s.member == nil || s.member.Role != idear.RoleAdmin { |
| + t.Fatalf("From = %+v, want an admin", s.member) |
| + } |
| +} |
| + |
| +// ---------------------------------------------------- fault injection |
| + |
| +// breakRoster drops the members table out from under the roster. It is |
| +// the bluntest possible storage failure and the only one that needs no |
| +// hook into gorm: every query idear makes about membership now errors. |
| +func breakRoster(t *testing.T, h *ideartest.Harness) { |
| + t.Helper() |
| + if err := h.DB.G.Exec("DROP TABLE idear_members").Error; err != nil { |
| + t.Fatalf("dropping idear_members: %v", err) |
| + } |
| +} |
| + |
| +// TestAuthorizeRefusesWhenTheStoreIsBroken: Authorize returns a bool |
| +// with no error channel, so "a database failure can never come back |
| +// true" is a property only a test can hold. Every early return in it |
| +// is a `return false`, and this is what stops one of them becoming a |
| +// `return true` in a later refactor. |
| +func TestAuthorizeRefusesWhenTheStoreIsBroken(t *testing.T) { |
| + h := guardedHarness(t) |
| + member := h.Owner() |
| + breakRoster(t, h) |
| + |
| + for _, addr := range []string{member.Email, "stranger@corp.test", ""} { |
| + if h.Roster.Authorize(addr) { |
| + t.Errorf("Authorize(%q) returned true against a broken store", addr) |
| + } |
| + } |
| +} |
| + |
| +// TestAdmittingRefusesWhenTheStoreIsBroken: admission's FIRST store |
| +// call is IsEmpty, so a broken roster is what proves that branch |
| +// refuses. It must also not reach the app's Create — a signup that |
| +// creates a user and then cannot write a member is the orphan this |
| +// design goes out of its way to avoid manufacturing. |
| +func TestAdmittingRefusesWhenTheStoreIsBroken(t *testing.T) { |
| + h := guardedHarness(t) |
| + h.Owner() |
| + breakRoster(t, h) |
| + |
| + u := newUsers() |
| + id, err := attempt(t, h.Roster, u, signupForm("anyone@corp.test", ""), true) |
| + if err == nil { |
| + t.Fatalf("admission returned id %d and no error against a broken store", id) |
| + } |
| + if id != 0 { |
| + t.Errorf("admission returned id %d, want 0", id) |
| + } |
| + if errors.Is(err, password.ErrRefused) { |
| + t.Error("a storage failure was reported as a policy refusal; it must stay an error so password logs it") |
| + } |
| + if calls := u.created(); len(calls) != 0 { |
| + t.Errorf("the app's Create ran %v before the roster was known to be writable; that manufactures an orphan", calls) |
| + } |
| +} |
| + |
| +// ------------------------------------- admission's own normalisation |
| + |
| +// TestAdmittingNormalisesTheSubmittedAddress hands admission the |
| +// address EXACTLY as it came off the form, with password's own |
| +// folding removed. |
| +// |
| +// Nothing is broken today — password/handlers.go normalises identically |
| +// before calling Create — but Admitting's doc comment claims it |
| +// normalises both sides of the email match, and until now every test |
| +// pre-folded the address in the harness, so the claim was never |
| +// exercised. Admission must not depend on its caller for the property |
| +// its own security rule rests on. |
| +func TestAdmittingNormalisesTheSubmittedAddress(t *testing.T) { |
| + h := guardedHarness(t) |
| + inv, token := invited(t, h, "admin@corp.test", idear.RoleAdmin) |
| + |
| + u := newUsers() |
| + id, err := attemptAsTyped(t, h.Roster, u, signupForm(" Admin@Corp.Test ", token), true) |
| + if err != nil { |
| + t.Fatalf("an invited signup was refused because of how it was typed: %v", err) |
| + } |
| + |
| + m := memberFor(t, h, "admin@corp.test") |
| + if m == nil { |
| + t.Fatal("no member row at the normalised address; the row was written under the typed spelling") |
| + } |
| + if m.Role != idear.RoleAdmin { |
| + t.Errorf("role = %s, want %s", m.Role, idear.RoleAdmin) |
| + } |
| + if want := strconv.FormatInt(id, 10); m.Subject != want { |
| + t.Errorf("Subject = %q, want %q", m.Subject, want) |
| + } |
| + if got := h.Invitation(inv.ID); got.AcceptedAt == nil { |
| + t.Error("the invitation was not consumed") |
| + } |
| +} |
| + |
| +// TestAdmittingRefusesAnUnnormalisedUninvitedAddress is the other half: |
| +// folding the address must not accidentally admit anyone. A typed |
| +// address that matches no invitation is refused however it is spelled. |
| +func TestAdmittingRefusesAnUnnormalisedUninvitedAddress(t *testing.T) { |
| + h := guardedHarness(t) |
| + invited(t, h, "admin@corp.test", idear.RoleAdmin) |
| + |
| + u := newUsers() |
| + id, err := attemptAsTyped(t, h.Roster, u, signupForm(" Admin@Corp.Test ", ""), true) |
| + mustRefuse(t, id, err) |
| + if got := memberFor(t, h, "admin@corp.test"); got != nil { |
| + t.Fatalf("a token-less signup for an invited address admitted %+v", got) |
| + } |
| +} |
| + |
| +// -------------------------------------------------------- the wiring |
| + |
| +// TestAdmittingRefusesWithNoCreateFunction: Admitting(nil) is a wiring |
| +// bug, and it must fail closed as a STORAGE error rather than as a |
| +// policy refusal — a visitor must never be told they are not invited |
| +// because the app forgot to pass its own Create. |
| +func TestAdmittingRefusesWithNoCreateFunction(t *testing.T) { |
| + h := guardedHarness(t) |
| + h.Owner() |
| + |
| + id, err := h.Roster.Admitting(nil)(context.Background(), "anyone@corp.test", "hash") |
| + if err == nil { |
| + t.Fatalf("Admitting(nil) returned id %d and no error", id) |
| + } |
| + if id != 0 { |
| + t.Errorf("Admitting(nil) returned id %d, want 0", id) |
| + } |
| + if errors.Is(err, password.ErrRefused) { |
| + t.Error("a nil Create was reported to the visitor as a policy refusal") |
| + } |
| + if n := h.CountMembers(); n != 1 { |
| + t.Errorf("roster has %d rows, want 1: nothing may be written", n) |
| + } |
| +} |
| + |
| +// TestCarryTokenIgnoresTheQueryString: the invitation token is a live |
| +// credential. Read from the query string it would ride in the URL, and |
| +// from there into access logs, browser history, and the Referer header |
| +// of every asset the signup page loads — leaking the credential to |
| +// third parties who were never sent it. |
| +// |
| +// r.PostFormValue reads the body only; r.FormValue would accept both, |
| +// and the two are one keystroke apart. |
| +func TestCarryTokenIgnoresTheQueryString(t *testing.T) { |
| + h := guardedHarness(t) |
| + _, token := invited(t, h, "admin@corp.test", idear.RoleAdmin) |
| + |
| + u := newUsers() |
| + admitted := h.Roster.Admitting(u.create) |
| + |
| + var ( |
| + id int64 |
| + err error |
| + ) |
| + inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + id, err = admitted(r.Context(), "admin@corp.test", "hash") |
| + }) |
| + // The token is in the URL and NOWHERE else. |
| + form := url.Values{"email": {"admin@corp.test"}, "password": {"a-long-enough-password"}} |
| + req := httptest.NewRequest(http.MethodPost, "/signup?invite="+url.QueryEscape(token), |
| + strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + h.Roster.CarryToken(inner).ServeHTTP(httptest.NewRecorder(), req) |
| + |
| + mustRefuse(t, id, err) |
| + if got := memberFor(t, h, "admin@corp.test"); got != nil { |
| + t.Fatalf("a token from the query string admitted %+v", got) |
| + } |
| +} |
| + |
| +// breakCounting fails every COUNT this roster issues, and nothing |
| +// else. |
| +// |
| +// It exists because DROP TABLE is too blunt to reach one branch: with |
| +// the table gone, Authorize's BySubject fails FIRST and returns before |
| +// the roster is ever counted, so the IsEmpty failure path is |
| +// unreachable by that fault. This one discriminates on the |
| +// destination gorm is scanning into — Count's is *int64, and no other |
| +// query idear makes has that shape — so a lookup still cleanly MISSES |
| +// while the count fails. That is the exact state the branch needs. |
| +func breakCounting(t *testing.T, h *ideartest.Harness) { |
| + t.Helper() |
| + err := h.DB.G.Callback().Query().Before("gorm:query"). |
| + Register("ideartest:break_counting", func(tx *gorm.DB) { |
| + if _, counting := tx.Statement.Dest.(*int64); counting { |
| + tx.AddError(errors.New("ideartest: injected count failure")) |
| + } |
| + }) |
| + if err != nil { |
| + t.Fatalf("registering the count fault: %v", err) |
| + } |
| +} |
| + |
| +// TestAuthorizeRefusesWhenCountingFails covers the branch DROP TABLE |
| +// cannot reach: the address resolves to no member (a clean miss), and |
| +// then the roster cannot be counted. |
| +// |
| +// Getting this wrong is the worst answer in the package. "Is the |
| +// roster empty" failing OPEN means an arbitrary stranger is handed |
| +// Owner of a populated instance the moment the database hiccups. |
| +func TestAuthorizeRefusesWhenCountingFails(t *testing.T) { |
| + h := guardedHarness(t) |
| + h.Owner() |
| + breakCounting(t, h) |
| + |
| + if h.Roster.Authorize("stranger@corp.test") { |
| + t.Fatal("Authorize returned true when it could not tell whether the roster was empty") |
| + } |
| + // Asserted through Members and Owners, not the harness's |
| + // CountMembers: the injected fault breaks every COUNT in this |
| + // process, the test's own included. |
| + rows, err := h.Roster.Members(h.Ctx()) |
| + if err != nil { |
| + t.Fatalf("listing members: %v", err) |
| + } |
| + if len(rows) != 1 { |
| + t.Errorf("roster has %d rows, want 1: nothing may have been written", len(rows)) |
| + } |
| + if owner := h.TheOwner(); owner.Email == "stranger@corp.test" { |
| + t.Fatal("a failed count handed ownership to a stranger") |
| + } |
| +} |
| + |
| +// TestSubjectIsCanonicalWhicheverSpellingWritesIt pins the WRITE half |
| +// of the Subject pair, which Authorize alone cannot pin because it |
| +// lowercases the address before it ever reaches the store. |
| +// |
| +// The caller that will hand the store a raw typed Subject is Task 5's |
| +// reconciliation route: POST /invitations/{token} writes a Member from |
| +// the LIVE session Subject, which under keymail is the address as the |
| +// visitor typed it. Without canonicalisation on the write, that route |
| +// recreates the same lockout on a different day — a member row nothing |
| +// can look up, or one human holding two rows past a unique index that |
| +// cannot see they are the same person. |
| +func TestSubjectIsCanonicalWhicheverSpellingWritesIt(t *testing.T) { |
| + t.Run("Claim", func(t *testing.T) { |
| + h := guardedHarness(t) |
| + m, err := h.Roster.Claim(h.Ctx(), " Alice@Corp.Test ", "Alice@Corp.Test", "Alice") |
| + if err != nil { |
| + t.Fatalf("Claim: %v", err) |
| + } |
| + if m.Subject != "alice@corp.test" { |
| + t.Errorf("stored Subject = %q, want the canonical form", m.Subject) |
| + } |
| + for _, spelling := range []string{"Alice@Corp.Test", "alice@corp.test", " ALICE@CORP.TEST "} { |
| + if _, err := h.Roster.BySubject(h.Ctx(), spelling); err != nil { |
| + t.Errorf("BySubject(%q) = %v, want the row Claim just wrote", spelling, err) |
| + } |
| + } |
| + }) |
| + |
| + t.Run("Accept", func(t *testing.T) { |
| + h := guardedHarness(t) |
| + _, token := invited(t, h, "bob@corp.test", idear.RoleAdmin) |
| + m, err := h.Roster.Accept(h.Ctx(), token, "Bob@Corp.Test", "Bob") |
| + if err != nil { |
| + t.Fatalf("Accept: %v", err) |
| + } |
| + if m.Subject != "bob@corp.test" { |
| + t.Errorf("stored Subject = %q, want the canonical form", m.Subject) |
| + } |
| + if _, err := h.Roster.BySubject(h.Ctx(), "Bob@Corp.Test"); err != nil { |
| + t.Errorf("BySubject(typed) = %v, want the row Accept just wrote", err) |
| + } |
| + }) |
| +} |
amadan