Merge branch 'master' into AO3-6587

app/controllers/feedbacks_controller.rb

@@ -3,8 +3,9 @@ class FeedbacksController < ApplicationController
   before_action :load_support_languages
 
   def new
-    @feedback = Feedback.new
     @admin_setting = AdminSetting.current
+    @feedback = Feedback.new
+    @feedback.referer = request.referer
     if logged_in_as_admin?
       @feedback.email = current_admin.email
     elsif is_registered_user?
@@ -19,7 +20,7 @@ def create
     @feedback.rollout = @feedback.rollout_string
     @feedback.user_agent = request.env["HTTP_USER_AGENT"]
     @feedback.ip_address = request.remote_ip
-    @feedback.referer = request.referer if request.referer && ArchiveConfig.PERMITTED_HOSTS.include?(URI(request.referer).host)
+    @feedback.referer = nil unless @feedback.referer && ArchiveConfig.PERMITTED_HOSTS.include?(URI(@feedback.referer).host)
     @feedback.site_skin = helpers.current_skin
     if @feedback.save
       @feedback.email_and_send
@@ -41,7 +42,7 @@ def load_support_languages
 
   def feedback_params
     params.require(:feedback).permit(
-      :comment, :email, :summary, :username, :language
+      :comment, :email, :summary, :username, :language, :referer
     )
   end
 end

app/controllers/languages_controller.rb

@@ -31,13 +31,13 @@ def update
     @language = Language.find_by(short: params[:id])
     authorize @language
 
-    if !policy(@language).can_edit_non_abuse_fields? && (@language.name != language_params[:name] || @language.short != language_params[:short] || @language.sortable_name != language_params[:sortable_name] || @language.support_available != (language_params[:support_available] == "1"))
+    if !policy(@language).can_edit_non_abuse_fields? && ((language_params[:name].present? && language_params[:name] != @language.name) || (language_params[:short].present? && @language.short != language_params[:short]) || (language_params[:sortable_name].present? && @language.sortable_name != language_params[:sortable_name]) || (language_params[:support_available].present? && @language.support_available != (language_params[:support_available] == "1")))
       flash[:error] = t("languages.update.non_abuse_field_error")
       redirect_to languages_path
       return
     end
 
-    if !policy(@language).can_edit_abuse_fields? && (@language.abuse_support_available != (language_params[:abuse_support_available] == "1"))
+    if !policy(@language).can_edit_abuse_fields? && language_params[:abuse_support_available].present? && (@language.abuse_support_available != (language_params[:abuse_support_available] == "1"))
       flash[:error] = t("languages.update.abuse_field_error")
       redirect_to languages_path
       return

app/views/feedbacks/new.html.erb

@@ -56,6 +56,7 @@
 
     <fieldset>
       <legend><%= t(".form.legend.feedback") %></legend>
+      <%= f.hidden_field :referer %>
       <dl>
         <dt class="required">
           <%= f.label :language, t(".form.language.label") %>

features/admins/admin_languages.feature

@@ -22,7 +22,7 @@ Scenario: Adding Abuse support for a language
     | name        | short |
     | Arabic      | ar    |
     | Espanol     | es    |
-  When I am logged in as a "translation" admin
+  When I am logged in as a "policy_and_abuse" admin
     And I go to the languages page
     # Languages are sorted by short name, so the first "Edit" is for Arabic
     And I follow "Edit"
@@ -39,7 +39,7 @@ Scenario: Adding a language to the Support form
       | name     | short |
       | Sindarin | sj    |
       | Klingon  | tlh   |
-  When I am logged in as a "translation" admin
+  When I am logged in as a "support" admin
     And I go to the languages page
     # Languages are sorted by short name, so the first "Edit" is for Sindarin
     And I follow "Edit"

features/other_b/support.feature

@@ -60,3 +60,28 @@ Feature: Filing a support request
     # The sanitizer adds the domain in front of relative image URLs as of AO3-6571
     And the email should not contain "<img src="http://www.example.org/foo.jpg" />"
     But the email should contain "http://www.example.org/foo.jpgHi"
