Browse Source

CLDC-1015: limit password reset (#356)

* Add a failing test (for a wrong reason)

* Is that correct?

* Add rack attack config for reset password

* Add support to read redis configuration

* Add PaaS configuration for redis

* Add too many requests page

* Redirect to the too many requests error page

* update manifest

Co-authored-by: Stéphane Meny <smeny@users.noreply.github.com>
pull/359/head
kosiakkatrina 3 years ago committed by GitHub
parent
commit
78c762b17f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      Gemfile
  2. 5
      Gemfile.lock
  3. 7
      app/controllers/errors_controller.rb
  4. 19
      app/services/paas_configuration_service.rb
  5. 8
      app/views/errors/too_many_requests.erb
  6. 20
      config/initializers/rack_attack.rb
  7. 1
      config/routes.rb
  8. 2
      manifest.yml
  9. 64
      spec/requests/rack_attack_spec.rb
  10. 57
      spec/services/paas_configuration_service_spec.rb

2
Gemfile

@ -50,6 +50,8 @@ gem "paper_trail"
# Store active record objects in version whodunnits
gem "paper_trail-globalid"
# Receive exceptions and configure alerts
gem "rack-attack"
gem "redis"
gem "sentry-rails"
gem "sentry-ruby"

5
Gemfile.lock

@ -290,6 +290,8 @@ GEM
nio4r (~> 2.0)
racc (1.6.0)
rack (2.2.3)
rack-attack (6.6.0)
rack (>= 1.0, < 3)
rack-mini-profiler (2.3.4)
rack (>= 1.2.0)
rack-proxy (0.7.2)
@ -332,6 +334,7 @@ GEM
rb-fsevent (0.11.1)
rb-inotify (0.10.1)
ffi (~> 1.0)
redis (4.6.0)
regexp_parser (2.2.1)
request_store (1.5.1)
rack (>= 1.4)
@ -490,8 +493,10 @@ DEPENDENCIES
postcodes_io
pry-byebug
puma (~> 5.0)
rack-attack
rack-mini-profiler (~> 2.0)
rails (~> 7.0.1)
redis
roo
rspec-rails
rubocop-govuk

7
app/controllers/errors_controller.rb

@ -22,4 +22,11 @@ class ErrorsController < ApplicationController
format.json { render json: { error: "Unprocessable entity" }, status: :unprocessable_entity }
end
end
def too_many_requests
respond_to do |format|
format.html { render status: :too_many_requests }
format.json { render json: { error: "Too many requests" }, status: :too_many_requests }
end
end
end

19
app/services/paas_configuration_service.rb

@ -1,10 +1,11 @@
class PaasConfigurationService
attr_reader :s3_buckets
attr_reader :s3_buckets, :redis_uris
def initialize(logger = Rails.logger)
@logger = logger
@paas_config = read_pass_config
@s3_buckets = read_s3_buckets
@redis_uris = read_redis_uris
end
def config_present?
@ -15,6 +16,10 @@ class PaasConfigurationService
config_present? && @paas_config.key?(:"aws-s3-bucket")
end
def redis_config_present?
config_present? && @paas_config.key?(:redis)
end
private
def read_pass_config
@ -42,4 +47,16 @@ private
end
s3_buckets
end
def read_redis_uris
return {} unless redis_config_present?
redis_uris = {}
@paas_config[:redis].each do |redis_config|
if redis_config.key?(:instance_name)
redis_uris[redis_config[:instance_name].to_sym] = redis_config.dig(:credentials, :uri)
end
end
redis_uris
end
end

8
app/views/errors/too_many_requests.erb

@ -0,0 +1,8 @@
<% content_for :title, "Sorry, you have sent too many requests to our service" %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<h1 class="govuk-heading-l"><%= content_for(:title) %></h1>
<p class="govuk-body">Try again in a minute.</p>
</div>
</div>

20
config/initializers/rack_attack.rb

@ -0,0 +1,20 @@
if Rails.env.development? || Rails.env.test?
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
Rack::Attack.enabled = false
else
redis_url = PaasConfigurationService.new.redis_uris[:"dluhc-core-#{Redis.env}-redis-rate-limit"]
Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(url: redis_url)
end
Rack::Attack.throttle("password reset requests", limit: 5, period: 60.seconds) do |request|
if request.params["user"].present? && request.path == "/users/password" && request.post?
request.params["user"]["email"].to_s.downcase.gsub(/\s+/, "")
end
end
Rack::Attack.throttled_responder = lambda do |_env|
headers = {
"Location" => "/429",
}
[301, headers, []]
end

1
config/routes.rb

@ -71,6 +71,7 @@ Rails.application.routes.draw do
scope via: :all do
match "/404", to: "errors#not_found"
match "/429", to: "errors#too_many_requests", status: 429
match "/422", to: "errors#unprocessable_entity"
match "/500", to: "errors#internal_server_error"
end

2
manifest.yml

@ -17,6 +17,7 @@ applications:
RAILS_ENV: staging
services:
- dluhc-core-staging-postgres
- dluhc-core-staging-redis-rate-limit
- name: dluhc-core-production
<<: *defaults
@ -30,3 +31,4 @@ applications:
host: submit-social-housing-lettings-sales-data
services:
- dluhc-core-production-postgres
- dluhc-core-production-redis-rate-limit

64
spec/requests/rack_attack_spec.rb

@ -0,0 +1,64 @@
require "rails_helper"
require_relative "../support/devise"
require "rack/attack"
describe "Rack::Attack" do
let(:limit) { 5 }
let(:under_limit) { limit / 2 }
let(:over_limit) { limit + 1 }
let(:page) { Capybara::Node::Simple.new(response.body) }
let(:notify_client) { instance_double(Notifications::Client) }
let(:devise_notify_mailer) { DeviseNotifyMailer.new }
let(:params) { { user: { email: } } }
let(:user) { FactoryBot.create(:user) }
let(:email) { user.email }
before do
Rack::Attack.enabled = true
allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer)
allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client)
allow(notify_client).to receive(:send_email).and_return(true)
end
after do
Rack::Attack.enabled = false
Rack::Attack.reset!
end
context "when a password reset is requested" do
context "when the number of requests is under the throttle limit" do
it "does not throttle" do
under_limit.times do
post "/users/password", params: params
follow_redirect!
end
last_response = response
expect(last_response.status).to eq(200)
end
end
context "when the number of requests is at the throttle limit" do
it "does not throttle" do
limit.times do
post "/users/password", params: params
follow_redirect!
end
last_response = response
expect(last_response.status).to eq(200)
end
end
context "when the number of requests is over the throttle limit" do
it "throttles" do
over_limit.times do
post "/users/password", params: params
follow_redirect!
end
last_response = response
expect(last_response.status).to eq(429)
end
end
end
end

