From a60b0d0bfac8c0b050b88ae9a9a3c70933a2bfec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Gaya?= <sebastien.gaya@gmail.com>
Date: Thu, 23 Feb 2023 10:59:13 +0100
Subject: [PATCH] Remove and legacy ActionText parts, include Sanitizable
pretty much everywhere and custom sanitize
---
app/models/communication/block.rb | 1 +
app/models/communication/extranet.rb | 15 +++++++++++++
app/models/communication/website.rb | 11 ++++++++++
app/models/communication/website/git_file.rb | 2 ++
app/models/communication/website/permalink.rb | 1 +
app/models/concerns/sanitizable.rb | 3 +++
app/models/education/cohort.rb | 1 +
app/models/education/diploma.rb | 1 +
app/models/education/school.rb | 1 +
app/models/language.rb | 1 +
app/models/research/hal/author.rb | 4 +++-
app/models/research/hal/publication.rb | 3 ++-
app/models/research/journal.rb | 1 +
app/models/research/journal/paper/kind.rb | 1 +
app/models/research/laboratory.rb | 1 +
app/models/university.rb | 16 ++++++++++++++
app/models/university/person/experience.rb | 1 +
app/models/university/person/involvement.rb | 1 +
app/models/university/role.rb | 1 +
app/models/user.rb | 2 ++
app/models/user/with_authentication.rb | 10 ++++-----
.../action_text/content/_layout.html.erb | 1 -
app/views/active_storage/blobs/_blob.html.erb | 22 -------------------
config/application.rb | 2 +-
24 files changed, 71 insertions(+), 32 deletions(-)
delete mode 100644 app/views/action_text/content/_layout.html.erb
delete mode 100644 app/views/active_storage/blobs/_blob.html.erb
diff --git a/app/models/communication/block.rb b/app/models/communication/block.rb
index 1369e18f4..aec48f232 100644
--- a/app/models/communication/block.rb
+++ b/app/models/communication/block.rb
@@ -24,6 +24,7 @@
# fk_rails_18291ef65f (university_id => universities.id)
#
class Communication::Block < ApplicationRecord
+ include Sanitizable
include WithUniversity
include WithPosition
include Accessible
diff --git a/app/models/communication/extranet.rb b/app/models/communication/extranet.rb
index d48d2a51c..da5242530 100644
--- a/app/models/communication/extranet.rb
+++ b/app/models/communication/extranet.rb
@@ -35,6 +35,7 @@
class Communication::Extranet < ApplicationRecord
self.filter_attributes += [:sso_cert]
+ # We don't include Sanitizable because too many complex attributes. We handle it below.
include WithAbouts
include WithLegal
include WithSso
@@ -49,6 +50,8 @@ class Communication::Extranet < ApplicationRecord
validates :logo, size: { less_than: 1.megabytes }
validates :favicon, size: { less_than: 1.megabytes }
+ before_validation :sanitize_fields
+
scope :ordered, -> { order(:name) }
scope :for_search_term, -> (term) {
where("
@@ -100,4 +103,16 @@ class Communication::Extranet < ApplicationRecord
def to_s
"#{name}"
end
+
+ private
+
+ def sanitize_fields
+ self.color = Osuny::Sanitizer.sanitize(self.color, 'string')
+ self.cookies_policy = Osuny::Sanitizer.sanitize(self.cookies_policy, 'text')
+ self.host = Osuny::Sanitizer.sanitize(self.host, 'string')
+ self.name = Osuny::Sanitizer.sanitize(self.name, 'string')
+ self.privacy_policy = Osuny::Sanitizer.sanitize(self.privacy_policy, 'text')
+ self.registration_contact = Osuny::Sanitizer.sanitize(self.registration_contact, 'string')
+ self.terms = Osuny::Sanitizer.sanitize(self.terms, 'text')
+ end
end
diff --git a/app/models/communication/website.rb b/app/models/communication/website.rb
index 52a48b87f..2ef448ae9 100644
--- a/app/models/communication/website.rb
+++ b/app/models/communication/website.rb
@@ -64,6 +64,8 @@ class Communication::Website < ApplicationRecord
validates :languages, length: { minimum: 1 }
validate :languages_must_include_default_language
+ before_validation :sanitize_fields
+
scope :ordered, -> { order(:name) }
scope :in_production, -> { where(in_production: true) }
scope :for_theme_version, -> (version) { where(theme_version: version) }
@@ -107,6 +109,15 @@ class Communication::Website < ApplicationRecord
protected
+ def sanitize_fields
+ self.git_branch = Osuny::Sanitizer.sanitize(self.git_branch, 'string')
+ self.git_endpoint = Osuny::Sanitizer.sanitize(self.git_endpoint, 'string')
+ self.name = Osuny::Sanitizer.sanitize(self.name, 'string')
+ self.plausible_url = Osuny::Sanitizer.sanitize(self.plausible_url, 'string')
+ self.repository = Osuny::Sanitizer.sanitize(self.repository, 'string')
+ self.url = Osuny::Sanitizer.sanitize(self.url, 'string')
+ end
+
def languages_must_include_default_language
errors.add(:languages, :must_include_default) unless language_ids.include?(default_language_id)
end
diff --git a/app/models/communication/website/git_file.rb b/app/models/communication/website/git_file.rb
index d055984b8..ca3bbce5e 100644
--- a/app/models/communication/website/git_file.rb
+++ b/app/models/communication/website/git_file.rb
@@ -21,6 +21,8 @@
# fk_rails_8505d649e8 (website_id => communication_websites.id)
#
class Communication::Website::GitFile < ApplicationRecord
+ # We don't include Sanitizable as this model is never handled by users directly.
+
belongs_to :website, class_name: 'Communication::Website'
belongs_to :about, polymorphic: true
diff --git a/app/models/communication/website/permalink.rb b/app/models/communication/website/permalink.rb
index 88f821fd2..5329fcc0c 100644
--- a/app/models/communication/website/permalink.rb
+++ b/app/models/communication/website/permalink.rb
@@ -40,6 +40,7 @@ class Communication::Website::Permalink < ApplicationRecord
"University::Person::Teacher" => Communication::Website::Permalink::Teacher
}
+ # We don't include Sanitizable as this model is never handled by users directly.
include WithUniversity
belongs_to :university
diff --git a/app/models/concerns/sanitizable.rb b/app/models/concerns/sanitizable.rb
index 1d9b39065..83f0cdaea 100644
--- a/app/models/concerns/sanitizable.rb
+++ b/app/models/concerns/sanitizable.rb
@@ -11,6 +11,9 @@ module Sanitizable
.select { |attr_name, attr_type|
[:string, :text].include?(attr_type) && public_send(attr_name).present?
}
+ .reject { |attr_name, _|
+ attr_name.ends_with?('_type') # Reject polymorphic type
+ }
attributes_to_sanitize.each do |attr_name, attr_type|
public_send "#{attr_name}=", Osuny::Sanitizer.sanitize(public_send(attr_name), attr_type)
diff --git a/app/models/education/cohort.rb b/app/models/education/cohort.rb
index e37605bda..482c32aad 100644
--- a/app/models/education/cohort.rb
+++ b/app/models/education/cohort.rb
@@ -26,6 +26,7 @@
# fk_rails_c2d725cabd (academic_year_id => education_academic_years.id)
#
class Education::Cohort < ApplicationRecord
+ include Sanitizable
include WithUniversity
belongs_to :school,
diff --git a/app/models/education/diploma.rb b/app/models/education/diploma.rb
index f01e1db3b..7d78ab089 100644
--- a/app/models/education/diploma.rb
+++ b/app/models/education/diploma.rb
@@ -23,6 +23,7 @@
# fk_rails_6cb2e9fa90 (university_id => universities.id)
#
class Education::Diploma < ApplicationRecord
+ include Sanitizable
include WithBlocks
include WithGit
include WithPermalink
diff --git a/app/models/education/school.rb b/app/models/education/school.rb
index 14dbac926..78ab3e1fc 100644
--- a/app/models/education/school.rb
+++ b/app/models/education/school.rb
@@ -24,6 +24,7 @@
# fk_rails_e01b37a3ad (university_id => universities.id)
#
class Education::School < ApplicationRecord
+ include Sanitizable
include WithGit
include Aboutable
include WithPrograms # must come before WithAlumni and WithTeam
diff --git a/app/models/language.rb b/app/models/language.rb
index 36cf5338a..212b40dce 100644
--- a/app/models/language.rb
+++ b/app/models/language.rb
@@ -10,6 +10,7 @@
# updated_at :datetime not null
#
class Language < ApplicationRecord
+ include Sanitizable
has_many :users
has_and_belongs_to_many :communication_websites,
diff --git a/app/models/research/hal/author.rb b/app/models/research/hal/author.rb
index 726c98e54..1a8e33ec2 100644
--- a/app/models/research/hal/author.rb
+++ b/app/models/research/hal/author.rb
@@ -17,6 +17,8 @@
# index_research_hal_authors_on_docid (docid)
#
class Research::Hal::Author < ApplicationRecord
+ include Sanitizable
+
has_and_belongs_to_many :publications,
foreign_key: 'research_hal_publication_id',
association_foreign_key: 'research_hal_author_id'
@@ -76,7 +78,7 @@ class Research::Hal::Author < ApplicationRecord
researchers << researcher
researcher.import_research_hal_publications!
end
-
+
def disconnect_researcher(researcher)
researchers.delete researcher
researcher.import_research_hal_publications!
diff --git a/app/models/research/hal/publication.rb b/app/models/research/hal/publication.rb
index 75d03b57a..577fd844b 100644
--- a/app/models/research/hal/publication.rb
+++ b/app/models/research/hal/publication.rb
@@ -20,9 +20,10 @@
# index_research_hal_publications_on_docid (docid)
#
class Research::Hal::Publication < ApplicationRecord
+ include Sanitizable
include WithGit
include WithSlug
-
+
DOI_PREFIX = 'http://dx.doi.org/'.freeze
has_and_belongs_to_many :researchers,
diff --git a/app/models/research/journal.rb b/app/models/research/journal.rb
index 21149af29..7919fd8eb 100644
--- a/app/models/research/journal.rb
+++ b/app/models/research/journal.rb
@@ -20,6 +20,7 @@
# fk_rails_96097d5f10 (university_id => universities.id)
#
class Research::Journal < ApplicationRecord
+ include Sanitizable
include Aboutable
include WithUniversity
include WithGit
diff --git a/app/models/research/journal/paper/kind.rb b/app/models/research/journal/paper/kind.rb
index da961c04f..f5895fd6f 100644
--- a/app/models/research/journal/paper/kind.rb
+++ b/app/models/research/journal/paper/kind.rb
@@ -21,6 +21,7 @@
# fk_rails_8e6f992b9d (university_id => universities.id)
#
class Research::Journal::Paper::Kind < ApplicationRecord
+ include Sanitizable
include WithUniversity
include WithGit
include WithSlug
diff --git a/app/models/research/laboratory.rb b/app/models/research/laboratory.rb
index 2daa8f561..d5e261d49 100644
--- a/app/models/research/laboratory.rb
+++ b/app/models/research/laboratory.rb
@@ -21,6 +21,7 @@
# fk_rails_f61d27545f (university_id => universities.id)
#
class Research::Laboratory < ApplicationRecord
+ include Sanitizable
include WithGit
include Aboutable
diff --git a/app/models/university.rb b/app/models/university.rb
index 15691d831..321f21333 100644
--- a/app/models/university.rb
+++ b/app/models/university.rb
@@ -38,6 +38,7 @@
class University < ApplicationRecord
self.filter_attributes += [:sso_cert]
+ # We don't include Sanitizable because too many complex attributes. We handle it below.
include WithPeopleAndOrganizations
include WithCommunication
include WithEducation
@@ -59,6 +60,7 @@ class University < ApplicationRecord
validates :sms_sender_name, presence: true, length: { maximum: 11 }
validates :logo, size: { less_than: 1.megabytes }
+ before_validation :sanitize_fields
after_destroy :destroy_remaining_blobs
scope :ordered, -> { order(:name) }
@@ -88,6 +90,20 @@ class University < ApplicationRecord
private
+ def sanitize_fields
+ self.address = Osuny::Sanitizer.sanitize(self.address, 'string')
+ self.city = Osuny::Sanitizer.sanitize(self.city, 'string')
+ self.country = Osuny::Sanitizer.sanitize(self.country, 'string')
+ self.identifier = Osuny::Sanitizer.sanitize(self.identifier, 'string')
+ self.invoice_amount = Osuny::Sanitizer.sanitize(self.invoice_amount, 'string')
+ self.mail_from_address = Osuny::Sanitizer.sanitize(self.mail_from_address, 'string')
+ self.mail_from_name = Osuny::Sanitizer.sanitize(self.mail_from_name, 'string')
+ self.name = Osuny::Sanitizer.sanitize(self.name, 'string')
+ self.sms_sender_name = Osuny::Sanitizer.sanitize(self.sms_sender_name, 'string')
+ self.sso_button_label = Osuny::Sanitizer.sanitize(self.sso_button_label, 'string')
+ self.zipcode = Osuny::Sanitizer.sanitize(self.zipcode, 'string')
+ end
+
def destroy_remaining_blobs
active_storage_blobs.delete_all
end
diff --git a/app/models/university/person/experience.rb b/app/models/university/person/experience.rb
index a5f0b47c7..60c2a4363 100644
--- a/app/models/university/person/experience.rb
+++ b/app/models/university/person/experience.rb
@@ -25,6 +25,7 @@
# fk_rails_923d0b71fd (university_id => universities.id)
#
class University::Person::Experience < ApplicationRecord
+ include Sanitizable
include WithUniversity
attr_accessor :organization_name
diff --git a/app/models/university/person/involvement.rb b/app/models/university/person/involvement.rb
index 988be24c9..e80bd448a 100644
--- a/app/models/university/person/involvement.rb
+++ b/app/models/university/person/involvement.rb
@@ -25,6 +25,7 @@
# fk_rails_5c704f6338 (university_id => universities.id)
#
class University::Person::Involvement < ApplicationRecord
+ include Sanitizable
include WithUniversity
include WithPosition
diff --git a/app/models/university/role.rb b/app/models/university/role.rb
index 3e740ac52..f631de252 100644
--- a/app/models/university/role.rb
+++ b/app/models/university/role.rb
@@ -21,6 +21,7 @@
# fk_rails_8e52293a38 (university_id => universities.id)
#
class University::Role < ApplicationRecord
+ include Sanitizable
include WithUniversity
include WithPosition
diff --git a/app/models/user.rb b/app/models/user.rb
index 569270a99..769136f69 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -56,6 +56,8 @@
# fk_rails_bd6f7212a9 (university_id => universities.id)
#
class User < ApplicationRecord
+ # We don't include Sanitizable because too many complex attributes.
+ # The sanitization is handled in User::WithAuthentication's sanitize_fields method.
include WithAdminTheme
include WithAvatar
include WithRegistrationContext
diff --git a/app/models/user/with_authentication.rb b/app/models/user/with_authentication.rb
index 86a880834..443bbb02e 100644
--- a/app/models/user/with_authentication.rb
+++ b/app/models/user/with_authentication.rb
@@ -94,13 +94,11 @@ module User::WithAuthentication
end
def sanitize_fields
- full_sanitizer = Rails::Html::FullSanitizer.new
-
# Only text allowed, and remove '=' to prevent excel formulas
- self.email = full_sanitizer.sanitize(self.email)&.gsub('=', '')
- self.first_name = full_sanitizer.sanitize(self.first_name)&.gsub('=', '')
- self.last_name = full_sanitizer.sanitize(self.last_name)&.gsub('=', '')
- self.mobile_phone = full_sanitizer.sanitize(self.mobile_phone)&.gsub('=', '')
+ self.email = Osuny::Sanitizer.sanitize(self.email, 'string')&.gsub('=', '')
+ self.first_name = Osuny::Sanitizer.sanitize(self.first_name, 'string')&.gsub('=', '')
+ self.last_name = Osuny::Sanitizer.sanitize(self.last_name, 'string')&.gsub('=', '')
+ self.mobile_phone = Osuny::Sanitizer.sanitize(self.mobile_phone, 'string')&.gsub('=', '')
end
def password_required?
diff --git a/app/views/action_text/content/_layout.html.erb b/app/views/action_text/content/_layout.html.erb
deleted file mode 100644
index 898a50667..000000000
--- a/app/views/action_text/content/_layout.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= render_action_text_content(content) %>
diff --git a/app/views/active_storage/blobs/_blob.html.erb b/app/views/active_storage/blobs/_blob.html.erb
deleted file mode 100644
index e823ece4c..000000000
--- a/app/views/active_storage/blobs/_blob.html.erb
+++ /dev/null
@@ -1,22 +0,0 @@
-<figure class="attachment attachment--<%= blob.variable? ? "preview" : "file" %> attachment--<%= blob.filename.extension %>">
- <% if blob.image? %>
- <%= kamifusen_tag blob, width: 800 %>
- <% elsif blob.video? %>
- <video controls>
- <source src="<%= rails_blob_path(blob) %>" type="<%= blob.content_type %>">
- </video>
- <% else %>
- <%= link_to polymorphic_path(blob), target: :blank do %>
- <p>
- <span class="attachment__name"><%= blob.filename %></span>
- <span class="attachment__size"><%= number_to_human_size blob.byte_size %></span>
- </p>
- <% end %>
- <% end %>
-
- <% if caption = blob.try(:caption) %>
- <figcaption class="attachment__caption">
- <%= caption %>
- </figcaption>
- <% end %>
-</figure>
diff --git a/config/application.rb b/config/application.rb
index fa9ee9cc7..d7ff6b357 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -9,7 +9,7 @@ require "active_storage/engine"
require "action_controller/railtie"
require "action_mailer/railtie"
require "action_mailbox/engine"
-require "action_text/engine"
+# require "action_text/engine"
require "action_view/railtie"
# require "action_cable/engine"
require "sprockets/railtie"
--
GitLab