Skip to content
  • Vantalon, Thibaud (CIAT-Vietnam)'s avatar
    0f5db599
    fix(auth): one failure-based attempt budget, and stop it locking everyone out · 0f5db599
    Vantalon, Thibaud (CIAT-Vietnam) authored
    The global attempt budget was charged first and unconditionally, so a source
    already blocked by its own budget kept spending the shared one with every
    further request. One address, no credentials, could therefore hold every
    account out of logging in, changing a password or minting a token for the
    rest of each minute. The budgets are now charged narrowest-scope first and
    charging stops at whichever one blocks, so the global budget cannot be spent
    by traffic the narrower budgets are already refusing.
    
    They also count failures only, and a success clears the account's counter, so
    ordinary use -- a morning login, a handful of token mints -- never approaches
    a limit. That in turn lets the account budget span 15 minutes rather than one,
    which is what makes it meaningful against a weak password: ten guesses a
    minute is fourteen thousand a day.
    
    That one mechanism replaces the in-process failed-login throttle as well. Two
    overlapping limiters with different windows, different keys and different
    notions of what to count could not be reasoned about during an incident, and
    the in-process one multiplied by the number of workers.
    
    `last_used_at` is now stamped in a transaction of its own. Leaving it pending
    made every authenticated request hold a write lock from its first line, which
    on a single-writer store blocked the attempt counters -- whose whole point is
    that they commit when the request does not. The counters keep a documented
    fallback to the request's connection for that case, and a PostgreSQL test
    asserts the real path: three failed logins stay counted although each request
    rolled back.
    
    Also, from the same review:
    
    * `revoke_user_tokens` no longer takes the `keep=` that nothing passed, and
      the dependency docstring no longer claims the requesting token survives a
      password change -- it does not, deliberately.
    * `SAMPLEEARTH_TOKEN_TTL_DAYS` cannot be 0, so the two branches that handled
      "never expires" are gone. Documented in the README and the compose file,
      since a deployment set to 0 now refuses to start.
    0f5db599
    fix(auth): one failure-based attempt budget, and stop it locking everyone out
    Vantalon, Thibaud (CIAT-Vietnam) authored
    The global attempt budget was charged first and unconditionally, so a source
    already blocked by its own budget kept spending the shared one with every
    further request. One address, no credentials, could therefore hold every
    account out of logging in, changing a password or minting a token for the
    rest of each minute. The budgets are now charged narrowest-scope first and
    charging stops at whichever one blocks, so the global budget cannot be spent
    by traffic the narrower budgets are already refusing.
    
    They also count failures only, and a success clears the account's counter, so
    ordinary use -- a morning login, a handful of token mints -- never approaches
    a limit. That in turn lets the account budget span 15 minutes rather than one,
    which is what makes it meaningful against a weak password: ten guesses a
    minute is fourteen thousand a day.
    
    That one mechanism replaces the in-process failed-login throttle as well. Two
    overlapping limiters with different windows, different keys and different
    notions of what to count could not be reasoned about during an incident, and
    the in-process one multiplied by the number of workers.
    
    `last_used_at` is now stamped in a transaction of its own. Leaving it pending
    made every authenticated request hold a write lock from its first line, which
    on a single-writer store blocked the attempt counters -- whose whole point is
    that they commit when the request does not. The counters keep a documented
    fallback to the request's connection for that case, and a PostgreSQL test
    asserts the real path: three failed logins stay counted although each request
    rolled back.
    
    Also, from the same review:
    
    * `revoke_user_tokens` no longer takes the `keep=` that nothing passed, and
      the dependency docstring no longer claims the requesting token survives a
      password change -- it does not, deliberately.
    * `SAMPLEEARTH_TOKEN_TTL_DAYS` cannot be 0, so the two branches that handled
      "never expires" are gone. Documented in the README and the compose file,
      since a deployment set to 0 now refuses to start.
Loading