diff --git a/app/assets/stylesheets/petitions/views/_home.scss b/app/assets/stylesheets/petitions/views/_home.scss index b38d5f3c5..2ac0d3bb2 100644 --- a/app/assets/stylesheets/petitions/views/_home.scss +++ b/app/assets/stylesheets/petitions/views/_home.scss @@ -3,23 +3,39 @@ } .actioned-petitions { - @extend %contain-floats; + ul { + display: flex; + margin: 30px 0 0 0; + } + li { - float: left; + margin: 0; + padding: 0; width: 50%; - @include box-sizing(border-box); - @include media(tablet) { - padding-right: $gutter*3; - } - } - li:first-child { - padding-right: $gutter-half; } + .count { + display: block; font-size: 36px; - line-height: 40/36; - font-weight: 700; + font-weight: bold; + line-height: 1; + margin-bottom: 5px; + + @include media(tablet) { + font-size: 48px; + } + } + + .suffix { display: block; + font-size: 14px; + padding-right: 10px; + text-wrap: balance; + + @include media(tablet) { + font-size: 19px; + padding-right: 20px; + } } } diff --git a/app/controllers/admin/rate_limits_controller.rb b/app/controllers/admin/rate_limits_controller.rb index fa4bca6ae..e687b7c77 100644 --- a/app/controllers/admin/rate_limits_controller.rb +++ b/app/controllers/admin/rate_limits_controller.rb @@ -31,7 +31,7 @@ def rate_limit_attributes geoblocking_enabled countries country_rate_limits_enabled country_burst_rate country_sustained_rate trending_items_notification_url threshold_for_logging_trending_items threshold_for_notifying_trending_items - enable_logging_of_trending_items ignored_domains + enable_logging_of_trending_items ignored_domains blocked_emails ] end diff --git a/app/controllers/parliaments_controller.rb b/app/controllers/parliaments_controller.rb index 55129bfc5..e9847eb5d 100644 --- a/app/controllers/parliaments_controller.rb +++ b/app/controllers/parliaments_controller.rb @@ -2,6 +2,7 @@ class ParliamentsController < ApplicationController before_action :set_cors_headers, if: :json_request? before_action :fetch_parliaments, only: [:index] before_action :fetch_parliament, only: [:show] + before_action :fetch_constituencies, only: [:show] def index expires_in 1.hour, public: true, @@ -32,4 +33,8 @@ def fetch_parliaments def fetch_parliament @parliament = Parliament.archived.find_by!(period: params[:period]) end + + def fetch_constituencies + @constituencies = @parliament.constituencies.by_ons_code + end end diff --git a/app/models/rate_limit.rb b/app/models/rate_limit.rb index ee656b89e..3d996217c 100644 --- a/app/models/rate_limit.rb +++ b/app/models/rate_limit.rb @@ -7,6 +7,7 @@ class RateLimit < ActiveRecord::Base SINGLE_GLOB = "*." SINGLE_PATTERN = "(?:[-a-z0-9]+\\.)" DOMAIN_PATTERN = /^(?:[a-z0-9][a-z0-9-]{0,61}[a-z0-9]\.)+[a-z]{2,}$/ + EMAIL_PATTERN = /\A[^@\s]+@[^@\s]+\z/ validates :burst_rate, presence: true, numericality: { only_integer: true, greater_than: 0 } validates :burst_period, presence: true, numericality: { only_integer: true, greater_than: 0 } @@ -26,6 +27,7 @@ class RateLimit < ActiveRecord::Base validates :threshold_for_notifying_trending_items, presence: true, numericality: { only_integer: true, greater_than: 0 } validates :trending_items_notification_url, length: { maximum: 100, allow_blank: true } validates :ignored_domains, length: { maximum: 10000, allow_blank: true } + validates :blocked_emails, length: { maximum: 50000, allow_blank: true } validate do unless sustained_rate.nil? || burst_rate.nil? @@ -75,6 +77,12 @@ class RateLimit < ActiveRecord::Base rescue StandardError => e errors.add :ignored_domains, :invalid end + + begin + blocked_emails_list + rescue StandardError => e + errors.add :blocked_emails, :invalid + end end def reload @@ -84,6 +92,7 @@ def reload @blocked_ips_list = nil @allowed_countries = nil @ignored_domains_list = nil + @blocked_emails_list = nil super end @@ -92,6 +101,7 @@ def exceeded?(signature) return true if ip_blocked?(signature.ip_address) return true if ip_geoblocked?(signature.ip_address) return true if domain_blocked?(signature.domain) + return true if email_blocked?(signature.email) return false if ip_allowed?(signature.ip_address) return false if domain_allowed?(signature.domain) @@ -170,6 +180,15 @@ def ignored_domains_list @ignored_domains_list ||= build_ignored_domains end + def blocked_emails=(value) + @blocked_emails_list + super(normalize_lines(value)) + end + + def blocked_emails_list + @blocked_emails_list || build_blocked_emails + end + private def strip_comments(list) @@ -228,6 +247,17 @@ def build_ignored_domains strip_blank_lines(strip_comments(ignored_domains)).map { |d| validate_domain!(d.strip) } end + def build_blocked_emails + strip_blank_lines(strip_comments(blocked_emails)).map { |e| validate_email!(e.strip) } + end + + def email_blocked?(email) + return false if email.blank? + + canonical_email = Domain.normalize(email) + blocked_emails_list.any?{ |e| e == canonical_email } + end + def build_allowed_countries strip_blank_lines(strip_comments(countries)).map(&:strip) end @@ -272,6 +302,14 @@ def validate_domain!(domain) end end + def validate_email!(email) + if email =~ EMAIL_PATTERN + email + else + raise ArgumentError, "Invalid email: #{email.inspect}" + end + end + def normalize_lines(value) value.to_s.strip.gsub(/\r\n|\r/, "\n") end diff --git a/app/models/site.rb b/app/models/site.rb index e17fcb3d7..ecd6115de 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -213,11 +213,11 @@ def signature_count_updated_at!(timestamp = Time.current) end def moderation_overdue_in_days - 7.days + 10.days end def moderation_near_overdue_in_days - 5.days + 8.days end def maximum_number_of_signatures diff --git a/app/views/admin/admin/index.html.erb b/app/views/admin/admin/index.html.erb index dad993509..9ff0b0809 100644 --- a/app/views/admin/admin/index.html.erb +++ b/app/views/admin/admin/index.html.erb @@ -115,7 +115,7 @@ <% unless recently_in_moderation_untagged_count.zero? %> / <%= recently_in_moderation_untagged_count %> <% end %> - 0-5 days + 0-7 days <% end %> @@ -125,7 +125,7 @@ <% unless nearly_overdue_in_moderation_untagged_count.zero? %> / <%= nearly_overdue_in_moderation_untagged_count %> <% end %> - 6-7 days + 8-10 days <% end %> @@ -135,7 +135,7 @@ <% unless overdue_in_moderation_untagged_count.zero? %> / <%= overdue_in_moderation_untagged_count %> <% end %> - > 7 days + > 10 days <% end %> diff --git a/app/views/admin/rate_limits/_blocked_emails.html.erb b/app/views/admin/rate_limits/_blocked_emails.html.erb new file mode 100644 index 000000000..80f01f850 --- /dev/null +++ b/app/views/admin/rate_limits/_blocked_emails.html.erb @@ -0,0 +1,7 @@ +<%= hidden_field_tag :tab, "blocked_emails" %> + +<%= form_row for: [form.object, :blocked_emails] do %> + <%= error_messages_for_field @rate_limit, :blocked_emails %> + <%= form.text_area :blocked_emails, tabindex: increment, rows: 15, class: 'form-control' %> +