+
+  Scenario: Submit a request with an on-Archive referer
+
+  Given I am logged in as "puzzled"
+    And basic languages
+    And Zoho ticket creation is enabled
+    And "www.example.com" is a permitted Archive host
+  When I go to the works page
+    And I follow "Support & Feedback"
+    And I fill in "Brief summary" with "Just a brief note"
+    And I fill in "Your question or problem" with "Hi, I came from the Archive"
+    And I press "Send"
+  Then a Zoho ticket should be created with referer "http://www.example.com/works"
+
+  Scenario: Submit a request with a referer that is not on-Archive
+
+  Given I am logged in as "puzzled"
+    And basic languages
+    And Zoho ticket creation is enabled
+  When I go to the works page
+    And I follow "Support & Feedback"
+    And I fill in "Brief summary" with "Just a brief note"
+    And I fill in "Your question or problem" with "Hi, I didn't come from the Archive"
+    And I press "Send"
+  Then a Zoho ticket should be created with referer "Unknown URL"

features/step_definitions/support_steps.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+Given "Zoho ticket creation is enabled" do
+  allow_any_instance_of(Feedback).to receive(:zoho_enabled?).and_return(true)
+  WebMock.stub_request(:get, %r{/contacts/search})
+    .to_return(headers: { content_type: "application/json" }, body: '{"data":[{"id":"1"}]}')
+  WebMock.stub_request(:post, %r{/tickets})
+    .to_return(headers: { content_type: "application/json" }, body: '{"id":"3"}')
+end
+
+Given "{string} is a permitted Archive host" do |host|
+  allow(ArchiveConfig).to receive(:PERMITTED_HOSTS).and_return([host])
+end
+
+Then "a Zoho ticket should be created with referer {string}" do |referer|
+  # rubocop:disable Lint/AmbiguousBlockAssociation
+  expect(WebMock).to have_requested(:post, "https://desk.zoho.com/api/v1/tickets")
+    .with { |req| JSON.parse(req.body)["cf"]["cf_url"] == referer }
+  # rubocop:enable Lint/AmbiguousBlockAssociation
+end

lib/css_cleaner.rb

@@ -227,7 +227,7 @@ def sanitize_css_content(value)
     return value if value =~ /^\"([^\"]*)\"$/
 
     # or a valid img url
-    return value if value.match(URL_FUNCTION_REGEX)
+    return value if value.match(Regexp.new("^#{URL_FUNCTION_REGEX}$"))
 
     # or "none"
     return value if value == "none"

spec/controllers/feedbacks_controller_spec.rb

@@ -83,39 +83,5 @@
         end
       end
     end
-
-    context "when accessed by a guest" do
-      context "when the referer is on the Archive" do
-        before do
-          request.env["HTTP_REFERER"] = "https://archiveofourown.org/works/1"
-        end
-
-        it "sets the referer in the Zoho ticket" do
-          expect(mock_zoho).to receive(:create_ticket).with(ticket_attributes: include(
-            "cf" => include(
-              "cf_referer" => "https://archiveofourown.org/works/1",
-              "cf_ip" => "0.0.0.0"
-            )
-          ))
-          post :create, params: default_parameters
-        end
-      end
-
-      context "when the referer is elsewhere" do
-        before do
-          request.env["HTTP_REFERER"] = "https://example.com/works/1"
-        end
-
-        it "does not set the referer in the Zoho ticket" do
-          expect(mock_zoho).to receive(:create_ticket).with(ticket_attributes: include(
-            "cf" => include(
-              "cf_referer" => "Unknown URL",
-              "cf_ip" => "0.0.0.0"
-            )
-          ))
-          post :create, params: default_parameters
-        end
-      end
-    end
   end
 end

spec/models/skin_spec.rb

@@ -177,7 +177,8 @@
       "errors when saving gradient with xss" => "div {background: -webkit-linear-gradient(url(xss.htc))}",
       "errors when saving dsf images" => "body {background: url(http://foo.com/bar.dsf)}",
       "errors when saving urls with invalid domain" => "body {background: url(http://foo.htc/bar.png)}",
-      "errors when saving xss interrupted with comments" => "div {xss:expr/*XSS*/ession(alert('XSS'))}"
+      "errors when saving xss interrupted with comments" => "div {xss:expr/*XSS*/ession(alert('XSS'))}",
+      "errors when saving url followed by something else" => 'a {content: url(/images/fakeimage.png) " (" attr(href) ")"}'
     }.each_pair do |condition, css|
       it condition do
         @skin.css = css