Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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