| index 32c316bbc..3b1458598 100644 |
| --- a/internal/console/browserupdates_test.go |
| +++ b/internal/console/browserupdates_test.go |
| @@ -3,10 +3,13 @@ package console |
| import ( |
| "context" |
| "fmt" |
| + "html" |
| "html/template" |
| "net/http" |
| "net/url" |
| "os" |
| + "regexp" |
| + "slices" |
| "strings" |
| "testing" |
| "time" |
| @@ -59,6 +62,37 @@ func updatesSection(body string) string { |
| return body[i : i+j] |
| } |
| |
| +var ( |
| + formInputTag = regexp.MustCompile(`<input [^>]*>`) |
| + formInputAttr = regexp.MustCompile(`([a-z_]+)="([^"]*)"`) |
| +) |
| + |
| +// browserForm is what a browser submits from the Updates section's Save form |
| +// as rendered, untouched: every hidden, text and number input with its |
| +// value, and a checkbox only when it is checked. A test edits the result the |
| +// way a member edits the form. |
| +func browserForm(t *testing.T, sec string) url.Values { |
| + t.Helper() |
| + i := strings.Index(sec, `<form method="post"`) |
| + if i < 0 { |
| + t.Fatalf("no Save form in:\n%s", sec) |
| + } |
| + form := sec[i:] |
| + form = form[:strings.Index(form, "</form>")] |
| + out := url.Values{} |
| + for _, tag := range formInputTag.FindAllString(form, -1) { |
| + attrs := map[string]string{} |
| + for _, m := range formInputAttr.FindAllStringSubmatch(tag, -1) { |
| + attrs[m[1]] = html.UnescapeString(m[2]) |
| + } |
| + if attrs["type"] == "checkbox" && !strings.Contains(tag, " checked") { |
| + continue |
| + } |
| + out.Add(attrs["name"], attrs["value"]) |
| + } |
| + return out |
| +} |
| + |
| // boxesEnforce rewrites box1, the deployment's only box, keeping its held |
| // rollout report and adding heldHost to its routes, so the box join has an |
| // update view for the host. on says whether box1's binary applies update |
| @@ -158,7 +192,7 @@ func TestPageSaveGoesThroughTheGate(t *testing.T) { |
| fx := updatesPageFixture(t) |
| fx.boxesEnforce(true) |
| c := signInPaul(t, fx.consoleFixture) |
| - manual := url.Values{"form": {"updates"}, "auto_shown": {"1"}} // the checkbox unticked |
| + manual := url.Values{"form": {"updates"}, "was_auto": {"1"}} // the page showed auto; the checkbox unticked |
| bad := url.Values{"csrf": {"wrong"}} |
| for k, v := range manual { |
| bad[k] = v |
| @@ -368,18 +402,20 @@ func TestClearingWorksFromThePageWhileTheGateIsClosed(t *testing.T) { |
| if strings.Contains(sec, "disabled") { |
| t.Fatal("a settings control is disabled while the gate is closed") |
| } |
| - for _, want := range []string{`name="auto"`, `name="auto_shown"`, `name="major" value="2"`, `name="pin" value="v1"`, esc(updatewords.Text("settings.page.refused-until-boxes"))} { |
| + for _, want := range []string{`name="auto"`, `name="was_auto" value="0"`, `name="major" value="2"`, `name="was_major" value="2"`, |
| + `name="pin" value="v1"`, `name="was_pin" value="v1"`, esc(updatewords.Text("settings.page.refused-until-boxes"))} { |
| if !strings.Contains(sec, want) { |
| t.Fatalf("section lacks %q:\n%s", want, sec) |
| } |
| } |
| - csrf := sessionToken(t, fx.consoleFixture, c) |
| - post := func(v url.Values) { |
| + // save loads the page, makes one edit to its Save form and posts the |
| + // whole form back, baselines and all, as the browser does. |
| + save := func(edit func(url.Values)) { |
| t.Helper() |
| - v.Set("csrf", csrf) |
| - v.Set("form", "updates") |
| - v.Set("auto_shown", "1") |
| - code, loc, _ := postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), v) |
| + _, page := fx.getPage(c, fx.u("/apps/kass/instances/"+heldHost)) |
| + f := browserForm(t, updatesSection(page)) |
| + edit(f) |
| + code, loc, _ := postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), f) |
| if code != http.StatusSeeOther { |
| t.Fatalf("save = %d", code) |
| } |
| @@ -387,66 +423,81 @@ func TestClearingWorksFromThePageWhileTheGateIsClosed(t *testing.T) { |
| t.Fatalf("save refused:\n%s", page) |
| } |
| } |
| - post(url.Values{"major": {"2"}, "pin": {""}}) // clear the pin; still manual (box unchecked), major unchanged |
| + save(func(f url.Values) { f.Set("pin", "") }) // clear the pin; still manual, major unchanged |
| if r := fx.record(heldHost); r.Pin != "" || r.Updates != "manual" || r.Major == nil || *r.Major != 2 { |
| t.Fatalf("clearing the pin: %+v", r) |
| } |
| - post(url.Values{"major": {""}, "pin": {""}}) // clear the major hold |
| + save(func(f url.Values) { f.Set("major", "") }) // clear the major hold |
| if r := fx.record(heldHost); r.Major != nil || r.Updates != "manual" { |
| t.Fatalf("clearing major: %+v", r) |
| } |
| - post(url.Values{"auto": {"1"}, "major": {""}, "pin": {""}}) // auto back on |
| + save(func(f url.Values) { f.Set("auto", "1") }) // auto back on |
| if r := fx.record(heldHost); r.Updates != "auto" { |
| t.Fatalf("switching auto on: %+v", r) |
| } |
| } |
| |
| func TestAnOmittedCheckboxIsNotManual(t *testing.T) { |
| - v := updatesView{} |
| - v.Settings.Updates = "auto" |
| - if req := updatesFormRequest(url.Values{"csrf": {"x"}}, v); len(req) != 0 { |
| + form := func(kv ...string) url.Values { |
| + f := url.Values{"form": {"updates"}} |
| + for i := 0; i < len(kv); i += 2 { |
| + f.Add(kv[i], kv[i+1]) |
| + } |
| + return f |
| + } |
| + if req := updatesFormRequest(url.Values{"csrf": {"x"}, "was_auto": {"1"}, "pin": {"v2"}, "was_pin": {""}}); len(req) != 0 { |
| t.Fatalf("a post without the form marker sent %v", req) |
| } |
| - if req := updatesFormRequest(url.Values{"form": {"updates"}, "auto_shown": {"1"}, "auto": {"1"}, "pin": {""}}, v); len(req) != 0 { |
| + if req := updatesFormRequest(form("was_auto", "1", "auto", "1", "pin", "", "was_pin", "")); len(req) != 0 { |
| t.Fatalf("an unchanged form sent %v", req) |
| } |
| - if req := updatesFormRequest(url.Values{"form": {"updates"}}, v); len(req) != 0 { |
| + if req := updatesFormRequest(form()); len(req) != 0 { |
| t.Fatalf("a form with no checkbox shown sent %v", req) |
| } |
| - if req := updatesFormRequest(url.Values{"form": {"updates"}, "auto": {"1"}}, v); len(req) != 0 { |
| + if req := updatesFormRequest(form("auto", "1")); len(req) != 0 { |
| t.Fatalf("an auto with no checkbox shown sent %v", req) |
| } |
| - if req := updatesFormRequest(url.Values{"form": {"updates"}, "auto_shown": {"1"}}, v); req["updates"] != "manual" || len(req) != 1 { |
| - t.Fatalf("unchecking auto sent %v", req) |
| + if req := updatesFormRequest(form("was_auto", "yes")); len(req) != 0 { |
| + t.Fatalf("a malformed baseline sent %v", req) |
| } |
| - two := 2 |
| - v.Explicit.Major, v.Explicit.Pin = &two, "v1" |
| - f := url.Values{"form": {"updates"}, "major": {" 2 "}, "pin": {" v1 "}} |
| - if req := updatesFormRequest(f, v); len(req) != 0 { |
| + if req := updatesFormRequest(form("was_auto", "1")); req["updates"] != "manual" || len(req) != 1 { |
| + t.Fatalf("unticking auto sent %v", req) |
| + } |
| + if req := updatesFormRequest(form("was_auto", "0", "auto", "1")); req["updates"] != "auto" || len(req) != 1 { |
| + t.Fatalf("ticking auto sent %v", req) |
| + } |
| + if req := updatesFormRequest(form("was_auto", "0")); len(req) != 0 { |
| + t.Fatalf("a box left unticked sent %v", req) |
| + } |
| + if req := updatesFormRequest(form("major", " 2 ", "was_major", "2", "pin", " v1 ", "was_pin", "v1")); len(req) != 0 { |
| t.Fatalf("an unchanged major and pin sent %v", req) |
| } |
| - f = url.Values{"form": {"updates"}, "major": {"3"}, "pin": {""}} |
| - if req := updatesFormRequest(f, v); req["major"] != float64(3) || req["pin"] != "" || len(req) != 2 { |
| + if req := updatesFormRequest(form("major", "02", "was_major", "2")); len(req) != 0 { |
| + t.Fatalf("the same major written differently sent %v", req) |
| + } |
| + if req := updatesFormRequest(form("major", "3", "was_major", "2", "pin", "", "was_pin", "v1")); req["major"] != float64(3) || req["pin"] != "" || len(req) != 2 { |
| t.Fatalf("a new major and a cleared pin sent %v", req) |
| } |
| - f = url.Values{"form": {"updates"}, "major": {""}} |
| - if req := updatesFormRequest(f, v); req["major"] != "" || len(req) != 1 { |
| + if req := updatesFormRequest(form("major", "", "was_major", "2")); req["major"] != "" || len(req) != 1 { |
| t.Fatalf("a cleared major sent %v", req) |
| } |
| - f = url.Values{"form": {"updates"}, "major": {"two"}} |
| - if req := updatesFormRequest(f, v); req["major"] != "two" { |
| + if req := updatesFormRequest(form("major", "two", "was_major", "")); req["major"] != "two" { |
| t.Fatalf("a bad major sent %v", req) |
| } |
| + // A control with no baseline was not rendered live: it sends nothing. |
| + if req := updatesFormRequest(form("major", "3", "pin", "v2")); len(req) != 0 { |
| + t.Fatalf("controls with no baseline sent %v", req) |
| + } |
| } |
| |
| -// R10: a form without auto_shown (the checkbox was not rendered, because the |
| +// R10: a form without was_auto (the checkbox was not rendered, because the |
| // app keeps updates) never reads the absent checkbox as manual. |
| func TestAnUnshownCheckboxIsNeverReadAsManual(t *testing.T) { |
| fx := updatesPageFixture(t) |
| fx.boxesEnforce(true) |
| c := signInPaul(t, fx.consoleFixture) |
| csrf := sessionToken(t, fx.consoleFixture, c) |
| - code, _, _ := postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), url.Values{"csrf": {csrf}, "form": {"updates"}, "pin": {""}}) |
| + code, _, _ := postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), url.Values{"csrf": {csrf}, "form": {"updates"}, "pin": {""}, "was_pin": {""}}) |
| if r := fx.record(heldHost); code != http.StatusSeeOther || r.Updates != "" { |
| t.Fatalf("save = %d, record %+v", code, r) |
| } |
| @@ -461,7 +512,7 @@ func TestKeptSettingsAreReadOnly(t *testing.T) { |
| c := signInPaul(t, fx.consoleFixture) |
| _, body := fx.getPage(c, fx.u("/apps/kass/instances/"+heldHost)) |
| sec := updatesSection(body) |
| - for _, bad := range []string{`name="auto"`, `name="auto_shown"`, `name="major"`, `name="pin"`, `name="form"`, esc(updatewords.Text("settings.page.save"))} { |
| + for _, bad := range []string{`name="auto"`, `name="was_auto"`, `name="major"`, `name="was_major"`, `name="pin"`, `name="was_pin"`, `name="form"`, esc(updatewords.Text("settings.page.save"))} { |
| if strings.Contains(sec, bad) { |
| t.Errorf("kept settings section has %q", bad) |
| } |
| @@ -477,6 +528,110 @@ func TestKeptSettingsAreReadOnly(t *testing.T) { |
| } |
| } |
| |
| +// staleSave posts f to the Save door and fails on anything but a clean save. |
| +func (fx *pageFx) staleSave(c *http.Client, f url.Values) { |
| + fx.t.Helper() |
| + code, loc, _ := postForm(fx.t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), f) |
| + if code != http.StatusSeeOther { |
| + fx.t.Fatalf("save = %d", code) |
| + } |
| + if _, page := fx.getPage(c, loc); !strings.Contains(page, esc(updatewords.Text("settings.page.saved"))) { |
| + fx.t.Fatalf("save not saved:\n%s", page) |
| + } |
| +} |
| + |
| +// updatesAudit is every instance-updates activity line for kass. |
| +func (fx *pageFx) updatesAudit() []string { |
| + fx.t.Helper() |
| + acts, err := fx.st.Events(OpsAccountID, "kass", 50) |
| + if err != nil { |
| + fx.t.Fatal(err) |
| + } |
| + var out []string |
| + for _, a := range acts { |
| + if a.Action == "instance-updates" { |
| + out = append(out, a.Detail) |
| + } |
| + } |
| + return out |
| +} |
| + |
| +// Review round 1 (R15): the form sends what the member changed on the page |
| +// they saw, not what differs from the record at post time. Member A's page |
| +// shows the instance pinned to v1; member B clears the pin behind it; A |
| +// unticks auto and saves. The pin A never touched stays cleared. |
| +func TestAStalePageDoesNotUndoAnotherMembersChange(t *testing.T) { |
| + fx := updatesPageFixture(t) |
| + fx.grant("updates", "major", "pin") |
| + fx.boxesEnforce(true) |
| + fx.pin(heldHost, "v1") |
| + c := signInPaul(t, fx.consoleFixture) |
| + _, body := fx.getPage(c, fx.u("/apps/kass/instances/"+heldHost)) |
| + f := browserForm(t, updatesSection(body)) |
| + if f.Get("pin") != "v1" || f.Get("auto") != "1" { |
| + t.Fatalf("rendered form %v", f) |
| + } |
| + fx.pin(heldHost, "") // member B, through the CLI, after A's page loaded |
| + f.Del("auto") // A unticks auto |
| + fx.staleSave(c, f) |
| + if r := fx.record(heldHost); r.Updates != release.UpdatesManual || r.Pin != "" { |
| + t.Fatalf("record after a stale save: updates %q pin %q", r.Updates, r.Pin) |
| + } |
| + want := []string{fmt.Sprintf(updatewords.Text("settings.console.audit-set"), heldHost, "updates manual")} |
| + if got := fx.updatesAudit(); !slices.Equal(got, want) { |
| + t.Fatalf("audit = %q, want %q", got, want) |
| + } |
| +} |
| + |
| +// The auto variant: A's page shows the instance manual and pinned; B switches |
| +// it to auto behind it; A only clears the pin. The unticked box A left alone |
| +// does not switch the instance back to manual. |
| +func TestAStalePageDoesNotUndoAnotherMembersSwitchToAuto(t *testing.T) { |
| + fx := updatesPageFixture(t) |
| + fx.grant("updates", "major", "pin") |
| + fx.boxesEnforce(true) |
| + fx.setRecordSettings(heldHost, func(r *provision.Record) { r.Updates, r.Pin = release.UpdatesManual, "v1" }) |
| + c := signInPaul(t, fx.consoleFixture) |
| + _, body := fx.getPage(c, fx.u("/apps/kass/instances/"+heldHost)) |
| + f := browserForm(t, updatesSection(body)) |
| + if f.Get("pin") != "v1" || f.Has("auto") { |
| + t.Fatalf("rendered form %v", f) |
| + } |
| + fx.setRecordUpdates(heldHost, release.UpdatesAuto) // member B |
| + f.Set("pin", "") // A clears the pin |
| + fx.staleSave(c, f) |
| + if r := fx.record(heldHost); r.Updates != release.UpdatesAuto || r.Pin != "" { |
| + t.Fatalf("record after a stale save: updates %q pin %q", r.Updates, r.Pin) |
| + } |
| + want := []string{fmt.Sprintf(updatewords.Text("settings.console.audit-set"), heldHost, "pin cleared")} |
| + if got := fx.updatesAudit(); !slices.Equal(got, want) { |
| + t.Fatalf("audit = %q, want %q", got, want) |
| + } |
| +} |
| + |
| +// Review round 1 (R16): both forms spend the shared write budget, and a |
| +// refusal is the page's reviewed words, not the API's. |
| +func TestPageSaveAndUpdateNowAreRateLimited(t *testing.T) { |
| + fx := updatesPageFixture(t) |
| + fx.srv.cli.write = newRateLimiter(1, time.Minute) |
| + c := signInPaul(t, fx.consoleFixture) |
| + csrf := sessionToken(t, fx.consoleFixture, c) |
| + limited := `<p class="banner error">` + esc(updatewords.Text("settings.page.rate-limited")) + `</p>` |
| + save := url.Values{"csrf": {csrf}, "form": {"updates"}} |
| + _, loc, _ := postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), save) |
| + if _, page := fx.getPage(c, loc); !strings.Contains(page, esc(updatewords.Text("settings.page.saved"))) { |
| + t.Fatalf("first save:\n%s", page) |
| + } |
| + _, loc, _ = postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates"), save) |
| + if _, page := fx.getPage(c, loc); !strings.Contains(page, limited) { |
| + t.Fatalf("second save lacks %q:\n%s", limited, page) |
| + } |
| + _, loc, _ = postForm(t, c, fx.u("/apps/kass/instances/"+heldHost+"/updates/install"), url.Values{"csrf": {csrf}}) |
| + if _, page := fx.getPage(c, loc); !strings.Contains(page, limited) || fx.record(heldHost).InstallRequest != nil { |
| + t.Fatalf("Update now on a spent budget lacks %q or wrote %+v:\n%s", limited, fx.record(heldHost).InstallRequest, page) |
| + } |
| +} |
| + |
| // TestUpdatesSectionPreview writes the instance page, Updates section and |
| // all, to $UPDATEWORDS_PREVIEW for Paul's copy review: an owner's view of a |
| // pinned instance on stable whose app grants updates, major and pin, with a |