Repository navigation
Split device code login into authorization and token exchange steps - #7217
Merged
Merged
Conversation
Collaborator
Integration test reportCommit: 84cdd4e
Top 6 slowest tests (at least 2 minutes):
|
This was referenced Oct 28, 2026
maksim-zuev-db
marked this pull request as ready for review
October 28, 2026 15:58
maksim-zuev-db
requested review from
renaudhartert-db
and removed request for
renaudhartert-db
October 28, 2026 15:58
maksim-zuev-db
force-pushed
the
device-code-split-steps
branch
from
October 29, 2026 12:15
113e772 to
74b6992
Compare
libs/auth/u2m now provides the two steps of device code login instead of the whole login. PersistentAuth.DeviceCodeLogin checks that the host supports it and fetches the OAuth endpoints, and its RequestDeviceAuthorization and RequestDeviceAccessToken methods make the two requests. A rejected code matches ErrInvalidConfirmationCode or ErrInvalidDeviceCode. The confirmation code prompt and its retries move from DeviceCodeChallenge to auth login. The login's output does not change. Co-authored-by: Isaac <no-reply@databricks.com>
maksim-zuev-db
force-pushed
the
device-code-split-steps
branch
from
October 29, 2026 12:19
74b6992 to
0ab827c
Compare
The retries moved from u2m to auth login without unit tests. The new test enters a wrong code followed by the right one, three wrong codes, and a wrong code followed by no input, and checks the number of token requests for each. Also rename PersistentAuth.DeviceCodeLogin to NewDeviceCodeLogin, which had the same name as the DeviceCodeLogin type it returns, rename TestRequestDeviceAccessToken_RetriesWrongConfirmationCode to TestRequestDeviceAccessToken_WrongConfirmationCode because it no longer retries, and check the token requests in TestDeviceCodeLogin_Errors again. Co-authored-by: Isaac <no-reply@databricks.com>
The retry test covered missing input only after a wrong code. The new case closes stdin at the first prompt and checks that no token request is sent. Co-authored-by: Isaac <no-reply@databricks.com>
NewDeviceCodeLogin fetches the OAuth endpoints, and used the context that PersistentAuth was created with. It now takes the context of the call, like the two steps of the login, and passes it to oauth2Config. Co-authored-by: Isaac <no-reply@databricks.com>
Each case now states how many token requests it expects: none when the device authorization is rejected, and one otherwise. The previous upper bound of one also passed if a case that should reach the token endpoint never did. Co-authored-by: Isaac <no-reply@databricks.com>
The server answers both a wrong confirmation code and an unknown device code with HTTP 400 and invalid_request, and only a wrong confirmation code lets the user enter the code again. Only the u2m tests checked that an unknown device code is told apart, so auth login could ask for the code again with no test failing. TestLoginDeviceCodeFailsOnTokenExchangeError now covers an unknown device code next to the rate limit, and checks that each sends a single token request. Co-authored-by: Isaac <no-reply@databricks.com>
mihaimitrea-db
left a comment
mihaimitrea-db
left a comment
Contributor
There was a problem hiding this comment.
Other than the ctx problem the refactoring looks good.
| switch argg := a.oAuthArgument.(type) { | ||
| case WorkspaceOAuthArgument: | ||
| endpoints, err = a.endpointSupplier.GetWorkspaceOAuthEndpoints(a.ctx, argg.GetWorkspaceHost()) | ||
| endpoints, err = a.endpointSupplier.GetWorkspaceOAuthEndpoints(ctx, argg.GetWorkspaceHost()) |
Contributor
There was a problem hiding this comment.
If PersistentAuth has its own context why are we using a different one? We should use one or the other and in this case the one from within PersistentAuth seems to be the correct one.
Member
Author
There was a problem hiding this comment.
agents/rules/style-guide-go.md says to pass context.Context as an argument and never store it in a struct, and this came up for the device code login in #7093 (#7093 (comment)). The stored ctx is only there because Token() (oauth2.TokenSource) and Challenge() don't take a context. The device code login now uses only the context passed to it, including for fetching the endpoints, so it no longer mixes the two. Both callers pass the context PersistentAuth was created with, so behavior is unchanged.
mihaimitrea-db
approved these changes
Oct 30, 2026
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.
Changes
A refactoring for the non-interactive device code login (#7220), with no user-visible change.
libs/auth/u2mnow provides the two steps of device code login instead of the whole login.PersistentAuth.NewDeviceCodeLogin(ctx)checks that the host supports device code login and fetches the OAuth endpoints once, with the caller's context. ItsRequestDeviceAuthorizationandRequestDeviceAccessTokenmethods make the two requests. A rejected code matchesErrInvalidConfirmationCodeorErrInvalidDeviceCode, so callers no longer compare the server's error description.The confirmation code prompt and its retries move from
DeviceCodeChallenge, which is removed, toauth login, built on the two steps.auth loginadds thetoken exchange:prefix to the errors it does not retry, so the login prints the same messages as before.Why
In the non-interactive login (#7220), one CLI invocation requests the device authorization and a later one exchanges the device code, so the two steps have to be callable separately.
Tests
The
u2mtests call the two steps instead ofDeviceCodeChallenge. They are edited in place, so the diff shows the changed calls and the error messages without thetoken exchange:prefix. The retry tests move out ofu2mwith the loop: thedevice-code/nominalanddevice-code/wrong-codeacceptance tests cover it and pass unchanged. Newtermtesttests check the loop inauth login. One enters a wrong confirmation code followed by the right one, three wrong codes, and a wrong code followed by no input, and checks the number of token requests for each. Another checks that a rate-limited token request and an unknown device code each end the login after one token request, without asking for the code again.This pull request and its description were written by Isaac.