auth: wire secure-storage cache into login/token/logout #5013
Open
simonfaltum wants to merge 8 commits intosimonfaltum/cli-ga-secure-storage-foundationfrom
Open
auth: wire secure-storage cache into login/token/logout #5013simonfaltum wants to merge 8 commits intosimonfaltum/cli-ga-secure-storage-foundationfrom
simonfaltum wants to merge 8 commits intosimonfaltum/cli-ga-secure-storage-foundationfrom
Conversation
Introduces a thin helper that resolves the CLI's token storage mode via libs/auth/storage.ResolveStorageMode and returns a cache.TokenCache. The helper is split into a public convenience form and an injectable core (newAuthCacheWith) so unit tests exercise the secure path with a fake cache without touching the real OS keyring. Follow-up commits wire this into the auth command call sites.
cache.NewFileTokenCache has a variadic signature (opts ...FileTokenCacheOption), which cannot be assigned directly to a non-variadic field. Adds a one-line explanation so future readers do not 'simplify' the closure back into a direct function reference and break the build.
Adds a hidden --secure-storage BoolVar to auth login and routes the resolved token cache into both NewPersistentAuth call sites (main flow and discoveryLogin). Users opt into OS-native secure storage either per invocation via the flag or for the whole shell via DATABRICKS_AUTH_STORAGE=secure. The default storage mode remains legacy. Flag is MarkHidden because MS1 is experimental; release notes document the env var for discovery.
Resolves the token cache via newAuthCache at the two NewPersistentAuth call sites inside cmd/auth/token.go (loadToken via newTokenCommand and runInlineLogin). No flag is added: per the rollout contract, auth token reads from whichever mode the resolver selects via DATABRICKS_AUTH_STORAGE or [__settings__].auth_storage.
Replaces the direct cache.NewFileTokenCache() call with newAuthCache so that logout targets whichever backend ResolveStorageMode selects. The resolver still defaults to legacy, so existing flows are unchanged; once a user opts into secure storage, logout clears the keyring entry instead of the file cache.
Adds three acceptance tests under acceptance/cmd/auth/storage-modes/: - invalid-env: DATABRICKS_AUTH_STORAGE=bogus surfaces a clear error - legacy-env-default: explicit legacy mode behaves like the default path - hidden-flag: --secure-storage is hidden from auth login --help Secure-mode end-to-end behavior is covered by unit tests in cmd/auth/cache_test.go because acceptance runners cannot safely exercise the real OS keyring on macOS (would write to Keychain) or Linux (no D-Bus session in CI).
Storage backend is a per-machine setting, not a per-invocation choice. Keeping a write-time flag on login while token/logout rely on the resolver creates an asymmetry: login --secure-storage writes to the keyring, but a later auth token without the env var defaults to the legacy file cache and can't find the token. Resolution is now always env -> [__settings__].auth_storage -> default. login.go matches token.go and logout.go: newAuthCache(ctx, "") with no per-call override from a flag. Also drops the hidden-flag acceptance test (no flag to hide) and updates NEXT_CHANGELOG to advertise the env var and config key instead of the flag.
Approval status: pending
|
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.
Why
Second and final PR of CLI GA MS1 (secure token storage). Plugs the
dormant building blocks from PR 1 into the user-facing auth commands.
Users opt into OS-native secure storage by setting
DATABRICKS_AUTH_STORAGE=secure(per-shell) or[__settings__].auth_storage = securein.databrickscfg(per-machine). Everyone else stays on the legacy file-backed cache.
The default does not change.
Storage backend is a per-machine setting, not a per-invocation choice.
This PR deliberately ships no
--secure-storageflag so that login,token, and logout can never drift apart on the same machine.
See
documents/fy2027-q2/cli-ga/2026-04-17-cli-ga-secure-storage-pr-plan.mdfor the two-PR split and
documents/fy2027-q2/cli-ga/2026-04-13-cli-ga-rollout-contract.mdfor the full rollout contract.
Stacks on #5008 (PR 1, Foundation). Once that merges, retarget to main.
Changes
Before:
auth login,auth token, andauth logoutalways wentthrough the SDK's default file-backed
TokenCache. The CLI had noway to honor the storage-mode resolver added in PR 1.
Now:
cmd/auth/cache.go:newAuthCache(ctx, override)resolvesthe mode via
storage.ResolveStorageModeand returns thecorresponding
cache.TokenCache. Split into a public form and aninjectable core (
newAuthCacheWith) so unit tests exercise thesecure path with a fake cache. Production always passes
override = ""and relies on env -> config -> default.
auth login,auth token(vialoadToken+runInlineLogin), andauth logoutall callnewAuthCache(ctx, "")and plumb the resolvedcache into
u2m.WithTokenCache(...)at everyNewPersistentAuthcall site.
acceptance/cmd/auth/storage-modes/coverthe two CLI-visible behaviors that don't require an OS keyring:
invalid env var surfaces a clear error, and explicit legacy mode
behaves identically to the default path.
Out of scope (documented in the rollout contract):
plaintextstorage implementation (MS3). The resolver alreadyaccepts
plaintext; a later milestone will route it to a non-dualwrite file backend.
[__settings__].auth_storage(MS4 or fast-follow).Users hand-edit the config for MS1.
token-cache.jsonentries. Contractsays never: users re-login after upgrading to GA.
secure(MS4).Test plan
newAuthCachecovering default legacy, explicitoverride, env-var selection, plaintext fallback, invalid override,
invalid env, and file-factory error propagation. Secure path uses
a fake
cache.TokenCacheso CI never touches the OS keyring.acceptance/cmd/auth/storage-modes/:invalid-env(bogusDATABRICKS_AUTH_STORAGEsurfaces a clearerror via
auth token) andlegacy-env-default(explicit legacymode clears the file cache via
auth logout, matching defaultbehavior).
make checks,make test,make lintpass.cmd/auth/login,cmd/auth/token,cmd/auth/logoutacceptance test suites still pass (regression).