Skip to content

feat: auth logout - #41

Closed
alukach wants to merge 1 commit into
feat/product-commandsfrom
feat/logout-command
Closed

alukach wants to merge 1 commit into
feat/product-commandsfrom
feat/logout-command

Conversation

@alukach

@alukach alukach commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #24 (feat/product-commands). Retarget to main once #24 merges.

Rebuilt on #24, under auth (#24 moved login to auth login). Replaces this PR's earlier top-level source-coop logout.

What

source-coop auth logout removes an environment's login session and one role's cached credentials, from the keyring and the file fallback.

  • --proxy-url / SOURCE_PROXY_URL and --role-arn / SOURCE_ROLE_ARN pick the environment and role, as for creds.
  • Order: the session first, then the role's credentials, each under its slot's lock. A creds that's minting finishes before its result is removed.
  • Old-format entries: for a non-production environment, an old-format entry of that environment left in production's slot is removed too. Otherwise read_credentials would copy it straight back.
  • Other roles: the keyring can't list its entries, so other roles' credentials stay until they expire. Without the session they can't be renewed. The README says so.
  • Revocation: tokens aren't revoked at the identity provider. That could be a follow-up.
  • Prints "Logged out of …" or "Not logged in to …" on stderr.

This and #40 (auth status) are independent, so either can land first. The other then has a one-line conflict in the auth command's description and in the README.

Testing

  • remove_file_reports_whether_it_existed.
  • cargo fmt, cargo clippy --all-targets -- -D warnings, cargo test and cargo test --features staging: 89 passed.
  • Ran it for a made-up environment with HOME in a scratch folder holding fake session and credential files. It removed both and said "Logged out", and a second run said "Not logged in". No real keychain entry or login was touched.

🤖 Generated with Claude Code

@claude

claude Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Claude finished @alukach's task in 18s —— View job


✅ No blocking issues — safe to merge.

I read the diff and the surrounding read_credentials and read_session code. I did not build the branch or run the tests or clippy.

Notes (non-blocking)

  • PR description is stale. It says source-coop logout, but the code and README use auth logout. This matches AGENTS.md ("signing in lives under auth"). Update the description so it doesn't confuse the squash title or release notes.
  • Legacy-slot race. In src/cache.rs:244-250, read_slot(&legacy) runs without the legacy slot's lock, and delete_slot then takes the lock. If another environment's creds rewrites that slot between the check and the delete, you could delete a still-valid entry. The cost is one extra re-mint, so this is harmless.
  • Message prints the proxy URL as given. run_logout (src/main.rs) prints args.proxy_url exactly as passed, so a trailing slash shows up as typed. This is cosmetic.

Simplify (ponytail)

  • src/cache.rs remove_file: fs::remove_file plus a NotFound match is already minimal. It is only used by delete_slot, so it could be inlined. The unit test for it is the only reason to keep it separate, so keep it.
  • src/main.rs LogoutArgs: duplicates the proxy_url and role_arn args from creds. If creds has a shared args struct, flatten it with #[command(flatten)] instead of redeclaring the arguments.
    · branch feat/logout-command

💰 Estimated review cost: $0.12 · 0m18s · 7 turns

Base automatically changed from feat/per-environment-cache to main October 13, 2026 22:36
…dentials

`source-coop auth logout` deletes the environment's login session and the
role's cached credentials from the keyring and the file fallback, under
their locks, session first so a `creds` that's minting finishes before
its result is removed. For a non-production environment it also removes
an old-format entry of that environment left in production's slot, which
would otherwise be copied straight back. Other roles' credentials can't
be listed, so they stay until they expire and can't be renewed. Tokens
aren't revoked at the IdP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alukach
alukach force-pushed the feat/logout-command branch from ec195fe to 9b6d51c Compare October 14, 2026 16:31
@alukach alukach changed the title feat: add logout command feat: auth logout Oct 14, 2026
@alukach
alukach changed the base branch from main to feat/product-commands October 14, 2026 16:31
@alukach

alukach commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #46 (auth commands: login, status, logout, token), built on #45 instead of #24. The branch is kept.

@alukach alukach closed this Oct 14, 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