2023-01-06 15:15:57 +00:00
|
|
|
package auth
|
|
|
|
|
|
|
|
import (
|
|
|
|
"context"
|
2023-11-23 09:04:54 +00:00
|
|
|
"errors"
|
2023-06-21 10:58:28 +00:00
|
|
|
"fmt"
|
2024-04-18 11:55:42 +00:00
|
|
|
"runtime"
|
2023-01-06 15:15:57 +00:00
|
|
|
"time"
|
|
|
|
|
2023-05-16 16:35:39 +00:00
|
|
|
"github.com/databricks/cli/libs/auth"
|
2023-06-21 10:58:28 +00:00
|
|
|
"github.com/databricks/cli/libs/cmdio"
|
|
|
|
"github.com/databricks/cli/libs/databrickscfg"
|
2023-11-09 16:38:45 +00:00
|
|
|
"github.com/databricks/cli/libs/databrickscfg/cfgpickers"
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
"github.com/databricks/cli/libs/databrickscfg/profile"
|
2023-06-21 15:51:59 +00:00
|
|
|
"github.com/databricks/databricks-sdk-go"
|
2023-06-21 10:58:28 +00:00
|
|
|
"github.com/databricks/databricks-sdk-go/config"
|
2023-01-06 15:15:57 +00:00
|
|
|
"github.com/spf13/cobra"
|
|
|
|
)
|
|
|
|
|
2024-08-14 13:01:00 +00:00
|
|
|
func promptForProfile(ctx context.Context, defaultValue string) (string, error) {
|
|
|
|
if !cmdio.IsInTTY(ctx) {
|
2024-09-04 07:14:21 +00:00
|
|
|
return "", nil
|
2023-07-12 15:36:09 +00:00
|
|
|
}
|
|
|
|
|
2024-08-14 13:01:00 +00:00
|
|
|
prompt := cmdio.Prompt(ctx)
|
|
|
|
prompt.Default = defaultValue
|
2025-03-09 20:17:56 +00:00
|
|
|
if defaultValue == "" {
|
|
|
|
defaultValue = "DEFAULT"
|
|
|
|
}
|
|
|
|
prompt.Label = fmt.Sprintf("Databricks profile name [%s]", defaultValue)
|
2024-08-14 13:01:00 +00:00
|
|
|
prompt.AllowEdit = true
|
|
|
|
return prompt.Run()
|
2023-07-12 15:36:09 +00:00
|
|
|
}
|
|
|
|
|
2023-11-09 16:38:45 +00:00
|
|
|
const (
|
|
|
|
minimalDbConnectVersion = "13.1"
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
defaultTimeout = 1 * time.Hour
|
2024-12-12 09:28:42 +00:00
|
|
|
)
|
2023-11-09 16:38:45 +00:00
|
|
|
|
2025-03-09 20:17:56 +00:00
|
|
|
func newLoginCommand(hidden bool, persistentAuth *auth.PersistentAuth) *cobra.Command {
|
2024-04-18 11:55:42 +00:00
|
|
|
defaultConfigPath := "~/.databrickscfg"
|
|
|
|
if runtime.GOOS == "windows" {
|
|
|
|
defaultConfigPath = "%USERPROFILE%\\.databrickscfg"
|
|
|
|
}
|
2023-07-27 10:03:08 +00:00
|
|
|
cmd := &cobra.Command{
|
2025-03-09 20:17:56 +00:00
|
|
|
Use: "login [HOST]",
|
|
|
|
Hidden: hidden,
|
|
|
|
Short: "Log into a Databricks workspace or account",
|
2024-04-18 11:55:42 +00:00
|
|
|
Long: fmt.Sprintf(`Log into a Databricks workspace or account.
|
|
|
|
This command logs you into the Databricks workspace or account and saves
|
|
|
|
the authentication configuration in a profile (in %s by default).
|
|
|
|
|
|
|
|
This profile can then be used to authenticate other Databricks CLI commands by
|
|
|
|
specifying the --profile flag. This profile can also be used to authenticate
|
|
|
|
other Databricks tooling that supports the Databricks Unified Authentication
|
|
|
|
Specification. This includes the Databricks Go, Python, and Java SDKs. For more information,
|
|
|
|
you can refer to the documentation linked below.
|
|
|
|
AWS: https://docs.databricks.com/dev-tools/auth/index.html
|
|
|
|
Azure: https://learn.microsoft.com/azure/databricks/dev-tools/auth
|
|
|
|
GCP: https://docs.gcp.databricks.com/dev-tools/auth/index.html
|
|
|
|
|
|
|
|
|
|
|
|
This command requires a Databricks Host URL (using --host or as a positional argument
|
|
|
|
or implicitly inferred from the specified profile name)
|
|
|
|
and a profile name (using --profile) to be specified. If you don't specify these
|
|
|
|
values, you'll be prompted for values at runtime.
|
|
|
|
|
|
|
|
While this command always logs you into the specified host, the runtime behaviour
|
|
|
|
depends on the existing profiles you have set in your configuration file
|
|
|
|
(at %s by default).
|
|
|
|
|
|
|
|
1. If a profile with the specified name exists and specifies a host, you'll
|
|
|
|
be logged into the host specified by the profile. The profile will be updated
|
|
|
|
to use "databricks-cli" as the auth type if that was not the case before.
|
|
|
|
|
|
|
|
2. If a profile with the specified name exists but does not specify a host,
|
|
|
|
you'll be prompted to specify a host. The profile will be updated to use the
|
|
|
|
specified host. The auth type will be updated to "databricks-cli" if that was
|
|
|
|
not the case before.
|
|
|
|
|
|
|
|
3. If a profile with the specified name exists and specifies a host, but you
|
|
|
|
specify a host using --host (or as the [HOST] positional arg), the profile will
|
|
|
|
be updated to use the newly specified host. The auth type will be updated to
|
|
|
|
"databricks-cli" if that was not the case before.
|
|
|
|
|
|
|
|
4. If a profile with the specified name does not exist, a new profile will be
|
|
|
|
created with the specified host. The auth type will be set to "databricks-cli".
|
|
|
|
`, defaultConfigPath, defaultConfigPath),
|
2023-07-27 10:03:08 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
var loginTimeout time.Duration
|
|
|
|
var configureCluster bool
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
cmd.Flags().DurationVar(&loginTimeout, "timeout", defaultTimeout,
|
2023-07-27 10:03:08 +00:00
|
|
|
"Timeout for completing login challenge in the browser")
|
|
|
|
cmd.Flags().BoolVar(&configureCluster, "configure-cluster", false,
|
|
|
|
"Prompts to configure cluster")
|
|
|
|
|
|
|
|
cmd.RunE = func(cmd *cobra.Command, args []string) error {
|
2023-07-12 15:36:09 +00:00
|
|
|
ctx := cmd.Context()
|
2024-08-14 13:01:00 +00:00
|
|
|
profileName := cmd.Flag("profile").Value.String()
|
2023-08-02 09:43:42 +00:00
|
|
|
|
2024-08-14 13:01:00 +00:00
|
|
|
// If the user has not specified a profile name, prompt for one.
|
|
|
|
if profileName == "" {
|
|
|
|
var err error
|
|
|
|
profileName, err = promptForProfile(ctx, persistentAuth.ProfileName())
|
2023-08-02 09:43:42 +00:00
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2024-08-14 13:01:00 +00:00
|
|
|
// Set the host and account-id based on the provided arguments and flags.
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
err := setHostAndAccountId(ctx, profileName, persistentAuth, args)
|
2023-08-02 09:43:42 +00:00
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
2023-07-12 15:36:09 +00:00
|
|
|
defer persistentAuth.Close()
|
|
|
|
|
|
|
|
// We need the config without the profile before it's used to initialise new workspace client below.
|
|
|
|
// Otherwise it will complain about non existing profile because it was not yet saved.
|
|
|
|
cfg := config.Config{
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
Host: persistentAuth.Host,
|
|
|
|
AccountID: persistentAuth.AccountID,
|
|
|
|
AuthType: "databricks-cli",
|
2023-07-12 15:36:09 +00:00
|
|
|
}
|
2023-06-21 15:51:59 +00:00
|
|
|
|
2023-07-12 15:36:09 +00:00
|
|
|
ctx, cancel := context.WithTimeout(ctx, loginTimeout)
|
2023-01-06 15:15:57 +00:00
|
|
|
defer cancel()
|
2023-06-21 10:58:28 +00:00
|
|
|
|
2023-08-02 09:43:42 +00:00
|
|
|
err = persistentAuth.Challenge(ctx)
|
2023-06-21 10:58:28 +00:00
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
|
|
|
|
2023-06-21 15:51:59 +00:00
|
|
|
if configureCluster {
|
|
|
|
w, err := databricks.NewWorkspaceClient((*databricks.Config)(&cfg))
|
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
|
|
|
ctx := cmd.Context()
|
2023-11-09 16:38:45 +00:00
|
|
|
clusterID, err := cfgpickers.AskForCluster(ctx, w,
|
|
|
|
cfgpickers.WithDatabricksConnect(minimalDbConnectVersion))
|
2023-06-21 15:51:59 +00:00
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
2023-11-09 16:38:45 +00:00
|
|
|
cfg.ClusterID = clusterID
|
2023-06-21 15:51:59 +00:00
|
|
|
}
|
2023-06-21 10:58:28 +00:00
|
|
|
|
2023-09-21 12:38:45 +00:00
|
|
|
if profileName != "" {
|
2023-11-10 14:03:57 +00:00
|
|
|
err = databrickscfg.SaveToProfile(ctx, &config.Config{
|
|
|
|
Profile: profileName,
|
|
|
|
Host: cfg.Host,
|
|
|
|
AuthType: cfg.AuthType,
|
|
|
|
AccountID: cfg.AccountID,
|
|
|
|
ClusterID: cfg.ClusterID,
|
|
|
|
})
|
2023-09-21 12:38:45 +00:00
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
|
|
|
|
|
|
|
cmdio.LogString(ctx, fmt.Sprintf("Profile %s was successfully saved", profileName))
|
2023-06-21 10:58:28 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
return nil
|
2023-07-27 10:03:08 +00:00
|
|
|
}
|
2023-06-21 15:51:59 +00:00
|
|
|
|
2023-07-27 10:03:08 +00:00
|
|
|
return cmd
|
2023-01-06 15:15:57 +00:00
|
|
|
}
|
2023-10-11 11:12:18 +00:00
|
|
|
|
2024-08-14 13:01:00 +00:00
|
|
|
// Sets the host in the persistentAuth object based on the provided arguments and flags.
|
|
|
|
// Follows the following precedence:
|
|
|
|
// 1. [HOST] (first positional argument) or --host flag. Error if both are specified.
|
|
|
|
// 2. Profile host, if available.
|
|
|
|
// 3. Prompt the user for the host.
|
|
|
|
//
|
|
|
|
// Set the account in the persistentAuth object based on the flags.
|
|
|
|
// Follows the following precedence:
|
|
|
|
// 1. --account-id flag.
|
|
|
|
// 2. account-id from the specified profile, if available.
|
|
|
|
// 3. Prompt the user for the account-id.
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
func setHostAndAccountId(ctx context.Context, profileName string, persistentAuth *auth.PersistentAuth, args []string) error {
|
2024-08-14 13:01:00 +00:00
|
|
|
// If both [HOST] and --host are provided, return an error.
|
|
|
|
if len(args) > 0 && persistentAuth.Host != "" {
|
2025-01-07 10:49:23 +00:00
|
|
|
return errors.New("please only provide a host as an argument or a flag, not both")
|
2024-08-14 13:01:00 +00:00
|
|
|
}
|
|
|
|
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
profiler := profile.GetProfiler(ctx)
|
2023-10-11 11:12:18 +00:00
|
|
|
// If the chosen profile has a hostname and the user hasn't specified a host, infer the host from the profile.
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
profiles, err := profiler.LoadProfiles(ctx, profile.WithName(profileName))
|
2023-11-23 09:04:54 +00:00
|
|
|
// Tolerate ErrNoConfiguration here, as we will write out a configuration as part of the login flow.
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
if err != nil && !errors.Is(err, profile.ErrNoConfiguration) {
|
2023-10-11 11:12:18 +00:00
|
|
|
return err
|
|
|
|
}
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
|
2023-10-11 11:12:18 +00:00
|
|
|
if persistentAuth.Host == "" {
|
2024-08-14 13:01:00 +00:00
|
|
|
if len(args) > 0 {
|
|
|
|
// If [HOST] is provided, set the host to the provided positional argument.
|
|
|
|
persistentAuth.Host = args[0]
|
|
|
|
} else if len(profiles) > 0 && profiles[0].Host != "" {
|
|
|
|
// If neither [HOST] nor --host are provided, and the profile has a host, use it.
|
2023-10-11 11:12:18 +00:00
|
|
|
persistentAuth.Host = profiles[0].Host
|
|
|
|
} else {
|
2024-08-14 13:01:00 +00:00
|
|
|
// If neither [HOST] nor --host are provided, and the profile does not have a host,
|
|
|
|
// then prompt the user for a host.
|
|
|
|
hostName, err := promptForHost(ctx)
|
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
|
|
|
persistentAuth.Host = hostName
|
2023-10-11 11:12:18 +00:00
|
|
|
}
|
|
|
|
}
|
2024-08-14 13:01:00 +00:00
|
|
|
|
|
|
|
// If the account-id was not provided as a cmd line flag, try to read it from
|
|
|
|
// the specified profile.
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
isAccountClient := (&config.Config{Host: persistentAuth.Host}).IsAccountClient()
|
|
|
|
if isAccountClient && persistentAuth.AccountID == "" {
|
|
|
|
if len(profiles) > 0 && profiles[0].AccountID != "" {
|
|
|
|
persistentAuth.AccountID = profiles[0].AccountID
|
|
|
|
} else {
|
2024-08-14 13:01:00 +00:00
|
|
|
// Prompt user for the account-id if it we could not get it from a
|
|
|
|
// profile.
|
Improve token refresh flow (#1434)
## Changes
Currently, there are a number of issues with the non-happy-path flows
for token refresh in the CLI.
If the token refresh fails, the raw error message is presented to the
user, as seen below. This message is very difficult for users to
interpret and doesn't give any clear direction on how to resolve this
issue.
```
Error: token refresh: Post "https://adb-<WSID>.azuredatabricks.net/oidc/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"}
```
When logging in again, I've noticed that the timeout for logging in is
very short, only 45 seconds. If a user is using a password manager and
needs to login to that first, or needs to do MFA, 45 seconds may not be
enough time. to an account-level profile, it is quite frustrating for
users to need to re-enter account ID information when that information
is already stored in the user's `.databrickscfg` file.
This PR tackles these two issues. First, the presentation of error
messages from `databricks auth token` is improved substantially by
converting the `error` into a human-readable message. When the refresh
token is invalid, it will present a command for the user to run to
reauthenticate. If the token fetching failed for some other reason, that
reason will be presented in a nice way, providing front-line debugging
steps and ultimately redirecting users to file a ticket at this repo if
they can't resolve the issue themselves. After this PR, the new error
message is:
```
Error: a new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run `.databricks/databricks auth login --host https://adb-<WSID>.azuredatabricks.net`
```
To improve the login flow, this PR modifies `databricks auth login` to
auto-complete the account ID from the profile when present.
Additionally, it increases the login timeout from 45 seconds to 1 hour
to give the user sufficient time to login as needed.
To test this change, I needed to refactor some components of the CLI
around profile management, the token cache, and the API client used to
fetch OAuth tokens. These are now settable in the context, and a
demonstration of how they can be set and used is found in
`auth_test.go`.
Separately, this also demonstrates a sort-of integration test of the CLI
by executing the Cobra command for `databricks auth token` from tests,
which may be useful for testing other end-to-end functionality in the
CLI. In particular, I believe this is necessary in order to set flag
values (like the `--profile` flag in this case) for use in testing.
## Tests
Unit tests cover the unhappy and happy paths using the mocked API
client, token cache, and profiler.
Manually tested
---------
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
2024-05-16 10:22:09 +00:00
|
|
|
accountId, err := promptForAccountID(ctx)
|
|
|
|
if err != nil {
|
|
|
|
return err
|
|
|
|
}
|
|
|
|
persistentAuth.AccountID = accountId
|
|
|
|
}
|
|
|
|
}
|
2023-10-11 11:12:18 +00:00
|
|
|
return nil
|
|
|
|
}
|