Skip to content

fix: block after Max failed attempts, not Max+1 - #175

Open
lamtrinhdev wants to merge 1 commit into
ivpn:mainfrom
lamtrinhdev:Fix_utils-id_limiter.go
Open

lamtrinhdev wants to merge 1 commit into
ivpn:mainfrom
lamtrinhdev:Fix_utils-id_limiter.go

Conversation

@lamtrinhdev

Copy link
Copy Markdown
Contributor

PR type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other... Please describe:

Problem

IDLimiter.IsAllowed() used failedAttemptsInt > l.Max, so the limiter only
kicked in on attempt Max+1 — one more failed attempt than ID_LIMITER_MAX
promises. The counter is incremented before the check (Tick on failure,
then IsAllowed), so with Max=5 the 6th failed attempt was still "allowed"
and only the 7th got blocked.

Both users of the limiter are affected: TOTP verification (user.go) and
recipient OTP activation (recipient.go).

Fix

One-character change: > → >=. After the Max-th failure the limiter now
reports blocked, and every further attempt — including one with a valid
code — is rejected until the key expires (ID_LIMITER_EXPIRATION).

@lamtrinhdev

Copy link
Copy Markdown
Contributor Author

Dear @jurajhilje ,

I have a small fix, when you have time, please help me to review it.

Thanks,
Lam

@jurajhilje jurajhilje self-assigned this Oct 19, 2026
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