diff --git a/app/decorators/investigation_decorator.rb b/app/decorators/investigation_decorator.rb index 07aab9530a..9d8c55579e 100644 --- a/app/decorators/investigation_decorator.rb +++ b/app/decorators/investigation_decorator.rb @@ -11,7 +11,6 @@ def title end def unformatted_description - # Bypasses `FormattedDescription` for situations where we want the raw value of the field. object.description end @@ -44,7 +43,7 @@ def case_title_key(viewing_user) def case_summary_values values = [] - if investigation.is_private? + if object.is_private? values << { text: "" } values << { text: "" } else @@ -53,7 +52,6 @@ def case_summary_values end values << { text: status } - values end @@ -62,25 +60,21 @@ def source_details_summary_list(view_protected_details: false) contact_details << h.tag.p(I18n.t("case.protected_details", data_type: "#{object.case_type} contact details"), class: "govuk-body-s govuk-!-margin-bottom-1 opss-secondary-text opss-text-align-right") rows = [ - should_display_date_received? ? { key: { text: "Received date" }, value: { text: date_received.to_formatted_s(:govuk) } } : nil, - should_display_received_by? ? { key: { text: "Received by" }, value: { text: received_type.upcase_first } } : nil, { key: { text: "Source type" }, value: { text: complainant.complainant_type } }, { key: { text: "Contact details" }, value: { text: contact_details } } ] - rows.compact! - h.govuk_summary_list(rows:, borders: false, classes: "opss-summary-list-mixed opss-summary-list-mixed--narrow-dt") end def contact_details_list h.tag.ul(class: "govuk-list govuk-list--bullet govuk-list--spaced") do lis = [] - lis << h.tag.li(complainant.name) if complainant.name.present? - lis << h.tag.li("Telephone: #{complainant.phone_number}") if complainant.phone_number.present? - lis << h.tag.li("Email: ".html_safe + h.mail_to(complainant.email_address, class: "govuk-link govuk-link--no-visited-state")) if complainant.email_address.present? - lis << h.tag.li(complainant.other_details) if complainant.other_details.present? - safe_join(lis) + lis << h.tag.li(h.sanitize(complainant.name)) if complainant.name.present? + lis << h.tag.li("Telephone: #{h.sanitize(complainant.phone_number)}") if complainant.phone_number.present? + lis << h.tag.li("Email: ".html_safe + h.mail_to(h.sanitize(complainant.email_address), class: "govuk-link govuk-link--no-visited-state")) if complainant.email_address.present? + lis << h.tag.li(h.sanitize(complainant.other_details)) if complainant.other_details.present? + h.safe_join(lis) end end @@ -95,7 +89,7 @@ def created_by end def owner_display_name_for(viewer:) - return "No notification owner" unless investigation.owner + return "No notification owner" unless object.owner owner.owner_short_name(viewer:) end @@ -125,22 +119,12 @@ def visibility_action private def category - @category ||= \ - if categories.size == 1 - h.simple_format(categories.first.downcase.upcase_first, class: "govuk-body") - else - h.tag.ul(class: "govuk-list") do - lis = categories.map { |cat| h.tag.li(cat.downcase.upcase_first) } - lis.join.html_safe - end - end - end - - def should_display_date_received? - false - end - - def should_display_received_by? - false + @category ||= if categories.size == 1 + h.simple_format(categories.first.downcase.upcase_first, class: "govuk-body") + else + h.tag.ul(class: "govuk-list") do + h.safe_join(categories.map { |cat| h.tag.li(cat.downcase.upcase_first) }) + end + end end end diff --git a/app/decorators/product_decorator.rb b/app/decorators/product_decorator.rb index b586bc8eeb..45b32b4b45 100644 --- a/app/decorators/product_decorator.rb +++ b/app/decorators/product_decorator.rb @@ -3,45 +3,52 @@ class ProductDecorator < ApplicationDecorator delegate_all decorates_association :investigations + # Include the sanitize helper + include ActionView::Helpers::SanitizeHelper + + # Safely combines the brand and name of the product def name_with_brand - [brand, name].compact.join(" ") + [sanitize(brand), sanitize(name)].compact.join(" ") end def pretty_description - "Product: #{name}" + "Product: #{sanitize(name)}" end def unformatted_description - # Bypasses `FormattedDescription` for situations where we want the raw value of the field. object.description end def details_list(date_case_closed: nil) timestamp = date_case_closed.to_i if date_case_closed + psd_ref_key_html = 'PSD ref'.html_safe psd_secondary_text_html = if date_case_closed.present? - " - The PSD reference number for this version of the product record - as recorded when the notification was closed: #{date_case_closed.to_formatted_s(:govuk)}." + " - The PSD reference number for this version of the product record - as recorded when the notification was closed: #{sanitize(date_case_closed.to_formatted_s(:govuk))}." else " - The PSD reference number for this product record" end.html_safe - webpage_html = "#{webpage}".html_safe - when_placed_on_market_value = when_placed_on_market == "unknown_date" ? nil : when_placed_on_market + + webpage_html = "#{sanitize(webpage.presence || '')}".html_safe + + when_placed_on_market_value = when_placed_on_market == "unknown_date" ? nil : sanitize(when_placed_on_market.presence || "") + psd_ref_value_html = date_case_closed.present? ? psd_ref(timestamp:, investigation_was_closed: true) : psd_ref(timestamp:, investigation_was_closed: false) rows = [ { key: { text: psd_ref_key_html }, value: { text: "#{psd_ref_value_html}#{psd_secondary_text_html}".html_safe } }, - { key: { text: "Brand name" }, value: { text: object.brand } }, - { key: { text: "Product name" }, value: { text: object.name } }, - { key: { text: "Category" }, value: { text: category } }, - { key: { text: "Subcategory" }, value: { text: subcategory } }, - { key: { text: "Barcode" }, value: { text: barcode } }, - { key: { text: "Description" }, value: { text: description } }, - { key: { text: "Webpage" }, value: { text: webpage_html } }, + { key: { text: "Brand name" }, value: { text: sanitize(object.brand.presence || "") } }, + { key: { text: "Product name" }, value: { text: sanitize(object.name.presence || "") } }, + { key: { text: "Category" }, value: { text: sanitize(category.presence || "") } }, + { key: { text: "Subcategory" }, value: { text: sanitize(subcategory.presence || "") } }, + { key: { text: "Barcode" }, value: { text: sanitize(barcode.presence || "") } }, + { key: { text: "Description" }, value: { text: sanitize(description.presence || "") } }, + { key: { text: "Webpage" }, value: { text: webpage_html } }, # Already sanitized { key: { text: "Market date" }, value: { text: when_placed_on_market_value } }, - { key: { text: "Country of origin" }, value: { text: country_from_code(country_of_origin) } }, - { key: { text: "Counterfeit" }, value: counterfeit_row_value }, - { key: { text: "Product marking" }, value: { text: markings } }, - { key: { text: "Other product identifiers" }, value: { text: product_code } }, + { key: { text: "Country of origin" }, value: { text: sanitize(country_from_code(country_of_origin.presence || "")) } }, + { key: { text: "Counterfeit" }, value: counterfeit_row_value }, # Assuming this is already safe + { key: { text: "Product marking" }, value: { text: sanitize(markings.presence || "") } }, + { key: { text: "Other product identifiers" }, value: { text: sanitize(product_code.presence || "") } }, ] h.govuk_summary_list(rows:) @@ -65,7 +72,7 @@ def when_placed_on_market end def subcategory_and_category_label - product_and_category = [subcategory.presence, category.presence].compact + product_and_category = [sanitize(subcategory.presence), sanitize(category.presence)].compact if product_and_category.length > 1 "#{product_and_category.first} (#{product_and_category.last.downcase})" @@ -79,7 +86,7 @@ def markings return I18n.t(".product.unknown") if object.markings_unknown? return I18n.t(".product.none") if object.markings_no? - object.markings.join(", ") + sanitize(object.markings.join(", ")) end def case_ids @@ -88,11 +95,11 @@ def case_ids def counterfeit_row_value if product.counterfeit? - return { text: "Yes - #{counterfeit_explanation}".html_safe } + return { text: "Yes - #{sanitize(counterfeit_explanation)}".html_safe } end if product.genuine? - return { text: "No - #{counterfeit_explanation}" } + return { text: "No - #{sanitize(counterfeit_explanation)}" } end { text: I18n.t(object.authenticity || :missing, scope: Product.model_name.i18n_key) } @@ -107,14 +114,14 @@ def counterfeit_value def counterfeit_explanation return if product.unsure? - product.counterfeit? ? I18n.t("products.counterfeit") : I18n.t("products.genuine") + sanitize(product.counterfeit? ? I18n.t("products.counterfeit") : I18n.t("products.genuine")) end def owning_team_link return "No owner" if owning_team.nil? return "Your team is the product record owner" if owning_team == h.current_user.team - h.link_to owning_team.name, h.owner_product_path(object), class: "govuk-link govuk-link--no-visited-state" + h.link_to sanitize(owning_team.name), h.owner_product_path(object), class: "govuk-link govuk-link--no-visited-state" end def unique_cases_except(investigation) @@ -149,7 +156,7 @@ def product_name_with_cases rows = [ { key: { text: "Product" }, - value: { text: "#{name_with_brand} (psd-#{id})".html_safe } + value: { text: "#{sanitize(name_with_brand)} (psd-#{sanitize(id.to_s)})".html_safe } } ] @@ -157,7 +164,7 @@ def product_name_with_cases object.investigations.each_with_index do |investigation, i| rows << { key: { text: i.zero? ? "Notification(s)" : "" }, - value: { text: "#{investigation.user_title} (#{investigation.pretty_id})".html_safe } + value: { text: "#{sanitize(investigation.user_title)} (#{sanitize(investigation.pretty_id)})".html_safe } } end end diff --git a/app/views/notifications/create/check_notification_details_and_submit.html.erb b/app/views/notifications/create/check_notification_details_and_submit.html.erb index 557bce5bf3..5c63784bfa 100644 --- a/app/views/notifications/create/check_notification_details_and_submit.html.erb +++ b/app/views/notifications/create/check_notification_details_and_submit.html.erb @@ -8,9 +8,9 @@ <%= govuk_inset_text do %>

