Skip to content

feat(auth): logout/clear commands and client_id mismatch diagnostic - #104

Merged
robpc merged 3 commits into
mainfrom
feat/auth-logout-clear
Oct 2, 2026
Merged

robpc merged 3 commits into
mainfrom
feat/auth-logout-clear

Conversation

@robpc

@robpc robpc commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Re-lands the useful part of #15 — Jess Finger's bebb0b5, cherry-picked from the archived yahoo-orion/desk#52 — on top of #83. Jess is credited as co-author on the commit.

Stacked on #83 (feat/scope-aware-commands): this needs ADR-037's granted-scope missing_scopes and edits the same auth status code, so the base is #83's branch. GitHub retargets to main when #83 merges; the diff shown here is only this PR's work.

What it adds — ADR-040

Recovering from stale keychain state (a token minted against a different OAuth client than the one now configured) used to mean security CLI surgery. The keyring helpers existed but nothing exposed them.

  • desk auth logout — removes the OAuth token, keeps the client credentials, scrubs secrets from the legacy ~/.desk/token.json. Idempotent. --json.
  • desk auth clear [--token|--client] [--yes] — removes token and/or client credentials. Confirmation prompt; --yes required when there's no TTY (same pattern as docs delete-tab). --json.
  • desk auth status now reports client_id (what the next login would use — keyring, then credentials.json, same order as login()), token_client_id (what minted the stored token), and token_source (keyring / file / gcloud_adc / none). When the two client ids differ, the human output says the token cannot refresh and names the fix.
  • desk auth set-client drops a stored token whose client_id differs from the new one, with a one-line note — the token could never have refreshed against it.
  • keyring_store.delete_client_credentials() mirrors delete_token().

What's deliberately dropped from #15

Kept from #15 but not on the original keep list: the set-client auto-invalidation. It is the proactive half of the same mismatch diagnostic, three lines, and already tested — easy to pull if unwanted.

clear composes delete_token() + delete_client_credentials() rather than calling clear_all(), so its result can say which key was actually removed (--json consumers get keyring_token_removed / keyring_client_removed separately).

Verification

  • 27 new tests (tests/test_auth_logout_clear.py): logout idempotence and legacy-file scrub (including that ADR-037's granted_scopes survives the scrub), the clear flag matrix, non-interactive --yes enforcement, status field shape and credentials.json fallback, the mismatch wording in human output, and the set-client invalidation path.
  • 1022 tests total, 1021 passing normally and under PYTHON_KEYRING_BACKEND=keyring.backends.fail.Keyring; the one failure (test_preexisting_directory_tightened_to_0o700) fails identically on main in a umask-077 sandbox and passes in CI.
  • ruff check clean.
  • Live, read-only: desk auth status on this build shows the configured and token client_id matching with token source: keyring; desk auth --help lists clear and logout. logout/clear were not run against the real keychain.

Docs

ADR-040 (Jess's ADR-017/019 text, renumbered past #83's ADR-039, with a "What changed in the re-land" section), decisions index, CLAUDE.md architecture tree, README "Signing out and resetting".

Refs #15 (to be closed once this merges).

🤖 Generated with Claude Code

Comment thread tests/test_auth_logout_clear.py Fixed
Base automatically changed from feat/scope-aware-commands to main October 1, 2026 23:57
robpc and others added 2 commits October 1, 2026 19:57
Recovering from stale keychain state — a token minted against a different
OAuth client than the one now configured — used to mean `security` CLI
surgery. The keyring helpers existed but nothing exposed them.

- `desk auth logout` removes the OAuth token (keeps client credentials),
  scrubs secrets from the legacy ~/.desk/token.json, idempotent, --json.
- `desk auth clear [--token|--client] [--yes]` removes token and/or client
  credentials; confirmation prompt, --yes required when non-interactive.
- `desk auth status` surfaces `client_id` (what the next login would use),
  `token_client_id` (what minted the stored token) and `token_source`, and
  says plainly when they differ and what to run.
- `desk auth set-client` drops a stored token whose client_id differs from
  the new one, with a one-line note.
- `keyring_store.delete_client_credentials()` mirrors `delete_token()`.

Re-lands the useful part of #15 (Jess Finger's bebb0b5, cherry-picked from
yahoo-orion/desk#52) on top of ADR-037. Two pieces of that commit are
dropped on purpose: the `get_bundled_credentials()` fallback, which only
exists in the yahoo-orion fork and is why #15's CI never passed here; and
the stored `scopes` display, which showed the *requested* set — ADR-037's
`missing_scopes` is the granted-scope signal and already exists. ADR-040
(was ADR-017/019) records both.

Co-authored-by: Jess Finger <jessica.finger@yahooinc.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CodeQL read `"old.apps.googleusercontent.com" in result.output` as an
incomplete URL-substring sanitization check. The assertion is a plain
substring match on CLI output, so swap the hostname-shaped fakes for
opaque ids and the rule no longer applies.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@robpc
robpc force-pushed the feat/auth-logout-clear branch from a38ae4a to 8f45297 Compare October 1, 2026 23:58
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@robpc
robpc force-pushed the feat/auth-logout-clear branch from 8f45297 to 9249ecb Compare October 2, 2026 00:04
@robpc
robpc merged commit 9ec923c into main Oct 2, 2026
7 checks passed
@robpc
robpc deleted the feat/auth-logout-clear branch October 2, 2026 00:06
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.

2 participants