Skip to content
Open
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
1 change: 1 addition & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Notable Changes

### CLI
* Auth commands now error when --profile and --host conflict ([#4841](https://github.com/databricks/cli/pull/4841))

### Bundles
* engine/direct: Fix drift in grants resource due to privilege reordering ([#4794](https://github.com/databricks/cli/pull/4794))
Expand Down

This file was deleted.

16 changes: 3 additions & 13 deletions acceptance/cmd/auth/login/custom-config-file/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,8 @@ host = https://old-host.cloud.databricks.com
auth_type = pat
token = old-token-123

=== Login with new host (should override old host)
=== Login with conflicting host (should error)
>>> [CLI] auth login --host [DATABRICKS_URL] --profile custom-test
Profile custom-test was successfully saved
Error: --profile "custom-test" has host "https://old-host.cloud.databricks.com", which conflicts with --host "[DATABRICKS_URL]". Use --profile only to select a profile

=== Default config file should NOT exist
OK: Default .databrickscfg does not exist

=== Custom config file after login (old host should be replaced)
; The profile defined in the DEFAULT section is to be used as a fallback when no profile is explicitly specified.
[DEFAULT]

[custom-test]
host = [DATABRICKS_URL]
auth_type = databricks-cli
workspace_id = [NUMID]
Exit code: 1
20 changes: 2 additions & 18 deletions acceptance/cmd/auth/login/custom-config-file/script
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ export BROWSER="browser.py"
export DATABRICKS_CONFIG_FILE="./home/custom.databrickscfg"

# Create an existing custom config file with a DIFFERENT host.
# The login command should use the host from --host argument, NOT from this file.
# If the wrong host (from the config file) were used, the OAuth flow would fail
# because the mock server only responds for $DATABRICKS_HOST.
# Since --profile and --host conflict, the CLI should error.
cat > "./home/custom.databrickscfg" <<EOF
[custom-test]
host = https://old-host.cloud.databricks.com
Expand All @@ -21,19 +19,5 @@ EOF
title "Initial custom config file (with old host)\n"
cat "./home/custom.databrickscfg"

title "Login with new host (should override old host)"
title "Login with conflicting host (should error)"
trace $CLI auth login --host $DATABRICKS_HOST --profile custom-test

title "Default config file should NOT exist\n"
if [ -f "./home/.databrickscfg" ]; then
echo "ERROR: Default .databrickscfg exists but should not"
cat "./home/.databrickscfg"
else
echo "OK: Default .databrickscfg does not exist"
fi

title "Custom config file after login (old host should be replaced)\n"
cat "./home/custom.databrickscfg"

# Track the custom config file to surface changes
mv "./home/custom.databrickscfg" "./out.databrickscfg"
50 changes: 50 additions & 0 deletions cmd/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ package auth
import (
"context"
"errors"
"fmt"

"github.com/databricks/cli/libs/auth"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/databrickscfg/profile"
"github.com/databricks/databricks-sdk-go/config"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -59,3 +62,50 @@ func promptForAccountID(ctx context.Context) (string, error) {
prompt.AllowEdit = true
return prompt.Run()
}

// validateProfileHostConflict checks that --profile and --host don't conflict.
// If the profile's host matches the provided host (after canonicalization),
// the flags are considered compatible. If the profile is not found or has no
// host, the check is skipped (let the downstream command handle it).
func validateProfileHostConflict(ctx context.Context, profileName, host string, profiler profile.Profiler) error {
p, err := loadProfileByName(ctx, profileName, profiler)
if err != nil {
return err
}
if p == nil || p.Host == "" {
return nil
}

profileHost := (&config.Config{Host: p.Host}).CanonicalHostName()
flagHost := (&config.Config{Host: host}).CanonicalHostName()

if profileHost != flagHost {
return fmt.Errorf(
"--profile %q has host %q, which conflicts with --host %q. Use --profile only to select a profile",
profileName, p.Host, host,
)
}
return nil
}

// profileHostConflictCheck is a PreRunE function that validates
// --profile and --host don't conflict.
func profileHostConflictCheck(cmd *cobra.Command, args []string) error {
profileFlag := cmd.Flag("profile")
hostFlag := cmd.Flag("host")

// Only validate when both flags are explicitly set by the user.
if profileFlag == nil || hostFlag == nil {
return nil
}
if !profileFlag.Changed || !hostFlag.Changed {
return nil
}

return validateProfileHostConflict(
cmd.Context(),
profileFlag.Value.String(),
hostFlag.Value.String(),
profile.DefaultProfiler,
)
}
134 changes: 134 additions & 0 deletions cmd/auth/auth_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package auth

import (
"testing"

"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/cmdctx"
"github.com/databricks/cli/libs/databrickscfg/profile"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestValidateProfileHostConflict(t *testing.T) {
profiler := profile.InMemoryProfiler{
Profiles: profile.Profiles{
{Name: "logfood", Host: "https://logfood.cloud.databricks.com"},
{Name: "staging", Host: "https://staging.cloud.databricks.com"},
{Name: "no-host", Host: ""},
},
}

cases := []struct {
name string
profileName string
host string
wantErr string
}{
{
name: "matching hosts are allowed",
profileName: "logfood",
host: "https://logfood.cloud.databricks.com",
},
{
name: "matching hosts with trailing slash",
profileName: "logfood",
host: "https://logfood.cloud.databricks.com/",
},
{
name: "conflicting hosts produce error",
profileName: "logfood",
host: "https://other.cloud.databricks.com",
wantErr: `--profile "logfood" has host "https://logfood.cloud.databricks.com", which conflicts with --host "https://other.cloud.databricks.com". Use --profile only to select a profile`,
},
{
name: "profile not found skips check",
profileName: "nonexistent",
host: "https://any.cloud.databricks.com",
},
{
name: "profile with no host skips check",
profileName: "no-host",
host: "https://any.cloud.databricks.com",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ctx := t.Context()
err := validateProfileHostConflict(ctx, tc.profileName, tc.host, profiler)
if tc.wantErr != "" {
assert.ErrorContains(t, err, tc.wantErr)
} else {
require.NoError(t, err)
}
})
}
}

// TestProfileHostConflictViaCobra verifies that the conflict check runs
// through Cobra's lifecycle (PreRunE on login) and that the root command's
// PersistentPreRunE is NOT shadowed (it initializes logging, IO, user agent).
func TestProfileHostConflictViaCobra(t *testing.T) {
// Point at a config file that has "profile-1" with host https://www.host1.com.
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/.databrickscfg")

ctx := cmdctx.GenerateExecId(t.Context())
cli := root.New(ctx)
cli.AddCommand(New())

// Set args: auth login --profile profile-1 --host https://other.host.com
cli.SetArgs([]string{
"auth", "login",
"--profile", "profile-1",
"--host", "https://other.host.com",
})

_, err := cli.ExecuteContextC(ctx)
require.Error(t, err)
assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.com", which conflicts with --host "https://other.host.com"`)
assert.Contains(t, err.Error(), "Use --profile only to select a profile")
}

// TestProfileHostConflictTokenViaCobra verifies the conflict check on the token subcommand.
func TestProfileHostConflictTokenViaCobra(t *testing.T) {
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/.databrickscfg")

ctx := cmdctx.GenerateExecId(t.Context())
cli := root.New(ctx)
cli.AddCommand(New())

cli.SetArgs([]string{
"auth", "token",
"--profile", "profile-1",
"--host", "https://other.host.com",
})

_, err := cli.ExecuteContextC(ctx)
require.Error(t, err)
assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.com", which conflicts with --host "https://other.host.com"`)
}

// TestProfileHostCompatibleViaCobra verifies that matching --profile and --host
// pass the conflict check (the command will fail later for other reasons, but
// NOT with a conflict error).
func TestProfileHostCompatibleViaCobra(t *testing.T) {
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/.databrickscfg")

ctx := cmdctx.GenerateExecId(t.Context())
cli := root.New(ctx)
cli.AddCommand(New())

cli.SetArgs([]string{
"auth", "login",
"--profile", "profile-1",
"--host", "https://www.host1.com",
})

_, err := cli.ExecuteContextC(ctx)
// The command may fail for other reasons (no browser, non-interactive, etc.)
// but it should NOT fail with a conflict error.
if err != nil {
assert.NotContains(t, err.Error(), "conflicts with --host")
}
}
2 changes: 2 additions & 0 deletions cmd/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ depends on the existing profiles you have set in your configuration file
cmd.Flags().StringVar(&scopes, "scopes", "",
"Comma-separated list of OAuth scopes to request (defaults to 'all-apis')")

cmd.PreRunE = profileHostConflictCheck

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
profileName := cmd.Flag("profile").Value.String()
Expand Down
2 changes: 2 additions & 0 deletions cmd/auth/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ using a client ID and secret is not supported.`,
cmd.Flags().DurationVar(&tokenTimeout, "timeout", defaultTimeout,
"Timeout for acquiring a token.")

cmd.PreRunE = profileHostConflictCheck

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
profileName := ""
Expand Down
Loading