FIX: Rate limit and hijack certificate generation. (#8215)

To eliminate a DDOS attack vector, we're taking the following measures:

The endpoint will be rate-limited to 3 requests every 60 seconds (per user).
A 24 hours max-age cache header is sent with the response.
The route will be hijacked to generate the certificate in the background.

plugins/discourse-narrative-bot/lib/discourse_narrative_bot/certificate_generator.rb

@@ -2,8 +2,9 @@
 
 module DiscourseNarrativeBot
   class CertificateGenerator
-    def initialize(user, date)
+    def initialize(user, date, avatar_url)
       @user = user
+      @avatar_url = avatar_url
 
       date =
         begin
@@ -21,29 +22,36 @@ module DiscourseNarrativeBot
     end
 
     def new_user_track
-      width = 538.583 # Default width for the SVG
+      svg_default_width = 538.583
-      ApplicationController.render(inline: File.read(File.expand_path('../templates/new_user.svg.erb', __FILE__)),
+      logo_container = logo_group(55, svg_default_width, 350)
-                                   assigns: { width: width,
+
-                                              discobot_user: @discobot_user,
+      ApplicationController.render(inline: read_template('new_user'), assigns: assign_options(svg_default_width, logo_container))
-                                              date: @date,
-                                              avatar_url: base64_image_link(avatar_url),
-                                              logo_group: logo_group(55, width, 350),
-                                              name: name })
     end
 
     def advanced_user_track
-      width = 722.8 # Default width for the SVG
+      svg_default_width = 722.8
-      ApplicationController.render(inline: File.read(File.expand_path('../templates/advanced_user.svg.erb', __FILE__)),
+      logo_container = logo_group(40, svg_default_width, 280)
-                                   assigns: { width: width,
+
-                                              discobot_user: @discobot_user,
+      ApplicationController.render(inline: read_template('advanced_user'), assigns: assign_options(svg_default_width, logo_container))
-                                              date: @date,
-                                              avatar_url: base64_image_link(avatar_url),
-                                              logo_group: logo_group(40, width, 280),
-                                              name: name })
     end
 
     private
 
+    def read_template(filename)
+      File.read(File.expand_path("../templates/#{filename}.svg.erb", __FILE__))
+    end
+
+    def assign_options(width, logo_group)
+      {
+        width: width,
+        discobot_user: @discobot_user,
+        date: @date,
+        avatar_url: base64_image_link(@avatar_url),
+        logo_group: logo_group,
+        name: name
+      }
+    end
+
     def name
       @user.username.titleize
     end
@@ -84,9 +92,5 @@ module DiscourseNarrativeBot
     rescue OpenURI::HTTPError
       # Ignore if fetching image returns a non 200 response
     end
-
-    def avatar_url
-      UrlHelper.absolute(Discourse.base_uri + @user.avatar_template.gsub('{size}', '250'))
-    end
   end
 end

plugins/discourse-narrative-bot/plugin.rb

@@ -80,31 +80,38 @@ after_initialize do
       requires_login
 
       def generate
-        unless params[:user_id]&.present?
+        immutable_for(24.hours)
-          raise Discourse::InvalidParameters.new('user_id must be present')
+
+        %i[date user_id].each do |key|
+          raise Discourse::InvalidParameters.new("#{key} must be present") unless params[key]&.present?
         end
 
+        rate_limiter = RateLimiter.new(current_user, 'svg_certificate', 3, 1.minute)
+        rate_limiter.performed! unless current_user.staff?
+
         user = User.find_by(id: params[:user_id])
         raise Discourse::NotFound if user.blank?
+        cdn_avatar_url = fetch_avatar_url(user)
 
-        unless params[:date]&.present?
+        hijack do
-          raise Discourse::InvalidParameters.new('date must be present')
+          generator = CertificateGenerator.new(user, params[:date], cdn_avatar_url)
-        end
 
-        generator = CertificateGenerator.new(user, params[:date])
+          svg = params[:type] == 'advanced' ? generator.advanced_user_track : generator.new_user_track
 
-        svg =
+          respond_to do |format|
-          case params[:type]
+            format.svg { render inline: svg }
-          when 'advanced'
-            generator.advanced_user_track
-          else
-            generator.new_user_track
           end
-
-        respond_to do |format|
-          format.svg { render inline: svg }
         end
       end
+
+      private
+
+      def fetch_avatar_url(user)
+        avatar_url = UrlHelper.absolute(Discourse.base_uri + user.avatar_template.gsub('{size}', '250'))
+        URI(avatar_url).open('rb', redirect: false).read
+      rescue OpenURI::HTTPError
+        # Ignore if fetching image returns a non 200 response
+      end
     end
   end
 

plugins/discourse-narrative-bot/spec/lib/certificate_generator_spec.rb

@@ -4,10 +4,12 @@ require 'rails_helper'
 
 RSpec.describe DiscourseNarrativeBot::CertificateGenerator do
   let(:user) { Fabricate(:user) }
+  let(:avatar_url) { 'http://test.localhost/cdn/avatar.png' }
+  let(:date) { "2017-00-10" }
 
   describe 'when an invalid date is given' do
     it 'should default to the current date' do
-      expect { described_class.new(user, "2017-00-10") }.to_not raise_error
+      expect { described_class.new(user, date, avatar_url) }.to_not raise_error
     end
   end
 
@@ -19,7 +21,7 @@ RSpec.describe DiscourseNarrativeBot::CertificateGenerator do
       end
 
       it 'should not try to fetch a image' do
-        expect(described_class.new(user, "2017-02-10").send(:logo_group, 1, 1, 1))
+        expect(described_class.new(user, date, avatar_url).send(:logo_group, 1, 1, 1))
           .to eq(nil)
       end
     end

plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb

@@ -27,7 +27,8 @@ describe "Discobot Certificate" do
       end
 
       it 'should return the right text' do
-        stub_request(:get, /letter_avatar_proxy/).to_return(status: 200)
+        stub_request(:get, /letter_avatar_proxy/).to_return(status: 200, body: 'http://test.localhost/cdn/avatar.png')
+        stub_request(:get, /avatar.png/).to_return(status: 200)
 
         stub_request(:get, SiteSetting.site_logo_small_url)
           .to_return(status: 200)