Skip to content

auth: wire secure-storage cache into login/token/logout #5013

Open
simonfaltum wants to merge 8 commits intosimonfaltum/cli-ga-secure-storage-foundationfrom
simonfaltum/cli-ga-secure-storage-wiring
Open

auth: wire secure-storage cache into login/token/logout #5013
simonfaltum wants to merge 8 commits intosimonfaltum/cli-ga-secure-storage-foundationfrom
simonfaltum/cli-ga-secure-storage-wiring

Conversation

@simonfaltum
Copy link
Copy Markdown
Member

@simonfaltum simonfaltum commented Apr 17, 2026

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 = secure in .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-storage flag 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.md
for the two-PR split and
documents/fy2027-q2/cli-ga/2026-04-13-cli-ga-rollout-contract.md
for the full rollout contract.

Stacks on #5008 (PR 1, Foundation). Once that merges, retarget to main.

Changes

Before: auth login, auth token, and auth logout always went
through the SDK's default file-backed TokenCache. The CLI had no
way to honor the storage-mode resolver added in PR 1.

Now:

  • New helper cmd/auth/cache.go:newAuthCache(ctx, override) resolves
    the mode via storage.ResolveStorageMode and returns the
    corresponding cache.TokenCache. Split into a public form and an
    injectable core (newAuthCacheWith) so unit tests exercise the
    secure path with a fake cache. Production always passes override = ""
    and relies on env -> config -> default.
  • auth login, auth token (via loadToken + runInlineLogin), and
    auth logout all call newAuthCache(ctx, "") and plumb the resolved
    cache into u2m.WithTokenCache(...) at every NewPersistentAuth
    call site.
  • Acceptance tests under acceptance/cmd/auth/storage-modes/ cover
    the 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):

  • plaintext storage implementation (MS3). The resolver already
    accepts plaintext; a later milestone will route it to a non-dual
    write file backend.
  • Write path for [__settings__].auth_storage (MS4 or fast-follow).
    Users hand-edit the config for MS1.
  • Automatic migration of existing token-cache.json entries. Contract
    says never: users re-login after upgrading to GA.
  • Flipping the default to secure (MS4).

Test plan

  • Unit tests for newAuthCache covering default legacy, explicit
    override, env-var selection, plaintext fallback, invalid override,
    invalid env, and file-factory error propagation. Secure path uses
    a fake cache.TokenCache so CI never touches the OS keyring.
  • Acceptance tests under acceptance/cmd/auth/storage-modes/:
    invalid-env (bogus DATABRICKS_AUTH_STORAGE surfaces a clear
    error via auth token) and legacy-env-default (explicit legacy
    mode clears the file cache via auth logout, matching default
    behavior).
  • make checks, make test, make lint pass.
  • Existing cmd/auth/login, cmd/auth/token, cmd/auth/logout
    acceptance test suites still pass (regression).

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.
@simonfaltum simonfaltum marked this pull request as ready for review April 17, 2026 13:36
@github-actions
Copy link
Copy Markdown

Approval status: pending

/cmd/auth/ - needs approval

6 files changed
Suggested: @mihaimitrea-db
Also eligible: @tanmay-db, @tejaskochar-db, @hectorcast-db, @renaudhartert-db, @parthban-db, @Divyansh-db, @chrisst, @rauchy

General files (require maintainer)

9 files changed
Based on git history:

  • @pietern -- recent work in cmd/auth/, ./

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @renaudhartert-db) can approve all areas.
See OWNERS for ownership rules.

@simonfaltum simonfaltum changed the title auth: wire secure-storage cache into login/token/logout (CLI GA MS1, 2/2) auth: wire secure-storage cache into login/token/logout Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant