Skip to content

api: report clear error when refresh token is rejected - #362

Open
NanShanFish wants to merge 1 commit into
doy:mainfrom
NanShanFish:fix/refresh-token-error-handling
Open

NanShanFish wants to merge 1 commit into
doy:mainfrom
NanShanFish:fix/refresh-token-error-handling

Conversation

@NanShanFish

Copy link
Copy Markdown

Problem

rbw sync (and any command that needs to refresh the access token) reports a confusing error when the server rejects the stored refresh token:

rbw sync: failed to sync database from server: failed to parse JSON:
missing field `access_token` at line 1 column 25

Cause: exchange_refresh_token and exchange_refresh_token_async deserialize the response body unconditionally. When the identity server returns an error like {"error":"invalid_grant"} (HTTP 400), serde fails with missing field 'access_token', masking the real cause. This is a common real-world scenario: the refresh token has been revoked or has expired (e.g. after changing the master password), leaving the user without an actionable recovery path.

Relates to #32.

Changes

  • src/api.rs: check the response status code before parsing the response in both exchange_refresh_token* functions. On a non-200 response, parse the standard ConnectErrorRes and classify it through a new classify_refresh_token_error helper (consistent with the existing classify_login_error pattern):
    • invalid_grant → new Error::RefreshTokenInvalid
    • invalid_client → Error::IncorrectApiKey
    • otherwise → Error::RequestFailed
  • src/error.rs: add Error::RefreshTokenInvalid, with an error message telling users how to recover (rbw purge followed by rbw login).
  • Tests: add unit tests and mock HTTP tests for the refresh path (success, invalid_grant, invalid_client).

Result

rbw sync: failed to sync database from server: refresh token is invalid or
has been revoked (the master password may have been changed); run `rbw purge`
and then `rbw login` to re-authenticate

Note: this fix targets doy/rbw upstream; the PR currently lives on the fork.

@LockeAG

LockeAG commented Sep 11, 2026

Copy link
Copy Markdown

Confirmed against a self-hosted Vaultwarden, and the red lint check is not this PR's doing.

Reproduction. rbw 1.15.0, Vaultwarden, refresh token no longer accepted:

$ rbw sync
rbw sync: failed to sync database from server: failed to parse JSON: missing field `access_token` at line 1 column 25

The identity endpoint answers exactly as this PR assumes:

$ curl -s -w '%{http_code}\n' -X POST https://<vault-host>/identity/connect/token \
    -d grant_type=refresh_token -d client_id=cli -d refresh_token=bogus
{"error":"invalid_grant"}
400

That body is 25 bytes, which is where "column 25" comes from. So classify_refresh_token_error takes the invalid_grant arm and a self-hosted user gets the new message instead of a serde error.

The lint failure is pre-existing drift. I checked out f51c504 (this PR) and 77464d4 (master) and ran the lint job's commands locally with clippy 0.1.98. Both fail the same four lints:

  • src/actions.rs:116 and :126 — this match expression can be replaced with ?
  • src/api.rs:1744 on master, :1784 here — same classify_login_error body, this if can be collapsed into the outer match
  • src/dirs.rs:100 — redundant reference in format! argument

None of them is in this diff; the api.rs one is the same pre-existing function, shifted by the lines added here. cargo fmt --check passes on the branch and cargo test passes (7 lib tests, including the two new mock-server ones). Master's lint run is from 2025-12-31; current clippy flags code that run accepted, so master would go red on a rerun too. #376 looks like the fix for that.

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