diff --git a/CHANGELOG.md b/CHANGELOG.md index 6438d4219..5d5a85182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,8 @@ Features: - Added GraphQL types for custom fields in the API - Adds parsed information about custom fields in the Proposals export - Adds parsed information bout private custom fields when admins exports private data - - Adds a maintenance menu with tools to remove old private data + - Adds a maintenance menu with tools to remove old private + - SQL vulnerability fix for admin accountability v0.10.2 ------ diff --git a/app/controllers/concerns/decidim/decidim_awesome/admin_accountability/admin/filterable_helper.rb b/app/controllers/concerns/decidim/decidim_awesome/admin_accountability/admin/filterable_helper.rb index 3da344bd9..f6445cc82 100644 --- a/app/controllers/concerns/decidim/decidim_awesome/admin_accountability/admin/filterable_helper.rb +++ b/app/controllers/concerns/decidim/decidim_awesome/admin_accountability/admin/filterable_helper.rb @@ -13,10 +13,11 @@ def extra_dropdown_submenu_options_items(_filter, _i18n_scope) end def applied_filters_tags(i18n_ctx) - if global? && params[:admin_role_type].present? + admin_role_type = PaperTrailVersion.safe_admin_role_type(params[:admin_role_type]) + if global? && admin_role_type.present? content_tag(:span, class: "label secondary") do concat "#{i18n_filter_label(:admin_role_type, filterable_i18n_scope_from_ctx(i18n_ctx))}: " - concat t("decidim.decidim_awesome.admin.admin_accountability.admin_roles.#{params[:admin_role_type]}", default: params[:admin_role_type]) + concat t("decidim.decidim_awesome.admin.admin_accountability.admin_roles.#{admin_role_type}", default: admin_role_type) concat icon_link_to( "circle-x", url_for(export_params.except(:admin_role_type)), diff --git a/app/controllers/concerns/decidim/decidim_awesome/proposals/orderable_override.rb b/app/controllers/concerns/decidim/decidim_awesome/proposals/orderable_override.rb index 36748f806..9d8623c71 100644 --- a/app/controllers/concerns/decidim/decidim_awesome/proposals/orderable_override.rb +++ b/app/controllers/concerns/decidim/decidim_awesome/proposals/orderable_override.rb @@ -14,6 +14,8 @@ module OrderableOverride private + alias_method :decidim_original_reorder, :reorder + # read order from session if available def order @order ||= detect_order(session[:order]) || default_order @@ -31,38 +33,23 @@ def possible_orders end end - # rubocop:disable Metrics/CyclomaticComplexity def reorder(proposals) + title_by_locale = Arel.sql(proposals.sanitize_sql(["decidim_proposals_proposals.title->>? #{collation}", locale])) + title_by_machine_translation = Arel.sql(proposals.sanitize_sql(["decidim_proposals_proposals.title->'machine_translations'->>? #{collation}", locale])) + title_by_default_locale = Arel.sql(proposals.sanitize_sql(["decidim_proposals_proposals.title->>? #{collation}", default_locale])) case order when "az" - proposals.order(Arel.sql("CONCAT(decidim_proposals_proposals.title->>'#{locale}', - decidim_proposals_proposals.title->'machine_translations'->>'#{locale}', - decidim_proposals_proposals.title->>'#{default_locale}') #{collation} ASC")) + proposals.order(title_by_locale => :asc, title_by_machine_translation => :asc, title_by_default_locale => :asc) when "za" - proposals.order(Arel.sql("CONCAT(decidim_proposals_proposals.title->>'#{locale}', - decidim_proposals_proposals.title->'machine_translations'->>'#{locale}', - decidim_proposals_proposals.title->>'#{default_locale}') #{collation} DESC")) + proposals.order(title_by_locale => :desc, title_by_machine_translation => :desc, title_by_default_locale => :desc) when "supported_first" proposals.joins(my_votes_join).group(:id).order(Arel.sql("COUNT(decidim_proposals_proposal_votes.id) DESC")) when "supported_last" proposals.joins(my_votes_join).group(:id).order(Arel.sql("COUNT(decidim_proposals_proposal_votes.id) ASC")) - when "most_commented" - proposals.left_joins(:comments).group(:id).order(Arel.sql("COUNT(decidim_comments_comments.id) DESC")) - when "most_endorsed" - proposals.order(endorsements_count: :desc) - when "most_followed" - proposals.left_joins(:follows).group(:id).order(Arel.sql("COUNT(decidim_follows.id) DESC")) - when "most_voted" - proposals.order(proposal_votes_count: :desc) - when "random" - proposals.order_randomly(random_seed) - when "recent" - proposals.order(published_at: :desc) - when "with_more_authors" - proposals.order(coauthorships_count: :desc) + else + decidim_original_reorder(proposals) end end - # rubocop:enable Metrics/CyclomaticComplexity def collation @collation ||= begin diff --git a/app/controllers/decidim/decidim_awesome/admin/admin_accountability_controller.rb b/app/controllers/decidim/decidim_awesome/admin/admin_accountability_controller.rb index 1aba9f348..ec3ffe3ce 100644 --- a/app/controllers/decidim/decidim_awesome/admin/admin_accountability_controller.rb +++ b/app/controllers/decidim/decidim_awesome/admin/admin_accountability_controller.rb @@ -5,7 +5,7 @@ module DecidimAwesome module Admin class AdminAccountabilityController < DecidimAwesome::Admin::ApplicationController include NeedsAwesomeConfig - include Decidim::DecidimAwesome::AdminAccountability::Admin::Filterable + include AdminAccountability::Admin::Filterable helper_method :admin_actions, :collection, :export_params, :global?, :global_users_missing_date @@ -20,9 +20,9 @@ def index; end def export filters = export_params[:q] - Decidim::DecidimAwesome::ExportAdminActionsJob.perform_later(current_user, - params[:format].to_s, - admin_actions.ransack(filters).result.ids) + ExportAdminActionsJob.perform_later(current_user, + params[:format].to_s, + admin_actions.ransack(filters).result.ids) redirect_back fallback_location: decidim_admin_decidim_awesome.admin_accountability_path, notice: t("decidim.decidim_awesome.admin.admin_accountability.exports.notice") @@ -39,11 +39,11 @@ def collection end def space_role_actions - @space_role_actions ||= Decidim::DecidimAwesome::PaperTrailVersion.space_role_actions(current_organization) + @space_role_actions ||= PaperTrailVersion.space_role_actions(current_organization) end def admin_role_actions - @admin_role_actions ||= Decidim::DecidimAwesome::PaperTrailVersion.in_organization(current_organization).admin_role_actions(params[:admin_role_type]) + @admin_role_actions ||= PaperTrailVersion.in_organization(current_organization).admin_role_actions(PaperTrailVersion.safe_admin_role_type(params[:admin_role_type])) end def export_params @@ -61,7 +61,7 @@ def global_users_missing_date return unless global? @global_users_missing_date ||= begin - first_version = Decidim::DecidimAwesome::PaperTrailVersion.where(item_type: "Decidim::UserBaseEntity").last + first_version = PaperTrailVersion.where(item_type: "Decidim::UserBaseEntity").last first_user = Decidim::User.first first_version.created_at if first_user && first_version && (first_version.created_at > first_user.created_at + 1.second) end diff --git a/app/models/decidim/decidim_awesome/paper_trail_version.rb b/app/models/decidim/decidim_awesome/paper_trail_version.rb index e2f38544e..fbb268f1c 100644 --- a/app/models/decidim/decidim_awesome/paper_trail_version.rb +++ b/app/models/decidim/decidim_awesome/paper_trail_version.rb @@ -9,6 +9,10 @@ def self.safe_user_roles DecidimAwesome.participatory_space_roles.filter(&:safe_constantize) end + def self.safe_admin_role_type(admin_role) + Decidim.user_roles.find { |role| role == admin_role } + end + scope :space_role_actions, lambda { |organization| role_changes = where(item_type: PaperTrailVersion.safe_user_roles, event: "create") user_ids_from_object_changes = role_changes.pluck(:object_changes).map { |change| change.match(/decidim_user_id:\n- ?\n- (\d+)/)[1].to_i } @@ -33,7 +37,7 @@ def self.admin_role_actions(filter = nil) when "admin" base.where("object_changes LIKE '%\nadmin:\n- false\n- true%'") else - base.where(Arel.sql("object_changes LIKE '%\nroles:\n- []\n- - #{filter}\n%'")) + base.where("object_changes LIKE ?", "%\nroles:\n- []\n- - #{filter}\n%") end end diff --git a/lib/decidim/decidim_awesome/awesome.rb b/lib/decidim/decidim_awesome/awesome.rb index c2806bd13..9f5de8679 100644 --- a/lib/decidim/decidim_awesome/awesome.rb +++ b/lib/decidim/decidim_awesome/awesome.rb @@ -356,11 +356,11 @@ def self.possible_additional_proposal_sortings def self.collation_for(locale) @collation_for ||= {} - @collation_for[locale] ||= begin - res = ActiveRecord::Base.connection.execute(Arel.sql("SELECT collname FROM pg_collation WHERE collname LIKE '#{locale}-x-icu' LIMIT 1")).first - res ||= ActiveRecord::Base.connection.execute(Arel.sql("SELECT collname FROM pg_collation WHERE collname LIKE '#{locale[0..1]}%' LIMIT 1")).first + @collation_for[locale] ||= ["#{locale}-x-icu", "#{locale[0..1]}%"].filter_map do |loc| + sql = ApplicationRecord.sanitize_sql(["SELECT collname FROM pg_collation WHERE collname LIKE ? LIMIT 1", loc]) + res = ActiveRecord::Base.connection.execute(sql).first res["collname"] if res - end + end.first end def self.enabled?(*config_vars) diff --git a/spec/models/paper_trail_version_spec.rb b/spec/models/paper_trail_version_spec.rb index 00ac2aa5a..c13bb3277 100644 --- a/spec/models/paper_trail_version_spec.rb +++ b/spec/models/paper_trail_version_spec.rb @@ -106,6 +106,10 @@ module Decidim::DecidimAwesome expect(PaperTrailVersion.in_organization(external_organization).admin_role_actions("valuator")).to include(external_paper_trail_version) expect(PaperTrailVersion.in_organization(external_organization).admin_role_actions("admin")).to eq([]) end + + it "ignores invalid filters" do + expect(PaperTrailVersion.in_organization(organization).admin_role_actions("%')")).to eq([]) + end end end end