Skip to content

Commit 9da7f7d

Browse files
authored
Respect BROWSER environment variable in "auth login" command (#3659)
## Changes Respect the `BROWSER` environment variable for the OAuth login flow. When set, the value is used as executable to launch the URL to authenticate the user. When set to "none", the CLI prints a message with the URL to open. When not set, it defers to the existing logic to open a browser via the `pkg/browser` package. ## Why Context in: * #3464 * databricks/databricks-vscode#1764 Also refer to issues linked in the description of these issues for broader context, and why `pkg/browser` doesn't respect `BROWSER` itself. It boils down to it not being a ratified standard. We choose to respect it to make the interactive login flow from VS Code running in a dev container work. ## Tests New acceptance test that goes through the login flow via the test server.
1 parent 269ccc4 commit 9da7f7d

File tree

11 files changed

+181
-12
lines changed

11 files changed

+181
-12
lines changed

acceptance/bin/browser.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#!/usr/bin/env python3
2+
"""
3+
This script fetches a URL.
4+
It follows redirects if applicable.
5+
6+
Usage: browser.py <url>
7+
"""
8+
9+
import urllib.request
10+
import sys
11+
12+
if len(sys.argv) < 2:
13+
sys.stderr.write("Usage: browser.py <url>\n")
14+
sys.exit(1)
15+
16+
url = sys.argv[1]
17+
try:
18+
response = urllib.request.urlopen(url)
19+
if response.status != 200:
20+
sys.stderr.write(f"Failed to fetch URL: {url} (status {response.status})\n")
21+
sys.exit(1)
22+
except Exception as e:
23+
sys.stderr.write(f"Failed to fetch URL: {url} ({e})\n")
24+
sys.exit(1)
25+
26+
sys.exit(0)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
; The profile defined in the DEFAULT section is to be used as a fallback when no profile is explicitly specified.
2+
[DEFAULT]
3+
4+
[test]
5+
host = [DATABRICKS_URL]
6+
auth_type = databricks-cli

acceptance/cmd/auth/login/out.test.toml

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
>>> [CLI] auth login --host [DATABRICKS_URL] --profile test
3+
Profile test was successfully saved
4+
5+
>>> [CLI] auth profiles
6+
Name Host Valid
7+
test [DATABRICKS_URL] YES

acceptance/cmd/auth/login/script

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
sethome "./home"
2+
3+
# Use a fake browser that performs a GET on the authorization URL
4+
# and follows the redirect back to localhost.
5+
export BROWSER="browser.py"
6+
7+
trace $CLI auth login --host $DATABRICKS_HOST --profile test
8+
trace $CLI auth profiles
9+
10+
# Track the .databrickscfg file that was created to surface changes.
11+
mv "./home/.databrickscfg" "./out.databrickscfg"
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Ignore = [
2+
"home"
3+
]

acceptance/script.prepare

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,14 @@ envsubst() {
9797
print_telemetry_bool_values() {
9898
jq -r 'select(.path? == "/telemetry-ext") | (.body.protoLogs // [])[] | fromjson | ( (.entry // .) | (.databricks_cli_log.bundle_deploy_event.experimental.bool_values // []) ) | map("\(.key) \(.value)") | .[]' out.requests.txt | sort
9999
}
100+
101+
sethome() {
102+
local home="$1"
103+
mkdir -p "$home"
104+
105+
# For macOS and Linux, use HOME.
106+
export HOME="$home"
107+
108+
# For Windows, use USERPROFILE.
109+
export USERPROFILE="$home"
110+
}

cmd/auth/login.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ import (
1414
"github.com/databricks/cli/libs/databrickscfg"
1515
"github.com/databricks/cli/libs/databrickscfg/cfgpickers"
1616
"github.com/databricks/cli/libs/databrickscfg/profile"
17+
"github.com/databricks/cli/libs/env"
18+
"github.com/databricks/cli/libs/exec"
1719
"github.com/databricks/databricks-sdk-go"
1820
"github.com/databricks/databricks-sdk-go/config"
1921
"github.com/databricks/databricks-sdk-go/credentials/u2m"
22+
browserpkg "github.com/pkg/browser"
2023
"github.com/spf13/cobra"
2124
)
2225

@@ -136,7 +139,11 @@ depends on the existing profiles you have set in your configuration file
136139
if err != nil {
137140
return err
138141
}
139-
persistentAuth, err := u2m.NewPersistentAuth(ctx, u2m.WithOAuthArgument(oauthArgument))
142+
persistentAuthOpts := []u2m.PersistentAuthOption{
143+
u2m.WithOAuthArgument(oauthArgument),
144+
}
145+
persistentAuthOpts = append(persistentAuthOpts, u2m.WithBrowser(getBrowserFunc(cmd)))
146+
persistentAuth, err := u2m.NewPersistentAuth(ctx, persistentAuthOpts...)
140147
if err != nil {
141148
return err
142149
}
@@ -288,3 +295,38 @@ func loadProfileByName(ctx context.Context, profileName string, profiler profile
288295
}
289296
return nil, nil
290297
}
298+
299+
// getBrowserFunc returns a function that opens the given URL in the browser.
300+
// It respects the BROWSER environment variable:
301+
// - empty string: uses the default browser
302+
// - "none": prints the URL to stdout without opening a browser
303+
// - custom command: executes the specified command with the URL as argument
304+
func getBrowserFunc(cmd *cobra.Command) func(url string) error {
305+
browser := env.Get(cmd.Context(), "BROWSER")
306+
switch browser {
307+
case "":
308+
return browserpkg.OpenURL
309+
case "none":
310+
return func(url string) error {
311+
fmt.Fprintf(cmd.OutOrStdout(), "Please open %s in the browser to continue authentication\n", url)
312+
return nil
313+
}
314+
default:
315+
return func(url string) error {
316+
// Run the browser command via a shell.
317+
// It can be a script or a binary and scripts cannot be executed directly on Windows.
318+
e, err := exec.NewCommandExecutor(".")
319+
if err != nil {
320+
return err
321+
}
322+
323+
e.WithInheritOutput()
324+
cmd, err := e.StartCommand(cmd.Context(), fmt.Sprintf("%q %q", browser, url))
325+
if err != nil {
326+
return err
327+
}
328+
329+
return cmd.Wait()
330+
}
331+
}
332+
}

libs/testserver/fake_oidc.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package testserver
2+
3+
import (
4+
"net/http"
5+
"net/url"
6+
)
7+
8+
// FakeOidc holds OAuth state for acceptance tests.
9+
type FakeOidc struct {
10+
url string
11+
}
12+
13+
func (s *FakeOidc) OidcEndpoints() Response {
14+
return Response{
15+
Body: map[string]string{
16+
"authorization_endpoint": s.url + "/oidc/v1/authorize",
17+
"token_endpoint": s.url + "/oidc/v1/token",
18+
},
19+
}
20+
}
21+
22+
func (s *FakeOidc) OidcAuthorize(req Request) Response {
23+
redirectURI, err := url.Parse(req.URL.Query().Get("redirect_uri"))
24+
if err != nil {
25+
return Response{
26+
StatusCode: http.StatusBadRequest,
27+
Body: err.Error(),
28+
}
29+
}
30+
31+
// Compile query parameters for the redirect URL.
32+
q := make(url.Values)
33+
34+
// Include an opaque authorization code that will be used to exchange for a token.
35+
q.Set("code", "oauth-code")
36+
37+
// Include the state parameter from the original request.
38+
q.Set("state", req.URL.Query().Get("state"))
39+
40+
// Update the redirect URI with the new query parameters.
41+
redirectURI.RawQuery = q.Encode()
42+
43+
return Response{
44+
StatusCode: http.StatusMovedPermanently,
45+
Headers: map[string][]string{
46+
"Location": {redirectURI.String()},
47+
},
48+
}
49+
}
50+
51+
func (s *FakeOidc) OidcToken(req Request) Response {
52+
return Response{
53+
Body: map[string]string{
54+
"access_token": "oauth-token",
55+
"expires_in": "3600",
56+
"scope": "all-apis",
57+
"token_type": "Bearer",
58+
},
59+
}
60+
}

libs/testserver/handlers.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,15 @@ func AddDefaultHandlers(server *Server) {
204204
})
205205

206206
server.Handle("GET", "/oidc/.well-known/oauth-authorization-server", func(_ Request) any {
207-
return map[string]string{
208-
"authorization_endpoint": server.URL + "oidc/v1/authorize",
209-
"token_endpoint": server.URL + "/oidc/v1/token",
210-
}
207+
return server.fakeOidc.OidcEndpoints()
211208
})
212209

213-
server.Handle("POST", "/oidc/v1/token", func(_ Request) any {
214-
return map[string]string{
215-
"access_token": "oauth-token",
216-
"expires_in": "3600",
217-
"scope": "all-apis",
218-
"token_type": "Bearer",
219-
}
210+
server.Handle("GET", "/oidc/v1/authorize", func(req Request) any {
211+
return server.fakeOidc.OidcAuthorize(req)
212+
})
213+
214+
server.Handle("POST", "/oidc/v1/token", func(req Request) any {
215+
return server.fakeOidc.OidcToken(req)
220216
})
221217

222218
server.Handle("POST", "/telemetry-ext", func(_ Request) any {

0 commit comments

Comments
 (0)