57
spec/services/paas_configuration_service_spec.rb

@ -20,12 +20,15 @@ RSpec.describe PaasConfigurationService do
expect(config_service.s3_buckets).to be_a(Hash)
expect(config_service.s3_buckets).to be_empty
end
it "does not retrieve any redis configuration" do
expect(config_service.redis_uris).to be_a(Hash)
expect(config_service.redis_uris).to be_empty
end
end
context "when configuration is present but invalid" do
let(:vcap_services) do
{ "aws-s3-bucket": [{ instance_name: "bucket_1" }, { instance_name: "bucket_2" }] }
end
let(:vcap_services) { "random text" }
before do
allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services)
@ -85,4 +88,52 @@ RSpec.describe PaasConfigurationService do
expect(config_service.s3_buckets).to be_empty
end
end
context "when the paas configuration is present with redis configurations" do
let(:vcap_services) do
<<-JSON
{"redis": [{"instance_name": "redis_1", "credentials": {"uri": "redis_uri" }}]}
JSON
end
before do
allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services)
end
it "returns the configuration as present" do
expect(config_service.config_present?).to be(true)
end
it "returns the redis configuration as present" do
expect(config_service.redis_config_present?).to be(true)
end
it "does retrieve the redis configurations" do
redis_uris = config_service.redis_uris
expect(redis_uris).not_to be_empty
expect(redis_uris.count).to be(1)
expect(redis_uris).to have_key(:redis_1)
expect(redis_uris[:redis_1]).to eq("redis_uri")
end
end
context "when the paas configuration is present without redis configuration" do
before do
allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("{}")
end
it "returns the configuration as present" do
expect(config_service.config_present?).to be(true)
end
it "returns the redis configuration as not present" do
expect(config_service.redis_config_present?).to be(false)
end
it "does not retrieve any redis uris from configuration" do
expect(config_service.redis_uris).to be_a(Hash)
expect(config_service.redis_uris).to be_empty
end
end
end

Loading…
Cancel
Save