FEATURE: use new site setting for onebox custom user agent. (#28045)

Previously, we couldn't change the user agent name dynamically for onebox requests. In this commit, a new hidden site setting `onebox_user_agent` is created to override the default user agent value specified in the [initializer](c333e9d6e6/config/initializers/100-onebox_options.rb (L15)).

Co-authored-by: Régis Hanol <regis@hanol.fr>

config/initializers/100-onebox_options.rb

@@ -5,14 +5,14 @@ Rails.application.config.to_prepare do
     Onebox.options = {
       twitter_client: TwitterApi,
       redirect_limit: 3,
-      user_agent: "Discourse Forum Onebox v#{Discourse::VERSION::STRING}",
+      user_agent: "Discourse Forum Onebox",
       allowed_ports: [80, 443, SiteSetting.port.to_i],
     }
   else
     Onebox.options = {
       twitter_client: TwitterApi,
       redirect_limit: 3,
-      user_agent: "Discourse Forum Onebox v#{Discourse::VERSION::STRING}",
+      user_agent: "Discourse Forum Onebox",
     }
   end
 end

config/site_settings.yml

@@ -2106,6 +2106,9 @@ onebox:
     list_type: compact
   enable_inline_onebox_on_all_domains:
     default: true
+  onebox_user_agent:
+    default: ""
+    hidden: true
   force_custom_user_agent_hosts:
     default: "http://codepen.io"
     type: list

lib/final_destination.rb

@@ -81,7 +81,7 @@ class FinalDestination
     @user_agent =
       (
         if @force_custom_user_agent_hosts.any? { |host| hostname_matches?(host) }
-          Onebox.options.user_agent
+          Onebox::Helpers.user_agent
         else
           @default_user_agent
         end

lib/onebox/helpers.rb

@@ -104,9 +104,7 @@ module Onebox
 
         headers ||= {}
 
-        if Onebox.options.user_agent && !headers["User-Agent"]
+        headers["User-Agent"] ||= user_agent if user_agent
-          headers["User-Agent"] = Onebox.options.user_agent
-        end
 
         request = Net::HTTP::Get.new(uri.request_uri, headers)
         start_time = Time.now
@@ -232,6 +230,12 @@ module Onebox
       end
     end
 
+    def self.user_agent
+      user_agent = SiteSetting.onebox_user_agent.presence || Onebox.options.user_agent
+      user_agent = "#{user_agent} v#{Discourse::VERSION::STRING}"
+      user_agent
+    end
+
     # Percent-encodes a URI string per RFC3986 - https://tools.ietf.org/html/rfc3986
     def self.uri_encode(url)
       return "" unless url

spec/lib/onebox/helpers_spec.rb

@@ -211,7 +211,7 @@ RSpec.describe Onebox::Helpers do
     context "with custom option" do
       around do |example|
         previous_options = Onebox.options.to_h
-        Onebox.options = { user_agent: "EvilTroutBot v0.1" }
+        Onebox.options = { user_agent: "EvilTroutBot" }
 
         example.run
 
@@ -221,7 +221,7 @@ RSpec.describe Onebox::Helpers do
       it "has the custom user agent" do
         stub_request(:get, "http://example.com/some-resource").with(
           headers: {
-            "user-agent" => "EvilTroutBot v0.1",
+            "user-agent" => "EvilTroutBot v#{Discourse::VERSION::STRING}",
           },
         ).to_return(status: 200, body: "test")
 

spec/lib/oneboxer_spec.rb

@@ -570,26 +570,59 @@ RSpec.describe Oneboxer do
     end
   end
 
-  it "uses the Onebox custom user agent on specified hosts" do
+  describe "onebox custom user agent" do
-    SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/"
+    let!(:default_onebox_user_agent) do
-    url = "https://video.discourse.org/presentation.mp4"
+      "#{Onebox.options.user_agent} v#{Discourse::VERSION::STRING}"
+    end
 
-    stub_request(:head, url).to_return(status: 403, body: "", headers: {})
+    it "uses the site setting value" do
-    stub_request(:get, url).to_return(status: 403, body: "", headers: {})
+      SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/"
-    stub_request(:head, url).with(headers: { "User-Agent" => Onebox.options.user_agent }).to_return(
+      url = "https://video.discourse.org/presentation.mp4"
-      status: 200,
+      custom_user_agent = "Custom User Agent"
-      body: "",
-      headers: {
-      },
-    )
-    stub_request(:get, url).with(headers: { "User-Agent" => Onebox.options.user_agent }).to_return(
-      status: 200,
-      body: "",
-      headers: {
-      },
-    )
 
-    expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to be_present
+      %i[head get].each do |method|
+        stub_request(method, url).with(
+          headers: {
+            "User-Agent" => default_onebox_user_agent,
+          },
+        ).to_return(status: 403, body: "", headers: {})
+        stub_request(method, url).with(
+          headers: {
+            "User-Agent" => "#{custom_user_agent} v#{Discourse::VERSION::STRING}",
+          },
+        ).to_return(status: 200, body: "", headers: {})
+      end
+
+      expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include("onebox-warning-message")
+
+      SiteSetting.onebox_user_agent = custom_user_agent
+
+      expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include(
+        "onebox-placeholder-container",
+      )
+    end
+
+    it "forcing on specified hosts" do
+      SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/"
+      url = "https://video.discourse.org/presentation.mp4"
+
+      stub_request(:head, url).to_return(status: 403, body: "", headers: {})
+      stub_request(:get, url).to_return(status: 403, body: "", headers: {})
+      stub_request(:head, url).with(
+        headers: {
+          "User-Agent" => default_onebox_user_agent,
+        },
+      ).to_return(status: 200, body: "", headers: {})
+      stub_request(:get, url).with(
+        headers: {
+          "User-Agent" => default_onebox_user_agent,
+        },
+      ).to_return(status: 200, body: "", headers: {})
+
+      expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include(
+        "onebox-placeholder-container",
+      )
+    end
   end
 
   context "with youtube stub" do