FEATURE: tighten rate limiting rules for forgot password

1. Total 6 attempts per day per user
2. Total of 5 per unique email/login that is not found per hour
3. If an admin blocks an IP that IP can not request a reset
This commit is contained in:
Sam Saffron 2020-05-08 13:30:16 +10:00
parent 1840023c28
commit d8d54a92f1
No known key found for this signature in database
GPG Key ID: B9606168D2FFD9F5
3 changed files with 59 additions and 3 deletions
app/controllers
config/locales
spec/requests

View File

@ -401,13 +401,21 @@ class SessionController < ApplicationController
def forgot_password
params.require(:login)
if ScreenedIpAddress.should_block?(request.remote_ip)
return render_json_error(I18n.t("login.reset_not_allowed_from_ip_address"))
end
RateLimiter.new(nil, "forgot-password-hr-#{request.remote_ip}", 6, 1.hour).performed!
RateLimiter.new(nil, "forgot-password-min-#{request.remote_ip}", 3, 1.minute).performed!
RateLimiter.new(nil, "forgot-password-login-hour-#{params[:login].to_s[0..100]}", 12, 1.hour).performed!
RateLimiter.new(nil, "forgot-password-login-min-#{params[:login].to_s[0..100]}", 3, 1.minute).performed!
user = User.find_by_username_or_email(params[:login])
if user
RateLimiter.new(nil, "forgot-password-login-day-#{user.username}", 6, 1.day).performed!
else
RateLimiter.new(nil, "forgot-password-login-hour-#{params[:login].to_s[0..100]}", 5, 1.hour).performed!
end
user_presence = user.present? && user.human? && !user.staged
if user_presence
email_token = user.email_tokens.create(email: user.email)

View File

@ -2385,6 +2385,7 @@ en:
not_activated: "You can't log in yet. We sent an activation email to you. Please follow the instructions in the email to activate your account."
not_allowed_from_ip_address: "You can't log in as %{username} from that IP address."
admin_not_allowed_from_ip_address: "You can't log in as admin from that IP address."
reset_not_allowed_from_ip_address: "You can't request a password reset from that IP address."
suspended: "You can't log in until %{date}."
suspended_with_reason: "Account suspended until %{date}: %{reason}"
errors: "%{errors}"

View File

@ -1825,6 +1825,53 @@ RSpec.describe SessionController do
expect(response.status).to eq(400)
end
it 'should correctly screen ips' do
ScreenedIpAddress.create!(
ip_address: '100.0.0.1',
action_type: ScreenedIpAddress.actions[:block]
)
post "/session/forgot_password.json",
params: { login: 'made_up' },
headers: { 'REMOTE_ADDR' => '100.0.0.1' }
expect(response.parsed_body).to eq({
"errors" => [I18n.t("login.reset_not_allowed_from_ip_address")]
})
end
it 'should correctly rate limits' do
RateLimiter.enable
RateLimiter.clear_all!
user = Fabricate(:user)
3.times do
post "/session/forgot_password.json", params: { login: user.username }
expect(response.status).to eq(200)
end
post "/session/forgot_password.json", params: { login: user.username }
expect(response.status).to eq(422)
3.times do
post "/session/forgot_password.json",
params: { login: user.username },
headers: { 'REMOTE_ADDR' => '10.1.1.1' }
expect(response.status).to eq(200)
end
post "/session/forgot_password.json",
params: { login: user.username },
headers: { 'REMOTE_ADDR' => '100.1.1.1' }
# not allowed, max 6 a day
expect(response.status).to eq(422)
end
context 'for a non existant username' do
it "doesn't generate a new token for a made up username" do
expect do