diff --git a/internal/account/account.go b/internal/account/account.go index 7b49f9e..d9419d0 100644 --- a/internal/account/account.go +++ b/internal/account/account.go @@ -81,18 +81,28 @@ func Balance(session api.Session, arguments []string) error { idWidth := len("ID") nameWidth := len("NAME") + nameValues := make([]string, len(balances)) + amountValues := make([]string, len(balances)) - for _, balance := range balances { + for index, balance := range balances { + name, known := fleetNames[balance.Fleet] + + if !known { + name = "-" + } + + formatted, _, _ := api.FormatBalance(balance) + + nameValues[index] = api.Printable(name) + amountValues[index] = api.Printable(formatted) idWidth = max(idWidth, len(strconv.FormatInt(balance.Fleet, 10))) - nameWidth = max(nameWidth, len(fleetNames[balance.Fleet])) + nameWidth = max(nameWidth, len(nameValues[index])) } fmt.Fprintf(session.Out, "%-*s %-*s %s\n", idWidth, "ID", nameWidth, "NAME", "BALANCE") - for _, balance := range balances { - formatted, _, _ := api.FormatBalance(balance) - - fmt.Fprintf(session.Out, "%-*d %-*s %s\n", idWidth, balance.Fleet, nameWidth, fleetNames[balance.Fleet], formatted) + for index, balance := range balances { + fmt.Fprintf(session.Out, "%-*d %-*s %s\n", idWidth, balance.Fleet, nameWidth, nameValues[index], amountValues[index]) } return nil @@ -132,13 +142,13 @@ func Topup(session api.Session, arguments []string) error { Url string `json:"url"` }{} - err = json.NewDecoder(response.Body).Decode(&opened) + err = api.Decode(response, &opened) if err != nil || opened.Url == "" { return errors.New("could not open the top-up page, try again") } - fmt.Fprintf(session.Out, "Open this link to choose an amount and pay:\n\n %s\n\nThe credit appears on the balance once the top-up completes.\nPress enter to open the browser.\n", opened.Url) + fmt.Fprintf(session.Out, "Open this link to choose an amount and pay:\n\n %s\n\nThe credit appears on the balance once the top-up completes.\nPress enter to open the browser.\n", api.Printable(opened.Url)) _, err = bufio.NewReader(session.In).ReadString('\n') @@ -170,6 +180,10 @@ func Delete(session api.Session, arguments []string) error { response.Body.Close() + if response.StatusCode == http.StatusUnauthorized { + return errors.New("you are not logged in, run login first") + } + fmt.Fprint(session.Out, "Delete your account, its logins, and your access to every fleet? This cannot be undone. [y/N] ") answer, _ := bufio.NewReader(session.In).ReadString('\n') @@ -199,6 +213,8 @@ func Delete(session api.Session, arguments []string) error { return api.ServerError(response) } + fmt.Fprintln(session.Out, "Account deleted.") + path, err := api.KeyPath() if err != nil { @@ -208,10 +224,8 @@ func Delete(session api.Session, arguments []string) error { err = os.Remove(path) if err != nil && !errors.Is(err, fs.ErrNotExist) { - return err + return fmt.Errorf("the login stored at %s is no longer valid but could not be removed, delete it yourself", path) } - fmt.Fprintln(session.Out, "Account deleted.") - return nil } diff --git a/internal/account/account_test.go b/internal/account/account_test.go index 32cc467..fdf7924 100644 --- a/internal/account/account_test.go +++ b/internal/account/account_test.go @@ -38,6 +38,14 @@ func TestAccountBalance(t *testing.T) { wantLines: []string{"pilot", "€0.00"}, wantAbsent: []string{"crew"}, }, + { + name: "a fleet name with control characters is escaped", + arguments: []string{}, + fleets: `[{"id":1,"name":"\u001b[2Kquiet","owner":true}]`, + balances: `[{"fleet":1,"balance":"15.000000","currency":"eur"}]`, + wantLines: []string{`\x1b[2Kquiet`}, + wantAbsent: []string{"\x1b"}, + }, { name: "machine readable", arguments: []string{"--json"}, @@ -394,3 +402,25 @@ func TestAccountDeleteAsksNothingWhenTheServerIsGone(t *testing.T) { t.Errorf("it asked %q before finding the server was gone", out.String()) } } + +func TestAccountDeleteStopsWhenTheStoredLoginIsNoLongerValid(t *testing.T) { + mux := http.NewServeMux() + + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + + session, out := apitest.LoggedInSession(t, mux) + + session.In = strings.NewReader("y\n") + + err := Delete(session, nil) + + if err == nil || !strings.Contains(err.Error(), "not logged in") { + t.Fatalf("error = %v, want it to say the login is not valid", err) + } + + if out.String() != "" { + t.Errorf("output = %q, want the question never to be put", out.String()) + } +} diff --git a/internal/api/api_test.go b/internal/api/api_test.go index b3bab3a..70bbe1d 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -3,12 +3,14 @@ package api_test import ( "bytes" "fmt" + "io" "net/http" "os" "path/filepath" "runtime" "strings" "testing" + "time" "github.com/siliconwitchery/superstack-cli/internal/api" "github.com/siliconwitchery/superstack-cli/internal/api/apitest" @@ -171,7 +173,8 @@ func TestFetchKeysFailures(t *testing.T) { wantError string }{ {name: "server refusal", status: http.StatusServiceUnavailable, body: "keys unavailable", wantError: "keys unavailable"}, - {name: "undecodable body", status: http.StatusOK, body: `{`, wantError: "unexpected EOF"}, + {name: "undecodable body", status: http.StatusOK, body: `{`, wantError: "could not be read"}, + {name: "a refusal carrying control characters", status: http.StatusServiceUnavailable, body: "\x1b[2Kgone", wantError: `\x1b[2Kgone`}, } for _, test := range tests { @@ -248,3 +251,87 @@ func TestFormatBalance(t *testing.T) { }) } } + +func TestDecode(t *testing.T) { + tests := []struct { + name string + body string + wantId int64 + wantError string + }{ + {name: "a whole object", body: `{"id":7}`, wantId: 7}, + {name: "a truncated object", body: `{"id":`, wantError: "could not be read"}, + {name: "a page from something that is not the server", body: "bad gateway", wantError: "could not be read"}, + {name: "nothing at all", body: "", wantError: "could not be read"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + response := &http.Response{Body: io.NopCloser(strings.NewReader(test.body))} + + value := struct { + Id int64 `json:"id"` + }{} + + err := api.Decode(response, &value) + + if test.wantError != "" { + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want it to mention %q", err, test.wantError) + } + + for _, machinery := range []string{"unexpected EOF", "invalid character", "json", "EOF"} { + if strings.Contains(err.Error(), machinery) { + t.Errorf("the error %q shows the user %q", err, machinery) + } + } + + return + } + + if err != nil { + t.Fatal(err) + } + + if value.Id != test.wantId { + t.Errorf("id = %d, want %d", value.Id, test.wantId) + } + }) + } +} + +type endlessBody struct{} + +func (endlessBody) Read(destination []byte) (int, error) { + for index := range destination { + destination[index] = 'x' + } + + return len(destination), nil +} + +func TestDecodeStopsReadingABodyThatNeverEnds(t *testing.T) { + response := &http.Response{ + Body: io.NopCloser(io.MultiReader(strings.NewReader(`{"name":"`), endlessBody{})), + } + + value := struct { + Name string `json:"name"` + }{} + + finished := make(chan error, 1) + + go func() { + finished <- api.Decode(response, &value) + }() + + select { + case err := <-finished: + if err == nil { + t.Fatal("Decode accepted a body that never ends") + } + + case <-time.After(30 * time.Second): + t.Fatal("Decode is still reading a body that never ends") + } +} diff --git a/internal/api/balances.go b/internal/api/balances.go index 5f6bcb4..d82258a 100644 --- a/internal/api/balances.go +++ b/internal/api/balances.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "errors" "fmt" "net/http" @@ -35,7 +34,7 @@ func FetchBalances(session Session) ([]BalanceEntry, error) { balances := []BalanceEntry{} - err = json.NewDecoder(response.Body).Decode(&balances) + err = Decode(response, &balances) if err != nil { return nil, err diff --git a/internal/api/client.go b/internal/api/client.go index aec8162..1fbe2b8 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -1,6 +1,7 @@ package api import ( + "encoding/json" "errors" "io" "io/fs" @@ -11,6 +12,10 @@ import ( "strings" ) +// Larger than any answer the server can produce: it caps fleets at 100 per +// user and keys at 100 per fleet, and only the device list is uncapped. +const maximumBody = 32 << 20 + func Request(session Session, method string, path string, body io.Reader) (*http.Request, error) { request, err := http.NewRequest(method, strings.TrimSuffix(session.Base, "/")+path, body) @@ -37,7 +42,7 @@ func AuthenticatedRequest(session Session, method string, path string, body io.R } if err != nil { - return nil, err + return nil, errors.New("the login stored on this computer could not be read") } key := strings.TrimSpace(string(keyBytes)) @@ -66,7 +71,17 @@ func ServerError(response *http.Response) error { return errors.New("that did not go through, try again in a moment") } - return errors.New(detail) + return errors.New(Printable(detail)) +} + +func Decode(response *http.Response, value any) error { + err := json.NewDecoder(io.LimitReader(response.Body, maximumBody)).Decode(value) + + if err != nil { + return errors.New("the server's answer could not be read, try again in a moment") + } + + return nil } func KeyPath() (string, error) { @@ -78,7 +93,7 @@ func KeyPath() (string, error) { home, err := os.UserHomeDir() if err != nil { - return "", err + return "", errors.New("your home folder could not be found, so the login has nowhere to live") } stateHome = filepath.Join(home, ".local", "state") @@ -90,7 +105,7 @@ func KeyPath() (string, error) { configDirectory, err := os.UserConfigDir() if err != nil { - return "", err + return "", errors.New("your settings folder could not be found, so the login has nowhere to live") } return filepath.Join(configDirectory, "superstack", "key"), nil diff --git a/internal/api/devices.go b/internal/api/devices.go index 99d4081..5df2cd5 100644 --- a/internal/api/devices.go +++ b/internal/api/devices.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "errors" "net/http" ) @@ -37,7 +36,7 @@ func FetchDevices(session Session) ([]DeviceEntry, error) { devices := []DeviceEntry{} - err = json.NewDecoder(response.Body).Decode(&devices) + err = Decode(response, &devices) if err != nil { return nil, err diff --git a/internal/api/fleets.go b/internal/api/fleets.go index 06e05b6..5c8593e 100644 --- a/internal/api/fleets.go +++ b/internal/api/fleets.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "errors" "net/http" ) @@ -33,7 +32,7 @@ func FetchFleets(session Session) ([]FleetEntry, error) { fleets := []FleetEntry{} - err = json.NewDecoder(response.Body).Decode(&fleets) + err = Decode(response, &fleets) if err != nil { return nil, err diff --git a/internal/api/keys.go b/internal/api/keys.go index a84c9c9..2e78885 100644 --- a/internal/api/keys.go +++ b/internal/api/keys.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "errors" "net/http" ) @@ -34,7 +33,7 @@ func FetchKeys(session Session) ([]KeyEntry, error) { keys := []KeyEntry{} - err = json.NewDecoder(response.Body).Decode(&keys) + err = Decode(response, &keys) if err != nil { return nil, err diff --git a/internal/api/session.go b/internal/api/session.go index c58fce0..9f18277 100644 --- a/internal/api/session.go +++ b/internal/api/session.go @@ -3,6 +3,7 @@ package api import ( "io" "net/http" + "net/url" "os/exec" "runtime" "time" @@ -20,7 +21,7 @@ type Session struct { Client *http.Client In io.Reader Out io.Writer - OpenBrowser func(url string) + OpenBrowser func(link string) } func NewSession(base string, version string, in io.Reader, out io.Writer) Session { @@ -36,18 +37,28 @@ func NewSession(base string, version string, in io.Reader, out io.Writer) Sessio } } -func openBrowser(url string) { +func openBrowser(link string) { + address, err := url.Parse(link) + + if err != nil { + return + } + + if address.Scheme != "http" && address.Scheme != "https" { + return + } + var command *exec.Cmd switch runtime.GOOS { case "darwin": - command = exec.Command("open", url) + command = exec.Command("open", link) case "windows": - command = exec.Command("rundll32", "url.dll,FileProtocolHandler", url) + command = exec.Command("rundll32", "url.dll,FileProtocolHandler", link) default: - command = exec.Command("xdg-open", url) + command = exec.Command("xdg-open", link) } _ = command.Start() diff --git a/internal/api/text.go b/internal/api/text.go new file mode 100644 index 0000000..8468895 --- /dev/null +++ b/internal/api/text.go @@ -0,0 +1,11 @@ +package api + +import "strconv" + +// Escapes with Go's own rules, which cover the control characters a terminal +// acts on and the bidirectional overrides that reorder what is around them. +func Printable(text string) string { + quoted := strconv.QuoteToGraphic(text) + + return quoted[1 : len(quoted)-1] +} diff --git a/internal/api/text_test.go b/internal/api/text_test.go new file mode 100644 index 0000000..c6c792a --- /dev/null +++ b/internal/api/text_test.go @@ -0,0 +1,52 @@ +package api_test + +import ( + "strings" + "testing" + "unicode" + + "github.com/siliconwitchery/superstack-cli/internal/api" +) + +func TestPrintable(t *testing.T) { + tests := []struct { + name string + text string + want string + }{ + {name: "plain text is untouched", text: "weather-north", want: "weather-north"}, + {name: "an empty string stays empty", text: "", want: ""}, + {name: "spaces are kept", text: "north south", want: "north south"}, + {name: "accented letters are kept", text: "Ekbläd-Fjärrsond", want: "Ekbläd-Fjärrsond"}, + {name: "other scripts are kept", text: "気象観測機", want: "気象観測機"}, + {name: "emoji are kept", text: "roof \U0001F6F0", want: "roof \U0001F6F0"}, + {name: "an erase-line sequence is escaped", text: "\x1b[2K\rhidden", want: `\x1b[2K\rhidden`}, + {name: "a newline is escaped", text: "north\nsouth", want: `north\nsouth`}, + {name: "a tab is escaped", text: "north\tsouth", want: `north\tsouth`}, + {name: "a bell is escaped", text: "north\asouth", want: `north\asouth`}, + {name: "a right-to-left override is escaped", text: "a\u202eb", want: "a\\u202eb"}, + {name: "a zero-width joiner is escaped", text: "a\u200db", want: "a\\u200db"}, + {name: "a quote is escaped", text: `my"device`, want: `my\"device`}, + {name: "a backslash is escaped", text: `my\device`, want: `my\\device`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := api.Printable(test.text) + + if got != test.want { + t.Errorf("Printable(%q) = %q, want %q", test.text, got, test.want) + } + + for _, letter := range got { + if unicode.IsControl(letter) { + t.Errorf("Printable(%q) left the control character %U in %q", test.text, letter, got) + } + } + + if strings.ContainsAny(got, "\x1b\r\n") { + t.Errorf("Printable(%q) = %q, which a terminal would still act on", test.text, got) + } + }) + } +} diff --git a/internal/device/device.go b/internal/device/device.go index 16dfc36..5c8cee8 100644 --- a/internal/device/device.go +++ b/internal/device/device.go @@ -180,21 +180,21 @@ func List(session api.Session, arguments []string) error { if device.LastSeenAt != nil { seenAt, err := time.Parse(time.RFC3339, *device.LastSeenAt) - if err != nil { - return err - } + lastSeen = "unknown" + + if err == nil { + age := time.Since(seenAt) - age := time.Since(seenAt) - - switch { - case age < 2*time.Minute: - lastSeen = "just now" - case age < time.Hour: - lastSeen = fmt.Sprintf("%d min ago", int(age.Minutes())) - case age < 24*time.Hour: - lastSeen = fmt.Sprintf("%d h ago", int(age.Hours())) - default: - lastSeen = fmt.Sprintf("%d d ago", int(age.Hours()/24)) + switch { + case age < 2*time.Minute: + lastSeen = "just now" + case age < time.Hour: + lastSeen = fmt.Sprintf("%d min ago", int(age.Minutes())) + case age < 24*time.Hour: + lastSeen = fmt.Sprintf("%d h ago", int(age.Hours())) + default: + lastSeen = fmt.Sprintf("%d d ago", int(age.Hours()/24)) + } } } @@ -228,9 +228,15 @@ func List(session api.Session, arguments []string) error { storage = fmt.Sprintf("%s of %s", formatBytes(*device.StorageUsed), formatBytes(*device.StorageTotal)) } - imeiValues[index] = device.Imei - nameValues[index] = name - fleetValues[index] = fleetNames[device.FleetId] + fleetName, known := fleetNames[device.FleetId] + + if !known { + fleetName = "-" + } + + imeiValues[index] = api.Printable(device.Imei) + nameValues[index] = api.Printable(name) + fleetValues[index] = api.Printable(fleetName) stateValues[index] = state storageValues[index] = storage lastSeenValues[index] = lastSeen diff --git a/internal/device/device_test.go b/internal/device/device_test.go index 3aa025c..d27bbc6 100644 --- a/internal/device/device_test.go +++ b/internal/device/device_test.go @@ -188,7 +188,9 @@ func TestDeviceList(t *testing.T) { {name: "unknown fleet", arguments: []string{"9"}, wantError: "no such fleet"}, {name: "two ids", arguments: []string{"3", "4"}, wantError: "takes at most one fleet id"}, {name: "wordy id", arguments: []string{"pilot"}, wantError: "shown by fleet list"}, - {name: "bad last seen time", devices: `[{"imei":"111111111111111","name":"roof","fleet_id":3,"last_seen_at":"yesterday"}]`, wantError: `cannot parse "yesterday"`}, + {name: "an unreadable last seen time leaves the rest of the table", devices: `[{"imei":"111111111111111","name":"roof","fleet_id":3,"last_seen_at":"yesterday"}]`, wantShown: []string{"111111111111111 roof pilot unknown - unknown"}}, + {name: "a fleet the list does not name", devices: `[{"imei":"888888888888888","name":"orphan","fleet_id":99}]`, wantShown: []string{"888888888888888 orphan - unknown - never"}}, + {name: "a name with control characters is escaped", devices: `[{"imei":"111111111111111","name":"\u001b[2K\rhidden","fleet_id":3}]`, wantShown: []string{`\x1b[2K\rhidden`}, wantHidden: []string{"\x1b"}}, {name: "minutes ago", devices: fmt.Sprintf(`[{"imei":"111111111111111","name":"roof","fleet_id":3,"last_seen_at":%q}]`, now.Add(-12*time.Minute).Format(time.RFC3339)), wantShown: []string{"12 min ago"}}, {name: "days ago", devices: fmt.Sprintf(`[{"imei":"111111111111111","name":"roof","fleet_id":3,"last_seen_at":%q}]`, now.Add(-49*time.Hour).Format(time.RFC3339)), wantShown: []string{"2 d ago"}}, {name: "stopped and undefined states", devices: `[{"imei":"444444444444444","name":"halted","fleet_id":3,"reported_state":3},{"imei":"555555555555555","name":"odd","fleet_id":3,"reported_state":1}]`, wantShown: []string{"stopped", "unknown"}}, diff --git a/internal/fleet/fleet.go b/internal/fleet/fleet.go index ea01162..44bf6b1 100644 --- a/internal/fleet/fleet.go +++ b/internal/fleet/fleet.go @@ -49,7 +49,7 @@ func Create(session api.Session, arguments []string) error { Name string `json:"name"` }{} - err = json.NewDecoder(response.Body).Decode(&created) + err = api.Decode(response, &created) if err != nil { return err @@ -84,22 +84,24 @@ func List(session api.Session, arguments []string) error { idWidth := len("ID") nameWidth := len("NAME") + nameValues := make([]string, len(fleets)) - for _, fleet := range fleets { + for index, fleet := range fleets { + nameValues[index] = api.Printable(fleet.Name) idWidth = max(idWidth, len(strconv.FormatInt(fleet.Id, 10))) - nameWidth = max(nameWidth, len(fleet.Name)) + nameWidth = max(nameWidth, len(nameValues[index])) } fmt.Fprintf(session.Out, "%-*s %-*s %s\n", idWidth, "ID", nameWidth, "NAME", "ROLE") - for _, fleet := range fleets { + for index, fleet := range fleets { role := "member" if fleet.Owner { role = "owner" } - fmt.Fprintf(session.Out, "%-*d %-*s %s\n", idWidth, fleet.Id, nameWidth, fleet.Name, role) + fmt.Fprintf(session.Out, "%-*d %-*s %s\n", idWidth, fleet.Id, nameWidth, nameValues[index], role) } return nil @@ -283,7 +285,7 @@ func Delete(session api.Session, arguments []string) error { } if value > 0 { - forfeited = formatted + forfeited = api.Printable(formatted) } } diff --git a/internal/fleet/fleet_test.go b/internal/fleet/fleet_test.go index e6b8327..031a42b 100644 --- a/internal/fleet/fleet_test.go +++ b/internal/fleet/fleet_test.go @@ -100,6 +100,12 @@ func TestFleetList(t *testing.T) { 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: "a name with control characters is escaped", + fleets: `[{"id":1,"name":"\u001b[2K\rhidden","owner":true}]`, + wantShown: []string{`\x1b[2K\rhidden`}, + wantAbsent: []string{"\x1b"}, + }, { name: "no fleets", fleets: `[]`, diff --git a/internal/key/key.go b/internal/key/key.go index 8f44fc2..830fb61 100644 --- a/internal/key/key.go +++ b/internal/key/key.go @@ -56,13 +56,17 @@ func Create(session api.Session, arguments []string) error { Key string `json:"key"` }{} - err = json.NewDecoder(response.Body).Decode(&created) + err = api.Decode(response, &created) if err != nil { return err } - fmt.Fprintf(session.Out, "Created fleet key %d.\n\n %s\n\nAnyone holding it can send data to the fleet, and you will not see it again.\n", created.Id, created.Key) + if created.Key == "" { + return errors.New("the fleet key was not created, try again") + } + + fmt.Fprintf(session.Out, "Created fleet key %d.\n\n %s\n\nAnyone holding it can send data to the fleet, and you will not see it again.\n", created.Id, api.Printable(created.Key)) return nil } @@ -135,19 +139,31 @@ func List(session api.Session, arguments []string) error { idWidth := len("ID") fleetIdWidth := len("FLEET") fleetNameWidth := len("FLEET NAME") + fleetNameValues := make([]string, len(keys)) + suffixValues := make([]string, len(keys)) + labelValues := make([]string, len(keys)) - for _, key := range keys { + for index, key := range keys { + fleetName, known := fleetNames[key.Fleet] + + if !known { + fleetName = "-" + } + + fleetNameValues[index] = api.Printable(fleetName) + suffixValues[index] = api.Printable(key.Suffix) + labelValues[index] = api.Printable(key.Label) idWidth = max(idWidth, len(strconv.FormatInt(key.Id, 10))) fleetIdWidth = max(fleetIdWidth, len(strconv.FormatInt(key.Fleet, 10))) - fleetNameWidth = max(fleetNameWidth, len(fleetNames[key.Fleet])) + fleetNameWidth = max(fleetNameWidth, len(fleetNameValues[index])) } fmt.Fprintf(session.Out, "%-*s %-*s %-*s %-8s %s\n", idWidth, "ID", fleetIdWidth, "FLEET", fleetNameWidth, "FLEET NAME", "KEY", "LABEL") - for _, key := range keys { + for index, key := range keys { fmt.Fprintf(session.Out, "%-*d %-*d %-*s ...%s %s\n", - idWidth, key.Id, fleetIdWidth, key.Fleet, fleetNameWidth, fleetNames[key.Fleet], key.Suffix, key.Label) + idWidth, key.Id, fleetIdWidth, key.Fleet, fleetNameWidth, fleetNameValues[index], suffixValues[index], labelValues[index]) } return nil diff --git a/internal/key/key_test.go b/internal/key/key_test.go index 70d082a..69dac1c 100644 --- a/internal/key/key_test.go +++ b/internal/key/key_test.go @@ -16,9 +16,18 @@ func TestKeyCreate(t *testing.T) { arguments []string wantPath string wantLabel string + answer string refusal string wantError string }{ + { + name: "the server answers without a key", + arguments: []string{"3", "deploy server"}, + wantPath: "/fleets/3/keys", + wantLabel: "deploy server", + answer: `{"id":1}`, + wantError: "was not created", + }, { name: "a labelled key", arguments: []string{"3", "deploy server"}, @@ -87,7 +96,13 @@ func TestKeyCreate(t *testing.T) { t.Errorf("the request carried label %q, want %q", sent.Label, test.wantLabel) } - fmt.Fprint(w, `{"id":1,"key":"ssf_testtesttestab2de"}`) + answer := test.answer + + if answer == "" { + answer = `{"id":1,"key":"ssf_testtesttestab2de"}` + } + + fmt.Fprint(w, answer) }) session, out := apitest.LoggedInSession(t, mux) @@ -150,6 +165,13 @@ func TestKeyList(t *testing.T) { wantShown: []string{"ID FLEET FLEET NAME", "crew", "...ab2de"}, wantHidden: []string{"skunkworks", "f9hjk", "lab sensor"}, }, + { + name: "a label with control characters is escaped", + arguments: []string{}, + keys: `[{"id":1,"fleet":3,"label":"\u001b[2Kquiet","suffix":"ab2de"}]`, + wantShown: []string{`\x1b[2Kquiet`}, + wantHidden: []string{"\x1b"}, + }, { name: "no keys", arguments: []string{}, diff --git a/internal/login/login.go b/internal/login/login.go index 8752288..0741002 100644 --- a/internal/login/login.go +++ b/internal/login/login.go @@ -49,7 +49,7 @@ func Login(session api.Session, arguments []string) error { GitlabClientId string `json:"gitlab_client_id"` }{} - err = json.NewDecoder(providersResponse.Body).Decode(&providers) + err = api.Decode(providersResponse, &providers) if err != nil { return err @@ -108,7 +108,7 @@ func Login(session api.Session, arguments []string) error { Error string `json:"error"` }{} - err = json.NewDecoder(codeResponse.Body).Decode(&code) + err = api.Decode(codeResponse, &code) if err != nil { return fmt.Errorf("%s would not start the login, try again", provider) @@ -124,8 +124,8 @@ func Login(session api.Session, arguments []string) error { enterAt = code.VerificationUriComplete } - fmt.Fprintf(session.Out, "Copy your one-time code: %s\n", code.UserCode) - fmt.Fprintf(session.Out, "Then enter it at %s\n", enterAt) + fmt.Fprintf(session.Out, "Copy your one-time code: %s\n", api.Printable(code.UserCode)) + fmt.Fprintf(session.Out, "Then enter it at %s\n", api.Printable(enterAt)) fmt.Fprintln(session.Out, "Press enter to open the browser.") go func() { @@ -181,7 +181,7 @@ func Login(session api.Session, arguments []string) error { Error string `json:"error"` }{} - err = json.NewDecoder(pollResponse.Body).Decode(&poll) + err = api.Decode(pollResponse, &poll) pollResponse.Body.Close() @@ -247,7 +247,7 @@ func Login(session api.Session, arguments []string) error { Email string `json:"email"` }{} - err = json.NewDecoder(loginResponse.Body).Decode(&login) + err = api.Decode(loginResponse, &login) if err != nil { return err @@ -263,19 +263,42 @@ func Login(session api.Session, arguments []string) error { return err } - err = os.MkdirAll(filepath.Dir(path), 0o700) + directory := filepath.Dir(path) + + err = os.MkdirAll(directory, 0o700) if err != nil { - return err + return fmt.Errorf("the login could not be saved to %s, so you are not logged in", path) } - err = os.WriteFile(path, []byte(login.Key+"\n"), 0o600) + temporary, err := os.CreateTemp(directory, "key") if err != nil { - return err + return fmt.Errorf("the login could not be saved to %s, so you are not logged in", path) + } + + defer os.Remove(temporary.Name()) + + _, err = temporary.WriteString(login.Key + "\n") + + if err != nil { + temporary.Close() + return fmt.Errorf("the login could not be saved to %s, so you are not logged in", path) } - fmt.Fprintf(session.Out, "Logged in as %s.\n", login.Email) + err = temporary.Close() + + if err != nil { + return fmt.Errorf("the login could not be saved to %s, so you are not logged in", path) + } + + err = os.Rename(temporary.Name(), path) + + if err != nil { + return fmt.Errorf("the login could not be saved to %s, so you are not logged in", path) + } + + fmt.Fprintf(session.Out, "Logged in as %s.\n", api.Printable(login.Email)) return nil } @@ -299,7 +322,14 @@ func Logout(session api.Session, arguments []string) error { } if err != nil { - return err + return errors.New("the login stored on this computer could not be read") + } + + key := strings.TrimSpace(string(keyBytes)) + + if key == "" { + fmt.Fprintln(session.Out, "Not logged in.") + return nil } revokeRequest, err := api.Request(session, http.MethodPost, "/logout", nil) @@ -308,7 +338,7 @@ func Logout(session api.Session, arguments []string) error { return err } - revokeRequest.Header.Set("Authorization", "Bearer "+strings.TrimSpace(string(keyBytes))) + revokeRequest.Header.Set("Authorization", "Bearer "+key) revokeResponse, err := session.Client.Do(revokeRequest) @@ -322,13 +352,13 @@ func Logout(session api.Session, arguments []string) error { return fmt.Errorf("you are still logged in: %s", api.ServerError(revokeResponse)) } + fmt.Fprintln(session.Out, "Logged out.") + err = os.Remove(path) if err != nil { - return err + return fmt.Errorf("the login stored at %s is no longer valid but could not be removed, delete it yourself", path) } - fmt.Fprintln(session.Out, "Logged out.") - return nil } diff --git a/internal/login/login_test.go b/internal/login/login_test.go index 8840533..cabbe96 100644 --- a/internal/login/login_test.go +++ b/internal/login/login_test.go @@ -449,6 +449,7 @@ func TestLogout(t *testing.T) { name string arguments []string storedKey string + storeEmptyKey bool serverDown bool revokeStatus int wantError string @@ -472,6 +473,12 @@ func TestLogout(t *testing.T) { name: "nothing stored", wantShown: "Not logged in.\n", }, + { + name: "an empty stored login", + storeEmptyKey: true, + wantShown: "Not logged in.\n", + wantKeyKept: true, + }, { name: "server refuses the revocation", storedKey: "ssk_test", @@ -520,7 +527,7 @@ func TestLogout(t *testing.T) { t.Fatal(err) } - if test.storedKey != "" { + if test.storedKey != "" || test.storeEmptyKey { err = os.MkdirAll(filepath.Dir(path), 0o700) if err != nil { @@ -568,3 +575,71 @@ func TestLogout(t *testing.T) { }) } } + +func TestLoginReplacesAStoredLoginLeftTooOpen(t *testing.T) { + apitest.IsolateKeyStorage(t) + + path, err := api.KeyPath() + + if err != nil { + t.Fatal(err) + } + + err = os.MkdirAll(filepath.Dir(path), 0o700) + + if err != nil { + t.Fatal(err) + } + + err = os.WriteFile(path, []byte("ssk_stale\n"), 0o600) + + if err != nil { + t.Fatal(err) + } + + err = os.Chmod(path, 0o644) + + if err != nil { + t.Fatal(err) + } + + _, providerBase := fakeProviderForLogin(t, "gitlab", 1, "", []string{`{"access_token": "glpat-test"}`}) + session, _ := fakeSuperstack(t, "", "") + session.GitlabBase = providerBase + + err = Login(session, []string{"gitlab"}) + + if err != nil { + t.Fatal(err) + } + + information, err := os.Stat(path) + + if err != nil { + t.Fatal(err) + } + + if information.Mode().Perm() != 0o600 { + t.Errorf("the stored login is mode %v, want it narrowed to 0600", information.Mode().Perm()) + } + + stored, err := os.ReadFile(path) + + if err != nil { + t.Fatal(err) + } + + if strings.TrimSpace(string(stored)) != "ssk_test" { + t.Errorf("the stored login is %q, want the one just issued", strings.TrimSpace(string(stored))) + } + + entries, err := os.ReadDir(filepath.Dir(path)) + + if err != nil { + t.Fatal(err) + } + + if len(entries) != 1 { + t.Errorf("the login folder holds %d files, want only the stored login", len(entries)) + } +} diff --git a/internal/member/member.go b/internal/member/member.go index 4993e84..37e8800 100644 --- a/internal/member/member.go +++ b/internal/member/member.go @@ -96,7 +96,7 @@ func List(session api.Session, arguments []string) error { Members []string `json:"members"` }{} - err = json.NewDecoder(response.Body).Decode(&people) + err = api.Decode(response, &people) if err != nil { return err @@ -106,17 +106,20 @@ func List(session api.Session, arguments []string) error { return json.NewEncoder(session.Out).Encode(people) } - emailWidth := max(len("EMAIL"), len(people.Owner)) + owner := api.Printable(people.Owner) + members := make([]string, len(people.Members)) + emailWidth := max(len("EMAIL"), len(owner)) - for _, email := range people.Members { - emailWidth = max(emailWidth, len(email)) + for index, email := range people.Members { + members[index] = api.Printable(email) + emailWidth = max(emailWidth, len(members[index])) } fmt.Fprintf(session.Out, "%-*s %s\n", emailWidth, "EMAIL", "ROLE") - fmt.Fprintf(session.Out, "%-*s owner\n", emailWidth, people.Owner) + fmt.Fprintf(session.Out, "%-*s owner\n", emailWidth, owner) - for _, email := range people.Members { + for _, email := range members { fmt.Fprintf(session.Out, "%-*s member\n", emailWidth, email) } diff --git a/internal/member/member_test.go b/internal/member/member_test.go index f8863ac..faa0036 100644 --- a/internal/member/member_test.go +++ b/internal/member/member_test.go @@ -108,6 +108,13 @@ func TestMemberList(t *testing.T) { wantFleet: "3", wantShown: []string{"EMAIL", "ROLE", "owner@example.com", "owner", "member@example.com", "member"}, }, + { + name: "addresses with control characters are escaped", + arguments: []string{"3"}, + people: `{"owner":"\u001bowner@example.com","members":["\u001bmember@example.com"]}`, + wantFleet: "3", + wantShown: []string{`\x1bowner@example.com`, `\x1bmember@example.com`}, + }, { name: "nobody but the owner", arguments: []string{"7"},