From 0b66d2c18b0881aad11dbd4f402d4c82f5eeb78e Mon Sep 17 00:00:00 2001 From: Raj Nakarja Date: Thu, 13 Aug 2026 10:39:21 +0200 Subject: [PATCH 1/4] Confirmed every destructive verb, added account delete, and closed the test holes key revoke and member remove now confirm on stdin like fleet delete, which CLAUDE.md already required of every destructive verb. Both name what they are about to do rather than echoing back the id that was typed, since catching a mistyped id is the point: revoking the wrong key kills a live integration and the secret is only ever shown once. Naming the key needed the key fetch lifted out of key list into its own file, matching fleets.go and balances.go. account delete is wired up: it confirms, relays the server's refusal while a fleet is still owned, and removes the stored login once the account is gone. The help advertised --json globally though eleven of the fifteen implemented commands reject it, so it moves into each command's own arguments and the usage line stops promising flags. member list --json printed the server's raw bytes with no trailing newline, unlike the other three. login's stdin goroutine outlived the read and touched os.Stdin after something else could have replaced it, which the race detector reports. The tests had holes wide enough to drive through: nothing asserted the stored key travelled, so nine commands could drop it and stay green; no command drove a non-success status; fleet list and member list asserted nothing about their output, and member list never checked which fleet it asked for. resolve was compared after joining, so collapsing every argument into one string passed. The version const drops to 0.0.3, the next release above v0.0.2. --- internal/commands/account_delete.go | 68 +++++++++++++ internal/commands/account_delete_test.go | 122 +++++++++++++++++++++++ internal/commands/client_test.go | 25 ++++- internal/commands/fleet_delete_test.go | 41 ++++++-- internal/commands/fleet_list_test.go | 43 ++++++-- internal/commands/key_list.go | 34 +------ internal/commands/key_revoke.go | 33 ++++++ internal/commands/key_revoke_test.go | 77 ++++++++++---- internal/commands/keys.go | 48 +++++++++ internal/commands/login.go | 8 +- internal/commands/member_list.go | 10 +- internal/commands/member_list_test.go | 37 ++++++- internal/commands/member_remove.go | 33 ++++++ internal/commands/member_remove_test.go | 36 +++++-- main.go | 19 ++-- main_test.go | 44 +++++++- 16 files changed, 579 insertions(+), 99 deletions(-) create mode 100644 internal/commands/account_delete.go create mode 100644 internal/commands/account_delete_test.go create mode 100644 internal/commands/keys.go diff --git a/internal/commands/account_delete.go b/internal/commands/account_delete.go new file mode 100644 index 0000000..fba83bf --- /dev/null +++ b/internal/commands/account_delete.go @@ -0,0 +1,68 @@ +package commands + +import ( + "bufio" + "errors" + "fmt" + "io" + "io/fs" + "net/http" + "os" + "strings" +) + +func AccountDelete(arguments []string) error { + + if len(arguments) != 0 { + return errors.New("account delete takes no arguments") + } + + fmt.Print("Delete your account, its logins, and your access to every fleet? This cannot be undone. [y/N] ") + + answer, _ := bufio.NewReader(os.Stdin).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Println("Nothing deleted.") + return nil + } + + request, err := authenticatedRequest(http.MethodDelete, "/account", nil) + + if err != nil { + return err + } + + response, err := apiClient.Do(request) + + if err != nil { + return fmt.Errorf("the server could not be reached: %w", err) + } + + defer response.Body.Close() + + if response.StatusCode != http.StatusNoContent { + message, _ := io.ReadAll(io.LimitReader(response.Body, 4096)) + + return fmt.Errorf("the server said: %s", strings.TrimSpace(string(message))) + } + + // The stored login died with the account, so it goes whether or not the + // file is still there + path, err := keyPath() + + if err != nil { + return err + } + + err = os.Remove(path) + + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + + fmt.Println("Account deleted.") + + return nil +} diff --git a/internal/commands/account_delete_test.go b/internal/commands/account_delete_test.go new file mode 100644 index 0000000..734dad5 --- /dev/null +++ b/internal/commands/account_delete_test.go @@ -0,0 +1,122 @@ +package commands + +import ( + "net/http" + "os" + "strings" + "testing" +) + +func TestAccountDelete(t *testing.T) { + tests := []struct { + name string + arguments []string + answer string + refusal string + refusalCode int + wantDeleted bool + wantShown string + wantError string + }{ + { + name: "confirmed with y", + answer: "y\n", + wantDeleted: true, + wantShown: "Account deleted", + }, + { + name: "confirmed with yes", + answer: "YES\n", + wantDeleted: true, + wantShown: "Account deleted", + }, + { + name: "declined by default", + answer: "\n", + wantShown: "Nothing deleted", + }, + { + name: "declined with n", + answer: "n\n", + wantShown: "Nothing deleted", + }, + { + name: "closed input", + wantShown: "Nothing deleted", + }, + { + name: "the server refuses while a fleet is owned", + answer: "y\n", + refusal: "you still own fleets, hand each one over or delete it first", + refusalCode: http.StatusConflict, + wantDeleted: true, + wantError: "you still own fleets, hand each one over or delete it first", + }, + { + name: "arguments are refused", + arguments: []string{"everything"}, + wantError: "takes no arguments", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + deleted := false + + mux := http.NewServeMux() + + mux.HandleFunc("DELETE /account", func(w http.ResponseWriter, r *http.Request) { + deleted = true + + if test.refusal != "" { + http.Error(w, test.refusal, test.refusalCode) + return + } + + w.WriteHeader(http.StatusNoContent) + }) + + loggedInTestServer(t, mux) + + path, err := keyPath() + + if err != nil { + t.Fatal(err) + } + + answerOnStdin(t, test.answer) + + printed, err := captureStdout(t, func() error { + return AccountDelete(test.arguments) + }) + + if test.wantError != "" { + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want it to mention %q", err, test.wantError) + } + } else if err != nil { + t.Fatal(err) + } + + if deleted != test.wantDeleted { + t.Errorf("the server saw the account deleted = %v, want %v", deleted, test.wantDeleted) + } + + if test.wantShown != "" && !strings.Contains(printed, test.wantShown) { + t.Errorf("the output %q does not show %q", printed, test.wantShown) + } + + // The stored login is worthless once the account is gone, and must + // survive anything short of a completed delete + _, statErr := os.Stat(path) + + switch { + case test.wantDeleted && test.wantError == "" && statErr == nil: + t.Error("the login is still stored although the account was deleted") + + case (!test.wantDeleted || test.wantError != "") && statErr != nil: + t.Errorf("the login was removed although the account was not deleted: %v", statErr) + } + }) + } +} diff --git a/internal/commands/client_test.go b/internal/commands/client_test.go index d9744fc..68de4af 100644 --- a/internal/commands/client_test.go +++ b/internal/commands/client_test.go @@ -74,7 +74,18 @@ func loggedInTestServer(t *testing.T, handler http.Handler) { t.Fatal(err) } - server := httptest.NewServer(handler) + // Every command reaching a logged-in server must carry the stored key, so + // the fixture proves it once rather than each command remembering to + authorized := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer ssk_test" { + t.Errorf("%s %s carried authorization %q, want the stored key", + r.Method, r.URL.Path, r.Header.Get("Authorization")) + } + + handler.ServeHTTP(w, r) + }) + + server := httptest.NewServer(authorized) t.Cleanup(server.Close) @@ -219,6 +230,12 @@ func TestTakeServerFlag(t *testing.T) { } func TestApiRequestBase(t *testing.T) { + previousVersion := CliVersion + + CliVersion = "1.2.3" + + t.Cleanup(func() { CliVersion = previousVersion }) + tests := []struct { name string chosenBase string @@ -251,8 +268,10 @@ func TestApiRequestBase(t *testing.T) { t.Errorf("url = %q, want %q", request.URL.String(), test.wantUrl) } - if request.Header.Get("User-Agent") != "superstack/"+CliVersion { - t.Errorf("User-Agent = %q, want the CLI version", request.Header.Get("User-Agent")) + // Pinned to a literal, not to CliVersion: the server's gate parses + // this exact shape, so deriving it here would agree with any value + if request.Header.Get("User-Agent") != "superstack/1.2.3" { + t.Errorf("User-Agent = %q, want superstack/1.2.3", request.Header.Get("User-Agent")) } }) } diff --git a/internal/commands/fleet_delete_test.go b/internal/commands/fleet_delete_test.go index a951c39..63c58c5 100644 --- a/internal/commands/fleet_delete_test.go +++ b/internal/commands/fleet_delete_test.go @@ -38,13 +38,22 @@ func TestFleetDelete(t *testing.T) { tests := []struct { name string answer string + refusal string wantDeleted bool + wantError string }{ - {"confirmed with y", "y\n", true}, - {"confirmed with yes", "YES\n", true}, - {"declined with n", "n\n", false}, - {"declined by default", "\n", false}, - {"closed input", "", false}, + {name: "confirmed with y", answer: "y\n", wantDeleted: true}, + {name: "confirmed with yes", answer: "YES\n", wantDeleted: true}, + {name: "declined with n", answer: "n\n"}, + {name: "declined by default", answer: "\n"}, + {name: "closed input", answer: ""}, + { + name: "the server refuses after the confirmation", + answer: "y\n", + refusal: "only the fleet's owner can delete it", + wantDeleted: true, + wantError: "only the fleet's owner can delete it", + }, } for _, test := range tests { @@ -64,6 +73,11 @@ func TestFleetDelete(t *testing.T) { mux.HandleFunc("DELETE /fleets/{id}", func(w http.ResponseWriter, r *http.Request) { deletedPath = r.URL.Path + if test.refusal != "" { + http.Error(w, test.refusal, http.StatusForbidden) + return + } + w.WriteHeader(http.StatusNoContent) }) @@ -71,9 +85,22 @@ func TestFleetDelete(t *testing.T) { answerOnStdin(t, test.answer) - err := FleetDelete([]string{"3"}) + printed, err := captureStdout(t, func() error { + return FleetDelete([]string{"3"}) + }) - if err != nil { + switch { + case test.wantError != "": + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want it to mention %q", err, test.wantError) + } + + // A refused delete must never claim the fleet is gone + if strings.Contains(printed, "Deleted") { + t.Errorf("the output %q says the fleet was deleted although the server refused", printed) + } + + case err != nil: t.Fatal(err) } diff --git a/internal/commands/fleet_list_test.go b/internal/commands/fleet_list_test.go index 95105de..2091f2f 100644 --- a/internal/commands/fleet_list_test.go +++ b/internal/commands/fleet_list_test.go @@ -9,23 +9,30 @@ import ( func TestFleetList(t *testing.T) { tests := []struct { - name string - arguments []string - fleets string - wantError string + name string + arguments []string + fleets string + wantShown []string + wantAbsent []string + wantExact string + wantError string }{ { - name: "some fleets", - fleets: `[{"id":1,"name":"field trial","owner":true},{"id":2,"name":"rooftop","owner":false}]`, + name: "some fleets", + fleets: `[{"id":1,"name":"field trial","owner":true},{"id":2,"name":"rooftop","owner":false}]`, + wantShown: []string{"ID", "NAME", "ROLE", "field trial", "owner", "rooftop", "member"}, }, { - name: "no fleets", - fleets: `[]`, + name: "no fleets", + fleets: `[]`, + wantShown: []string{"No fleets yet"}, + wantAbsent: []string{"ID", "NAME"}, }, { name: "machine-readable output", arguments: []string{"--json"}, fleets: `[{"id":1,"name":"field trial","owner":true}]`, + wantExact: `[{"id":1,"name":"field trial","owner":true}]` + "\n", }, { name: "an unknown argument", @@ -44,7 +51,9 @@ func TestFleetList(t *testing.T) { loggedInTestServer(t, mux) - err := FleetList(test.arguments) + printed, err := captureStdout(t, func() error { + return FleetList(test.arguments) + }) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { @@ -57,6 +66,22 @@ func TestFleetList(t *testing.T) { if err != nil { t.Fatal(err) } + + if test.wantExact != "" && printed != test.wantExact { + t.Errorf("the output is %q, want exactly %q", printed, test.wantExact) + } + + for _, want := range test.wantShown { + if !strings.Contains(printed, want) { + t.Errorf("the output %q does not show %q", printed, want) + } + } + + for _, absent := range test.wantAbsent { + if strings.Contains(printed, absent) { + t.Errorf("the output %q shows %q although there is nothing to list", printed, absent) + } + } }) } } diff --git a/internal/commands/key_list.go b/internal/commands/key_list.go index 4cd6cfa..295dd30 100644 --- a/internal/commands/key_list.go +++ b/internal/commands/key_list.go @@ -4,20 +4,10 @@ import ( "encoding/json" "errors" "fmt" - "io" - "net/http" "os" "strconv" - "strings" ) -type keyEntry struct { - Id int64 `json:"id"` - Fleet int64 `json:"fleet"` - Label string `json:"label"` - Suffix string `json:"suffix"` -} - func KeyList(arguments []string) error { jsonOutput := false @@ -67,29 +57,7 @@ func KeyList(arguments []string) error { } } - request, err := authenticatedRequest(http.MethodGet, "/keys", nil) - - if err != nil { - return err - } - - response, err := apiClient.Do(request) - - if err != nil { - return fmt.Errorf("the server could not be reached: %w", err) - } - - defer response.Body.Close() - - if response.StatusCode != http.StatusOK { - message, _ := io.ReadAll(io.LimitReader(response.Body, 4096)) - - return fmt.Errorf("the server said: %s", strings.TrimSpace(string(message))) - } - - fetched := []keyEntry{} - - err = json.NewDecoder(response.Body).Decode(&fetched) + fetched, err := fetchKeys() if err != nil { return err diff --git a/internal/commands/key_revoke.go b/internal/commands/key_revoke.go index 0d14fcc..edf4364 100644 --- a/internal/commands/key_revoke.go +++ b/internal/commands/key_revoke.go @@ -1,10 +1,12 @@ package commands import ( + "bufio" "errors" "fmt" "io" "net/http" + "os" "strconv" "strings" ) @@ -21,6 +23,37 @@ func KeyRevoke(arguments []string) error { return errors.New("the key id is the number shown by key list") } + keys, err := fetchKeys() + + if err != nil { + return err + } + + label := "" + found := false + + for _, key := range keys { + if key.Id == keyId { + label = key.Label + found = true + } + } + + if !found { + return errors.New("no such key") + } + + fmt.Printf("Revoke %q? Anything still using it stops reaching the fleet. [y/N] ", label) + + answer, _ := bufio.NewReader(os.Stdin).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Println("Nothing revoked.") + return nil + } + request, err := authenticatedRequest(http.MethodDelete, "/keys/"+strconv.FormatInt(keyId, 10), nil) diff --git a/internal/commands/key_revoke_test.go b/internal/commands/key_revoke_test.go index 0ec5e63..37ac2f3 100644 --- a/internal/commands/key_revoke_test.go +++ b/internal/commands/key_revoke_test.go @@ -1,6 +1,7 @@ package commands import ( + "fmt" "net/http" "strings" "testing" @@ -8,23 +9,51 @@ import ( func TestKeyRevoke(t *testing.T) { tests := []struct { - name string - arguments []string - wantPath string - refusal string - wantError string + name string + arguments []string + answer string + refusal string + wantRevoked string + wantShown string + wantError string }{ { - name: "revoke a key", + name: "revoke a key", + arguments: []string{"3"}, + answer: "y\n", + wantRevoked: "/keys/3", + wantShown: "production", + }, + { + name: "declined by default", + arguments: []string{"3"}, + answer: "\n", + wantShown: "Nothing revoked", + }, + { + name: "declined with n", arguments: []string{"3"}, - wantPath: "/keys/3", + answer: "n\n", + wantShown: "Nothing revoked", + }, + { + name: "closed input", + arguments: []string{"3"}, + wantShown: "Nothing revoked", + }, + { + name: "the server refuses after the confirmation", + arguments: []string{"3"}, + answer: "y\n", + refusal: "no such key", + wantRevoked: "/keys/3", + wantError: "the server said: no such key", }, { - name: "a key out of reach", + name: "a key that is not yours", arguments: []string{"9"}, - wantPath: "/keys/9", - refusal: "no such key", - wantError: "the server said: no such key", + answer: "y\n", + wantError: "no such key", }, { name: "no key id", @@ -45,12 +74,16 @@ func TestKeyRevoke(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + revokedPath := "" + mux := http.NewServeMux() + mux.HandleFunc("GET /keys", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"id":3,"fleet":1,"label":"production","suffix":"a1b2c"}]`) + }) + mux.HandleFunc("DELETE /keys/{id}", func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != test.wantPath { - t.Errorf("the request went to %s, want %s", r.URL.Path, test.wantPath) - } + revokedPath = r.URL.Path if test.refusal != "" { http.Error(w, test.refusal, http.StatusNotFound) @@ -62,18 +95,26 @@ func TestKeyRevoke(t *testing.T) { loggedInTestServer(t, mux) - err := KeyRevoke(test.arguments) + answerOnStdin(t, test.answer) + + printed, err := captureStdout(t, func() error { + return KeyRevoke(test.arguments) + }) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { t.Fatalf("error = %v, want it to mention %q", err, test.wantError) } + } else if err != nil { + t.Fatal(err) + } - return + if revokedPath != test.wantRevoked { + t.Errorf("the server saw %q revoked, want %q", revokedPath, test.wantRevoked) } - if err != nil { - t.Fatal(err) + if test.wantShown != "" && !strings.Contains(printed, test.wantShown) { + t.Errorf("the output %q does not show %q", printed, test.wantShown) } }) } diff --git a/internal/commands/keys.go b/internal/commands/keys.go new file mode 100644 index 0000000..6b64800 --- /dev/null +++ b/internal/commands/keys.go @@ -0,0 +1,48 @@ +package commands + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "strings" +) + +type keyEntry struct { + Id int64 `json:"id"` + Fleet int64 `json:"fleet"` + Label string `json:"label"` + Suffix string `json:"suffix"` +} + +func fetchKeys() ([]keyEntry, error) { + request, err := authenticatedRequest(http.MethodGet, "/keys", nil) + + if err != nil { + return nil, err + } + + response, err := apiClient.Do(request) + + if err != nil { + return nil, fmt.Errorf("the server could not be reached: %w", err) + } + + defer response.Body.Close() + + if response.StatusCode != http.StatusOK { + message, _ := io.ReadAll(io.LimitReader(response.Body, 4096)) + + return nil, fmt.Errorf("the server said: %s", strings.TrimSpace(string(message))) + } + + keys := []keyEntry{} + + err = json.NewDecoder(response.Body).Decode(&keys) + + if err != nil { + return nil, err + } + + return keys, nil +} diff --git a/internal/commands/login.go b/internal/commands/login.go index f0582e1..e206872 100644 --- a/internal/commands/login.go +++ b/internal/commands/login.go @@ -138,9 +138,13 @@ func Login(arguments []string) error { fmt.Println("Press enter to open the browser.") // The read sits in a goroutine so an unpressed key never stalls the - // poll: the code may just as well be entered on another device. + // poll: the code may just as well be entered on another device. The + // stream is captured here, because the goroutine outlives the read and + // must not touch os.Stdin once something else may have replaced it. + prompt := os.Stdin + go func() { - _, err := bufio.NewReader(os.Stdin).ReadString('\n') + _, err := bufio.NewReader(prompt).ReadString('\n') if err == nil { openBrowser(enterAt) diff --git a/internal/commands/member_list.go b/internal/commands/member_list.go index 90d4a30..bc79cdd 100644 --- a/internal/commands/member_list.go +++ b/internal/commands/member_list.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "os" "strconv" "strings" ) @@ -60,11 +61,6 @@ func MemberList(arguments []string) error { return fmt.Errorf("the server said: %s", strings.TrimSpace(string(body))) } - if jsonOutput { - fmt.Print(string(body)) - return nil - } - people := struct { Owner string `json:"owner"` Members []string `json:"members"` @@ -76,6 +72,10 @@ func MemberList(arguments []string) error { return err } + if jsonOutput { + return json.NewEncoder(os.Stdout).Encode(people) + } + emailWidth := max(len("EMAIL"), len(people.Owner)) for _, email := range people.Members { diff --git a/internal/commands/member_list_test.go b/internal/commands/member_list_test.go index 7edce6f..9867d1b 100644 --- a/internal/commands/member_list_test.go +++ b/internal/commands/member_list_test.go @@ -12,27 +12,38 @@ func TestMemberList(t *testing.T) { name string arguments []string people string + wantFleet string + wantShown []string + wantExact string wantError string }{ { name: "the people table", arguments: []string{"3"}, people: `{"owner":"owner@example.com","members":["member@example.com"]}`, + wantFleet: "3", + wantShown: []string{"EMAIL", "ROLE", "owner@example.com", "owner", "member@example.com", "member"}, }, { name: "nobody but the owner", - arguments: []string{"3"}, + arguments: []string{"7"}, people: `{"owner":"owner@example.com","members":[]}`, + wantFleet: "7", + wantShown: []string{"owner@example.com", "owner"}, }, { name: "machine-readable output", arguments: []string{"3", "--json"}, people: `{"owner":"owner@example.com","members":["member@example.com"]}`, + wantFleet: "3", + wantExact: `{"owner":"owner@example.com","members":["member@example.com"]}` + "\n", }, { name: "the flag before the id", - arguments: []string{"--json", "3"}, + arguments: []string{"--json", "5"}, people: `{"owner":"owner@example.com","members":[]}`, + wantFleet: "5", + wantExact: `{"owner":"owner@example.com","members":[]}` + "\n", }, { name: "no fleet id", @@ -55,13 +66,19 @@ func TestMemberList(t *testing.T) { t.Run(test.name, func(t *testing.T) { mux := http.NewServeMux() + askedFleet := "" + mux.HandleFunc("GET /fleets/{id}/members", func(w http.ResponseWriter, r *http.Request) { + askedFleet = r.PathValue("id") + fmt.Fprint(w, test.people) }) loggedInTestServer(t, mux) - err := MemberList(test.arguments) + printed, err := captureStdout(t, func() error { + return MemberList(test.arguments) + }) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { @@ -74,6 +91,20 @@ func TestMemberList(t *testing.T) { if err != nil { t.Fatal(err) } + + if askedFleet != test.wantFleet { + t.Errorf("the people of fleet %q were listed, want fleet %q", askedFleet, test.wantFleet) + } + + if test.wantExact != "" && printed != test.wantExact { + t.Errorf("the output is %q, want exactly %q", printed, test.wantExact) + } + + for _, want := range test.wantShown { + if !strings.Contains(printed, want) { + t.Errorf("the output %q does not show %q", printed, want) + } + } }) } } diff --git a/internal/commands/member_remove.go b/internal/commands/member_remove.go index dd3b803..1b81a37 100644 --- a/internal/commands/member_remove.go +++ b/internal/commands/member_remove.go @@ -1,11 +1,13 @@ package commands import ( + "bufio" "errors" "fmt" "io" "net/http" "net/url" + "os" "strconv" "strings" ) @@ -24,6 +26,37 @@ func MemberRemove(arguments []string) error { return errors.New("the fleet id is the number shown by fleet list") } + fleets, err := fetchFleets() + + if err != nil { + return err + } + + name := "" + found := false + + for _, fleet := range fleets { + if fleet.Id == fleetId { + name = fleet.Name + found = true + } + } + + if !found { + return errors.New("no such fleet") + } + + fmt.Printf("Take away %s's access to %q? [y/N] ", email, name) + + answer, _ := bufio.NewReader(os.Stdin).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Println("Nothing changed.") + return nil + } + request, err := authenticatedRequest(http.MethodDelete, "/fleets/"+strconv.FormatInt(fleetId, 10)+"/members/"+url.PathEscape(email), nil) diff --git a/internal/commands/member_remove_test.go b/internal/commands/member_remove_test.go index f21bc82..1d4f34b 100644 --- a/internal/commands/member_remove_test.go +++ b/internal/commands/member_remove_test.go @@ -1,6 +1,7 @@ package commands import ( + "fmt" "net/http" "strings" "testing" @@ -8,11 +9,18 @@ import ( func TestMemberRemove(t *testing.T) { tests := []struct { - name string - email string + name string + email string + answer string + wantRemoved bool + wantShown string }{ - {"a plain address", "member@example.com"}, - {"an address with a hash", "a#b@example.com"}, + {name: "a plain address", email: "member@example.com", answer: "y\n", wantRemoved: true}, + {name: "an address with a hash", email: "a#b@example.com", answer: "yes\n", wantRemoved: true}, + {name: "the prompt names the fleet", email: "member@example.com", answer: "y\n", wantRemoved: true, wantShown: `access to "pilot"`}, + {name: "declined by default", email: "member@example.com", answer: "\n", wantShown: "Nothing changed"}, + {name: "declined with n", email: "member@example.com", answer: "n\n", wantShown: "Nothing changed"}, + {name: "closed input", email: "member@example.com", wantShown: "Nothing changed"}, } for _, test := range tests { @@ -22,6 +30,10 @@ func TestMemberRemove(t *testing.T) { mux := http.NewServeMux() + mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"id":3,"name":"pilot","owner":true}]`) + }) + mux.HandleFunc("DELETE /fleets/{id}/members/{email}", func(w http.ResponseWriter, r *http.Request) { removedFleet = r.PathValue("id") removedEmail = r.PathValue("email") @@ -31,15 +43,27 @@ func TestMemberRemove(t *testing.T) { loggedInTestServer(t, mux) - err := MemberRemove([]string{test.email, "3"}) + answerOnStdin(t, test.answer) + + printed, err := captureStdout(t, func() error { + return MemberRemove([]string{test.email, "3"}) + }) if err != nil { t.Fatal(err) } - if removedFleet != "3" || removedEmail != test.email { + switch { + case test.wantRemoved && (removedFleet != "3" || removedEmail != test.email): t.Errorf("the server saw %q removed from fleet %q, want %q from fleet %q", removedEmail, removedFleet, test.email, "3") + + case !test.wantRemoved && removedEmail != "": + t.Errorf("the server saw %q removed although the confirmation was declined", removedEmail) + } + + if test.wantShown != "" && !strings.Contains(printed, test.wantShown) { + t.Errorf("the output %q does not show %q", printed, test.wantShown) } }) } diff --git a/main.go b/main.go index 250e528..f34d43a 100644 --- a/main.go +++ b/main.go @@ -9,7 +9,7 @@ import ( "github.com/siliconwitchery/superstack-cli/internal/commands" ) -const version = "0.0.4" +const version = "0.0.3" type command struct { name string @@ -35,7 +35,7 @@ var sections = []section{ title: "Fleets", commands: []command{ {name: "fleet create", arguments: "", summary: "Create a fleet", run: commands.FleetCreate}, - {name: "fleet list", summary: "List the fleets you can reach", run: commands.FleetList}, + {name: "fleet list", arguments: "[--json]", summary: "List the fleets you can reach", run: commands.FleetList}, {name: "fleet rename", arguments: " ", summary: "Rename a fleet", run: commands.FleetRename}, {name: "fleet transfer", arguments: " ", summary: "Hand a fleet to a new owner", run: commands.FleetTransfer}, {name: "fleet delete", arguments: "", summary: "Delete a fleet and release its devices", run: commands.FleetDelete}, @@ -71,7 +71,7 @@ var sections = []section{ title: "People", commands: []command{ {name: "member add", arguments: " ", summary: "Give someone access to a fleet", run: commands.MemberAdd}, - {name: "member list", arguments: "", summary: "List the people who can reach a fleet", run: commands.MemberList}, + {name: "member list", arguments: " [--json]", summary: "List the people who can reach a fleet", run: commands.MemberList}, {name: "member remove", arguments: " ", summary: "Take away someone's access", run: commands.MemberRemove}, }, }, @@ -79,16 +79,16 @@ var sections = []section{ title: "Keys", commands: []command{ {name: "key create", arguments: "