normalize the email address according to the <%= link_to "domain rules", admin_domains_path %>

+<% end %> diff --git a/app/views/admin/rate_limits/_tabs.html.erb b/app/views/admin/rate_limits/_tabs.html.erb index 40dc8fb08..c4241d005 100644 --- a/app/views/admin/rate_limits/_tabs.html.erb +++ b/app/views/admin/rate_limits/_tabs.html.erb @@ -1,6 +1,16 @@

- <% if params[:tab] == "allowed_domains" %> + <% if params[:tab] == "blocked_emails" %> <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + Blocked Emails | + <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | + <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | + <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | + <%= link_to "Blocked IPs", edit_admin_rate_limits_path(tab: 'blocked_ips') %> | + <%= link_to "Countries", edit_admin_rate_limits_path(tab: 'countries') %> | + <%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %> + <% elsif params[:tab] == "allowed_domains" %> + <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | Allowed Domains | <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | @@ -9,6 +19,7 @@ <%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %> <% elsif params[:tab] == "blocked_domains" %> <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | Blocked Domains | <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | @@ -17,6 +28,7 @@ <%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %> <% elsif params[:tab] == "allowed_ips" %> <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | Allowed IPs | @@ -25,6 +37,7 @@ <%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %> <% elsif params[:tab] == "blocked_ips" %> <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | @@ -33,6 +46,7 @@ <%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %> <% elsif params[:tab] == "countries" %> <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | @@ -41,6 +55,7 @@ <%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %> <% elsif params[:tab] == "trending_items" %> <%= link_to "Rate Limits", edit_admin_rate_limits_path %> | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | @@ -49,6 +64,7 @@ Trending Items <% else %> Rate Limits | + <%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> | <%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> | <%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> | <%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> | diff --git a/app/views/admin/rate_limits/edit.html.erb b/app/views/admin/rate_limits/edit.html.erb index c26874485..423654a5d 100644 --- a/app/views/admin/rate_limits/edit.html.erb +++ b/app/views/admin/rate_limits/edit.html.erb @@ -6,7 +6,9 @@

