Skip to content

fix(auth): delete jurisdiction tokens on logout - #1869

Merged
stiak merged 2 commits into
mainfrom
delete-tokens-on-logout
Jul 29, 2026
Merged

fix(auth): delete jurisdiction tokens on logout#1869
stiak merged 2 commits into
mainfrom
delete-tokens-on-logout

Conversation

@stiak

@stiak stiak commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

I saw today the jurisdiction tokens survive a logout. That was unexpected. Track them in the context file so they can be cleared on logout.

`git-remote-entire` caches its jurisdiction (data-plane) access token in the
credential store at `entire-jurisdiction:<audience>`, keyed by the context
handle, so a fresh helper process per git command doesn't re-run the RFC 8693
exchange. Logout deleted only the `entire-core:<core>` access + refresh slots,
so that token — a bearer for every repo the account can reach in its
jurisdiction, with an 8h server-side TTL — survived `entire logout`,
`--everywhere` and `--all-contexts` alike.

The credential store has no enumeration API and the audience isn't derivable
offline, so track it: `contexts.Context.JurisdictionAudiences` records the
audiences a context has a token filed for, and `deleteContextKeychain` walks
that list. Deletion order is longest-lived-first (refresh, jurisdiction,
access) so a mid-sequence failure leaves behind only the shorter-lived
credential, and a failed delete still aborts the logout rather than reporting
success over a surviving credential.

`jurisdictionTokenSource.persistToken` records the audience *before* writing
the token, and skips the write when recording fails: a persisted-but-unrecorded
token is invisible to logout, whereas skipping costs one exchange. The recorder
being non-nil now also selects the persisted flavour, replacing the `persist`
flag, so the two can't disagree — the ENTIRE_TOKEN path passes none and stays
in-process-only. `RecordLoginContext` carries the audience list across the
re-login upsert, since the tokens are keyed by audience + handle rather than by
login session and outlive a fresh login.

Deletion is scoped to the outgoing context: only its recorded audiences, only
under its own handle, so another account's tokens are untouched. Slots written
before this bookkeeping are unreachable and expire with their TTL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01KYNRX5J3PM8D1SD0RQ0ZXCCR
Copilot AI review requested due to automatic review settings July 29, 2026 01:49
@stiak
stiak requested a review from a team as a code owner July 29, 2026 01:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR closes a security/usability gap in logout by ensuring data-plane “jurisdiction” bearer tokens cached by git-remote-entire are discoverable and deleted during entire logout, despite the credential store lacking enumeration.

Changes:

  • Add JurisdictionAudiences tracking to login contexts so logout can locate jurisdiction-token keychain slots.
  • Update git-remote-entire jurisdiction token caching to record the audience before persisting the token (and skip persistence if recording fails).
  • Extend logout keychain deletion to remove jurisdiction tokens per recorded audience, and add focused tests for ordering, scope, and failure behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/entireclient/tokenstore/tokenstore.go Adds standardized service-name helper for jurisdiction token slots.
internal/entireclient/tokenstore/testing.go Adds an observing test backend to assert credential-store operation ordering.
internal/entireclient/contexts/contexts.go Extends stored context schema with JurisdictionAudiences.
cmd/git-remote-entire/main.go Wires audience-recording into jurisdiction token source creation for interactive git auth.
cmd/git-remote-entire/main_test.go Updates env-token path assertion to the new persistence predicate.
cmd/git-remote-entire/jurisdictionauth.go Reworks persistence selection and enforces record-before-persist semantics for jurisdiction tokens.
cmd/git-remote-entire/jurisdictionauth_test.go Adds/updates tests for audience recording order and “don’t persist if unrecordable” behavior.
cmd/entire/cli/auth/contexts.go Preserves JurisdictionAudiences across context upserts on re-login.
cmd/entire/cli/auth/context_store.go Expands logout deletion to include jurisdiction tokens for recorded audiences.
cmd/entire/cli/auth/context_store_test.go Adds tests covering multi-audience deletion, cross-account isolation, abort-on-delete-failure, and audience retention on re-login.

Comment thread cmd/entire/cli/auth/context_store.go
Comment thread internal/entireclient/tokenstore/tokenstore.go
Rename `JurisdictionPrefix` to `JurisdictionKeyringPrefix` so it matches its
siblings `ClusterKeyringPrefix` and `CoreKeyringPrefix`.

Skip blank recorded audiences in `deleteContextKeychain`: one would resolve to
the bare `entire-jurisdiction:` service prefix, and while no token is ever
filed there — so the delete was a no-op rather than a wrong deletion — a
hand-edited or corrupted contexts.json shouldn't cost a keyring round-trip.
`RememberJurisdictionAudience` already rejects blank audiences, so this only
guards entries the CLI didn't write.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01KYNXD6MABCV4V77GJ82F9HX0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/entireclient/tokenstore/tokenstore.go:60

  • JurisdictionService normalizes trailing slashes but not surrounding whitespace. If a context ever records an audience with leading/trailing spaces (e.g., a hand-edited/corrupted contexts.json, or input that wasn’t trimmed upstream), logout may compute a different service name and fail to delete the persisted token, leaving a valid jurisdiction bearer behind.
// JurisdictionService returns the service name for a jurisdiction (data-plane)
// access token, keyed by the jurisdiction audience so tokens for different
// jurisdictions — and for prod vs staging — can't be confused. The account key
// is the login context's handle. Trailing slashes are normalized away so
// callers don't have to.
func JurisdictionService(audience string) string {
	return JurisdictionKeyringPrefix + strings.TrimRight(audience, "/")
}

@stiak
stiak merged commit 683a10d into main Jul 29, 2026
11 checks passed
@stiak
stiak deleted the delete-tokens-on-logout branch July 29, 2026 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants