From a71928ae317f30d42ae43ddf1cb384383c378d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 22 Apr 2019 13:35:59 +0200 Subject: [PATCH 1/6] Update share message interpolation variables Having translations with the old interpolation variables was causing the application to crash. --- config/locales/ar/budgets.yml | 2 +- config/locales/de-DE/budgets.yml | 2 +- config/locales/gl/budgets.yml | 2 +- config/locales/gl/general.yml | 2 +- config/locales/nl/budgets.yml | 2 +- config/locales/ru/general.yml | 2 +- config/locales/so-SO/budgets.yml | 2 +- config/locales/so-SO/general.yml | 2 +- config/locales/sq-AL/budgets.yml | 2 +- config/locales/sq-AL/general.yml | 2 +- config/locales/val/budgets.yml | 2 +- config/locales/val/general.yml | 2 +- config/locales/zh-CN/budgets.yml | 2 +- config/locales/zh-CN/general.yml | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) diff --git a/config/locales/ar/budgets.yml b/config/locales/ar/budgets.yml index e9faed45b..7ca2652f8 100644 --- a/config/locales/ar/budgets.yml +++ b/config/locales/ar/budgets.yml @@ -97,7 +97,7 @@ ar: confidence_score: اعلى تقييم price: حسب السعر share: - message: "لقد أنشأت مشروع استثماري %{title} في %{org}. أنشىء مشروع الاستثماري أنت أيضاً!" + message: "لقد أنشأت مشروع استثماري %{title} في %{handle}. أنشىء مشروع الاستثماري أنت أيضاً!" show: author_deleted: تم حذف المستخدم price_explanation: توضيح الاسعار diff --git a/config/locales/de-DE/budgets.yml b/config/locales/de-DE/budgets.yml index 0879b1b44..446e3e7d0 100644 --- a/config/locales/de-DE/budgets.yml +++ b/config/locales/de-DE/budgets.yml @@ -106,7 +106,7 @@ de: confidence_score: am besten bewertet price: nach Preis share: - message: "Ich habe einen Ausgabenvorschlag %{title} in %{org} erstellt. Erstellen Sie auch einen Ausgabenvorschlag!" + message: "Ich habe einen Ausgabenvorschlag %{title} in %{handle} erstellt. Erstellen Sie auch einen Ausgabenvorschlag!" show: author_deleted: Benutzer gelöscht price_explanation: Preiserklärung diff --git a/config/locales/gl/budgets.yml b/config/locales/gl/budgets.yml index cecded9bf..8dfde80a2 100644 --- a/config/locales/gl/budgets.yml +++ b/config/locales/gl/budgets.yml @@ -106,7 +106,7 @@ gl: confidence_score: Máis apoiados price: por custo share: - message: "Acabo de crear o proxecto de investimento %{title} en %{org}. Anímote a crear o teu proxecto de investimento!" + message: "Acabo de crear o proxecto de investimento %{title} en %{handle}. Anímote a crear o teu proxecto de investimento!" show: author_deleted: Usuario/a borrado/a price_explanation: Informe de custo (opcional, dato público) diff --git a/config/locales/gl/general.yml b/config/locales/gl/general.yml index 6ddfcbdee..99ae07ab3 100644 --- a/config/locales/gl/general.yml +++ b/config/locales/gl/general.yml @@ -446,7 +446,7 @@ gl: form: submit_button: Gardar so cambios share: - message: "Veño de apoiar a proposta %{summary} en %{handle}. Se che interesa, apoia ti tamén!" + message: "Veño de apoiar a proposta %{title} en %{handle}. Se che interesa, apoia ti tamén!" polls: all: "Todo" no_dates: "sen data asignada" diff --git a/config/locales/nl/budgets.yml b/config/locales/nl/budgets.yml index e0ea47b2f..cc9a79d51 100644 --- a/config/locales/nl/budgets.yml +++ b/config/locales/nl/budgets.yml @@ -106,7 +106,7 @@ nl: confidence_score: best gescored price: op bedrag share: - message: "Ik heb investeringsvoorstel %{title} gemaakt in %{org}. Jij kunt ook een investeringsvoorstel doen!" + message: "Ik heb investeringsvoorstel %{title} gemaakt in %{handle}. Jij kunt ook een investeringsvoorstel doen!" show: author_deleted: Gebruiker verwijderd price_explanation: Price explanation diff --git a/config/locales/ru/general.yml b/config/locales/ru/general.yml index e08f45cf0..3c1cf8568 100644 --- a/config/locales/ru/general.yml +++ b/config/locales/ru/general.yml @@ -423,7 +423,7 @@ ru: form: submit_button: Сохранить изменения share: - message: "Я поддержал(а) предложение %{summary} в %{org}. Если вам интересно, поддержите его тоже!" + message: "Я поддержал(а) предложение %{title} в %{handle}. Если вам интересно, поддержите его тоже!" polls: all: "Все" no_dates: "дата не назначена" diff --git a/config/locales/so-SO/budgets.yml b/config/locales/so-SO/budgets.yml index c46e81034..8f7fd2c08 100644 --- a/config/locales/so-SO/budgets.yml +++ b/config/locales/so-SO/budgets.yml @@ -106,7 +106,7 @@ so: confidence_score: ugu sareeya price: qiime ahaan share: - message: "Waxaan abuuray mashruuca maalgashiga%{title} ee%{org} Samee mashruuc maal-gashi aad adigu leedahay!" + message: "Waxaan abuuray mashruuca maalgashiga%{title} ee%{handle} Samee mashruuc maal-gashi aad adigu leedahay!" show: author_deleted: Isticmalaha la tirtiray price_explanation: Faahfaahinta qiimaha diff --git a/config/locales/so-SO/general.yml b/config/locales/so-SO/general.yml index d3af4b54c..531489395 100644 --- a/config/locales/so-SO/general.yml +++ b/config/locales/so-SO/general.yml @@ -445,7 +445,7 @@ so: form: submit_button: Badbaadi beddelka share: - message: "Waxaan taageeray hindisaha%{summary} ee%{handle} Haddii aad xiiseyneyso, sidoo kale waa inaad taageertaa!" + message: "Waxaan taageeray hindisaha%{title} ee%{handle} Haddii aad xiiseyneyso, sidoo kale waa inaad taageertaa!" polls: all: "Dhamaan" no_dates: "tirada taariikhda la magacaabay" diff --git a/config/locales/sq-AL/budgets.yml b/config/locales/sq-AL/budgets.yml index 4a5b5c992..7d69875df 100644 --- a/config/locales/sq-AL/budgets.yml +++ b/config/locales/sq-AL/budgets.yml @@ -106,7 +106,7 @@ sq: confidence_score: më të vlerësuarat price: nga çmimi share: - message: "Kam krijuar projektin e investimeve%{title} në %{org}. Krijoni një projekt investimi edhe ju!" + message: "Kam krijuar projektin e investimeve%{title} në %{handle}. Krijoni një projekt investimi edhe ju!" show: author_deleted: Përdoruesi u fshi price_explanation: Shpjegimi i çmimit diff --git a/config/locales/sq-AL/general.yml b/config/locales/sq-AL/general.yml index 6c286d289..45d990f16 100644 --- a/config/locales/sq-AL/general.yml +++ b/config/locales/sq-AL/general.yml @@ -445,7 +445,7 @@ sq: form: submit_button: Ruaj ndryshimet share: - message: "Kam përkrahur propozimin %{summary} në%{handle}. Nëse jeni të interesuar, përkrahu gjithashtu!" + message: "Kam përkrahur propozimin %{title} në%{handle}. Nëse jeni të interesuar, përkrahu gjithashtu!" polls: all: "Të gjithë" no_dates: "Asnjë datë e caktuar" diff --git a/config/locales/val/budgets.yml b/config/locales/val/budgets.yml index b0fae8311..4d0602a03 100644 --- a/config/locales/val/budgets.yml +++ b/config/locales/val/budgets.yml @@ -106,7 +106,7 @@ val: confidence_score: millor valorats price: per cost share: - message: "Acabe de crear una proposta %{title} en %{org}. Crea una tu també!" + message: "Acabe de crear una proposta %{title} en %{handle}. Crea una tu també!" show: author_deleted: Usuari eliminat price_explanation: Informe de cost diff --git a/config/locales/val/general.yml b/config/locales/val/general.yml index 948c90661..487bb119b 100644 --- a/config/locales/val/general.yml +++ b/config/locales/val/general.yml @@ -445,7 +445,7 @@ val: form: submit_button: Guardar canvis share: - message: "He avalat la proposta %{summary} en %{handle}. Si t'interessa, avala tu també!" + message: "He avalat la proposta %{title} en %{handle}. Si t'interessa, avala tu també!" polls: all: "Totes" no_dates: "sense data asignada" diff --git a/config/locales/zh-CN/budgets.yml b/config/locales/zh-CN/budgets.yml index 0c8c30c63..59914871c 100644 --- a/config/locales/zh-CN/budgets.yml +++ b/config/locales/zh-CN/budgets.yml @@ -103,7 +103,7 @@ zh-CN: confidence_score: 最高评分 price: 按价格 share: - message: "我在%{org} 里创建了投资项目%{title}。请您也创建一个投资项目吧!" + message: "我在%{handle} 里创建了投资项目%{title}。请您也创建一个投资项目吧!" show: author_deleted: 已删除的用户 price_explanation: 价格说明 diff --git a/config/locales/zh-CN/general.yml b/config/locales/zh-CN/general.yml index c27f4da17..0abf00ed6 100644 --- a/config/locales/zh-CN/general.yml +++ b/config/locales/zh-CN/general.yml @@ -428,7 +428,7 @@ zh-CN: form: submit_button: 保存更改 share: - message: "我支持%{handle} 中的提议%{summary}。如果您感兴趣,也请支持它!" + message: "我支持%{handle} 中的提议%{title}。如果您感兴趣,也请支持它!" polls: all: "所有" no_dates: "没指定日期" From 4b1cbb7db68e3b7a8eb4d3e95898c9f92bf8259a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2019 19:22:39 +0200 Subject: [PATCH 2/6] Use Rails 5 conventions in ballot migrations These migrations and models were added after the Rails 5 branch was created but before it was merged. --- app/models/poll/ballot.rb | 2 +- app/models/poll/ballot_sheet.rb | 2 +- db/migrate/20170514192303_add_balloted_heading_id_to_users.rb | 2 +- db/migrate/20180604173248_add_budget_to_polls.rb | 2 +- db/migrate/20180621182723_create_poll_ballot_sheets.rb | 2 +- db/migrate/20180704093831_create_poll_ballot.rb | 2 +- db/migrate/20180704095538_add_physical_to_budget_ballot.rb | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/poll/ballot.rb b/app/models/poll/ballot.rb index 0a8a55d85..9572c7cfc 100644 --- a/app/models/poll/ballot.rb +++ b/app/models/poll/ballot.rb @@ -1,4 +1,4 @@ -class Poll::Ballot < ActiveRecord::Base +class Poll::Ballot < ApplicationRecord belongs_to :ballot_sheet, class_name: Poll::BallotSheet validates :ballot_sheet_id, presence: true diff --git a/app/models/poll/ballot_sheet.rb b/app/models/poll/ballot_sheet.rb index 883cb004b..70e058dac 100644 --- a/app/models/poll/ballot_sheet.rb +++ b/app/models/poll/ballot_sheet.rb @@ -1,4 +1,4 @@ -class Poll::BallotSheet < ActiveRecord::Base +class Poll::BallotSheet < ApplicationRecord belongs_to :poll belongs_to :officer_assignment has_many :ballots, class_name: Poll::Ballot diff --git a/db/migrate/20170514192303_add_balloted_heading_id_to_users.rb b/db/migrate/20170514192303_add_balloted_heading_id_to_users.rb index d1fcd04cb..25362e303 100644 --- a/db/migrate/20170514192303_add_balloted_heading_id_to_users.rb +++ b/db/migrate/20170514192303_add_balloted_heading_id_to_users.rb @@ -1,4 +1,4 @@ -class AddBallotedHeadingIdToUsers < ActiveRecord::Migration +class AddBallotedHeadingIdToUsers < ActiveRecord::Migration[4.2] def change add_column :users, :balloted_heading_id, :integer, default: nil end diff --git a/db/migrate/20180604173248_add_budget_to_polls.rb b/db/migrate/20180604173248_add_budget_to_polls.rb index 74d45e1a8..923f5f3ed 100644 --- a/db/migrate/20180604173248_add_budget_to_polls.rb +++ b/db/migrate/20180604173248_add_budget_to_polls.rb @@ -1,4 +1,4 @@ -class AddBudgetToPolls < ActiveRecord::Migration +class AddBudgetToPolls < ActiveRecord::Migration[4.2] def change add_reference :polls, :budget, index: { unique: true }, foreign_key: true end diff --git a/db/migrate/20180621182723_create_poll_ballot_sheets.rb b/db/migrate/20180621182723_create_poll_ballot_sheets.rb index ee06c5724..ed10e6ac4 100644 --- a/db/migrate/20180621182723_create_poll_ballot_sheets.rb +++ b/db/migrate/20180621182723_create_poll_ballot_sheets.rb @@ -1,4 +1,4 @@ -class CreatePollBallotSheets < ActiveRecord::Migration +class CreatePollBallotSheets < ActiveRecord::Migration[4.2] def change create_table :poll_ballot_sheets do |t| t.text :data diff --git a/db/migrate/20180704093831_create_poll_ballot.rb b/db/migrate/20180704093831_create_poll_ballot.rb index 137b71255..12f55df49 100644 --- a/db/migrate/20180704093831_create_poll_ballot.rb +++ b/db/migrate/20180704093831_create_poll_ballot.rb @@ -1,4 +1,4 @@ -class CreatePollBallot < ActiveRecord::Migration +class CreatePollBallot < ActiveRecord::Migration[4.2] def change create_table :poll_ballots do |t| t.integer :ballot_sheet_id diff --git a/db/migrate/20180704095538_add_physical_to_budget_ballot.rb b/db/migrate/20180704095538_add_physical_to_budget_ballot.rb index a06889d1b..eb853d523 100644 --- a/db/migrate/20180704095538_add_physical_to_budget_ballot.rb +++ b/db/migrate/20180704095538_add_physical_to_budget_ballot.rb @@ -1,4 +1,4 @@ -class AddPhysicalToBudgetBallot < ActiveRecord::Migration +class AddPhysicalToBudgetBallot < ActiveRecord::Migration[4.2] def change add_column :budget_ballots, :physical, :boolean, default: false add_column :budget_ballots, :poll_ballot_id, :integer From c63c06b7ec10fb1844f9ccf3f47dec55dc288e88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Wed, 14 Sep 2016 13:13:23 +0200 Subject: [PATCH 3/6] protect_from_forgery is not prepended by default so it has to appear before devise's auth methods --- app/controllers/application_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 38ce20f34..d22ec7269 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,6 +4,8 @@ class ApplicationController < ActionController::Base include HasFilters include HasOrders + protect_from_forgery with: :exception + before_action :authenticate_http_basic, if: :http_basic_auth_site? before_action :ensure_signup_complete @@ -15,8 +17,6 @@ class ApplicationController < ActionController::Base check_authorization unless: :devise_controller? self.responder = ApplicationResponder - protect_from_forgery with: :exception - rescue_from CanCan::AccessDenied do |exception| respond_to do |format| format.html { redirect_to main_app.root_url, alert: exception.message } From 286e0ca878b80c321d165c460dc625407cec2a30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2019 15:44:05 +0200 Subject: [PATCH 4/6] Handle AccessDenied in management sessions We were raising a `CanCan::AcessDenied` and were getting a 500 Internal Server Error. I've chosen to do the same thing we do in the ApplicationController. There are other options to handle this request, like redirecting to the login page or returning a 401 Unauthorized HTTP status. --- app/controllers/application_controller.rb | 8 +------- app/controllers/concerns/access_denied_handler.rb | 12 ++++++++++++ app/controllers/management/sessions_controller.rb | 1 + .../management/sessions_controller_spec.rb | 13 +++++++++---- 4 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 app/controllers/concerns/access_denied_handler.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d22ec7269..09dc42034 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,7 @@ require "application_responder" class ApplicationController < ActionController::Base include HasFilters include HasOrders + include AccessDeniedHandler protect_from_forgery with: :exception @@ -17,13 +18,6 @@ class ApplicationController < ActionController::Base check_authorization unless: :devise_controller? self.responder = ApplicationResponder - rescue_from CanCan::AccessDenied do |exception| - respond_to do |format| - format.html { redirect_to main_app.root_url, alert: exception.message } - format.json { render json: {error: exception.message}, status: :forbidden } - end - end - layout :set_layout respond_to :html helper_method :current_budget diff --git a/app/controllers/concerns/access_denied_handler.rb b/app/controllers/concerns/access_denied_handler.rb new file mode 100644 index 000000000..d924baf56 --- /dev/null +++ b/app/controllers/concerns/access_denied_handler.rb @@ -0,0 +1,12 @@ +module AccessDeniedHandler + extend ActiveSupport::Concern + + included do + rescue_from CanCan::AccessDenied do |exception| + respond_to do |format| + format.html { redirect_to main_app.root_url, alert: exception.message } + format.json { render json: { error: exception.message }, status: :forbidden } + end + end + end +end diff --git a/app/controllers/management/sessions_controller.rb b/app/controllers/management/sessions_controller.rb index 6db303a39..a88c1de9f 100644 --- a/app/controllers/management/sessions_controller.rb +++ b/app/controllers/management/sessions_controller.rb @@ -1,6 +1,7 @@ require "manager_authenticator" class Management::SessionsController < ActionController::Base + include AccessDeniedHandler def create destroy_session diff --git a/spec/controllers/management/sessions_controller_spec.rb b/spec/controllers/management/sessions_controller_spec.rb index 650f53246..2d58b0202 100644 --- a/spec/controllers/management/sessions_controller_spec.rb +++ b/spec/controllers/management/sessions_controller_spec.rb @@ -3,11 +3,13 @@ require "rails_helper" describe Management::SessionsController do describe "Sign in" do + it "denies access if wrong manager credentials" do allow_any_instance_of(ManagerAuthenticator).to receive(:auth).and_return(false) - expect { - get :create, params: { login: "nonexistent", clave_usuario: "wrong" } - }.to raise_error CanCan::AccessDenied + get :create, params: { login: "nonexistent", clave_usuario: "wrong" } + + expect(response).to redirect_to "/" + expect(flash[:alert]).to eq "You do not have permission to access this page." expect(session[:manager]).to be_nil end @@ -42,7 +44,10 @@ describe Management::SessionsController do it "denies access if user is not admin or manager" do sign_in create(:user) - expect { get :create}.to raise_error CanCan::AccessDenied + get :create + + expect(response).to redirect_to "/" + expect(flash[:alert]).to eq "You do not have permission to access this page." expect(session[:manager]).to be_nil end end From d90efa15e4e671b4316a7436bb4f56d9cd78cd64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 24 Apr 2019 16:44:24 +0200 Subject: [PATCH 5/6] Fix InvalidCrossOriginRequest response When requesting files like `/hackattempt.js`, the pages controller was responding with 404 status code. However, since the request was considered a JavaScript request (because of the `.js` extension), the response was also considered to be a JavaScript one, and since the request wasn't an AJAX request, our protection from forgery was preventing a potential security issue by raising an InvalidCrossOriginRequest exception. By setting HTML as content type, we correctly respond with a 404 status code. More info: https://die-antwort.eu/techblog/2018-08-avoid-invalid-cross-origin-request-with-catch-all-route/ --- app/controllers/pages_controller.rb | 2 +- spec/controllers/pages_controller_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 88ed65582..a4b01cf09 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -15,6 +15,6 @@ class PagesController < ApplicationController render action: params[:id] end rescue ActionView::MissingTemplate - head 404 + head 404, content_type: "text/html" end end diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index ac0678691..00d3e1962 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -42,6 +42,11 @@ describe PagesController do get :show, params: { id: "nonExistentPage" } expect(response).to be_missing end + + it "returns a 404 message for a JavaScript request" do + get :show, params: { id: "nonExistentJavaScript.js" } + expect(response).to be_missing + end end end From ca40e657fb2bff4226b2059d9374d6f5807ca95e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 25 Apr 2019 11:36:29 +0200 Subject: [PATCH 6/6] Check page after an AJAX call removing a ballot We were sending a new request without checking the previous one had finished, and if it hadn't finished, the test failed. This test started to fail after upgrading to Rails 5, since we removed the change done in commit eda47eff which set `config.allow_concurrency` to `false` in the test environment. While we could re-introduce that configuration option, which might have side effects, an easier solution is to check an AJAX request has been completed before sending a new request depending on the previous one seems to be a more solid option. Note this test failed with two possible errors: "undefined method `heading' for nil:NilClass" and "stale element reference: element is not attached to the page document". This change fixes the second error; it might fix the first error as well, but since I couldn't reproduce it locally, we'll only be sure when this test stops failing in travis builds. --- spec/features/budgets/ballots_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/budgets/ballots_spec.rb b/spec/features/budgets/ballots_spec.rb index 639c4e75a..d849ccfc5 100644 --- a/spec/features/budgets/ballots_spec.rb +++ b/spec/features/budgets/ballots_spec.rb @@ -307,6 +307,7 @@ feature "Ballots" do within("#budget_investment_#{investment1.id}") do find(".remove a").click + expect(page).to have_link "Vote" end visit budget_investments_path(budget, heading_id: new_york.id)