Skip to content

requestCredentials: rotated authorization data is persisted only on the branch that actually fetched credentials #1951

Description

@mc1100

Version: read on 0.99.0; re-checked against main @ 145e9e1a — the write-back placement is unchanged, though main has narrowed the exposure (see below).

This is a code reading, not an observed failure. We hit a refresh failure that we first attributed to this and were wrong — the real cause there was a 401 invalid_client from the issuer. We are reporting the code path itself, and we have not reproduced a data-loss case. Please weigh it accordingly.

What the code does

requestCredentials calls documentProvisioningHandler.updateDocument(..., documentAuthorizationData = provisioningClient.getAuthorizationData()) in two places, both of them inside branches that obtained credentials:

If a session opens, exchanges a token (rotating the refresh token, since this issuer rotates on use) and then finds nothing to fetch, the rotated authorization data is never written back. The stored refresh token would then be the spent one.

Why it is narrower on main than it was

openID4VCIRefreshCredentials now short-circuits before opening a session:

if (!documentProvisioningHandler.haveCredentialsToRefresh(document)) {

That guard (from #1873) removes the common route into the no-op case, and its docstring now promises no network I/O unless credentials need fetching. Two things it does not do: launchOpenID4VCIRefreshCredentials (L147) carries no equivalent check, and requestCredentials is reachable directly through launch(...). So the guard hides the case rather than removing it.

Suggested fix

Persist the authorization data whenever the provisioning client's authorization state changed, independently of whether credentials were fetched — e.g. a single updateDocument after the try block rather than one per success branch.

Workaround we ship

We ask DocumentUtil.managedCredentialHelper(dryRun = true) first and skip the session entirely when the answer is 0 — effectively the same guard main now has internally.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions