feat(cli): redesign bfabric-cli auth UX - #573
Draft
leoschwarz wants to merge 15 commits into
Draft
Conversation
Design document for the bfabric-cli auth redesign: the two-modes diagnosis, the decisions taken and their justification, and the implementation order. Committed so the design can be reviewed before any code is written; removed before this PR is ready to merge.
Caushi
reviewed
Aug 6, 2026
An expired token was renewed by retyping the base URL and scope, because a config env stored only half a login: `scope` was written nowhere durable and `base_url` had no fallback. The env now records the requested scope, so `bfabric-cli login` with no arguments replays it and prompts for nothing. - Config: new `scope` env field (the *requested* value, kept separate from the granted scope in the token cache so drift stays visible). - `write_environment_to_config` merges instead of replacing, so hand-written keys survive a re-login; auth-owned keys are replaced wholesale so a stale `pat` can't outlive the method that wrote it. Round-trip validation moves to the merged env. - New `clear_environment_credentials` + `auth logout` / `auth remove` split: logout drops credentials per auth method (token cache, or inline pat / password) and keeps the env replayable; remove deletes the env. - `auth default` -> `auth activate`; handlers normalised to `cmd_auth_*`; top-level `bfabric-cli login` alias. - `auth list` groups by instance with account / scope / expiry; `list` and `status` say why an env is active. Account read from the access token locally, so no extra scope is needed. - Every auth command resolves --config-env > BFABRICPY_CONFIG_ENV > default; config-writing commands refuse under BFABRICPY_CONFIG_OVERRIDE. - Refuse to silently repoint an env's base_url; normalise URLs offline and pre-flight them against OIDC discovery before the browser opens (advisory: a miss warns and proceeds). - PKCE fallback and timeout messages name the loopback redirect and point at device-code; `--no-browser` wired through. `logout` states that B-Fabric has no revocation endpoint, so an issued token stays valid until expiry. No deprecation shims: `auth` shipped 3 days ago and is marked experimental.
Remove the OIDC discovery pre-flight from login flows and the JWT-based account identity display from auth list/status. Base URLs are now normalized purely offline in the new _urls.py, and auth list/status show scope and expiry only.
- Inline single-use helpers (`select_instance`, `print_environments`, `_scope_menu_label`) into their callers - Move `require_mutable_config()` checks into `_load_config(mutating=...)` and `_resolve_params` so mutating commands check once - Cache the known-instance host reverse index in `_urls`
Centralize file reading and validation; `_load_for_edit` now returns the raw mapping directly, and validation runs on a copy to avoid mutating the write data.
Unify the selection-with-custom-entry flow in resolve_base_url and resolve_scope behind a single helper; behavior is unchanged.
Both Unreleased sections had grown into design-doc prose: multi-sentence bullets carrying the rationale for each decision. Cut ~40% of the words, keeping one line per user-visible change. The reasoning already lives in docs/design/oauth_integration.md, oauth_usage_and_troubleshooting.md and the new authentication user guide.
Instances do advertise one: trace's OIDC discovery document publishes
revocation_endpoint = {base_url}/rest/oauth/revoke. Whether it is
implemented is unverified, so logout now states what the command does
(it does not revoke server-side) instead of what the server cannot do.
Affects the notice printed on every logout, 'auth logout --help', the
authentication user guide and the OAuth design doc.
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
2 participants
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.
bfabric-cli loginbecomes top-level, and zero-argument on re-login (the env records the requested scope)auth default→auth activate(no alias)auth logoutremoves credentials only, per auth method;auth removedeletes the environment (breaking)auth listgroups by instance and shows account / scope / expiry;listandstatussay why an env is active--no-browser, plus remote-host guidance in the PKCE fallback and timeout messagesBFABRICPY_CONFIG_ENV; config-writing ones refuse underBFABRICPY_CONFIG_OVERRIDEbfabric-cliauthentication🤖 Prepared with assistance from Claude Opus 5 via Claude Code.