mirror of
https://github.com/heartcombo/devise.git
synced 2026-01-13 08:48:00 -05:00
62703943be
As reported in #4981, the method `#increment_failed_attempts` of `Devise::Models::Lockable` was not concurrency safe. The increment operation was being done in two steps: first the value was read from the database, and then incremented by 1. This may result in wrong values if two requests try to update the value concurrently. For example: ``` Browser1 -------> Read `failed_attempts` from DB (1) -------> Increment `failed_attempts` to 2 Browser2 -------> Read `failed_attempts` from DB (1) -------> Increment `failed_attempts` to 2 ``` In the example above, `failed_attempts` should have been set to 3, but it will be set to 2. This commit handles this case by calling `ActiveRecord::CounterCache.increment_counter` method, which will do both steps at once, reading the value straight from the database. This commit also adds a `ActiveRecord::AttributeMethods::Dirty#reload` call to ensure that the application gets the updated value - i.e. that other request might have updated. Although this does not ensure that the value is in fact the most recent one - other request could've updated it after the `reload` call - it seems good enough for this implementation. Even if a request does not locks the account because it has a stale value, the next one - that updated that value - will do it. That's why we decided not to use a pessimistic lock here. Closes #4981.