Repository navigation
Add a non-interactive device code login - #7220
Open
maksim-zuev-db wants to merge 13 commits into
Open
maksim-zuev-db wants to merge 13 commits into
maksim-zuev-db wants to merge 13 commits into
Conversation
Contributor
|
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 (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): TestAccept/cmd/auth/login/device-code/non-interactive/errors: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): TestAccept/cmd/auth/login/device-code/non-interactive/flags: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/cmd/auth/login/device-code/non-interactive/nominal: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/cmd/auth/login/device-code/unsupported-host: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/cmd/auth/logout/pending-device-code-login: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) |
Collaborator
Integration test reportCommit: 71775ba
Top 6 slowest tests (at least 2 minutes):
|
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 28, 2026 14:05
4ff0b1e to
1b2be13
Compare
maksim-zuev-db
changed the base branch from
device-code-split-steps
to
device-code-interactive-tests
October 28, 2026 14:06
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 28, 2026 14:35
1b2be13 to
26349d5
Compare
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 28, 2026 14:45
26349d5 to
90d5979
Compare
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 28, 2026 15:42
cf4b0e9 to
adfe6a4
Compare
maksim-zuev-db
changed the base branch from
device-code-interactive-tests
to
device-code-pending-login-storage
October 28, 2026 15:48
maksim-zuev-db
changed the base branch from
device-code-pending-login-storage
to
device-code-interactive-tests
October 28, 2026 15:57
maksim-zuev-db
requested review from
simonfaltum
and removed request for
simonfaltum
October 28, 2026 16:03
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 29, 2026 12:15
adfe6a4 to
b80d829
Compare
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 29, 2026 12:19
b80d829 to
64abb91
Compare
maksim-zuev-db
changed the base branch from
device-code-interactive-tests
to
device-code-split-steps
October 29, 2026 12:22
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
2 times, most recently
from
October 29, 2026 13:37
fb2c74d to
fa4c726
Compare
When stdin is not a terminal, e.g. when an AI agent runs it, auth login --use-device-code stores the device code as a pending login in the token store and prints the URL and the user code. Rerunning the command with the hidden --user-code and --confirmation-code flags completes the login. The device code stays on the machine, so a confirmation code in an agent's transcript cannot complete the login elsewhere. The pending login is a new PendingLogin field of storage.Entry, and auth logout deletes it together with the token. The flow is picked with the new cmdio.IsInputTTY. The interactive login tests piped the code into stdin, which now starts the non-interactive login, so they move from acceptance tests to the terminal emulator. Co-authored-by: Isaac <no-reply@databricks.com>
The interactive and the non-interactive device code login printed the same lines that show where to approve the login. approvalInstructions returns them for both, so they cannot drift apart. Co-authored-by: Isaac <no-reply@databricks.com>
Only an acceptance test covered the check. The unit test completes a login with the user code of another login, checks that no token request is sent, and then completes it with the right user code in another format. Co-authored-by: Isaac <no-reply@databricks.com>
A pending device code login stayed in the token store after the profile or host logged in another way, e.g. in the browser or with a device code login in a terminal. storeLoginToken, which every login goes through, now deletes the pending login of the same profile or host, so the non-interactive completion no longer deletes it itself. Co-authored-by: Isaac <no-reply@databricks.com>
An empty --user-code failed with "a newer login ... replaced this one", and an empty --confirmation-code was sent to the server. Both now fail before the login with an error that names the empty flag. Co-authored-by: Isaac <no-reply@databricks.com>
TestLoginDeviceCodeWrongConfirmationCode checks three wrong codes and a login without input, and also counts the token requests, so the tests for the same cases are no longer needed. Co-authored-by: Isaac <no-reply@databricks.com>
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>
maksim-zuev-db
force-pushed
the
device-code-agent-login
branch
from
October 29, 2026 15:53
fa4c726 to
bda3307
Compare
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>
simonfaltum
reviewed
Oct 29, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Left a few comments. Nothing blocking from my side. The one I care most about is the #device-code key collision, the rest are small.
| // pendingLoginKeySuffix marks the token store entry that holds a | ||
| // non-interactive device code login until it completes. The entry sits next to | ||
| // the login's token, so that a token that still works is kept until then. | ||
| const pendingLoginKeySuffix = "#device-code" |
Member
There was a problem hiding this comment.
[nice to have] The #device-code suffix shares the keyspace with profile names, and INI accepts [a#device-code] as a section. If profiles a and a#device-code both exist, a non-interactive login for a overwrites the other profile's token, and every successful login or logout of a deletes it. databricks auth token -p 'a#device-code' then panics: loadToken (libs/auth/u2m/persistent_auth.go:303) returns a nil token with no error, and refresh reads oldToken.RefreshToken.
I know 1af032a fixed this and was reverted. At minimum I think loadToken should treat e.Token == nil as ErrNotFound, and login should reject profile names ending in #device-code.
| } | ||
| token, err := login.RequestDeviceAccessToken(ctx, pending.DeviceCode, confirmationCode) | ||
| if err != nil { | ||
| err = fmt.Errorf("token exchange: %w", err) |
Member
There was a problem hiding this comment.
[nice to have] A 429 "Device grant rate limit exceeded" doesn't match either sentinel, so complete keeps the pending login and returns the bare error without startNewLoginHint. The troubleshooting table says to start a new login, but an agent that reruns the same completion command just gets the 429 again. I think we should map 429 to its own sentinel and handle it like ErrInvalidDeviceCode: delete the pending login and append the hint.
| // started with, or returns "" if none does. | ||
| func (l nonInteractiveDeviceCodeLogin) changedSetting(pending storage.PendingLogin) string { | ||
| switch { | ||
| case pending.Host != l.host: |
Member
There was a problem hiding this comment.
[nit] changedSetting compares pending.Host != l.host as raw strings, but the cache key goes through CanonicalHostName. Step 1 with --host adb-1.azuredatabricks.net and step 2 with --host https://adb-1.azuredatabricks.net find the same pending login, then fail with "started with a different host". Comparing canonical hosts (or storing the canonical host in PendingLogin) would fix it.
| // with the wrong device code, which the server would reject as a wrong | ||
| // confirmation code. | ||
| if !sameCode(pending.UserCode, userCode) { | ||
| return nil, fmt.Errorf("a newer login for %s replaced this one. Complete the newer login with its user code. %s", l.target(), startNewLoginHint) |
Member
There was a problem hiding this comment.
[nit] Every mismatched --user-code gets "a newer login ... replaced this one", including a typo or swapped --user-code/--confirmation-code values. In those cases the message points at a newer login that doesn't exist. Something like "--user-code does not match the pending login for X. A newer login may have replaced it" would cover both cases.
| // is not a terminal, e.g. when an AI agent runs the login as a one-off | ||
| // command, nobody can type the code in. The non-interactive login then | ||
| // stores the device code, and a rerun of the command with --user-code | ||
| // and --confirmation-code completes the login. Neither step prompts. |
Member
There was a problem hiding this comment.
[nit] After the revert of bda3307, a second step that runs in a terminal can still show the workspace picker through shouldPromptWorkspace. Maybe "Neither step prompts for the profile or the host".
| } | ||
| // A pending device code login for the same profile or host is left over | ||
| // once a login succeeds. | ||
| if err := tokenStore.Delete(pendingLoginKey(key)); err != nil { |
Member
There was a problem hiding this comment.
[nit] storeLoginToken now deletes <key>#device-code after every login, browser and discovery included. fileStore.Delete rewrites token-cache.json even when the key is missing. Skipping the write when the key isn't in f.Tokens would leave normal logins unchanged for users who never use device code.
| } | ||
| // A pending device code login could still complete and restore the | ||
| // credentials that the user is logging out of. | ||
| if err := tokenStore.Delete(pendingLoginKey(p.Name)); err != nil { |
Member
There was a problem hiding this comment.
[nit] Logout only calls clearTokenStore when auth_type = databricks-cli (isCreatedByLogin, line 207), so a pending login started for a PAT profile survives auth logout --profile dev. The docs say logout removes the profile's pending login. Either delete pendingLoginKey(p.Name) regardless of auth type, or narrow the docs.
|
|
||
| func cloneEntry(e Entry) Entry { | ||
| e.Token = cloneToken(e.Token) | ||
| if e.PendingLogin != nil { |
Member
There was a problem hiding this comment.
[nit] cloneEntry copies the PendingLogin struct, but Scopes still shares its backing array, so changing the caller's slice after Put changes the stored entry. slices.Clone(pending.Scopes) fixes it, and TestMemoryStore_PendingLoginUsesCopies could check Scopes too.
| func TestLoginDeviceCodeCompletesInTerminal(t *testing.T) { | ||
| server := newDeviceCodeServer(t) | ||
| setUpDeviceCodeLogin(t, "") | ||
| ctx := cmdio.InContext(t.Context(), cmdio.NewIO(t.Context(), flags.OutputText, strings.NewReader(""), &bytes.Buffer{}, &bytes.Buffer{}, "", "")) |
Member
There was a problem hiding this comment.
[nit] This and the tests at lines 477 and 531 build cmdio.InContext(..., cmdio.NewIO(..., strings.NewReader(""), ...)) by hand. That's what cmdio.MockDiscard(t.Context()) already does, and logout_test.go and switch_test.go use it.
| title "A second login for the same profile replaces the first\n" | ||
| start_login LOG.first $CLI auth login --host $DATABRICKS_HOST --profile test --use-device-code | ||
| start_login LOG.second $CLI auth login --host $DATABRICKS_HOST --profile test --use-device-code | ||
| trace update_file.py home/.databricks/token-cache.json WDJBMJHT NEWRCODE |
Member
There was a problem hiding this comment.
[nit] The script rewrites token-cache.json with update_file.py because the fake server always issues WDJBMJHT. The testing rules prefer modeling this in libs/testserver/. For example, the fake could issue a new user code from a fixed sequence for each authorization, so the test exercises the replace behavior directly.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #7217.
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.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.