Skip to content

Token refresh: Do not report transient failures as expired token - #237

Open
paolostivanin wants to merge 1 commit into
opencloud-eu:mainfrom
paolostivanin:fix-token-refresh-relogin
Open

paolostivanin wants to merge 1 commit into
opencloud-eu:mainfrom
paolostivanin:fix-token-refresh-relogin

Conversation

@paolostivanin

@paolostivanin paolostivanin commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Users are often sent back to the login screen with "The access token has expired or become invalid". Any failed silent token refresh (network error, timeout, IdP 5xx, failed OIDC discovery) was reported that way. The authenticator returned nothing, the request was retried with an empty Bearer header, got a second 401 and surfaced as an expired session. Background uploads were marked as permanently failed. With mTLS there were two more problems on the same path.

Changes

  • AccountAuthenticator tells a refresh token rejected by the IdP (invalid_grant, invalid_client, unauthorized_client, access_denied) from a transient failure. Only a rejection asks for a new login. A transient failure throws NetworkErrorException, which reaches the callers as a connection problem (TokenRefreshFailedException, a SocketException) so it is retried and the stored tokens are kept.
  • TokenRequestRemoteOperation maps a rejected refresh grant to OAUTH2_ERROR. The login (authorization code) grant keeps its mapping.
  • ConnectionValidator no longer counts a failed refresh as a success and does not retry with stale credentials.
  • A known IdP whose discovery fails is a transient failure, instead of falling back to the legacy ownCloud token endpoint.
  • mTLS: the validation probe, OIDC discovery and the refresh request present the account's client certificate (before, the probe had none, so the expired token was never detected on an mTLS server).
  • Refreshes are serialized with a lock, and the rotated refresh token is stored before the access token. The validator no longer calls clearPassword for OAuth accounts, which wiped a token refreshed by a concurrent request.
  • FileDisplayActivity checks AccountUtils.isOAuth2Account instead of loading credentials, which could trigger another refresh.
  • Tokens are no longer logged.

Testing

  • New unit tests: ConnectionValidatorTest, AccountAuthenticatorTest, and refresh error mapping in OAuthRemoteOperationTest. The two behavioural validator tests fail on the old code.
  • Library, domain, data and app unit tests and detekt pass.
  • Installed on a device as an update over the previous build. Not tested against a real IdP, only with an mTLS setup.

Any failed silent token refresh (network error, timeout, IdP 5xx, failed
OIDC discovery) ended in an empty bearer token, a second 401 and the
"token expired, sign in again" prompt. Background uploads were marked as
permanently failed.

- AccountAuthenticator now tells a refresh token rejected by the IdP
  (invalid_grant and similar) from a transient failure. Only the former
  asks for a new login. The latter is reported as a connection problem
  and the stored tokens are kept.
- ConnectionValidator no longer counts a failed refresh as a success and
  does not retry with stale credentials.
- mTLS: the validation probe and the OIDC discovery and refresh requests
  now present the account's client certificate.
- Serialize refreshes with a lock and store the rotated refresh token
  before the access token.
- Stop logging tokens.
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