Repository navigation
Add a non-interactive device code login - #7220
Conversation
|
Commit: Regression test reportSelected 6 changed tests (limit=20, 0 not selected) 6 tests: regression=6 unreleased=0 coverage=0 failing=0
TestAccept/cmd/auth/login/device-code/non-interactive/concurrent: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌main (3aa5c96): latest (v1.23.0): TestAccept/cmd/auth/login/device-code/non-interactive/errors: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌main (3aa5c96): latest (v1.23.0): TestAccept/cmd/auth/login/device-code/non-interactive/flags: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌main (3aa5c96): latest (v1.23.0): same output as main (3aa5c96) TestAccept/cmd/auth/login/device-code/non-interactive/nominal: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌main (3aa5c96): latest (v1.23.0): same output as main (3aa5c96) TestAccept/cmd/auth/login/device-code/unsupported-host: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌main (3aa5c96): latest (v1.23.0): same output as main (3aa5c96) TestAccept/cmd/auth/logout/pending-device-code-login: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌main (3aa5c96): latest (v1.23.0): same output as main (3aa5c96) |
Integration test reportCommit: 275c55c
Top 4 slowest tests (at least 2 minutes):
|
4ff0b1e to
1b2be13
Compare
1b2be13 to
26349d5
Compare
26349d5 to
90d5979
Compare
cf4b0e9 to
adfe6a4
Compare
adfe6a4 to
b80d829
Compare
b80d829 to
64abb91
Compare
NewDeviceCodeLogin now takes a context. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
A pending login was a token store entry under the cache key with a #device-code suffix, which a profile can also be named. A pending login for profile a then replaced the token of profile a#device-code, auth token for that profile failed with a nil pointer dereference, and logging out of a deleted it. The stores now keep pending logins in their own namespace through a new PendingLoginStore interface, under the plain cache key: the pending_logins map of the token cache file, the databricks-cli-pending-login keyring service, and a separate map in the memory store. Entry holds a token again, and deleting a missing pending login no longer rewrites the token cache file on every login and logout. Co-authored-by: Isaac <no-reply@databricks.com>
The second step of the non-interactive login used the host, --client-id and --scopes of its own command. The token gets the scopes of the device authorization, so a second step without the --scopes of the first saved a profile whose scopes did not match its token, and a different host or client sent the device code to a server or for a client that did not issue it. The pending login now stores the host, client ID and scopes, and the second step fails before the token exchange if any of them differs. Co-authored-by: Isaac <no-reply@databricks.com>
The second step of the non-interactive login asked for the workspace when it ran in a terminal and the host had an account ID but no workspace ID, although neither step is meant to prompt. It now shows the hint that a login without a terminal shows instead, and saves the profile without a workspace ID. Co-authored-by: Isaac <no-reply@databricks.com>
This reverts commit 1af032a. Pending logins are token store entries under the cache key with a #device-code suffix again, and keep the host, client ID and scopes that a later commit added. The README describes the settings check in its own paragraph. Co-authored-by: Isaac <no-reply@databricks.com>
…login" This reverts commit bda3307. The picker only prompted when the second step ran in a terminal for a host whose metadata has an account ID but no workspace ID, and the prompt can be skipped, so the extra branch is not worth it. Co-authored-by: Isaac <no-reply@databricks.com>
A pending device code login is stored under the cache key with a #device-code suffix, which a profile can also be named. Looking up the token of such a profile found the pending login, and auth token failed with a nil pointer dereference when it read the refresh token. The entry is now reported as storage.ErrNotFound. Co-authored-by: Isaac <no-reply@databricks.com>
The pending device code login of profile a is stored under the token store key a#device-code, which is also the key of the token of profile a#device-code. A login for a then replaced that token, and every login and logout of a deleted it. auth login now rejects such profile names, both at the profile name prompt and when they are passed with --profile. Co-authored-by: Isaac <no-reply@databricks.com>
The server answers further token requests for a device code with HTTP 429 after too many confirmation codes were sent for it. The second step of the non-interactive login kept the pending login and returned the error without a hint, so rerunning the command got the same error. The token exchange error now matches ErrDeviceCodeRateLimited, and the second step deletes the pending login and tells how to start a new one, as it does for an invalid device code. Co-authored-by: Isaac <no-reply@databricks.com>
The second step of the non-interactive login found the pending login with the cache key, which uses the canonical host, but compared the host it was started with as passed. Starting the login with adb-1.azuredatabricks.net and completing it with https://adb-1.azuredatabricks.net failed with "was started with a different host". The login now keeps the canonical host. Co-authored-by: Isaac <no-reply@databricks.com>
The second step of the non-interactive login said that a newer login replaced the pending one whenever --user-code did not match it, also when the user code was mistyped or swapped with the confirmation code. The error now says that the user code does not match, and mentions a newer login as one possible cause. Co-authored-by: Isaac <no-reply@databricks.com>
The second step can still show the workspace picker when it runs in a terminal, so the comment no longer says that neither step prompts at all. Co-authored-by: Isaac <no-reply@databricks.com>
Every successful login now deletes the key of its pending device code login, which is missing unless a non-interactive device code login was started. Deleting a missing key rewrote token-cache.json anyway, so logins without device code wrote the file one more time. Deleting a missing key now leaves the file as it is. Co-authored-by: Isaac <no-reply@databricks.com>
auth logout only cleared the token cache for profiles with auth_type databricks-cli, so a device code login started for another profile, such as a PAT profile, survived the logout and could still complete. The docs say that logout removes the profile's pending login. Logout now deletes it for every profile, and still leaves the tokens of other profiles alone. Co-authored-by: Isaac <no-reply@databricks.com>
The memory store copied the PendingLogin struct, but its Scopes slice still shared the backing array with the caller, so changing the caller's slice after Put changed the stored entry. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
b6cd53d to
33e396a
Compare
The logout of a profile that auth login did not create, e.g. a PAT profile, failed when the token store could not delete its pending device code login, e.g. with an unavailable keyring, and --delete then kept the profile. Such a profile does not use the token store otherwise, so the error is now only logged. Co-authored-by: Isaac <no-reply@databricks.com>
Completing a non-interactive device code login deleted the pending login when the server rate limited the token request, so an approved login had to be started again. A rate limit can be lifted, so the pending login is now kept, and the error asks to rerun the command later. Co-authored-by: Isaac <no-reply@databricks.com>
Completing a non-interactive device code login failed with "started with different scopes" when --scopes listed the same scopes in another order than the first step. The scopes are now compared as a set. Co-authored-by: Isaac <no-reply@databricks.com>
Every successful login, including the browser login, deleted the pending device code login of its profile or host, which added a token store write, and a keyring call in secure mode, to every login. The pending login is now deleted when its device code is exchanged, because the server no longer accepts the device code afterwards. Other logins for the same target keep it. Co-authored-by: Isaac <no-reply@databricks.com>
The fake server issued the same user code for every login, so the acceptance tests edited token-cache.json with update_file.py to tell logins apart. It now issues WDJB0001, WDJB0002, ... in order, like the real server issues a new user code for every login, and the tests use the codes it issues. The test of a device code the server no longer knows restores the pending login from before its device code was exchanged, instead of replacing the device code in the cache file. Co-authored-by: Isaac <no-reply@databricks.com>
- Sort the scopes once when the login is set up, instead of sorting both lists in every comparison. - Read --use-device-code from the command in validateDeviceCodeCompletionFlags instead of passing it in. Co-authored-by: Isaac <no-reply@databricks.com>
simonfaltum
left a comment
There was a problem hiding this comment.
No blocking findings from this pass. Reviewed the two-step login, token storage, logout cleanup, and previous review feedback. Focused auth unit tests and all eight device-code/logout acceptance tests pass; CI is green.
The legacy profile-name collision discussed in the earlier thread remains a nonblocking edge case: pending-login operations can still affect an existing profile whose name ends in #device-code. The new validation prevents future logins under those names.
Changes
When stdin is not a terminal,
auth login --use-device-coderuns in two steps. The first step stores the device code as a pending login and prints the URL and the user code. Running the same command with the hidden--user-codeand--confirmation-codeflags exchanges the device code and saves the login. In a terminal, the login stays interactive.A new login for the same profile or host replaces its pending login. The second step checks the user code, and the host, client ID and scopes the login was started with.
auth logoutalso deletes pending logins. Everyauth login, not only the device code login, now rejects a profile name that ends with#device-code, because the token of that profile would share its key with the pending login of another profile.The pending login is a new
PendingLoginfield ofstorage.Entry, stored under the cache key with a#device-codesuffix. Piped stdin no longer completes the login: it starts the two-step login instead.Why
An AI agent or a script that runs the login as a one-off command cannot type the confirmation code into a running
auth login.Tests
Acceptance tests under
device-code/non-interactiveandlogout/pending-device-code-login, unit tests for the stores andIsInputTTY, andtermtesttests for the second step in a terminal. Thedevice-code/nominalanddevice-code/wrong-codeacceptance tests piped the code into stdin, so they move totermtesttests.No changelog fragment:
--use-device-codeis still behindDATABRICKS_CLI_DEVICE_CODE.This pull request and its description were written by Isaac.