From bc73f6b27d3870d41ec0d81ca64bb0f4efd870f2 Mon Sep 17 00:00:00 2001 From: chas Date: Thu, 10 Sep 2026 18:30:38 -0400 Subject: [PATCH 1/5] [CFX-7608] Split 403 from 401 in auth status classification Why: A 403 from GET /version/ authenticated the key, then the account or agreement refused it (deactivated account, or an unsigned clickthrough agreement). A fresh login mints another key that 403s the same way, so relaunching login is futile. Before this, a 403 blamed the token like a 401 and reopened the browser login flow. Changes: - fprintServerStatus: 401 alone returns false (relaunch). 403 reports that the account lacks access and returns true, so the gate stops the relaunch through ReportUnjudged. 401 wording unchanged. - Tests: 403 row flips in TestReportUnjudged and TestReportEnvCredentialsError; new TestEnsureAuthenticated_StoredProfile403. - docs: authentication.md flow and auth.md example output. --- docs/commands/auth.md | 7 +++- docs/development/authentication.md | 4 +-- internal/auth/auth.go | 18 +++++++--- internal/auth/auth_test.go | 56 ++++++++++++++++++++++++------ 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/docs/commands/auth.md b/docs/commands/auth.md index f131eec92..9e59068cf 100644 --- a/docs/commands/auth.md +++ b/docs/commands/auth.md @@ -176,11 +176,16 @@ $ dr auth check ❌ Could not connect to https://app.example.com: dial tcp: lookup app.example.com: no such host Check DATAROBOT_ENDPOINT and your network, then try again. -# The instance answered, but not with a credential verdict (only 401/403 blame the token) +# The instance answered, but not with a credential verdict (only 401 blames the token) $ dr auth check ❌ https://app.example.com answered HTTP 503, so the CLI could not verify your credentials. Check DATAROBOT_ENDPOINT, and the instance's status if it persists. +# Credentials accepted, but the account lacks API access (a fresh login would not help) +$ dr auth check +❌ https://app.example.com accepted your credentials but your account lacks API access (HTTP 403). +Check that your account is activated and any required agreement is signed. + # Endpoint scheme the CLI cannot use $ dr auth check ❌ DATAROBOT_ENDPOINT environment variable is invalid: unsupported URL scheme "ftp", use https:// diff --git a/docs/development/authentication.md b/docs/development/authentication.md index 52ebd86bf..7ca7384d0 100644 --- a/docs/development/authentication.md +++ b/docs/development/authentication.md @@ -29,10 +29,10 @@ var MyCmd = &cobra.Command{ The hook functions are outlined below. -1. **Checks environment credentials first**: A complete `DATAROBOT_ENDPOINT` (or `DATAROBOT_API_ENDPOINT`) and `DATAROBOT_API_TOKEN` pair takes precedence over the config file. If the pair fails verification, the command fails with the reason (timeout, malformed endpoint, unreachable endpoint, a non-2xx status from the instance, or an invalid token; only a 401 or 403 blames the token). It never falls back to the stored profile and never starts the login flow, because that would silently run the command against a different DataRobot instance than the one requested. +1. **Checks environment credentials first**: A complete `DATAROBOT_ENDPOINT` (or `DATAROBOT_API_ENDPOINT`) and `DATAROBOT_API_TOKEN` pair takes precedence over the config file. If the pair fails verification, the command fails with the reason (timeout, malformed endpoint, unreachable endpoint, a non-2xx status from the instance, or an invalid token; only a 401 blames the token, and a 403 means the credentials authenticated but the account lacks API access). It never falls back to the stored profile and never starts the login flow, because that would silently run the command against a different DataRobot instance than the one requested. 2. **Checks for valid credentials**: With no complete environment pair, checks if a valid API key already exists in the config file. 3. **Auto-configures URL if missing**: If no DataRobot URL is configured, prompts you to set it up. -4. **Retrieves new credentials**: If the stored credentials are missing, or DataRobot rejected them with a 401 or 403, the hook automatically triggers the browser-based login flow. A timeout, an unreachable host, or any other status means DataRobot never judged the credentials, so the login flow does not start and the stored token is left intact. +4. **Retrieves new credentials**: If the stored credentials are missing, or DataRobot rejected them with a 401, the hook automatically triggers the browser-based login flow. A 403 authenticated the credentials but the account lacks access, so the login flow does not start (a fresh login would 403 the same way). A timeout, an unreachable host, or any other status means DataRobot never judged the credentials, so the login flow does not start and the stored token is left intact. 5. **Fails early**: If authentication cannot be established, the command will not run and returns an error. Credential failures are explained on stderr, so `--output-format json` leaves stdout empty when the gate rejects your credentials. The interactive paths still use stdout: the browser login flow and the URL prompt. ### Direct call for non-command code diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 0ed525fd2..6454aa6e1 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -243,19 +243,27 @@ func fprintTransportError(w io.Writer, endpoint, endpointName string, err error) return true } -// Only 401 and 403 mean the token was judged and rejected. Blaming it for -// a 404, 429, or 5xx would tell the user to unset working credentials. +// 401 returns false so the caller relaunches login. 403 authenticated but the +// account lacks access, so it reports here and returns true (no relaunch). func fprintServerStatus(w io.Writer, endpoint, endpointName string, err error) bool { var statusErr *config.HTTPStatusError - if !errors.As(err, &statusErr) || - statusErr.StatusCode == http.StatusUnauthorized || - statusErr.StatusCode == http.StatusForbidden { + if !errors.As(err, &statusErr) || statusErr.StatusCode == http.StatusUnauthorized { return false } base, info := writerStyles(w) + // A 403 authenticates then refuses; a fresh login would 403 the same way. + if statusErr.StatusCode == http.StatusForbidden { + fmt.Fprint(w, base.Render("❌ ")) + fmt.Fprint(w, info.Render(hostOrEndpoint(endpoint))) + fmt.Fprintln(w, base.Render(" accepted your credentials but your account lacks API access (HTTP 403).")) + fmt.Fprintln(w, base.Render("Check that your account is activated and any required agreement is signed.")) + + return true + } + fmt.Fprint(w, base.Render("❌ ")) fmt.Fprint(w, info.Render(hostOrEndpoint(endpoint))) fmt.Fprintln(w, base.Render(fmt.Sprintf(" answered HTTP %d, so the CLI could not verify your credentials.", diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index ffe3e7109..e9bb414c2 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -318,20 +318,27 @@ func TestReportEnvCredentialsError(t *testing.T) { assert.NotContains(t, buf.String(), "Could not connect") }) - // Only 401 and 403 may blame the token; every other status is the server - // failing to answer the version check. + // Only 401 blames the token; 403 authenticated but the account lacks access, + // and every other status is the server failing to answer the version check. creds := &EnvCredentials{Endpoint: "https://app.example.com/api/v2", Token: "some-token"} - for _, status := range []int{http.StatusUnauthorized, http.StatusForbidden} { - t.Run(fmt.Sprintf("%d blames the token", status), func(t *testing.T) { - var buf bytes.Buffer + t.Run("401 blames the token", func(t *testing.T) { + var buf bytes.Buffer - ReportEnvCredentialsError(&buf, creds, &config.HTTPStatusError{StatusCode: status}) + ReportEnvCredentialsError(&buf, creds, &config.HTTPStatusError{StatusCode: http.StatusUnauthorized}) - assert.Contains(t, buf.String(), "DATAROBOT_API_TOKEN environment variable is invalid or expired") - assert.Contains(t, buf.String(), "unset DATAROBOT_API_TOKEN") - }) - } + assert.Contains(t, buf.String(), "DATAROBOT_API_TOKEN environment variable is invalid or expired") + assert.Contains(t, buf.String(), "unset DATAROBOT_API_TOKEN") + }) + + t.Run("403 reports lacking access, not a bad token", func(t *testing.T) { + var buf bytes.Buffer + + ReportEnvCredentialsError(&buf, creds, &config.HTTPStatusError{StatusCode: http.StatusForbidden}) + + assert.Contains(t, buf.String(), "lacks API access") + assert.NotContains(t, buf.String(), "unset DATAROBOT_API_TOKEN") + }) for _, status := range []int{ http.StatusNotFound, http.StatusTooManyRequests, http.StatusInternalServerError, @@ -363,7 +370,7 @@ func TestReportUnjudged(t *testing.T) { {"429", endpoint, &config.HTTPStatusError{StatusCode: http.StatusTooManyRequests}, true, "answered HTTP 429"}, {"503", endpoint, &config.HTTPStatusError{StatusCode: http.StatusServiceUnavailable}, true, "answered HTTP 503"}, {"401 is a verdict", endpoint, &config.HTTPStatusError{StatusCode: http.StatusUnauthorized}, false, ""}, - {"403 is a verdict", endpoint, &config.HTTPStatusError{StatusCode: http.StatusForbidden}, false, ""}, + {"403 stops the relaunch", endpoint, &config.HTTPStatusError{StatusCode: http.StatusForbidden}, true, "lacks API access"}, // An absent token carries no status, so it must read as a verdict and // let the caller start the login flow. {"empty token is a verdict", endpoint, errors.New("empty token"), false, ""}, @@ -478,6 +485,33 @@ func TestEnsureAuthenticated_StoredProfileRejected(t *testing.T) { assert.Equal(t, "fresh-token", viperx.GetString(config.DataRobotAPIKey)) } +// TestEnsureAuthenticated_StoredProfile403 proves a 403 stops the relaunch: the +// key authenticated, so a fresh login would mint another key that 403s the same. +func TestEnsureAuthenticated_StoredProfile403(t *testing.T) { + _, cleanup := setupTestEnvironment(t) + defer cleanup() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer server.Close() + + viperx.Set(config.DataRobotURL, server.URL+"/api/v2") + viperx.Set(config.DataRobotAPIKey, "valid-token") + + APIKeyCallbackFunc = func(_ context.Context, _ string) (string, error) { + t.Error("login flow must not start on a 403; a fresh key would 403 too") + + return "", errors.New("unexpected login flow") + } + + result := EnsureAuthenticated(context.Background()) + + assert.False(t, result, "Expected EnsureAuthenticated to fail on a 403 from the stored profile") + assert.Equal(t, "valid-token", viperx.GetString(config.DataRobotAPIKey), + "Expected the stored token to survive a 403 (the account, not the key, was refused)") +} + // TestEnsureAuthenticated_NoStoredToken keeps a fresh install working: an absent // token carries no status, so it must still reach the login flow. func TestEnsureAuthenticated_NoStoredToken(t *testing.T) { From 5e0ec1b690957b9cd0d3a0b4a777ef8bb776ca59 Mon Sep 17 00:00:00 2001 From: chas Date: Thu, 10 Sep 2026 18:31:05 -0400 Subject: [PATCH 2/5] [CFX-7608] Classify server errors on the dr auth check legs Why: The .env and stored-profile legs of dr auth check blamed the token for any non-200. A 404, 429, or 5xx told the user their token was bad when the instance was what failed. PR #776 fixed this for the DATAROBOT_ENDPOINT/DATAROBOT_API_TOKEN pair; this extends the same classification to the other two legs. Changes: - checkCLICredentials and verifyDotenvToken defer to auth.ReportUnjudged; only a real 401 falls through to the dr auth login / dr dotenv update advice. - verifyDotenvToken and the dotenv print helpers take an io.Writer for testing. - Tests: per-status tables for both legs (401/403/404/503). --- cmd/auth/check/cmd.go | 98 ++++++++++++++++----------------- cmd/auth/check/cmd_test.go | 108 +++++++++++++++++++++++++++---------- 2 files changed, 124 insertions(+), 82 deletions(-) diff --git a/cmd/auth/check/cmd.go b/cmd/auth/check/cmd.go index 22749201b..4fa5f48bb 100644 --- a/cmd/auth/check/cmd.go +++ b/cmd/auth/check/cmd.go @@ -26,6 +26,7 @@ import ( "github.com/datarobot/cli/internal/auth" "github.com/datarobot/cli/internal/cli" "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/config/viperx" "github.com/datarobot/cli/internal/envbuilder" "github.com/datarobot/cli/internal/repo" "github.com/datarobot/cli/tui" @@ -66,11 +67,8 @@ func checkCLICredentials(w io.Writer) bool { _, err = config.GetAPIKey(context.Background()) if err != nil { - if errors.Is(err, context.DeadlineExceeded) { - fmt.Fprint(w, tui.BaseTextStyle.Render("❌ Connection to ")) - fmt.Fprint(w, tui.InfoStyle.Render(datarobotHost)) - fmt.Fprintln(w, tui.BaseTextStyle.Render(" timed out. Check your network and try again.")) - } else { + // ReportUnjudged handles timeout, unreachable, and non-401 statuses; only a real 401 needs the login advice. + if !auth.ReportUnjudged(w, viperx.GetString(config.DataRobotURL), auth.StoredEndpointName, err) { fmt.Fprintln(w, tui.BaseTextStyle.Render("❌ No valid API key found in CLI config.")) fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) fmt.Fprint(w, tui.InfoStyle.Render("dr auth login")) @@ -85,31 +83,31 @@ func checkCLICredentials(w io.Writer) bool { return allValid } -func printDotenvMissingError() { - fmt.Println(tui.BaseTextStyle.Render("⚠️ No '.env' file found in repository.")) - fmt.Print(tui.BaseTextStyle.Render("Run ")) - fmt.Print(tui.InfoStyle.Render("dr start")) - fmt.Print(tui.BaseTextStyle.Render(" or ")) - fmt.Print(tui.InfoStyle.Render("dr dotenv setup")) - fmt.Println(tui.BaseTextStyle.Render(" to create one.")) +func printDotenvMissingError(w io.Writer) { + fmt.Fprintln(w, tui.BaseTextStyle.Render("⚠️ No '.env' file found in repository.")) + fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr start")) + fmt.Fprint(w, tui.BaseTextStyle.Render(" or ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr dotenv setup")) + fmt.Fprintln(w, tui.BaseTextStyle.Render(" to create one.")) } -func printDotenvReadError() { - fmt.Println(tui.BaseTextStyle.Render("❌ Failed to read '.env' file.")) - fmt.Print(tui.BaseTextStyle.Render("Run ")) - fmt.Print(tui.InfoStyle.Render("dr start")) - fmt.Print(tui.BaseTextStyle.Render(" or ")) - fmt.Print(tui.InfoStyle.Render("dr dotenv setup")) - fmt.Println(tui.BaseTextStyle.Render(" to create one.")) +func printDotenvReadError(w io.Writer) { + fmt.Fprintln(w, tui.BaseTextStyle.Render("❌ Failed to read '.env' file.")) + fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr start")) + fmt.Fprint(w, tui.BaseTextStyle.Render(" or ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr dotenv setup")) + fmt.Fprintln(w, tui.BaseTextStyle.Render(" to create one.")) } -func printMissingEnvVarError(varName string) { - fmt.Println(tui.BaseTextStyle.Render(fmt.Sprintf("⚠️ No %s found in '.env'.", varName))) - fmt.Print(tui.BaseTextStyle.Render("Run ")) - fmt.Print(tui.InfoStyle.Render("dr start")) - fmt.Print(tui.BaseTextStyle.Render(" or ")) - fmt.Print(tui.InfoStyle.Render("dr dotenv setup")) - fmt.Println(tui.BaseTextStyle.Render(" to configure the '.env' file.")) +func printMissingEnvVarError(w io.Writer, varName string) { + fmt.Fprintln(w, tui.BaseTextStyle.Render(fmt.Sprintf("⚠️ No %s found in '.env'.", varName))) + fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr start")) + fmt.Fprint(w, tui.BaseTextStyle.Render(" or ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr dotenv setup")) + fmt.Fprintln(w, tui.BaseTextStyle.Render(" to configure the '.env' file.")) } func extractDotenvVars(dotenvPath string) (string, string, error) { @@ -141,30 +139,24 @@ func extractDotenvVars(dotenvPath string) (string, string, error) { return dotenvToken, dotenvEndpoint, nil } -func verifyDotenvToken(dotenvEndpoint, dotenvToken string) bool { - dotenvBaseURL, err := config.SchemeHostOnly(dotenvEndpoint) - if err != nil { - fmt.Println(tui.BaseTextStyle.Render("❌ Invalid DATAROBOT_ENDPOINT in '.env'.")) - fmt.Print(tui.BaseTextStyle.Render("Run ")) - fmt.Print(tui.InfoStyle.Render("dr dotenv update")) - fmt.Println(tui.BaseTextStyle.Render(" to fix the configuration.")) +func verifyDotenvToken(w io.Writer, dotenvEndpoint, dotenvToken string) bool { + if _, err := config.SchemeHostOnly(dotenvEndpoint); err != nil { + fmt.Fprintln(w, tui.BaseTextStyle.Render("❌ Invalid DATAROBOT_ENDPOINT in '.env'.")) + fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr dotenv update")) + fmt.Fprintln(w, tui.BaseTextStyle.Render(" to fix the configuration.")) return false } - err = config.VerifyToken(context.Background(), dotenvEndpoint, dotenvToken) + err := config.VerifyToken(context.Background(), dotenvEndpoint, dotenvToken) if err != nil { - if errors.Is(err, context.DeadlineExceeded) { - fmt.Print(tui.BaseTextStyle.Render("❌ Connection to ")) - fmt.Print(tui.InfoStyle.Render(dotenvBaseURL)) - fmt.Println(tui.BaseTextStyle.Render(" timed out. Check your network and try again.")) - } else { - // Blames the token for any non-200, unlike the env-var leg. - // Splitting on the status here is a planned follow-up. - fmt.Println(tui.BaseTextStyle.Render("❌ DATAROBOT_API_TOKEN in '.env' is invalid or expired.")) - fmt.Print(tui.BaseTextStyle.Render("Run ")) - fmt.Print(tui.InfoStyle.Render("dr dotenv update")) - fmt.Println(tui.BaseTextStyle.Render(" to refresh credentials.")) + // ReportUnjudged handles timeout, unreachable, and non-401 statuses; only a real 401 needs the dotenv-update advice. + if !auth.ReportUnjudged(w, dotenvEndpoint, "DATAROBOT_ENDPOINT in '.env'", err) { + fmt.Fprintln(w, tui.BaseTextStyle.Render("❌ DATAROBOT_API_TOKEN in '.env' is invalid or expired.")) + fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) + fmt.Fprint(w, tui.InfoStyle.Render("dr dotenv update")) + fmt.Fprintln(w, tui.BaseTextStyle.Render(" to refresh credentials.")) } return false @@ -173,40 +165,40 @@ func verifyDotenvToken(dotenvEndpoint, dotenvToken string) bool { return true } -func checkDotenvCredentials(repoRoot string) bool { +func checkDotenvCredentials(w io.Writer, repoRoot string) bool { dotenvPath := filepath.Join(repoRoot, ".env") _, statErr := os.Stat(dotenvPath) if statErr != nil { - printDotenvMissingError() + printDotenvMissingError(w) return false } dotenvToken, dotenvEndpoint, err := extractDotenvVars(dotenvPath) if err != nil { - printDotenvReadError() + printDotenvReadError(w) return false } if dotenvToken == "" { - printMissingEnvVarError("DATAROBOT_API_TOKEN") + printMissingEnvVarError(w, "DATAROBOT_API_TOKEN") return false } if dotenvEndpoint == "" { - printMissingEnvVarError("DATAROBOT_ENDPOINT") + printMissingEnvVarError(w, "DATAROBOT_ENDPOINT") return false } - if !verifyDotenvToken(dotenvEndpoint, dotenvToken) { + if !verifyDotenvToken(w, dotenvEndpoint, dotenvToken) { return false } - fmt.Println(tui.BaseTextStyle.Render("✅ '.env' credentials are valid.")) + fmt.Fprintln(w, tui.BaseTextStyle.Render("✅ '.env' credentials are valid.")) return true } @@ -223,7 +215,7 @@ func RunE(_ *cobra.Command, _ []string) error { return cli.ErrSilent } - if checkDotenvCredentials(repoRoot) { + if checkDotenvCredentials(os.Stdout, repoRoot) { return nil } diff --git a/cmd/auth/check/cmd_test.go b/cmd/auth/check/cmd_test.go index aeae17e10..9fa626142 100644 --- a/cmd/auth/check/cmd_test.go +++ b/cmd/auth/check/cmd_test.go @@ -16,12 +16,12 @@ package check import ( "bytes" - "io" "net/http" "net/http/httptest" - "os" "testing" + "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/config/viperx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -43,32 +43,82 @@ func TestCheckCLICredentials_QuotedEndpointNamesEndpointNotToken(t *testing.T) { assert.NotContains(t, buf.String(), "DATAROBOT_API_TOKEN environment variable is invalid or expired") } -// The '.env' leg consumes VerifyToken's error as an opaque non-nil; the typed -// *config.HTTPStatusError must leave its message and verdict unchanged. -func TestVerifyDotenvToken_StatusErrorKeepsMessage(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusUnauthorized) - })) - t.Cleanup(server.Close) - - orig := os.Stdout - - t.Cleanup(func() { os.Stdout = orig }) - - r, w, err := os.Pipe() - require.NoError(t, err) - - os.Stdout = w - - valid := verifyDotenvToken(server.URL+"/api/v2", "expired-token") - - os.Stdout = orig - - require.NoError(t, w.Close()) - - out, err := io.ReadAll(r) - require.NoError(t, err) +// The stored-profile leg blames the token only on a real 401; 403 reports the +// account lacking access, and other statuses blame the instance. +func TestCheckCLICredentials_ClassifiesStoredProfileStatus(t *testing.T) { + cases := []struct { + name string + status int + wantContains string + wantNotContains string + }{ + {"401 blames the token", http.StatusUnauthorized, "No valid API key found", ""}, + {"403 reports lacking access", http.StatusForbidden, "lacks API access", "No valid API key found"}, + {"503 blames the instance", http.StatusServiceUnavailable, "answered HTTP 503", "No valid API key found"}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(c.status) + })) + t.Cleanup(server.Close) + + t.Setenv("DATAROBOT_ENDPOINT", "") + t.Setenv("DATAROBOT_API_ENDPOINT", "") + t.Setenv("DATAROBOT_API_TOKEN", "") + + viperx.Reset() + viperx.Set(config.DataRobotURL, server.URL+"/api/v2") + viperx.Set(config.DataRobotAPIKey, "stored-token") + t.Cleanup(viperx.Reset) + + var buf bytes.Buffer + + valid := checkCLICredentials(&buf) + + require.False(t, valid) + assert.Contains(t, buf.String(), c.wantContains) + + if c.wantNotContains != "" { + assert.NotContains(t, buf.String(), c.wantNotContains) + } + }) + } +} - require.False(t, valid) - assert.Contains(t, string(out), "DATAROBOT_API_TOKEN in '.env' is invalid or expired") +// The '.env' leg used to blame the token for every non-200; now 401 blames the +// token, 403 reports lacking access, and other statuses blame the instance. +func TestVerifyDotenvToken_ClassifiesStatus(t *testing.T) { + cases := []struct { + name string + status int + wantContains string + wantNotContains string + }{ + {"401 blames the token", http.StatusUnauthorized, "DATAROBOT_API_TOKEN in '.env' is invalid or expired", ""}, + {"403 reports lacking access", http.StatusForbidden, "lacks API access", "is invalid or expired"}, + {"404 blames the instance", http.StatusNotFound, "answered HTTP 404", "is invalid or expired"}, + {"503 blames the instance", http.StatusServiceUnavailable, "answered HTTP 503", "is invalid or expired"}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(c.status) + })) + t.Cleanup(server.Close) + + var buf bytes.Buffer + + valid := verifyDotenvToken(&buf, server.URL+"/api/v2", "some-token") + + require.False(t, valid) + assert.Contains(t, buf.String(), c.wantContains) + + if c.wantNotContains != "" { + assert.NotContains(t, buf.String(), c.wantNotContains) + } + }) + } } From 50362fd38c50d0a6abe9373db740fe02728df8ff Mon Sep 17 00:00:00 2001 From: chas Date: Thu, 10 Sep 2026 18:59:50 -0400 Subject: [PATCH 3/5] [CFX-7608] Keep the login advice for an unconfigured profile in dr auth check Why: Routing every GetAPIKey failure through ReportUnjudged sent a fresh install (empty stored endpoint and token) into unusableEndpoint, which reported "missing URL scheme" and dropped the "Run dr auth login" advice. EnsureAuthenticated guards its ReportUnjudged call on a stored token being present; checkCLICredentials did not. Changes: - Skip ReportUnjudged when no token is stored; print the login advice instead. - Test: a fresh install keeps the login advice and shows no missing-URL-scheme. --- cmd/auth/check/cmd.go | 5 +++-- cmd/auth/check/cmd_test.go | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/cmd/auth/check/cmd.go b/cmd/auth/check/cmd.go index 4fa5f48bb..64426f5ce 100644 --- a/cmd/auth/check/cmd.go +++ b/cmd/auth/check/cmd.go @@ -67,8 +67,9 @@ func checkCLICredentials(w io.Writer) bool { _, err = config.GetAPIKey(context.Background()) if err != nil { - // ReportUnjudged handles timeout, unreachable, and non-401 statuses; only a real 401 needs the login advice. - if !auth.ReportUnjudged(w, viperx.GetString(config.DataRobotURL), auth.StoredEndpointName, err) { + // An absent stored token is a fresh install, not a verdict, so keep the login advice (as EnsureAuthenticated does). + if viperx.GetString(config.DataRobotAPIKey) == "" || + !auth.ReportUnjudged(w, viperx.GetString(config.DataRobotURL), auth.StoredEndpointName, err) { fmt.Fprintln(w, tui.BaseTextStyle.Render("❌ No valid API key found in CLI config.")) fmt.Fprint(w, tui.BaseTextStyle.Render("Run ")) fmt.Fprint(w, tui.InfoStyle.Render("dr auth login")) diff --git a/cmd/auth/check/cmd_test.go b/cmd/auth/check/cmd_test.go index 9fa626142..90fae05a9 100644 --- a/cmd/auth/check/cmd_test.go +++ b/cmd/auth/check/cmd_test.go @@ -87,6 +87,27 @@ func TestCheckCLICredentials_ClassifiesStoredProfileStatus(t *testing.T) { } } +// A fresh install (no stored endpoint or token) keeps the dr auth login advice +// instead of misreporting the empty endpoint as a bad URL. +func TestCheckCLICredentials_FreshInstallKeepsLoginAdvice(t *testing.T) { + t.Setenv("DATAROBOT_ENDPOINT", "") + t.Setenv("DATAROBOT_API_ENDPOINT", "") + t.Setenv("DATAROBOT_API_TOKEN", "") + + viperx.Reset() + viperx.Set(config.DataRobotURL, "") + viperx.Set(config.DataRobotAPIKey, "") + t.Cleanup(viperx.Reset) + + var buf bytes.Buffer + + valid := checkCLICredentials(&buf) + + require.False(t, valid) + assert.Contains(t, buf.String(), "No valid API key found") + assert.NotContains(t, buf.String(), "missing URL scheme") +} + // The '.env' leg used to blame the token for every non-200; now 401 blames the // token, 403 reports lacking access, and other statuses blame the instance. func TestVerifyDotenvToken_ClassifiesStatus(t *testing.T) { From fac5234b47d3a70e9e8df9a7038d28107ee4219a Mon Sep 17 00:00:00 2001 From: chas Date: Thu, 10 Sep 2026 18:59:50 -0400 Subject: [PATCH 4/5] [CFX-7608] Fix the ReportUnjudged docstring for the 403 return The old contract said false means the instance rejected the credentials, but a 403 is a verdict that now returns true to suppress the login relaunch. Restate what true and false mean so a caller does not read true as unjudged-only. --- internal/auth/auth.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 6454aa6e1..981805b4c 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -175,8 +175,8 @@ func ReportEnvCredentialsError(w io.Writer, creds *EnvCredentials, err error) { // It avoids naming dr auth set-url, since DATAROBOT_CLI_ENDPOINT can override the file. const StoredEndpointName = "the configured DataRobot endpoint" -// ReportUnjudged explains a verification failure that produced no verdict on the -// credentials and reports whether it did. False means the instance rejected them. +// ReportUnjudged writes a diagnostic and reports whether to suppress the login flow: +// true for an unjudged failure or a 403 (credentials valid, account lacks access). func ReportUnjudged(w io.Writer, endpoint, endpointName string, err error) bool { base, info := writerStyles(w) From 5f65ef3edf0c9fce802727b0c0b293684725b27e Mon Sep 17 00:00:00 2001 From: AJ Alon Date: Wed, 16 Sep 2026 07:44:34 -0700 Subject: [PATCH 5/5] Update cmd/auth/check/cmd_test.go --- cmd/auth/check/cmd_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/auth/check/cmd_test.go b/cmd/auth/check/cmd_test.go index 90fae05a9..8a59d1f4f 100644 --- a/cmd/auth/check/cmd_test.go +++ b/cmd/auth/check/cmd_test.go @@ -54,6 +54,7 @@ func TestCheckCLICredentials_ClassifiesStoredProfileStatus(t *testing.T) { }{ {"401 blames the token", http.StatusUnauthorized, "No valid API key found", ""}, {"403 reports lacking access", http.StatusForbidden, "lacks API access", "No valid API key found"}, + {"404 blames the instance", http.StatusNotFound, "answered HTTP 404", "No valid API key found"}, {"503 blames the instance", http.StatusServiceUnavailable, "answered HTTP 503", "No valid API key found"}, }