Skip to content

[login] revoke existing auth before starting login - #27674

Merged
cooper-oai merged 4 commits into
mainfrom
cooper/revoke-before-login
Jun 12, 2026
Merged

[login] revoke existing auth before starting login#27674
cooper-oai merged 4 commits into
mainfrom
cooper/revoke-before-login

Conversation

@cooper-oai

@cooper-oai cooper-oai commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Why

codex login previously persisted newly issued OAuth credentials and only then attempted to revoke the superseded refresh token. The old credential must be revoked before a replacement browser or device-code flow starts, and successful login must not perform any post-login revocation attempt.

What changed

  • Revoke and clear existing stored auth before browser or device-code CLI login begins.
  • Remove superseded-token detection and revocation from the shared token persistence path; successful login now only saves the new credentials.
  • Read the raw configured auth store during CLI cleanup so environment-provided auth cannot mask the stored refresh token.
  • Preserve auto storage fallback semantics when keyring deletion fails by clearing the fallback auth file.
  • Add a process-level CLI regression test that requires the revoke request to precede every device-login request and occur exactly once.

If replacement login is canceled or fails, the previous local credentials have already been cleared. Remote revocation remains best effort, matching explicit logout behavior.

Validation

Process-level before/after reproduction

I compiled the real codex CLI from the pre-fix parent (14df0e8833) and from the PR implementation (25c002f23b; the login behavior is unchanged at the current head), then ran the same device-code flow against a local HTTP mock OAuth authority.

Each run:

  1. Used a fresh temporary CODEX_HOME configured with cli_auth_credentials_store = "file".

  2. Seeded that temporary home with managed ChatGPT auth containing old-access and old-refresh tokens.

  3. Pointed CODEX_REVOKE_TOKEN_URL_OVERRIDE at the mock /oauth/revoke endpoint.

  4. Ran the compiled CLI as:

    CODEX_HOME=<temporary-home> \
      CODEX_REVOKE_TOKEN_URL_OVERRIDE=<mock-issuer>/oauth/revoke \
      <compiled-codex> login --device-auth --experimental_issuer <mock-issuer>
  5. Recorded every request received by the mock authority. The mock marked new-access valid when /oauth/token issued it and invalidated it if /oauth/revoke arrived afterward, reproducing the observed session-invalidating failure mode. After login exited, the harness also verified the persisted refresh token and probed a protected endpoint with new-access.

Build Observed request order CLI/persistence result new-access probe
Pre-fix usercode → device token → OAuth token → revoke(old-refresh) Exit 0; new-refresh persisted 401
PR revoke(old-refresh) → usercode → device token → OAuth token Exit 0; new-refresh persisted 200

The PR run therefore issued exactly one revocation request, before any request that initiated the replacement login, and issued no revocation after token exchange.

Regression coverage

codex-rs/cli/tests/login.rs::device_login_revokes_existing_auth_before_requesting_new_tokens runs the real first-party codex binary against a wiremock OAuth server with an isolated temporary CODEX_HOME. It asserts:

  • the exact request sequence is /oauth/revoke, /api/accounts/deviceauth/usercode, /api/accounts/deviceauth/token, then /oauth/token;
  • there is exactly one revoke request and its body contains old-refresh with the refresh_token hint;
  • the completed login persists new-refresh.

Local validation:

  • just test -p codex-login — 130 passed
  • just test -p codex-cli — 280 passed, including the new process-level regression test
  • just bazel-lock-check

@cooper-oai
cooper-oai marked this pull request as ready for review June 11, 2026 21:03

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 462c54e418

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/cli/src/login.rs Outdated
@celia-oai
celia-oai self-requested a review June 11, 2026 22:35
Comment thread codex-rs/cli/src/login.rs Outdated
Comment thread codex-rs/cli/src/login.rs Outdated
Comment thread codex-rs/login/src/auth/storage.rs Outdated
Comment thread codex-rs/login/src/auth/storage_tests.rs Outdated
@cooper-oai
cooper-oai force-pushed the cooper/revoke-before-login branch from 3efaf9c to 21c4565 Compare June 12, 2026 18:50
@cooper-oai
cooper-oai merged commit d1aaf78 into main Jun 12, 2026
31 checks passed
@cooper-oai
cooper-oai deleted the cooper/revoke-before-login branch June 12, 2026 19:38
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants