Invalidate cached OAuth tokens when profiles change - #6427
mihaimitrea-db wants to merge 14 commits into
Conversation
Integration test reportCommit: a1df656
Top 15 slowest tests (at least 2 minutes):
|
Approval status: pending
|
db84530 to
c69fa5d
Compare
renaudhartert-db
left a comment
There was a problem hiding this comment.
Thanks @mihaimitrea-db, first small round.
| type ProfileFingerprintError struct { | ||
| Profile string | ||
| Missing bool | ||
| } |
There was a problem hiding this comment.
Could this be two different errors so that we can easily test them with errors.Is?
| // WorkspaceIDNone is a sentinel value persisted to .databrickscfg when the | ||
| // user explicitly skips workspace selection for SPOG account-level access. | ||
| const WorkspaceIDNone = "none" | ||
| const WorkspaceIDNone = databrickscfg.WorkspaceIDNone |
There was a problem hiding this comment.
Should we just use databricks.WorkspaceIDNone everywhere WorkspaceIDNone is used?
|
|
||
| // ProfileFingerprintStore stamps token writes and rejects reads whose metadata | ||
| // does not match the current profile. | ||
| type ProfileFingerprintStore struct { |
There was a problem hiding this comment.
I'm not sure this type actually makes the code easier to follow. It is weird to me that the fingerprint is passed at the construction level but not at the lookup level. I wonder if the we should adapt the Store interface directly. You could also keep a wrapper if you want but that wrapper does not have to actually implement the Store interface.
| // setTokenProfileFingerprint runs after profile saving because OAuth-dependent | ||
| // workspace and compute selection can change the final profile contents. |
There was a problem hiding this comment.
Should this be a call site comment? It does not tell me much about the function itself.
Summary
Bind cached OAuth credentials to the profile configuration used during login.
When the profile changes, the CLI rejects the cached token and asks the user to log in again. This prevents a token created for one profile configuration from being reused with another.
How it works
Login
After login finishes saving the profile, the CLI computes a fingerprint from every field in the simplified profile and stores it with the cached token.
Token use and refresh
Before
auth tokenor another command uses the cached token, the CLI computes the current fingerprint and compares it with the stored value.Refreshed tokens keep the fingerprint.
Dependency loop
Fingerprint validation requires the auth package to use the simplified profile type. The profile package previously imported auth only for the legacy
workspace_id = nonevalue, which would make the two packages depend on each other.The shared value now lives in the parent
databrickscfgpackage, so profiles no longer need to import auth and the dependency only goes in one direction.Configuration behavior
Ordinary commands resolve the profile by also taking into account environment variables. An environment variable that changes a fingerprinted profile field is treated like temporarily editing that field in
.databrickscfgand invalidates the cached token.Hostless profiles remain usable with other authentication methods, such as an environment-provided PAT or client credentials. Cached
databricks-cliOAuth profiles are expected to have a host because login always saves one.