Skip to content

feat(auth): add logout/clear commands and stale-token detection - #15

Closed
robpc wants to merge 2 commits into
mainfrom
feat/auth-logout-clear-and-stale-detection
Closed

robpc wants to merge 2 commits into
mainfrom
feat/auth-logout-clear-and-stale-detection

Conversation

@robpc

@robpc robpc commented May 5, 2026

Copy link
Copy Markdown
Owner

Cherry-picked from yahoo-orion/desk#52 (now archived) by Jess Finger.

Why this is here

The yahoo-orion/desk repo was archived as part of the OSS migration to robpc/desk. Jess's #52 was open at the time of archive and is genuinely novel work — robpc/desk doesn't have an equivalent today (no desk auth logout / desk auth clear commands, no stale-token detection, ADR-017 in this repo is "Paragraph Spacing", not auth-logout-clear).

Bringing the work over here with attribution preserved. Tagging Jess for review/take-over if she wants.

Changes

New commands

When desk's keychain state goes stale (e.g. the stored OAuth token was minted against a different OAuth client than the one currently configured, or scopes drift after a SCOPES change), the only way to recover today is manual keychain surgery via the security CLI. The underlying keyring helpers exist in keyring_store.py but aren't exposed via the CLI, and auth status doesn't surface enough to even diagnose the mismatch.

This adds:

  • desk auth logout — clears the OAuth token only (keeps the configured client)
  • desk auth clear — clears both the token and the client config (full reset)
  • Stale-token detection in auth status — surfaces client/token mismatch and scope drift

Files

  • docs/decisions/019-auth-logout-clear-and-stale-token-detection.md — ADR for the design (renumbered from 017 in the source PR to avoid colliding with this repo's ADR-017 Paragraph Spacing)
  • src/desk/auth.py, src/desk/cli.py, src/desk/keyring_store.py — implementation
  • tests/test_auth_logout_clear.py — new test suite

Commits

  1. bebb0b5 — original commit by Jess Finger (cherry-picked with -x from yahoo-orion/desk@e7302a8)
  2. 166f3c8 — small follow-up: renumber the ADR file and heading from 017 to 019 to avoid collision

Notes

  • Original PR body, comments, and review threads are at yahoo-orion/desk#52 — preserved in archive.
  • Authorship trailer on the cherry-pick preserves Jess's identity. CC @jfinger or whoever should drive review.

🤖 Generated with Claude Code

Jess Finger and others added 2 commits May 5, 2026 12:26
Today, recovering from stale keychain state (e.g. token issued for a
different OAuth client than the one currently configured) requires manual
keychain surgery via the `security` CLI. The underlying keyring helpers
exist but are not exposed via the CLI.

Adds:

- `desk auth logout` — removes the OAuth token from the keychain
  (preserves client config); idempotent; scrubs legacy ~/.desk/token.json
  secrets; --json supported.
- `desk auth clear` — removes token and/or client credentials with
  --token / --client flags; confirmation prompt by default; --yes to
  skip; non-interactive mode requires --yes (matches `docs delete-tab`);
  --json supported.
- `desk auth status` — surfaces client_id, token_client_id, token_source
  (keyring/file/gcloud_adc/none), and scopes so users can spot stale
  state.
- `desk auth set-client` — when the new client_id differs from the
  stored token's client_id, automatically invalidate the token (it
  cannot refresh against a different client) and print a one-line note.

Also adds keyring_store.delete_client_credentials() to mirror
delete_token().

Documented in ADR-017.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(cherry picked from commit e7302a88bf9cbb158a38e49dcb26246e1bdc1493)
The cherry-picked auth-logout-clear ADR was numbered 017 in the source
repo (yahoo-orion/desk), but robpc/desk already has ADR-017 (Paragraph
Spacing Controls) referenced from ideas/047 and ideas/048. Renumbering
to 019 (018 is taken by tab-identifier-resolution).

No content changes — just the file name and the heading.
robpc added a commit that referenced this pull request Oct 1, 2026
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>
@robpc

robpc commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner Author

Superseded by #104, which re-landed logout/clear and the client_id mismatch diagnostic (crediting Jess Finger's original commit). It drops the bundled-credentials fallback that kept this PR's CI red, and uses #83's granted-scope tracking for scope drift. Closing in favour of #104.

robpc added a commit that referenced this pull request Oct 2, 2026
…104)

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

- **The bundled-credentials fallback.** `_get_configured_client_id()`
fell through to `get_bundled_credentials()`, which exists in the
yahoo-orion fork and was never part of robpc/desk — that import is why
#15's CI never passed here.
- **The stored `scopes` field and its display.** It printed the token's
*requested* scope list, the same requested-vs-granted confusion #83
fixed (#82). `missing_scopes` from ADR-037 is the actionable signal and
is already on `auth status`, so this PR leans on it instead. A test pins
that `scopes` stays absent and `missing_scopes` stays present.

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](https://claude.com/claude-code)

---------

Co-authored-by: Robert Cannon <robpc@users.noreply.github.com>
Co-authored-by: Jess Finger <jessica.finger@yahooinc.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@robpc robpc closed this Oct 2, 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