Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 46 additions & 53 deletions cmd/auth/check/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -66,11 +67,9 @@ 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 {
Comment thread
cursor[bot] marked this conversation as resolved.
// 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"))
Expand All @@ -85,31 +84,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) {
Expand Down Expand Up @@ -141,30 +140,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
Expand All @@ -173,40 +166,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
}
Expand All @@ -223,7 +216,7 @@ func RunE(_ *cobra.Command, _ []string) error {
return cli.ErrSilent
}

if checkDotenvCredentials(repoRoot) {
if checkDotenvCredentials(os.Stdout, repoRoot) {
return nil
}

Expand Down
120 changes: 96 additions & 24 deletions cmd/auth/check/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -43,32 +43,104 @@ 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
// 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"},
{"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"},
Comment thread
ajalon1 marked this conversation as resolved.
}

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)
}
})
}
}

valid := verifyDotenvToken(server.URL+"/api/v2", "expired-token")
// 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", "")

os.Stdout = orig
viperx.Reset()
viperx.Set(config.DataRobotURL, "")
viperx.Set(config.DataRobotAPIKey, "")
t.Cleanup(viperx.Reset)

require.NoError(t, w.Close())
var buf bytes.Buffer

out, err := io.ReadAll(r)
require.NoError(t, err)
valid := checkCLICredentials(&buf)

require.False(t, valid)
assert.Contains(t, string(out), "DATAROBOT_API_TOKEN in '.env' is invalid or expired")
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) {
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)
}
})
}
}
7 changes: 6 additions & 1 deletion docs/commands/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -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://
Expand Down
4 changes: 2 additions & 2 deletions docs/development/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -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, **including over an active named profile** (`--profile`/`DATAROBOT_CLI_PROFILE`) — the same rule, applied consistently. 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 credentials 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, **including over an active named profile** (`--profile`/`DATAROBOT_CLI_PROFILE`) — the same rule, applied consistently. 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 credentials 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
Expand Down
22 changes: 15 additions & 7 deletions internal/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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.",
Expand Down
Loading
Loading