Skip to content

Commit 64e4c2d

Browse files
maksim-zuev-dbIsaac
andauthored
Reject an invalid host in auth login and auth token (#7195)
## Changes `databricks auth login` and `databricks auth token` reject a host that is not an http(s) URL with a hostname, e.g. `/`, `?o=123`, `https://` or `ftp://x.test`, with `invalid host "<input>". Expected a URL like https://<databricks-instance>.cloud.databricks.com`. The check covers `--host`, the positional argument, profile hosts and the host prompt, which shows the error and asks again. `auth login` runs it before the keyring probe and the profile name prompt. Behavior changes: - An empty host after "Enter a host URL manually" in the login profile picker no longer starts discovery login. - `auth login --skip-workspace --host '?a=123'` fails. On main this started discovery login. - Spaces around a host are trimmed, so `" https://x.test/?o=1 "` works and keeps the workspace ID. ## Why Follow-up to a review comment on #7179. An invalid host made login and token resolve the host from `DATABRICKS_HOST`, save a profile without a `host` key, connect to a host named `https`, or fail later with `host must start with 'https://': `. ## Tests Unit tests cover host validation for each host source, the host prompt, and the early rejection in browser, device code and `--skip-workspace` login. New acceptance tests check the error output of `auth login` and `auth token`. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 4b902e8 commit 64e4c2d

17 files changed

Lines changed: 326 additions & 47 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* `databricks auth login` and `databricks auth token` now reject a host that is not a URL with a hostname, such as `/`, `?o=123` or `https://`, instead of falling back to `DATABRICKS_HOST` or failing later. An empty host after "Enter a host URL manually" in the login profile picker no longer starts discovery login. ([#7195](https://github.com/databricks/cli/pull/7195))
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Cloud = false
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
>>> musterr [CLI] auth login --host ?o=123 --profile test
3+
Error: invalid host "?o=123". Expected a URL like https://<databricks-instance>.cloud.databricks.com
4+
5+
>>> musterr [CLI] auth login --host https:// --profile test
6+
Error: invalid host "https://". Expected a URL like https://<databricks-instance>.cloud.databricks.com
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
sethome "./home"
2+
3+
# A host that is empty without its query params is rejected before login
4+
# can fall back to discovery or DATABRICKS_HOST.
5+
trace musterr $CLI auth login --host '?o=123' --profile test
6+
7+
# A host without a hostname is rejected instead of being read as a host
8+
# named "https".
9+
trace musterr $CLI auth login --host 'https://' --profile test
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Ignore = [
2+
"home"
3+
]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Cloud = false
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
>>> musterr [CLI] auth token --host ?o=123
3+
Error: invalid host "?o=123". Expected a URL like https://<databricks-instance>.cloud.databricks.com
4+
5+
>>> musterr [CLI] auth token --host https://
6+
Error: invalid host "https://". Expected a URL like https://<databricks-instance>.cloud.databricks.com
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
sethome "./home"
2+
3+
unset DATABRICKS_HOST
4+
unset DATABRICKS_TOKEN
5+
unset DATABRICKS_CONFIG_PROFILE
6+
7+
# A host that is empty without its query params is rejected instead of
8+
# falling back to DATABRICKS_HOST.
9+
trace musterr $CLI auth token --host '?o=123'
10+
11+
# A host without a hostname is rejected instead of being read as a host
12+
# named "https".
13+
trace musterr $CLI auth token --host 'https://'
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Ignore = [
2+
"home"
3+
]

‎cmd/auth/auth.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"strings"
78

89
"github.com/databricks/cli/libs/auth"
910
"github.com/databricks/cli/libs/cmdio"
@@ -50,10 +51,20 @@ func promptForHost(ctx context.Context) (string, error) {
5051
// promptui's screenbuf does not account for terminal line wrapping, and a
5152
// long "label + value" line that wraps causes each keystroke to leave a
5253
// stale render on screen instead of overwriting the previous one.
53-
cmdio.LogString(ctx, "Example: https://<databricks-instance>.cloud.databricks.com")
54-
return cmdio.RunPrompt(ctx, cmdio.PromptOptions{
54+
cmdio.LogString(ctx, "Example: "+exampleHost)
55+
result, err := cmdio.RunPrompt(ctx, cmdio.PromptOptions{
5556
Label: "Databricks host",
57+
Validate: func(input string) error {
58+
if strings.TrimSpace(input) == "" {
59+
return errors.New("host is required")
60+
}
61+
// Reject hosts that setHost would reject after the prompt,
62+
// e.g. "/" or "?o=123", while the user can still edit them.
63+
_, err := parseLoginHost(input)
64+
return err
65+
},
5666
})
67+
return strings.TrimSpace(result), err
5768
}
5869

5970
func promptForAccountID(ctx context.Context) (string, error) {

0 commit comments

Comments
 (0)