Skip to content

Fix lost HOTP counter increments during queued updates - #561

Open
frankii91 wants to merge 1 commit into
privacyidea:masterfrom
frankii91:fix/increment-hotp-from-current-counter
Open

frankii91 wants to merge 1 commit into
privacyidea:masterfrom
frankii91:fix/increment-hotp-from-current-counter

Conversation

@frankii91

Copy link
Copy Markdown
Contributor

Summary

Apply token updates to the latest state while holding the existing state mutex. This prevents overlapping HOTP increment requests from persisting the same next counter value.

Current behavior

The HOTP tile starts incrementCounter without waiting for its returned Future. The button's fixed one-second cooldown can expire while the previous repository write is still pending, so another press can pass the same token snapshot.

In the current implementation, _updateToken resolves the token before _replaceToken acquires _stateMutex, while incrementCounter calculates the next value from the supplied snapshot. Two overlapping calls can therefore both persist N + 1, losing one increment. The issue is timing-dependent and only manifests when the operations actually overlap, for example during a slower token write.

The user can then see the same HOTP value again instead of the expected next value, which can also leave the app counter out of step with the server.

Fix

  • Resolve the current token inside the existing _stateMutex.
  • Apply the updater and persist the result in the same critical section.
  • Calculate the next HOTP value from current.counter.
  • Preserve the existing missing-token and save-failure return behavior.

Regression test

The test starts two increments with the same counter-522 snapshot and verifies that the repository receives 523 followed by 524, with a final state of 524.

Validation

  • Focused regression test: passed.
  • Complete TokenNotifier test file: 14/14 passed.
  • Full Flutter test suite: 1320/1320 passed.
  • flutter analyze --no-pub: no issues found.
  • git diff --check: passed.

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