I would like to automatically and silently lock users who fail to sign in too many times in a row, as per NIST 800-63b.
In the Rails world, Devise has a Lockable model that can be specified on account models.
This works, in essence, by a monkeypatch of the authentication chain. Because Devise calls valid_for_authentication? on the model without regard to what it is, Lockable simply overrides the method to perform a state change on the user model if the password didn't match. 1 2 3 4
However, there is no similarly overridable method in Pow.
At the bottom, there is Pow.Ecto.Schema.Changeset.verify_password/3, but this does not give access to any information about the user struct to the hash methods. (This would also be undesirable to overwrite, because it could trigger on password _confirmation_ forms and lock a user who is currently signed in out of the account.)
At the top, there is Pow.Plug.authenticate_user/2, but this does not return a user struct if authentication fails, by design.
This means that the only way to determine if a user existed, but should increment failed attempts, is to unconditionally attempt to reload the user struct from the database based on the params. While totally doable, since the session creation endpoint is not exactly high traffic in most applications, this seems undesirable and IMO, there should be a better way to deal with this.
I attempted to implement rate limitation per NIST 800-63b in #289 based on https://github.com/danschultzer/pow/issues/284#issuecomment-536031168, but ultimately decided against it being part of Pow.
While working on it I did think about this exact problem, and decided that the context should be where this is handled rather than at plug level. The idea (with #6 in mind) is that credentials rather than being a user id and password is a list of one or more attributes that is handled together. Either there is a user that exists with the full credentials (in this case email and password), or no such user exists. At least this has been my thoughts so far, but I'm open to any counter arguments.
With the current release of Pow, you can set up a custom context to record number of attempts in the DB, and lock the account if necessary:
defmodule MyApp.Users do
use Pow.Ecto.Context,
repo: MyApp.Repo,
user: MyApp.Users.User
import Ecto.Query
alias MyApp.{Users.User, Repo}
@impl Pow.Ecto.Context
def authenticate(params) do
case pow_authenticate(params) do
nil ->
increment_failed_attempts(params)
user ->
reset_failed_attempts(user)
end
end
defp increment_failed_attempts(params) do
user_id_field = User.pow_user_id_field()
user_id_value = params[Atom.to_string(user_id_field)]
query = from(u in User, update: [inc: [failed_login_attempts: 1]], where: field(u, ^user_id_field) == ^user_id_value)
Repo.update_all(query, [])
nil
end
defp reset_failed_attempts(user) do
case locked?(user) do
true ->
user # Could also be return nil here if you want to prevent auth at context level
false ->
user =
user
|> Ecto.Changeset.change(failed_login_attempts: 0)
|> Repo.update!()
user
end
end
defp locked?(%{failed_login_attempts: failed}) when failed > 100, do: true
defp locked?(_user), do: false
end
Add users_context: MyApp.Users to the config, and it should be working. From here you can easily lock the user by preventing successful sign in if the failed login attempts are too high, and add mechanism to unlock the user by resetting the count.
I've dealt with this issue many times on many systems.
First you have to ask the question, what is the problem I'm solving and for who.
Example, if all I care about is blocking bots from brute force attacks, I can just have fail2ban scrape my logs and ban anyone after n attempts which maybe a little too extreme for the normal users.
In which case you can just make a flag on the account and send a password reset email upon n failed logins and then lift the flag upon successful sign in.
Though I think my favorite is to set a flood limit via a failed_at timestamp and just don't allow retries within 1 min, normal users will opt for the reset password link and bots will pass you by due to time required.
Great input @joshchernoff. The various methods you explain is why I ultimately decided not to implement #289. Another common way is to use a CAPTCHA after X failed attempts for an IP. It's pretty context dependent, and as I tried to implement this basic rate limiter I realized that most times you probably want to deal with this at the gateway rather than the application especially when dealing with distribution.
I mean if someone wants to make an expansion lib thats one thing, but yeah I agree its too wide of a scope to satisfy everyone.
I love the fact you provided a guide instead, I feel there is probably more value in that than there is in code.
I decided to write an extension, PowLockout, that satisfied my needs. It works in a way very similar to the solution employed by Devise, with the caveat that it always reloads the user.
The reason I decided to make an extension and not just implement a few custom context methods (I actually did do the custom context, but for unrelated reasons) is because I needed a separate controller and a mailer to handle the email tokens that are generated by the locking system, and doing this as part of a controller inside my application would just needlessly clutter up the app logic with something that isn't really relevant to its normal use.
I also added a separate extension module, PowCaptcha, that is installed as a controller callback on certain actions to deal more directly with the rate-limiting aspect of account abuse. I didn't do this by overriding the sessions/registrations controllers, because that is a messy ordeal and the only thing I need to do is to fail the attempt if the captcha doesn't match. I don't have any custom authentication flow and using the default controllers and routes is fine.
Both of these extensions are actually app-agnostic, but they are tailored for my specific usecase. As such, I agree there isn't really any reason to include them in Pow.
That looks great @liamwhite!
Most helpful comment
I mean if someone wants to make an expansion lib thats one thing, but yeah I agree its too wide of a scope to satisfy everyone.
I love the fact you provided a guide instead, I feel there is probably more value in that than there is in code.