For

<% end %>

Notification details

@@ -22,39 +22,39 @@ end summary_list.with_row do |row| row.with_key(text: "Notification title") - row.with_value(text: @notification.user_title) + row.with_value(text: sanitize(@notification.user_title)) row.with_action(text: "Change", href: wizard_path(:add_notification_details), visually_hidden_text: "notification title") end summary_list.with_row do |row| row.with_key(text: "Notification summary") - row.with_value(text: @notification.description || "Not provided", classes: "opss-text-limit-scroll-s") + row.with_value(text: sanitize(@notification.description.presence || "Not provided"), classes: "opss-text-limit-scroll-s") row.with_action(text: "Change", href: wizard_path(:add_notification_details), visually_hidden_text: "notification summary") end summary_list.with_row do |row| row.with_key(text: "Notification reason") - row.with_value(text: @notification.safe_and_compliant? ? "Safe and compliant product(s)" : "Unsafe or non-compliant product(s)") + row.with_value(text: sanitize(@notification.safe_and_compliant? ? "Safe and compliant product(s)" : "Unsafe or non-compliant product(s)")) row.with_action(text: "Change", href: wizard_path(:add_notification_details), visually_hidden_text: "notification reason") end unless @notification&.reported_reason&.safe_and_compliant? summary_list.with_row do |row| row.with_key(text: "Specific product safety issues") - row.with_value(text: specific_product_safety_issues.html_safe) + row.with_value(text: sanitize(specific_product_safety_issues).html_safe) row.with_action(text: "Change", href: wizard_path(:add_product_safety_and_compliance_details), visually_hidden_text: "specific product safety issues") end end summary_list.with_row do |row| row.with_key(text: "Reported by overseas regulator") - row.with_value(text: @notification.is_from_overseas_regulator ? "Yes: #{country_from_code(@notification.notifying_country)}" : "No") + row.with_value(text: sanitize(@notification.is_from_overseas_regulator ? "Yes: #{country_from_code(@notification.notifying_country)}" : "No")) row.with_action(text: "Change", href: wizard_path(:add_product_safety_and_compliance_details), visually_hidden_text: "reported by overseas regulator") end summary_list.with_row do |row| row.with_key(text: "Internal reference number") - row.with_value(text: @notification.complainant_reference.presence || "Not provided") + row.with_value(text: sanitize(@notification.complainant_reference.presence || "Not provided")) row.with_action(text: "Change", href: wizard_path(:add_product_safety_and_compliance_details), visually_hidden_text: "internal reference number") end summary_list.with_row do |row| row.with_key(text: "Number of affected products") - row.with_value(text: number_of_affected_units(@notification.investigation_products).html_safe) + row.with_value(text: sanitize(number_of_affected_units(@notification.investigation_products)).html_safe) row.with_action(text: "Change", href: wizard_path(:add_number_of_affected_units), visually_hidden_text: "number of affected products") end end @@ -63,11 +63,11 @@ <% @notification.investigation_products.decorate.each do |investigation_product| %> <%= govuk_summary_list( - card: { title: investigation_product.product.name_with_brand }, + card: { title: sanitize(investigation_product.product.name_with_brand) }, rows: [ { key: { text: "Batch numbers" }, - value: { text: investigation_product.batch_number.presence || "Not provided" }, + value: { text: sanitize(investigation_product.batch_number.presence || "Not provided") }, actions: [ { text: "Change", @@ -78,7 +78,7 @@ }, { key: { text: "Customs codes" }, - value: { text: investigation_product.customs_code.presence || "Not provided" }, + value: { text: sanitize(investigation_product.customs_code.presence || "Not provided") }, actions: [ { text: "Change", @@ -89,7 +89,7 @@ }, { key: { text: "Unique Consignment Reference (UCR) numbers" }, - value: { text: investigation_product.ucr_numbers.pluck(:number).join(", ").presence || "Not provided" }, + value: { text: sanitize(investigation_product.ucr_numbers.pluck(:number).join(", ").presence || "Not provided") }, actions: [ { text: "Change", @@ -104,24 +104,23 @@ <% end %>

Businesses and their role in the supply chain

<% @notification.investigation_businesses.decorate.each do |investigation_business| %> - <%# TODO: add change links once pages are ready %> <%= govuk_summary_list( - card: { title: investigation_business.business.trading_name }, + card: { title: sanitize(investigation_business.business.trading_name) }, rows: [ { key: { text: "Business role in the supply chain" }, - value: { text: investigation_business.relationship.present? ? investigation_business.pretty_relationship : "Not provided" }, + value: { text: sanitize(investigation_business.pretty_relationship.presence || "Not provided") }, actions: [] }, { key: { text: "Registered or legal name" }, - value: { text: investigation_business.business.legal_name.presence || "Not provided" }, + value: { text: sanitize(investigation_business.business.legal_name.presence || "Not provided") }, actions: [] }, { key: { text: "Companies House number" }, - value: { text: investigation_business.business.company_number.presence || "Not provided" }, + value: { text: sanitize(investigation_business.business.company_number.presence || "Not provided") }, actions: [] }, { @@ -131,7 +130,7 @@ }, { key: { text: "Contact details" }, - value: { text: investigation_business.business.contacts.map { |contact| formatted_business_contact(contact) }.join("

").html_safe.presence || "Not provided" }, + value: { text: investigation_business.business.contacts.map { |contact| sanitize(formatted_business_contact(contact)) }.join("

").html_safe.presence || "Not provided" }, actions: [] } ] @@ -142,11 +141,11 @@ <% @notification.investigation_products.decorate.each do |investigation_product| %> <%= govuk_summary_list( - card: { title: investigation_product.product.name_with_brand }, + card: { title: sanitize(investigation_product.product.name_with_brand) }, rows: [ { key: { text: "Test reports" }, - value: { text: formatted_test_results(investigation_product.test_results).html_safe.presence || "Not provided" }, + value: { text: sanitize(formatted_test_results(investigation_product.test_results)).html_safe.presence || "Not provided" }, actions: [ { text: "Change", @@ -157,7 +156,7 @@ }, { key: { text: "Risk assessments" }, - value: { text: formatted_risk_assessments(investigation_product.prism_risk_assessments, investigation_product.risk_assessments, nil).html_safe.presence || "Not provided" }, + value: { text: sanitize(formatted_risk_assessments(investigation_product.prism_risk_assessments, investigation_product.risk_assessments, nil)).html_safe.presence || "Not provided" }, actions: [ { text: "Change", @@ -174,17 +173,17 @@ govuk_summary_list do |summary_list| summary_list.with_row do |row| row.with_key(text: "Supporting images") - row.with_value(text: formatted_uploads(@notification.image_uploads.map(&:file_upload)).html_safe.presence || "Not provided") + row.with_value(text: sanitize(formatted_uploads(@notification.image_uploads.map(&:file_upload))).html_safe.presence || "Not provided") row.with_action(text: "Change", href: wizard_path(:add_supporting_images), visually_hidden_text: "supporting images") end summary_list.with_row do |row| row.with_key(text: "Supporting documents") - row.with_value(text: formatted_uploads(@notification.documents).html_safe.presence || "Not provided") + row.with_value(text: sanitize(formatted_uploads(@notification.documents)).html_safe.presence || "Not provided") row.with_action(text: "Change", href: wizard_path(:add_supporting_documents), visually_hidden_text: "supporting documents") end summary_list.with_row do |row| row.with_key(text: "Notification risk level") - row.with_value(text: risk_level_tag) + row.with_value(text: sanitize(risk_level_tag)) row.with_action(text: "Change", href: wizard_path(:determine_notification_risk_level), visually_hidden_text: "notification risk level") end end @@ -196,47 +195,47 @@ <%= govuk_summary_list( card: { - title: corrective_action.investigation_product.product.decorate.name_with_brand, + title: sanitize(corrective_action.investigation_product.product.decorate.name_with_brand), actions: [govuk_link_to("Change", with_entity_notification_create_index_path(@notification, entity_id: corrective_action.id))] }, rows: [ { key: { text: "Corrective action" }, - value: { text: corrective_action.page_title } + value: { text: sanitize(corrective_action.page_title) } }, { key: { text: "Effective date" }, - value: { text: corrective_action.date_of_activity } + value: { text: sanitize(corrective_action.date_of_activity) } }, { key: { text: "Legislation" }, - value: { text: corrective_action.legislation } + value: { text: sanitize(corrective_action.legislation) } }, { key: { text: "Responsible business" }, - value: { text: corrective_action.business&.trading_name || "Not provided" } + value: { text: sanitize(corrective_action.business&.trading_name.presence || "Not provided") } }, if corrective_action.recall_of_the_product_from_end_users? { key: { text: "Recall information" }, - value: { text: corrective_action.has_online_recall_information == "has_online_recall_information_yes" ? "Yes: #{corrective_action.online_recall_information}" : "No" } + value: { text: sanitize(corrective_action.has_online_recall_information == "has_online_recall_information_yes" ? "Yes: #{sanitize(corrective_action.online_recall_information)}" : "No") } } end, { key: { text: "Action type" }, - value: { text: corrective_action.measure_type } + value: { text: sanitize(corrective_action.measure_type) } }, { key: { text: "Geographic scope" }, - value: { text: corrective_action.geographic_scopes } + value: { text: sanitize(corrective_action.geographic_scopes) } }, { key: { text: "Further details" }, - value: { text: corrective_action.details.presence || "Not provided" } + value: { text: sanitize(corrective_action.details.presence || "Not provided") } }, { key: { text: "Related file" }, - value: { text: formatted_uploads([corrective_action.document]).html_safe.presence || "Not provided" } + value: { text: sanitize(formatted_uploads([corrective_action.document])).html_safe.presence || "Not provided" } } ].compact ) @@ -258,7 +257,7 @@ govuk_summary_list do |summary_list| summary_list.with_row do |row| row.with_key(text: "Corrective action taken?") - row.with_value(text: "No: #{corrective_action_not_taken_reasons}") + row.with_value(text: "No: #{sanitize(corrective_action_not_taken_reasons)}") row.with_action(text: "Change", href: wizard_path(:record_a_corrective_action), visually_hidden_text: "corrective action taken") end end