diff --git a/internal/api/client.go b/internal/api/client.go index 5ff51cc9..f0e8187c 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -333,10 +333,14 @@ func (c *Client) ListClients(ctx context.Context) ([]ProvisionedClient, error) { if status < 200 || status >= 300 { return nil, &APIError{StatusCode: status, Body: string(raw), URL: reqURL} } - // Unpaginated deployment → a bare array; return it as-is. - var bare []ProvisionedClient - if err := json.Unmarshal(raw, &bare); err == nil { - return append(all, bare...), nil + // Unpaginated deployment → a bare array. Only valid as the sole response + // (a paginated chain is a `{next,results}` object on every page), so guard + // to page 0 — a stray bare body mid-chain must not silently end the loop. + if pageNum == 0 { + var bare []ProvisionedClient + if err := json.Unmarshal(raw, &bare); err == nil { + return bare, nil + } } var body struct { Next string `json:"next"` diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 8cad599f..e091b79c 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -234,3 +234,22 @@ func TestListClients_UnparseableNextLink_IsError(t *testing.T) { t.Fatal("expected an error on an unparseable next link, got nil (silent truncation)") } } + +// TestListClients_BareArrayUnpaginated covers the unpaginated deployment shape +// (a bare JSON array) — still returned as-is after the bare-decode was guarded +// to the first page. +func TestListClients_BareArrayUnpaginated(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`[{"id":1,"first_name":"a","namespace":"a"},{"id":2,"first_name":"b","namespace":"b"}]`)) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + got, err := c.ListClients(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(got) != 2 || got[0].ID != 1 || got[1].ID != 2 { + t.Fatalf("want 2 clients [1,2] from a bare array, got %+v", got) + } +} diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 6f159653..836d4797 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -123,7 +123,9 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { case errors.Is(err, api.ErrAuthorizationPending): // not approved yet — keep polling case errors.Is(err, api.ErrSlowDown): - interval++ + // RFC 8628 §3.5: on slow_down the client MUST increase the poll + // interval by 5 seconds for this and all subsequent polls. + interval += 5 case errors.Is(err, api.ErrExpiredToken): return &exitError{code: 1, err: errors.New("the sign-in code expired — re-run `tracebloc login`")} case errors.Is(err, api.ErrAccessDenied): diff --git a/internal/cli/auth_test.go b/internal/cli/auth_test.go index 5be742b6..c4bba27a 100644 --- a/internal/cli/auth_test.go +++ b/internal/cli/auth_test.go @@ -87,6 +87,55 @@ func TestLogin_FullFlow(t *testing.T) { } } +// TestLogin_SlowDownBacksOffByFive pins RFC 8628 §3.5: on `slow_down` the poll +// interval must increase by 5 seconds, not 1. Captures the durations handed to +// the pollAfter seam. +func TestLogin_SlowDownBacksOffByFive(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + var polls int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/device/code": + _, _ = w.Write([]byte(`{"device_code":"dc","user_code":"X","verification_uri":"https://x/activate","expires_in":600,"interval":5}`)) + case "/device/token": + polls++ + if polls == 1 { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"slow_down"}`)) + return + } + _, _ = w.Write([]byte(`{"token":"cat_ok"}`)) + case "/userinfo/": + _, _ = w.Write([]byte(`{"email":"e@co","account":"A"}`)) + } + })) + t.Cleanup(srv.Close) + + origClient, origAfter := newAPIClient, pollAfter + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + var waits []time.Duration + pollAfter = func(d time.Duration) <-chan time.Time { + waits = append(waits, d) + ch := make(chan time.Time, 1) + ch <- time.Time{} + return ch + } + t.Cleanup(func() { newAPIClient = origClient; pollAfter = origAfter }) + + if _, err := runCmd(t, "login"); err != nil { + t.Fatalf("login: %v", err) + } + if len(waits) < 2 { + t.Fatalf("expected >=2 polls, got waits=%v", waits) + } + if waits[0] != 5*time.Second { + t.Errorf("first poll wait = %v, want 5s (server interval)", waits[0]) + } + if waits[1] != 10*time.Second { + t.Errorf("post-slow_down wait = %v, want 10s (interval+5 per RFC 8628), not 6s", waits[1]) + } +} + func TestLogin_BackendUnsupported(t *testing.T) { withTestBackend(t, func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) diff --git a/internal/cli/client.go b/internal/cli/client.go index c4d06a3f..328bbc32 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -167,6 +167,13 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien var existing []string if clients, lerr := client.ListClients(ctx); lerr == nil { for _, c := range clients { + // Skip the client already anchored to this cluster: a re-run adopts it + // (the backend keys on cluster_id), so its namespace isn't a collision. + // Counting it would bump the derived slug and show a namespace in the + // review that doesn't match the one actually adopted. + if clusterID != "" && c.ClusterID == clusterID { + continue + } if c.Namespace != "" { existing = append(existing, c.Namespace) } diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index bd79021d..dcdcc1cf 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -521,3 +521,31 @@ func TestClientCreate_NoClusterAnchorWarns(t *testing.T) { t.Errorf("no-anchor fallback should still print the credential, got:\n%s", out.String()) } } + +// TestClientCreate_ReRunReviewShowsAdoptedNamespace pins that on an idempotent +// re-run, the client already anchored to this cluster is excluded from collision +// detection — so the review shows the namespace that's actually adopted +// (lab-one), not a bumped lab-one-2. +func TestClientCreate_ReRunReviewShowsAdoptedNamespace(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": + // An existing client already anchored to THIS cluster (uid-1). + _, _ = w.Write([]byte(`[{"id":1,"first_name":"Lab One","username":"u-1","namespace":"lab-one","location":"DE","cluster_id":"uid-1"}]`)) + case r.Method == http.MethodPost && r.URL.Path == "/edge-device/": + w.WriteHeader(http.StatusOK) // adopt + _, _ = w.Write([]byte(`{"id":1,"first_name":"Lab One","username":"u-1","namespace":"lab-one","location":"DE","cluster_id":"uid-1"}`)) + } + }) + stubClusterID(t, "uid-1", nil) + confirmYes := true + pr := &fakePrompter{answers: map[string]string{}, confirm: &confirmYes} + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), pr, + clientCreateOpts{name: "Lab One", location: "DE"}); err != nil { + t.Fatalf("re-run create: %v", err) + } + if strings.Contains(out.String(), "lab-one-2") { + t.Errorf("review showed a bumped namespace — the cluster's own client wasn't excluded from collision detection:\n%s", out.String()) + } +} diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index fdcc4ecd..d69ce9c8 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -288,7 +288,10 @@ func checkBackendEgress(ctx context.Context, env map[string]string, probe func(c // runtime's own mapping (controller.py). Unset/unknown defaults to prod, the // chart's CLIENT_ENV default. func backendHost(clientEnv string) string { - switch clientEnv { + // Normalize like the API client (api.ResolveEnv/BaseURL lower-case), so a + // non-lowercase CLIENT_ENV on the edge box doesn't fall through to prod and + // make the doctor probe the wrong backend. + switch strings.ToLower(strings.TrimSpace(clientEnv)) { case "dev": return "dev-api.tracebloc.io" case "stg": diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index 766fe933..7eaaa22b 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -232,6 +232,11 @@ func TestBackendHost(t *testing.T) { "prod": "api.tracebloc.io", "": "api.tracebloc.io", "weird": "api.tracebloc.io", + // Case/space-insensitive, matching the API client's env resolution — a + // non-lowercase CLIENT_ENV must not fall through to prod. + "DEV": "dev-api.tracebloc.io", + "Stg": "stg-api.tracebloc.io", + " dev ": "dev-api.tracebloc.io", } for in, want := range tests { if got := backendHost(in); got != want {