- <% if params[:tab] == "allowed_domains" %> + <% if params[:tab] == "blocked_emails" %> + <%= render "form", tab: "blocked_emails" %> + <% elsif params[:tab] == "allowed_domains" %> <%= render "form", tab: "allowed_domains" %> <% elsif params[:tab] == "blocked_domains" %> <%= render "form", tab: "blocked_domains" %> diff --git a/app/views/parliaments/show.json.jbuilder b/app/views/parliaments/show.json.jbuilder index 7d8a85e91..b9f899322 100644 --- a/app/views/parliaments/show.json.jbuilder +++ b/app/views/parliaments/show.json.jbuilder @@ -1,14 +1,16 @@ json.period @parliament.period json.dissolution_at @parliament.dissolution_at +json.government @parliament.government +json.response_threshold @parliament.threshold_for_response +json.debate_threshold @parliament.threshold_for_debate -json.attributes do - json.government @parliament.government - json.response_threshold @parliament.threshold_for_response - json.debate_threshold @parliament.threshold_for_debate -end -json.constituencies @parliament.constituencies.each do |constituency| - json.constituency constituency.name - json.ons_code constituency.ons_code - json.start_date constituency.start_date - json.end_date constituency.end_date +json.constituencies do + @constituencies.each do |constituency| + json.set! constituency.ons_code do + json.constituency constituency.name + json.ons_code constituency.ons_code + json.start_date constituency.start_date + json.end_date constituency.end_date + end + end end diff --git a/app/views/trackers/show.gif.ruby b/app/views/trackers/show.gif.ruby deleted file mode 100644 index 90e5c0ec8..000000000 --- a/app/views/trackers/show.gif.ruby +++ /dev/null @@ -1,8 +0,0 @@ -[ - 0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, - 0x00, 0x01, 0x00, 0x80, 0xff, 0x00, 0xc0, - 0xc0, 0xc0, 0x00, 0x00, 0x00, 0x21, 0xf9, - 0x04, 0x01, 0x00, 0x00, 0x00, 0x00, 0x2c, - 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, - 0x00, 0x00, 0x01, 0x01, 0x32, 0x00, 0x3b -].pack("C*") diff --git a/config/locales/activerecord.en-GB.yml b/config/locales/activerecord.en-GB.yml index 75d645d71..1eaf7ec1d 100644 --- a/config/locales/activerecord.en-GB.yml +++ b/config/locales/activerecord.en-GB.yml @@ -188,6 +188,8 @@ en-GB: invalid: "Blocked IPs list is invalid" ignored_domains: invalid: "Ignored domains list is invalid" + blocked_emails: + invalid: "Blocked emails list is invalid" tag: attributes: diff --git a/config/locales/petitions.en-GB.yml b/config/locales/petitions.en-GB.yml index 3c72d7e5c..7d6b3464d 100644 --- a/config/locales/petitions.en-GB.yml +++ b/config/locales/petitions.en-GB.yml @@ -242,20 +242,20 @@ en-GB: counts: awaiting_response: html: - one: "%{formatted_count} petition is waiting for a response from the Government" - other: "%{formatted_count} petitions are waiting for responses from the Government" + one: "%{formatted_count} petition from the current Parliament is waiting for a response from the Government" + other: "%{formatted_count} petitions from the current Parliament are waiting for responses from the Government" with_response: html: - one: "%{formatted_count} petition got a response from the Government" - other: "%{formatted_count} petitions got a response from the Government" + one: "%{formatted_count} petition from the current Parliament got a response from the Government" + other: "%{formatted_count} petitions from the current Parliament got a response from the Government" awaiting_debate: html: - one: "%{formatted_count} petition is waiting to be considered for debate" - other: "%{formatted_count} petitions are waiting to be considered for debate" + one: "%{formatted_count} petition from the current Parliament is waiting to be considered for debate" + other: "%{formatted_count} petitions from the current Parliament are waiting to be considered for debate" with_debated_outcome: html: - one: "%{formatted_count} petition was debated in the House of Commons" - other: "%{formatted_count} petitions were debated in the House of Commons" + one: "%{formatted_count} petition from the current Parliament was debated in the House of Commons" + other: "%{formatted_count} petitions from the current Parliament were debated in the House of Commons" awaiting_response_explanation: html: "See all petitions waiting for a government response (%{formatted_count})" with_response_explanation: diff --git a/db/migrate/20241221160446_add_blocked_emails_to_rate_limit.rb b/db/migrate/20241221160446_add_blocked_emails_to_rate_limit.rb new file mode 100644 index 000000000..5e3337169 --- /dev/null +++ b/db/migrate/20241221160446_add_blocked_emails_to_rate_limit.rb @@ -0,0 +1,5 @@ +class AddBlockedEmailsToRateLimit < ActiveRecord::Migration[7.2] + def change + add_column :rate_limits, :blocked_emails, :string, limit: 50000, null: false, default: "" + end +end diff --git a/db/schema.rb b/db/schema.rb index 9f6688eb2..a0ae3699d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_10_21_142904) do +ActiveRecord::Schema[7.2].define(version: 2024_12_21_160446) do # These are extensions that must be enabled in order to support this database enable_extension "intarray" enable_extension "plpgsql" @@ -617,6 +617,7 @@ t.integer "creator_rate", default: 2, null: false t.integer "sponsor_rate", default: 5, null: false t.integer "feedback_rate", default: 2, null: false + t.string "blocked_emails", limit: 50000, default: "", null: false end create_table "regions", force: :cascade do |t| diff --git a/features/admin/updating_rate_limits.feature b/features/admin/updating_rate_limits.feature index da4e787a9..d2b79681c 100644 --- a/features/admin/updating_rate_limits.feature +++ b/features/admin/updating_rate_limits.feature @@ -17,6 +17,20 @@ Feature: Sysadmin updates the rate limits And I press "Save" Then I should see "Rate limits updated successfully" + Scenario: Sysadmin updates the blocked emails list + When I am logged in as a sysadmin + And I am on the admin home page + And I follow "Rate Limits" + Then I should see "Edit Rate Limits" + When I follow "Blocked Emails" + Then I should see "normalize the email address according to the domain rules" + When I fill in "rate_limit_blocked_emails" with "foo" + And I press "Save" + Then I should see "Blocked emails list is invalid" + When I fill in "rate_limit_blocked_emails" with "user@example.com" + And I press "Save" + Then I should see "Rate limits updated successfully" + Scenario: Sysadmin updates the allowed domains list When I am logged in as a sysadmin And I am on the admin home page diff --git a/features/charlie_creates_a_petition.feature b/features/charlie_creates_a_petition.feature index 475ac6f8e..4734466ee 100644 --- a/features/charlie_creates_a_petition.feature +++ b/features/charlie_creates_a_petition.feature @@ -188,7 +188,7 @@ Scenario: Charlie creates a petition when his email is autocorrected wrongly Then a petition should exist with action: "The wombats of wimbledon rock.", state: "pending" And a signature should exist with email: "charlie@hotmial.com", state: "pending" -Scenario: Charlie creates a petition when blocked +Scenario: Charlie creates a petition when his IP address is blocked Given the IP address 127.0.0.1 is blocked And I start a new petition And I fill in the petition details @@ -202,6 +202,20 @@ Scenario: Charlie creates a petition when blocked Then a petition should not exist with action: "The wombats of wimbledon rock.", state: "pending" And a signature should not exist with email: "womboid@wimbledon.com", state: "pending" +Scenario: Charlie creates a petition when his email address is blocked + Given the email address "womboid@wimbledon.com" is blocked + And I start a new petition + And I fill in the petition details + And I press "Preview petition" + And I press "This looks good" + And I fill in my details + When I press "Continue" + Then the markup should be valid + And I am asked to review my email address + When I press "Yes – this is my email address" + Then a petition should not exist with action: "The wombats of wimbledon rock.", state: "pending" + And a signature should not exist with email: "womboid@wimbledon.com", state: "pending" + Scenario: Charlie creates a petition when his IP address is rate limited Given the creator rate limit is 1 per hour And there are no allowed IPs diff --git a/features/step_definitions/rate_limit_steps.rb b/features/step_definitions/rate_limit_steps.rb index d613b0770..f98ea61bf 100644 --- a/features/step_definitions/rate_limit_steps.rb +++ b/features/step_definitions/rate_limit_steps.rb @@ -38,6 +38,10 @@ RateLimit.first_or_create!.update!(allowed_domains: domain) end +Given(/^the email address "(.*?)" is blocked$/) do |email| + RateLimit.first_or_create!.update!(blocked_emails: email) +end + Given(/^there is a signature already from this IP address$/) do steps %Q( When I go to the new signature page for "Do something!" diff --git a/features/suzie_signs_a_petition.feature b/features/suzie_signs_a_petition.feature index 276fc3b04..7692e11d7 100644 --- a/features/suzie_signs_a_petition.feature +++ b/features/suzie_signs_a_petition.feature @@ -221,6 +221,57 @@ Feature: Suzie signs a petition And I should see "This petition is closed" And I should see "2 signatures" + Scenario: Suzie cannot validate her signature when the domain is blocked + Given the domain "wimbledon.com" is blocked + When I am on the new signature page + And I fill in my details + And I try to sign + Then I should be on the new signature page + When I say I am happy with my email address + Then I am told to check my inbox to complete signing + And "womboid@wimbledon.com" should receive 1 email + And I confirm my email address + Then I should see "We've added your signature to the petition" + And I should see "2 signatures" + When I follow "Do something!" + Then I should be on the petition page + And I should see "1 signature" + And the signature "womboid@wimbledon.com" is marked as fraudulent + + Scenario: Suzie cannot validate her signature when the email address is blocked + Given the email address "womboid@wimbledon.com" is blocked + When I am on the new signature page + And I fill in my details + And I try to sign + Then I should be on the new signature page + When I say I am happy with my email address + Then I am told to check my inbox to complete signing + And "womboid@wimbledon.com" should receive 1 email + And I confirm my email address + Then I should see "We've added your signature to the petition" + And I should see "2 signatures" + When I follow "Do something!" + Then I should be on the petition page + And I should see "1 signature" + And the signature "womboid@wimbledon.com" is marked as fraudulent + + Scenario: Suzie can validate her signature when another email address on the same domain is blocked + Given the email address "bulgaria@wimbledon.com" is blocked + When I am on the new signature page + And I fill in my details + And I try to sign + Then I should be on the new signature page + When I say I am happy with my email address + Then I am told to check my inbox to complete signing + And "womboid@wimbledon.com" should receive 1 email + And I confirm my email address + Then I should see "We've added your signature to the petition" + And I should see "2 signatures" + When I follow "Do something!" + Then I should be on the petition page + And I should see "2 signatures" + And the signature "womboid@wimbledon.com" is marked as validated + Scenario: Suzie cannot validate her signature when IP address is rate limited Given the burst rate limit is 1 per minute And there are no allowed IPs diff --git a/features/user_sends_feedback.feature b/features/user_sends_feedback.feature index 4a775b195..deceba25c 100644 --- a/features/user_sends_feedback.feature +++ b/features/user_sends_feedback.feature @@ -33,6 +33,17 @@ Feature: User sends feedback And the site owners should not be notified And a feedback should not exist with comment: "I must protest" + Scenario: User is blocked by email address + Given the email address "bob@example.com" is blocked + And I am on the feedback page + When I fill in "Comments" with "I must protest" + And I fill in "Email address" with "bob@example.com" + And I press "Send feedback" + Then I should see "Your feedback has been sent" + Then the markup should be valid + And the site owners should not be notified + And a feedback should not exist with comment: "I must protest" + Scenario: User is blocked by IP address rate limiting Given the feedback rate limit is 1 per hour And there are no allowed IPs diff --git a/spec/controllers/admin/rate_limits_controller_spec.rb b/spec/controllers/admin/rate_limits_controller_spec.rb index cb7dd7037..f68242e75 100644 --- a/spec/controllers/admin/rate_limits_controller_spec.rb +++ b/spec/controllers/admin/rate_limits_controller_spec.rb @@ -85,6 +85,20 @@ end end + context "when submitting just the blocked emails list" do + let :params do + { blocked_emails: "user@example.com" } + end + + it "redirects to the edit page" do + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/rate-limits/edit") + end + + it "sets the flash notice message" do + expect(flash[:notice]).to eq("Rate limits updated successfully") + end + end + context "when submitting just the allowed domains list" do let :params do { allowed_domains: "example.com" } diff --git a/spec/factories.rb b/spec/factories.rb index d9b477763..4b3332bb3 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -751,6 +751,8 @@ mp_name { "#{Faker::Name.name} MP" } mp_id { generate(:mp_id) } example_postcode { Faker::Address.postcode } + start_date { "2010-04-13"} + end_date { nil } end factory :constituency_petition_journal do diff --git a/spec/helpers/home_helper_spec.rb b/spec/helpers/home_helper_spec.rb index 75b47a6fb..d4b5248a0 100644 --- a/spec/helpers/home_helper_spec.rb +++ b/spec/helpers/home_helper_spec.rb @@ -47,19 +47,19 @@ context "when the petition count is 1" do it "returns a correctly formatted petition count" do - expect(helper.petition_count(:with_response, 1)).to eq("1 petition got a response from the Government") + expect(helper.petition_count(:with_response, 1)).to eq("1 petition from the current Parliament got a response from the Government") end end context "when the petition count is 100" do it "returns a correctly formatted petition count" do - expect(helper.petition_count(:with_response, 100)).to eq("100 petitions got a response from the Government") + expect(helper.petition_count(:with_response, 100)).to eq("100 petitions from the current Parliament got a response from the Government") end end context "when the petition count is 1000" do it "returns a correctly formatted petition count" do - expect(helper.petition_count(:with_response, 1000)).to eq("1,000 petitions got a response from the Government") + expect(helper.petition_count(:with_response, 1000)).to eq("1,000 petitions from the current Parliament got a response from the Government") end end end @@ -71,19 +71,19 @@ context "when the petition count is 1" do it "returns a correctly formatted petition count" do - expect(helper.petition_count(:with_debated_outcome, 1)).to eq("1 petition was debated in the House of Commons") + expect(helper.petition_count(:with_debated_outcome, 1)).to eq("1 petition from the current Parliament was debated in the House of Commons") end end context "when the petition count is 100" do it "returns a correctly formatted petition count" do - expect(helper.petition_count(:with_debated_outcome, 100)).to eq("100 petitions were debated in the House of Commons") + expect(helper.petition_count(:with_debated_outcome, 100)).to eq("100 petitions from the current Parliament were debated in the House of Commons") end end context "when the petition count is 1000" do it "returns a correctly formatted petition count" do - expect(helper.petition_count(:with_debated_outcome, 1000)).to eq("1,000 petitions were debated in the House of Commons") + expect(helper.petition_count(:with_debated_outcome, 1000)).to eq("1,000 petitions from the current Parliament were debated in the House of Commons") end end end diff --git a/spec/models/petition_spec.rb b/spec/models/petition_spec.rb index deb8b931e..c4dbf755b 100644 --- a/spec/models/petition_spec.rb +++ b/spec/models/petition_spec.rb @@ -668,47 +668,47 @@ context "with a :from argument" do it "returns all petitions awaiting moderation after the timestamp" do - expect(Petition.in_moderation(from: 5.days.ago)).to include(recent_petition) + expect(Petition.in_moderation(from: 7.days.ago)).to include(recent_petition) end it "doesn't return petitions awaiting moderation before the timestamp" do - expect(Petition.in_moderation(from: 5.days.ago)).not_to include(overdue_petition, nearly_overdue_petition) + expect(Petition.in_moderation(from: 7.days.ago)).not_to include(overdue_petition, nearly_overdue_petition) end it "doesn't return petitions in other states" do - expect(Petition.in_moderation(from: 5.days.ago)).not_to include(open_petition) + expect(Petition.in_moderation(from: 7.days.ago)).not_to include(open_petition) end end context "with a :to argument" do it "returns all petitions awaiting moderation before the timestamp" do - expect(Petition.in_moderation(to: 7.days.ago)).to include(overdue_petition) + expect(Petition.in_moderation(to: 10.days.ago)).to include(overdue_petition) end it "doesn't return petitions awaiting moderation after the timestamp" do - expect(Petition.in_moderation(to: 7.days.ago)).not_to include(recent_petition, nearly_overdue_petition) + expect(Petition.in_moderation(to: 10.days.ago)).not_to include(recent_petition, nearly_overdue_petition) end it "doesn't return petitions in other states" do - expect(Petition.in_moderation(to: 7.days.ago)).not_to include(open_petition) + expect(Petition.in_moderation(to: 10.days.ago)).not_to include(open_petition) end end context "with both a :from and :to argument" do it "returns all petitions awaiting moderation between the timestamps" do - expect(Petition.in_moderation(from: 7.days.ago, to: 5.days.ago)).to include(nearly_overdue_petition) + expect(Petition.in_moderation(from: 10.days.ago, to: 8.days.ago)).to include(nearly_overdue_petition) end it "doesn't return petitions awaiting moderation before the timestamp" do - expect(Petition.in_moderation(from: 7.days.ago, to: 5.days.ago)).not_to include(overdue_petition) + expect(Petition.in_moderation(from: 10.days.ago, to: 8.days.ago)).not_to include(overdue_petition) end it "doesn't return petitions awaiting moderation after the timestamp" do - expect(Petition.in_moderation(from: 7.days.ago, to: 5.days.ago)).not_to include(recent_petition) + expect(Petition.in_moderation(from: 10.days.ago, to: 8.days.ago)).not_to include(recent_petition) end it "doesn't return petitions in other states" do - expect(Petition.in_moderation(from: 7.days.ago, to: 5.days.ago)).not_to include(open_petition) + expect(Petition.in_moderation(from: 10.days.ago, to: 8.days.ago)).not_to include(open_petition) end end end diff --git a/spec/models/rate_limit_spec.rb b/spec/models/rate_limit_spec.rb index d3a9c0e3a..594d6f177 100644 --- a/spec/models/rate_limit_spec.rb +++ b/spec/models/rate_limit_spec.rb @@ -114,6 +114,17 @@ expect(subject.errors[:ignored_domains]).to include("Ignored domains list is invalid") end end + + context "when the blocked email list is invalid" do + before do + subject.update(blocked_emails: "foo") + end + + it "adds an error message" do + expect(subject.valid?).to eq(false) + expect(subject.errors[:blocked_emails]).to include("Blocked emails list is invalid") + end + end end describe "#exceeded?" do @@ -125,6 +136,8 @@ let(:blocked_domains) { "" } let(:blocked_ips) { "" } + let(:ignored_domains) { "" } + let(:blocked_emails) { "" } let(:countries) { "" } let(:geoblocking_enabled) { false } @@ -136,6 +149,7 @@ sustained_rate: 20, sustained_period: 300, allowed_domains: allowed_domains, allowed_ips: allowed_ips, blocked_domains: blocked_domains, blocked_ips: blocked_ips, + ignored_domains: ignored_domains, blocked_emails: blocked_emails, countries: countries, geoblocking_enabled: geoblocking_enabled, country_rate_limits_enabled: country_rate_limits_enabled, country_burst_rate: 20, country_sustained_rate: 120 @@ -270,6 +284,20 @@ end end + shared_examples_for "blocked emails" do + let(:blocked_emails) { "user@foo.com\n" } + + it "returns false when the email is not blocked" do + allow(signature).to receive(:email).and_return("user@example.com") + expect(subject.exceeded?(signature)).to eq(false) + end + + it "returns true when the domain is blocked" do + allow(signature).to receive(:email).and_return("user@foo.com") + expect(subject.exceeded?(signature)).to eq(true) + end + end + context "when both rates are below the threshold" do before do allow(signature).to receive(:rate).with(60).and_return(5) @@ -283,6 +311,7 @@ it_behaves_like "blocked domains" it_behaves_like "blocked IPs" it_behaves_like "GeoIP blocking" + it_behaves_like "blocked emails" end context "when the burst rate is above the threshold" do @@ -557,6 +586,16 @@ end end + describe "#blocked_emails=" do + subject do + described_class.new(blocked_emails: " user@foo.com\r\nuser@bar.com\r\n") + end + + it "normalizes line endings and strips whitespace" do + expect(subject.blocked_emails).to eq("user@foo.com\nuser@bar.com") + end + end + describe "#countries=" do subject do described_class.new(countries: " United Kingdom\r\nIreland\r\n") @@ -903,6 +942,72 @@ end end + describe "#blocked_emails_list" do + subject do + described_class.create!(blocked_emails: blocked_emails) + end + + context "when there is extra whitespace" do + let :blocked_emails do + <<-EOF + user@foo.com + user@bar.com + + EOF + end + + it "is is stripped" do + expect(subject.blocked_emails_list).to eq(%w[user@foo.com user@bar.com]) + end + end + + context "when there are blank lines" do + let :blocked_emails do + <<-EOF + user@foo.com + + user@bar.com + + EOF + end + + it "they are stripped" do + expect(subject.blocked_emails_list).to eq(%w[user@foo.com user@bar.com]) + end + end + + context "when there are line comments" do + let :blocked_emails do + <<-EOF + # This is a test + user@foo.com + + user@bar.com + + EOF + end + + it "they are stripped" do + expect(subject.blocked_emails_list).to eq(%w[user@foo.com user@bar.com]) + end + end + + context "when there are inline comments" do + let :blocked_emails do + <<-EOF + user@foo.com # This is a test + + user@bar.com + + EOF + end + + it "they are stripped" do + expect(subject.blocked_emails_list).to eq(%w[user@foo.com user@bar.com]) + end + end + end + describe "#allowed_countries" do subject do described_class.create!(countries: countries) diff --git a/spec/requests/parliaments_spec.rb b/spec/requests/parliaments_spec.rb index 29d6ccfb0..91ad284b6 100644 --- a/spec/requests/parliaments_spec.rb +++ b/spec/requests/parliaments_spec.rb @@ -35,30 +35,97 @@ end describe "data fields" do - it "returns a list of parliaments", :skip_before_hook do - parliament = FactoryBot.create(:parliament, :dissolved, state_opening_at: "2021/05/23", opening_at: "2021/05/30", dissolution_at: "2022/06/30", period: "2021-2022", archived_at: "2022/08/30") - parliament_2 = FactoryBot.create(:parliament, :dissolved, state_opening_at: "2022/05/23", opening_at: "2022/05/30", dissolution_at: "2023/06/30", period: "2023-2024", archived_at: "2023/08/30") + before do + FactoryBot.create( + :parliament, :dissolved, + state_opening_at: "2021/05/23", + opening_at: "2021/05/30", + dissolution_at: "2022/06/30", + archived_at: "2022/08/30" + ) + FactoryBot.create( + :parliament, :dissolved, + state_opening_at: "2022/05/23", + opening_at: "2022/05/30", + dissolution_at: "2023/06/30", + archived_at: "2023/08/30" + ) + end + + subject(:json) { JSON.parse(response.body) } + + it "returns a list of parliaments" do get "/parliaments.json" - expect(JSON.parse(response.body).first).to match({"debate_threshold"=>100000, "dissolution_at"=>"2023-06-30T00:00:00.000+01:00", "government"=>"Conservative", "period"=>"2022-2023", "response_threshold"=>10000}) + expect(json).to match([ + { + "period" => "2022-2023", + "dissolution_at" => "2023-06-30T00:00:00.000+01:00", + "government" => "Conservative", + "response_threshold" => 10000, + "debate_threshold" => 100000 + }, + { + "period" => "2021-2022", + "dissolution_at" => "2022-06-30T00:00:00.000+01:00", + "government" => "Conservative", + "response_threshold" => 10000, + "debate_threshold" => 100000 + } + ]) end end - describe "data fields for constituencies", :skip_before_hook do - it "returns a list of constituencies for a specific parliament" do - parliament = FactoryBot.create(:parliament, :dissolved, state_opening_at: "2021/05/23", opening_at: "2021/05/30", dissolution_at: "2022/06/30", period: "2021-2022", archived_at: "2022/08/30") - constituency = FactoryBot.create(:constituency, name: "Buckinghamshire", ons_code: "E00000001") - constituency_2 = FactoryBot.create(:constituency, name: "Aberafan") + describe "data fields for constituencies" do + before do + parliament = FactoryBot.build( + :parliament, :dissolved, + state_opening_at: "2021/05/23", + opening_at: "2021/05/30", + dissolution_at: "2022/06/30", + archived_at: "2022/08/30" + ) - parliament.constituencies << constituency - parliament.constituencies << constituency_2 + parliament.constituencies << FactoryBot.build(:constituency, name: "Altrincham and Sale West", ons_code: "E14000532") + parliament.constituencies << FactoryBot.build(:constituency, name: "Aldershot", ons_code: "E14000530") + parliament.constituencies << FactoryBot.build(:constituency, name: "Aldridge-Brownhills", ons_code: "E14000531") + parliament.save! + end + + subject(:json) { JSON.parse(response.body) } + + it "returns a list of constituencies for a specific parliament" do get "/parliaments/2021-2022.json" - expect(JSON.parse(response.body)["period"]).to match("2021-2022") - expect(JSON.parse(response.body)["constituencies"].length).to match(2) - expect(JSON.parse(response.body)["constituencies"].first).to match({"constituency"=>"Buckinghamshire", "end_date"=>nil, "ons_code"=>"E00000001", "start_date"=>nil}) + expect(json).to match({ + "period" => "2021-2022", + "dissolution_at" => "2022-06-30T00:00:00.000+01:00", + "government" => "Conservative", + "response_threshold" => 10000, + "debate_threshold" => 100000, + "constituencies" => { + "E14000530" => { + "constituency" =>"Aldershot", + "ons_code" => "E14000530", + "start_date" => "2010-04-13", + "end_date" => nil + }, + "E14000531" => { + "constituency" => "Aldridge-Brownhills", + "ons_code" => "E14000531", + "start_date" => "2010-04-13", + "end_date" => nil + }, + "E14000532" => { + "constituency" => "Altrincham and Sale West", + "ons_code" => "E14000532", + "start_date" => "2010-04-13", + "end_date" => nil + } + } + }) end end end diff --git a/spec/support/parliament.rb b/spec/support/parliament.rb index 9486e372c..e910601bb 100644 --- a/spec/support/parliament.rb +++ b/spec/support/parliament.rb @@ -5,8 +5,6 @@ end config.before(:each) do |example| - unless example.metadata[:skip_before_hook] - Parliament.reset!(government: "TBC", opening_at: 2.weeks.ago) - end + Parliament.reset!(government: "TBC", opening_at: 2.weeks.ago) end end