diff --git a/.github/workflows/flake.yml b/.github/workflows/flake.yml new file mode 100644 index 0000000..44a56b1 --- /dev/null +++ b/.github/workflows/flake.yml @@ -0,0 +1,58 @@ +name: flake + +on: + push: + tags: ["v*"] + pull_request: + paths: + - .github/workflows/flake.yml + - flake.lock + - flake.nix + - go.mod + - main.go + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: flake-${{ github.ref }} + cancel-in-progress: true + +jobs: + build: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: cachix/install-nix-action@13d8dd58da0234aa297dedd986986ccb8e7f3e24 # v31.11.1 + with: + extra_nix_config: | + experimental-features = nix-command flakes + + - name: Build the package the flake defines + run: nix build --print-build-logs .#superstack + + - name: Verify the flake and the binary it built agree with main.go + run: | + declared=$(sed -n 's/^const version = "\(.*\)"$/\1/p' main.go) + + if [ -z "$declared" ]; then + echo "could not read 'const version' from main.go" + exit 1 + fi + + evaluated=$(nix eval --raw .#superstack.version) + + if [ "$evaluated" != "$declared" ]; then + echo "main.go declares $declared, but flake.nix reads $evaluated" + exit 1 + fi + + reported=$(./result/bin/superstack --version) + + if [ "$reported" != "$declared" ]; then + echo "main.go declares $declared, but the built binary reports $reported" + exit 1 + fi diff --git a/.gitignore b/.gitignore index 33b981f..7023785 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,9 @@ /superstack /superstack-cli +# Compiled test binaries, which go test leaves behind when a run is interrupted +*.test + # GoReleaser output /dist/ diff --git a/README.md b/README.md index 33e9c5c..a68a36b 100644 --- a/README.md +++ b/README.md @@ -42,19 +42,19 @@ Uploading Lua code and streaming logs are not available yet. ## Local development -1. Install the toolchain: - - - **Any platform:** [Go](https://go.dev) 1.25 or newer. - - **Nix:** `nix develop`, or `direnv allow` once with - [direnv](https://direnv.net) hooked into your shell. - 1. Clone the repository: ```sh - git clone git@github.com:siliconwitchery/superstack-cli.git ~/projects/superstack-cli + git clone https://github.com/siliconwitchery/superstack-cli.git ~/projects/superstack-cli cd ~/projects/superstack-cli ``` +1. Install the toolchain: + + - **Any platform:** [Go](https://go.dev) 1.25 or newer. + - **Nix:** `nix develop` from inside the clone, or `direnv allow` once with + [direnv](https://direnv.net) hooked into your shell. + 1. Build and run: ```sh @@ -88,7 +88,9 @@ Uploading Lua code and streaming logs are not available yet. git switch -C dev origin/main ``` -1. Change `version` in `main.go`, run the checks, commit, and push: +1. Change `version` in `main.go`. + +1. Run every check: ```sh gofmt -l . @@ -97,6 +99,11 @@ Uploading Lua code and streaming logs are not available yet. CGO_ENABLED=0 go vet ./... CGO_ENABLED=0 go test ./... git diff --check + ``` + +1. Commit and push: + + ```sh git add main.go git commit -m "Version " git push -u origin dev diff --git a/internal/account/account.go b/internal/account/account.go index d9419d0..2a04a92 100644 --- a/internal/account/account.go +++ b/internal/account/account.go @@ -66,7 +66,9 @@ func Balance(session api.Session, arguments []string) error { } if jsonOutput { - return json.NewEncoder(session.Out).Encode(balances) + err = json.NewEncoder(session.Out).Encode(balances) + + return err } if len(balances) == 0 { @@ -166,7 +168,7 @@ func Delete(session api.Session, arguments []string) error { return errors.New("account delete takes no arguments") } - request, err := api.AuthenticatedRequest(session, http.MethodGet, "/", nil) + request, err := api.AuthenticatedRequest(session, http.MethodGet, "/fleets", nil) if err != nil { return err @@ -178,12 +180,16 @@ func Delete(session api.Session, arguments []string) error { return errors.New("the server could not be reached, check your connection") } - response.Body.Close() + if response.StatusCode != http.StatusOK { + refusal := api.ServerError(response) - if response.StatusCode == http.StatusUnauthorized { - return errors.New("you are not logged in, run login first") + response.Body.Close() + + return refusal } + response.Body.Close() + 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') diff --git a/internal/account/account_test.go b/internal/account/account_test.go index fdf7924..9dffb63 100644 --- a/internal/account/account_test.go +++ b/internal/account/account_test.go @@ -18,6 +18,7 @@ func TestAccountBalance(t *testing.T) { arguments []string fleets string balances string + refusal string wantLines []string wantAbsent []string wantExact string @@ -81,6 +82,13 @@ func TestAccountBalance(t *testing.T) { balances: `[]`, wantExact: "No credit on that fleet yet.\n", }, + { + name: "a fleet the list does not name", + arguments: []string{}, + fleets: `[{"id":1,"name":"crew","owner":true}]`, + balances: `[{"fleet":99,"balance":"15.000000","currency":"eur"}]`, + wantLines: []string{"99 -"}, + }, { name: "an unknown fleet", arguments: []string{"9"}, @@ -88,6 +96,13 @@ func TestAccountBalance(t *testing.T) { balances: `[]`, wantError: "no such fleet", }, + { + name: "server refusal", + arguments: []string{}, + fleets: `[{"id":1,"name":"crew","owner":true}]`, + refusal: "balances unavailable", + wantError: "balances unavailable", + }, { name: "a wordy id", arguments: []string{"crew"}, @@ -109,6 +124,11 @@ func TestAccountBalance(t *testing.T) { }) mux.HandleFunc("GET /balance", func(w http.ResponseWriter, r *http.Request) { + if test.refusal != "" { + http.Error(w, test.refusal, http.StatusServiceUnavailable) + return + } + fmt.Fprint(w, test.balances) }) @@ -330,6 +350,12 @@ func TestAccountDelete(t *testing.T) { mux := http.NewServeMux() + // The command asks the server whether the stored login still works + // before it puts the question, so every case has to answer this. + mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[]`) + }) + mux.HandleFunc("DELETE /account", func(w http.ResponseWriter, r *http.Request) { deleted = true @@ -404,10 +430,17 @@ func TestAccountDeleteAsksNothingWhenTheServerIsGone(t *testing.T) { } func TestAccountDeleteStopsWhenTheStoredLoginIsNoLongerValid(t *testing.T) { + deleted := false + mux := http.NewServeMux() - mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusUnauthorized) + // What the server's own auth middleware answers for a revoked login. + mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "the login is no longer valid, log in again", http.StatusUnauthorized) + }) + + mux.HandleFunc("DELETE /account", func(w http.ResponseWriter, r *http.Request) { + deleted = true }) session, out := apitest.LoggedInSession(t, mux) @@ -416,11 +449,15 @@ func TestAccountDeleteStopsWhenTheStoredLoginIsNoLongerValid(t *testing.T) { 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 err == nil || !strings.Contains(err.Error(), "no longer valid") { + t.Fatalf("error = %v, want the server's own refusal", err) } if out.String() != "" { t.Errorf("output = %q, want the question never to be put", out.String()) } + + if deleted { + t.Error("the server saw the account deleted although the login was refused") + } } diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 70bbe1d..10be098 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -165,6 +165,36 @@ func TestFetchFleetsFailures(t *testing.T) { } } +func TestFetchDevicesFailures(t *testing.T) { + tests := []struct { + name string + status int + body string + wantError string + }{ + {name: "server refusal", status: http.StatusServiceUnavailable, body: "devices unavailable", wantError: "devices unavailable"}, + {name: "undecodable body", status: http.StatusOK, body: `[{`, wantError: "could not be read"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /devices", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(test.status) + fmt.Fprint(w, test.body) + }) + + session, _ := apitest.LoggedInSession(t, mux) + + _, err := api.FetchDevices(session) + + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want it to mention %q", err, test.wantError) + } + }) + } +} + func TestFetchKeysFailures(t *testing.T) { tests := []struct { name string diff --git a/internal/api/client.go b/internal/api/client.go index 1fbe2b8..8485720 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -12,12 +12,8 @@ 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) +func Request(session Session, method string, path string, reader io.Reader) (*http.Request, error) { + request, err := http.NewRequest(method, strings.TrimSuffix(session.Base, "/")+path, reader) if err != nil { return nil, err @@ -28,7 +24,7 @@ func Request(session Session, method string, path string, body io.Reader) (*http return request, nil } -func AuthenticatedRequest(session Session, method string, path string, body io.Reader) (*http.Request, error) { +func AuthenticatedRequest(session Session, method string, path string, reader io.Reader) (*http.Request, error) { storedKeyPath, err := KeyPath() if err != nil { @@ -51,7 +47,7 @@ func AuthenticatedRequest(session Session, method string, path string, body io.R return nil, errors.New("you are not logged in, run login first") } - request, err := Request(session, method, path, body) + request, err := Request(session, method, path, reader) if err != nil { return nil, err @@ -75,7 +71,8 @@ func ServerError(response *http.Response) error { } func Decode(response *http.Response, value any) error { - err := json.NewDecoder(io.LimitReader(response.Body, maximumBody)).Decode(value) + // 32 MiB is past every capped list and is the ceiling on the uncapped ones. + err := json.NewDecoder(io.LimitReader(response.Body, 32<<20)).Decode(value) if err != nil { return errors.New("the server's answer could not be read, try again in a moment") @@ -93,7 +90,7 @@ func KeyPath() (string, error) { home, err := os.UserHomeDir() if err != nil { - return "", errors.New("your home folder could not be found, so the login has nowhere to live") + return "", errors.New("your home folder could not be found, so the login cannot be read or saved") } stateHome = filepath.Join(home, ".local", "state") @@ -105,7 +102,7 @@ func KeyPath() (string, error) { configDirectory, err := os.UserConfigDir() if err != nil { - return "", errors.New("your settings folder could not be found, so the login has nowhere to live") + return "", errors.New("your settings folder could not be found, so the login cannot be read or saved") } return filepath.Join(configDirectory, "superstack", "key"), nil diff --git a/internal/api/session.go b/internal/api/session.go index 9f18277..5852251 100644 --- a/internal/api/session.go +++ b/internal/api/session.go @@ -10,8 +10,6 @@ import ( ) const DefaultBase = "https://supernext.siliconwitchery.com" -const defaultGithubBase = "https://github.com" -const defaultGitlabBase = "https://gitlab.com" type Session struct { Base string @@ -27,8 +25,8 @@ type Session struct { func NewSession(base string, version string, in io.Reader, out io.Writer) Session { return Session{ Base: base, - GithubBase: defaultGithubBase, - GitlabBase: defaultGitlabBase, + GithubBase: "https://github.com", + GitlabBase: "https://gitlab.com", Version: version, Client: &http.Client{Timeout: 30 * time.Second}, In: in, diff --git a/internal/device/device.go b/internal/device/device.go index 5c8cee8..fbba942 100644 --- a/internal/device/device.go +++ b/internal/device/device.go @@ -15,8 +15,6 @@ import ( ) func Claim(session api.Session, arguments []string) error { - claimClient := &http.Client{Timeout: 90 * time.Second} - if len(arguments) != 2 && len(arguments) != 3 { return errors.New("device claim takes an IMEI, a fleet id, and an optional name") } @@ -74,6 +72,8 @@ func Claim(session api.Session, arguments []string) error { request.Header.Set("Content-Type", "application/json") + claimClient := &http.Client{Timeout: 90 * time.Second} + response, err := claimClient.Do(request) if err != nil { @@ -143,7 +143,9 @@ func List(session api.Session, arguments []string) error { } if jsonOutput { - return json.NewEncoder(session.Out).Encode(filtered) + err = json.NewEncoder(session.Out).Encode(filtered) + + return err } if len(filtered) == 0 { diff --git a/internal/dispatch/dispatch.go b/internal/dispatch/dispatch.go index 926c2f1..21d5eac 100644 --- a/internal/dispatch/dispatch.go +++ b/internal/dispatch/dispatch.go @@ -9,39 +9,6 @@ import ( "github.com/siliconwitchery/superstack-cli/internal/api" ) -func takeServerFlag(arguments []string) ([]string, string, error) { - remaining := []string{} - - base := api.DefaultBase - - for index := 0; index < len(arguments); index++ { - switch { - case arguments[index] == "--server": - if index+1 == len(arguments) { - return nil, "", errors.New("--server needs an address") - } - - index++ - - base = arguments[index] - - case strings.HasPrefix(arguments[index], "--server="): - base = strings.TrimPrefix(arguments[index], "--server=") - - default: - remaining = append(remaining, arguments[index]) - } - } - - base = strings.TrimRight(base, "/") - - if base == "" { - return nil, "", errors.New("--server needs an address") - } - - return remaining, base, nil -} - type Command struct { Name string Arguments string @@ -127,12 +94,36 @@ func printHelp(session api.Session, sections []Section) { } func Dispatch(sections []Section, version string, arguments []string, in io.Reader, out io.Writer) error { - arguments, base, err := takeServerFlag(arguments) + remaining := []string{} + base := api.DefaultBase - if err != nil { - return err + for index := 0; index < len(arguments); index++ { + switch { + case arguments[index] == "--server": + if index+1 == len(arguments) { + return errors.New("--server needs an address") + } + + index++ + + base = arguments[index] + + case strings.HasPrefix(arguments[index], "--server="): + base = strings.TrimPrefix(arguments[index], "--server=") + + default: + remaining = append(remaining, arguments[index]) + } + } + + base = strings.TrimRight(strings.TrimSpace(base), "/") + + if base == "" { + return errors.New("--server needs an address") } + arguments = remaining + session := api.NewSession(base, version, in, out) if len(arguments) == 0 { @@ -187,7 +178,7 @@ func Dispatch(sections []Section, version string, arguments []string, in io.Read return fmt.Errorf("%s is not available yet", entry.Name) } - err = entry.Run(session, rest) + err := entry.Run(session, rest) return err } diff --git a/internal/dispatch/dispatch_test.go b/internal/dispatch/dispatch_test.go index 628661a..2f2c620 100644 --- a/internal/dispatch/dispatch_test.go +++ b/internal/dispatch/dispatch_test.go @@ -10,42 +10,57 @@ import ( "github.com/siliconwitchery/superstack-cli/internal/api" ) -func TestTakeServerFlag(t *testing.T) { +func TestDispatchTakesTheServerFlag(t *testing.T) { tests := []struct { - name string - arguments []string - wantRemaining string - wantBase string - wantError string + name string + arguments []string + wantCommand string + wantRest string + wantBase string + wantError string }{ { - name: "no flag", - arguments: []string{"login"}, - wantRemaining: "login", + name: "no flag", + arguments: []string{"login"}, + wantCommand: "login", + wantBase: api.DefaultBase, }, { - name: "a url", - arguments: []string{"--server", "http://localhost:8080", "login"}, - wantRemaining: "login", - wantBase: "http://localhost:8080", + name: "a url", + arguments: []string{"--server", "http://localhost:8080", "login"}, + wantCommand: "login", + wantBase: "http://localhost:8080", }, { - name: "a url in equals form", - arguments: []string{"logout", "--server=https://staging.example.com"}, - wantRemaining: "logout", - wantBase: "https://staging.example.com", + name: "a url in equals form", + arguments: []string{"login", "--server=https://staging.example.com"}, + wantCommand: "login", + wantBase: "https://staging.example.com", }, { - name: "a trailing slash is trimmed", - arguments: []string{"--server", "http://localhost:8080/", "login"}, - wantRemaining: "login", - wantBase: "http://localhost:8080", + name: "a trailing slash is trimmed", + arguments: []string{"--server", "http://localhost:8080/", "login"}, + wantCommand: "login", + wantBase: "http://localhost:8080", }, { - name: "the flag between command words", - arguments: []string{"fleet", "--server=http://localhost:9999", "list"}, - wantRemaining: "fleet list", - wantBase: "http://localhost:9999", + name: "surrounding spaces are trimmed", + arguments: []string{"--server", " http://localhost:8080/ ", "login"}, + wantCommand: "login", + wantBase: "http://localhost:8080", + }, + { + name: "the flag between command words", + arguments: []string{"fleet", "--server=http://localhost:9999", "list"}, + wantCommand: "fleet list", + wantBase: "http://localhost:9999", + }, + { + name: "the flag never reaches the command", + arguments: []string{"fleet", "list", "--server=http://localhost:9999", "3"}, + wantCommand: "fleet list", + wantRest: "3", + wantBase: "http://localhost:9999", }, { name: "a missing value", @@ -58,8 +73,8 @@ func TestTakeServerFlag(t *testing.T) { wantError: "needs an address", }, { - name: "an empty value from an unset shell variable", - arguments: []string{"--server", "", "login"}, + name: "a value that is only spaces", + arguments: []string{"login", "--server", " "}, wantError: "needs an address", }, { @@ -71,13 +86,38 @@ func TestTakeServerFlag(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - remaining, base, err := takeServerFlag(test.arguments) + ranCommand := "" + seenRest := []string{} + seenBase := "" + + record := func(name string) func(api.Session, []string) error { + return func(session api.Session, arguments []string) error { + ranCommand = name + seenRest = arguments + seenBase = session.Base + + return nil + } + } + + sections := []Section{ + {Title: "Things", Commands: []Command{ + {Name: "login", Summary: "Log in", Run: record("login")}, + {Name: "fleet list", Summary: "List fleets", Run: record("fleet list")}, + }}, + } + + err := Dispatch(sections, "1.2.3", test.arguments, strings.NewReader(""), &bytes.Buffer{}) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { t.Fatalf("error = %v, want it to mention %q", err, test.wantError) } + if ranCommand != "" { + t.Errorf("%q ran although the address was refused", ranCommand) + } + return } @@ -85,18 +125,16 @@ func TestTakeServerFlag(t *testing.T) { t.Fatal(err) } - if strings.Join(remaining, " ") != test.wantRemaining { - t.Errorf("remaining = %q, want %q", strings.Join(remaining, " "), test.wantRemaining) + if ranCommand != test.wantCommand { + t.Errorf("%q ran, want %q", ranCommand, test.wantCommand) } - wantBase := test.wantBase - - if wantBase == "" { - wantBase = api.DefaultBase + if strings.Join(seenRest, " ") != test.wantRest { + t.Errorf("the command was handed %q, want %q", strings.Join(seenRest, " "), test.wantRest) } - if base != wantBase { - t.Errorf("base = %q, want %q", base, wantBase) + if seenBase != test.wantBase { + t.Errorf("the session base is %q, want %q", seenBase, test.wantBase) } }) } diff --git a/internal/fleet/fleet.go b/internal/fleet/fleet.go index 44bf6b1..5fbca86 100644 --- a/internal/fleet/fleet.go +++ b/internal/fleet/fleet.go @@ -74,7 +74,9 @@ func List(session api.Session, arguments []string) error { } if jsonOutput { - return json.NewEncoder(session.Out).Encode(fleets) + err = json.NewEncoder(session.Out).Encode(fleets) + + return err } if len(fleets) == 0 { diff --git a/internal/fleet/fleet_test.go b/internal/fleet/fleet_test.go index 031a42b..fd8f3e5 100644 --- a/internal/fleet/fleet_test.go +++ b/internal/fleet/fleet_test.go @@ -90,11 +90,17 @@ func TestFleetList(t *testing.T) { name string arguments []string fleets string + refusal string wantShown []string wantAbsent []string wantExact string wantError string }{ + { + name: "server refusal", + refusal: "fleets unavailable", + wantError: "fleets unavailable", + }, { name: "some fleets", fleets: `[{"id":1,"name":"field trial","owner":true},{"id":2,"name":"rooftop","owner":false}]`, @@ -130,6 +136,11 @@ func TestFleetList(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { + if test.refusal != "" { + http.Error(w, test.refusal, http.StatusServiceUnavailable) + return + } + fmt.Fprint(w, test.fleets) }) diff --git a/internal/key/key.go b/internal/key/key.go index 830fb61..81ce208 100644 --- a/internal/key/key.go +++ b/internal/key/key.go @@ -123,7 +123,9 @@ func List(session api.Session, arguments []string) error { } if jsonOutput { - return json.NewEncoder(session.Out).Encode(keys) + err = json.NewEncoder(session.Out).Encode(keys) + + return err } if len(keys) == 0 { @@ -158,6 +160,8 @@ func List(session api.Session, arguments []string) error { fleetNameWidth = max(fleetNameWidth, len(fleetNameValues[index])) } + // KEY is fixed at eight: the server sends a five-character suffix and the + // cell prefixes it with three dots, so a shorter suffix would skew LABEL. fmt.Fprintf(session.Out, "%-*s %-*s %-*s %-8s %s\n", idWidth, "ID", fleetIdWidth, "FLEET", fleetNameWidth, "FLEET NAME", "KEY", "LABEL") diff --git a/internal/key/key_test.go b/internal/key/key_test.go index 69dac1c..1ac1ba9 100644 --- a/internal/key/key_test.go +++ b/internal/key/key_test.go @@ -153,7 +153,14 @@ func TestKeyList(t *testing.T) { wantHidden []string wantError string keys string + refusal string }{ + { + name: "server refusal", + arguments: []string{}, + refusal: "keys unavailable", + wantError: "keys unavailable", + }, { name: "every fleet's keys", arguments: []string{}, @@ -196,6 +203,12 @@ func TestKeyList(t *testing.T) { wantShown: []string{`"id":1`}, wantHidden: []string{`"id":2`, "ID FLEET"}, }, + { + name: "a fleet the list does not name", + arguments: []string{}, + keys: `[{"id":1,"fleet":99,"label":"orphan","suffix":"ab2de"}]`, + wantShown: []string{"99 -"}, + }, { name: "a fleet out of reach", arguments: []string{"9"}, @@ -228,6 +241,11 @@ func TestKeyList(t *testing.T) { }) mux.HandleFunc("GET /keys", func(w http.ResponseWriter, r *http.Request) { + if test.refusal != "" { + http.Error(w, test.refusal, http.StatusServiceUnavailable) + return + } + fmt.Fprint(w, servedKeys) }) diff --git a/internal/login/login.go b/internal/login/login.go index 0741002..5c135c7 100644 --- a/internal/login/login.go +++ b/internal/login/login.go @@ -18,8 +18,6 @@ import ( ) func Login(session api.Session, arguments []string) error { - oauthClient := &http.Client{Timeout: 30 * time.Second} - if len(arguments) != 1 || (arguments[0] != "github" && arguments[0] != "gitlab") { return errors.New("login takes a provider: github or gitlab") } @@ -90,6 +88,8 @@ func Login(session api.Session, arguments []string) error { codeRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded") codeRequest.Header.Set("Accept", "application/json") + oauthClient := &http.Client{Timeout: 30 * time.Second} + codeResponse, err := oauthClient.Do(codeRequest) if err != nil { @@ -138,11 +138,10 @@ func Login(session api.Session, arguments []string) error { deadline := time.Now().Add(time.Duration(code.ExpiresIn) * time.Second) - const defaultPollInterval = 5 interval := code.Interval if interval <= 0 { - interval = defaultPollInterval + interval = 5 // RFC 8628 section 3.2: the default when a provider omits it } accessToken := "" @@ -200,7 +199,7 @@ func Login(session api.Session, arguments []string) error { case "authorization_pending": case "slow_down": - interval += 5 + interval += 5 // RFC 8628 section 3.5: each slow_down adds five seconds case "expired_token": return errors.New("the code expired before it was entered, run login again") @@ -356,7 +355,7 @@ func Logout(session api.Session, arguments []string) error { err = os.Remove(path) - if err != nil { + if err != nil && !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("the login stored at %s is no longer valid but could not be removed, delete it yourself", path) } diff --git a/internal/login/login_test.go b/internal/login/login_test.go index cabbe96..341c47a 100644 --- a/internal/login/login_test.go +++ b/internal/login/login_test.go @@ -643,3 +643,134 @@ func TestLoginReplacesAStoredLoginLeftTooOpen(t *testing.T) { t.Errorf("the login folder holds %d files, want only the stored login", len(entries)) } } + +func TestLoginShowsTheCodeAndWhereToEnterIt(t *testing.T) { + apitest.IsolateKeyStorage(t) + + _, providerBase := fakeProviderForLogin(t, "gitlab", 1, "", []string{`{"access_token": "glpat-test"}`}) + session, out := fakeSuperstack(t, "", "") + session.GitlabBase = providerBase + + err := Login(session, []string{"gitlab"}) + + if err != nil { + t.Fatal(err) + } + + want := "Copy your one-time code: WDJB-MJHT\n" + + "Then enter it at https://gitlab.com/-/user_settings/device?user_code=WDJB-MJHT\n" + + "Press enter to open the browser.\n" + + "Logged in as someone@example.com.\n" + + if out.String() != want { + t.Errorf("output = %q, want %q", out.String(), want) + } +} + +func TestLoginKeepsAWorkingLoginWhenTheNewOneCannotBeSaved(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores the folder mode this test rests on") + } + + apitest.IsolateKeyStorage(t) + + path, err := api.KeyPath() + + if err != nil { + t.Fatal(err) + } + + directory := filepath.Dir(path) + + err = os.MkdirAll(directory, 0o700) + + if err != nil { + t.Fatal(err) + } + + err = os.WriteFile(path, []byte("ssk_working\n"), 0o600) + + if err != nil { + t.Fatal(err) + } + + err = os.Chmod(directory, 0o500) + + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { os.Chmod(directory, 0o700) }) + + _, providerBase := fakeProviderForLogin(t, "gitlab", 1, "", []string{`{"access_token": "glpat-test"}`}) + session, out := fakeSuperstack(t, "", "") + session.GitlabBase = providerBase + + err = Login(session, []string{"gitlab"}) + + if err == nil || !strings.Contains(err.Error(), "could not be saved") { + t.Fatalf("error = %v, want it to say the login could not be saved", err) + } + + stored, err := os.ReadFile(path) + + if err != nil { + t.Fatal(err) + } + + if strings.TrimSpace(string(stored)) != "ssk_working" { + t.Errorf("the stored login is %q, want the working one left untouched", strings.TrimSpace(string(stored))) + } + + if strings.Contains(out.String(), "Logged in as") { + t.Errorf("output = %q, want no claim that the login succeeded", out.String()) + } +} + +func TestLogoutToleratesALoginAlreadyRemoved(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_test\n"), 0o600) + + if err != nil { + t.Fatal(err) + } + + mux := http.NewServeMux() + + // Another shell logs out, or deletes the account, during the round trip. + mux.HandleFunc("POST /logout", func(w http.ResponseWriter, r *http.Request) { + os.Remove(path) + + w.WriteHeader(http.StatusNoContent) + }) + + server := httptest.NewServer(mux) + + defer server.Close() + + out := &bytes.Buffer{} + session := api.NewSession(server.URL, "test", strings.NewReader(""), out) + + err = Logout(session, nil) + + if err != nil { + t.Fatalf("error = %v, want a login already removed to be no failure", err) + } + + if out.String() != "Logged out.\n" { + t.Errorf("output = %q, want %q", out.String(), "Logged out.\n") + } +} diff --git a/internal/member/member.go b/internal/member/member.go index 37e8800..4c85dff 100644 --- a/internal/member/member.go +++ b/internal/member/member.go @@ -103,7 +103,9 @@ func List(session api.Session, arguments []string) error { } if jsonOutput { - return json.NewEncoder(session.Out).Encode(people) + err = json.NewEncoder(session.Out).Encode(people) + + return err } owner := api.Printable(people.Owner) diff --git a/internal/member/member_test.go b/internal/member/member_test.go index faa0036..442b76c 100644 --- a/internal/member/member_test.go +++ b/internal/member/member_test.go @@ -217,6 +217,7 @@ func TestMemberRemove(t *testing.T) { name string email string answer string + fleets string refusal string wantRemoved bool wantShown string @@ -230,6 +231,7 @@ func TestMemberRemove(t *testing.T) { {name: "declined with n", email: "member@example.com", answer: "n\n", wantShown: "Nothing removed"}, {name: "closed input", email: "member@example.com", wantShown: "Nothing removed"}, {name: "server refusal", email: "member@example.com", answer: "y\n", refusal: "only an owner can remove members", wantRemoved: true, wantError: "only an owner can remove members"}, + {name: "a fleet that is not yours", email: "member@example.com", answer: "y\n", fleets: `[{"id":7,"name":"someone else's","owner":true}]`, wantError: "no such fleet"}, } for _, test := range tests { @@ -239,8 +241,14 @@ func TestMemberRemove(t *testing.T) { mux := http.NewServeMux() + fleets := test.fleets + + if fleets == "" { + fleets = `[{"id":3,"name":"pilot","owner":true}]` + } + mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, `[{"id":3,"name":"pilot","owner":true}]`) + fmt.Fprint(w, fleets) }) mux.HandleFunc("DELETE /fleets/{id}/members/{email}", func(w http.ResponseWriter, r *http.Request) { diff --git a/main_test.go b/main_test.go index 0baaf90..845ed07 100644 --- a/main_test.go +++ b/main_test.go @@ -2,9 +2,12 @@ package main import ( "bytes" + "errors" "go/parser" "go/token" "io/fs" + "os" + "os/exec" "path/filepath" "slices" "strings" @@ -203,3 +206,87 @@ func TestHelpRendersTheRealTable(t *testing.T) { } } } + +// Re-runs this test binary as a child so main's own exit codes are observed +// rather than the error its command returned. +func TestMainReportsFailureWithANonZeroExit(t *testing.T) { + arguments, isChild := os.LookupEnv("SUPERSTACK_MAIN_ARGUMENTS") + + if isChild { + os.Args = append([]string{"superstack"}, strings.Fields(arguments)...) + + main() + + return + } + + tests := []struct { + name string + arguments string + wantCode int + wantSays string + }{ + {name: "no arguments", arguments: " ", wantCode: 0, wantSays: "Usage: superstack"}, + {name: "the version", arguments: "version", wantCode: 0}, + {name: "an unknown command", arguments: "nonsense", wantCode: 1, wantSays: "unknown command"}, + {name: "a command nobody has built yet", arguments: "tail 111111111111111", wantCode: 1, wantSays: "not available yet"}, + {name: "a command that needs a login", arguments: "fleet list", wantCode: 1, wantSays: "not logged in"}, + {name: "a flag with no value", arguments: "--server", wantCode: 1, wantSays: "needs an address"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + home := t.TempDir() + + command := exec.Command(os.Args[0], "-test.run=TestMainReportsFailureWithANonZeroExit") + + command.Env = append(os.Environ(), + "SUPERSTACK_MAIN_ARGUMENTS="+test.arguments, + "HOME="+home, + "XDG_STATE_HOME="+home, + "AppData="+home, + ) + + output, err := command.CombinedOutput() + + code := 0 + exitError, wasExit := err.(*exec.ExitError) + + switch { + case wasExit: + code = exitError.ExitCode() + + case err != nil: + t.Fatal(err) + } + + if code != test.wantCode { + t.Errorf("superstack %s exited %d, want %d, having said %q", test.arguments, code, test.wantCode, output) + } + + if test.wantSays != "" && !strings.Contains(string(output), test.wantSays) { + t.Errorf("superstack %s said %q, want it to mention %q", test.arguments, output, test.wantSays) + } + }) + } +} + +func TestTheModuleStaysDependencyFree(t *testing.T) { + module, err := os.ReadFile("go.mod") + + if err != nil { + t.Fatal(err) + } + + for _, line := range strings.Split(string(module), "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "require") { + t.Errorf("go.mod says %q, so flake.nix cannot keep vendorHash = null", strings.TrimSpace(line)) + } + } + + _, err = os.Stat("go.sum") + + if !errors.Is(err, fs.ErrNotExist) { + t.Error("go.sum exists, so something is vendored and flake.nix needs a real vendorHash") + } +}