ensure we allow self oneboxing of login required sites

lib/cooked_post_processor.rb

@@ -300,10 +300,10 @@ class CookedPostProcessor
     }
 
     # apply oneboxes
-    Oneboxer.apply(@doc, topic_id: @post.topic_id) { |url|
+    Oneboxer.apply(@doc, topic_id: @post.topic_id) do |url|
       @has_oneboxes = true
       Oneboxer.onebox(url, args)
-    }
+    end
 
     # make sure we grab dimensions for oneboxed images
     oneboxed_images.each { |img| limit_size!(img) }

lib/onebox/engine/discourse_local_onebox.rb

@@ -3,118 +3,84 @@ module Onebox
     class DiscourseLocalOnebox
       include Engine
 
-      # we need to allow for multisite here
-      def self.is_on_site?(url)
-        Regexp.new("^#{Discourse.base_url.gsub(".","\\.")}.*$", true) === url.to_s
-      end
-
       # Use this onebox before others
       def self.priority
         1
       end
 
       def self.===(other)
-        if other.kind_of?(URI)
+        url = other.to_s
-          uri = other
+        return false unless url[Discourse.base_url]
-          begin
+
-            route = Rails.application.routes.recognize_path(uri.path.sub(Discourse.base_uri, ""))
+        path = url.sub(Discourse.base_url, "")
-            case route[:controller]
+        route = Rails.application.routes.recognize_path(path)
-            when 'uploads'
+
-              is_on_site?(other)
+        !!(route[:controller] =~ /topics|uploads/)
-            when 'topics'
+      rescue ActionController::RoutingError
-              # super will use matches_regexp to match the domain name
+        false
-              is_on_site?(other)
-            else
-              false
-            end
-          rescue ActionController::RoutingError
-            false
-          end
-        else
-          is_on_site?(other)
-        end
       end
 
       def to_html
-        uri = URI::parse(@url)
+        path = @url.sub(Discourse.base_url, "")
-        route = Rails.application.routes.recognize_path(uri.path.sub(Discourse.base_uri, ""))
+        route = Rails.application.routes.recognize_path(path)
-        url = @url.sub(/[&?]source_topic_id=(\d+)/, "")
-        source_topic_id = $1.to_i
 
-        # Figure out what kind of onebox to show based on the URL
         case route[:controller]
-        when 'uploads'
+        when "uploads" then upload_html(path)
+        when "topics"  then topic_html(path, route)
+        end
+      end
 
-          url.gsub!("http:", "https:") if SiteSetting.force_https
+      private
-          if File.extname(uri.path) =~ /^.(mov|mp4|webm|ogv)$/
+
-            return "<video width='100%' height='100%' controls><source src='#{url}'><a href='#{url}'>#{url}</a></video>"
+        def upload_html(path)
-          elsif File.extname(uri.path) =~ /^.(mp3|ogg|wav)$/
+          case File.extname(path)
-            return "<audio controls><source src='#{url}'><a href='#{url}'>#{url}</a></audio>"
+          when /^\.(mov|mp4|webm|ogv)$/
-          else
+            "<video width='100%' height='100%' controls><source src='#{@url}'><a href='#{@url}'>#{@url}</a></video>"
-            return false
+          when /^\.(mp3|ogg|wav)$/
+            "<audio controls><source src='#{@url}'><a href='#{@url}'>#{@url}</a></audio>"
           end
-        when 'topics'
+        end
+
+        def topic_html(path, route)
+          link = "<a href='#{@url}'>#{@url}</a>"
+          source_topic_id = @url[/[&?]source_topic_id=(\d+)/, 1].to_i
 
-          linked = "<a href='#{url}'>#{url}</a>"
           if route[:post_number].present? && route[:post_number].to_i > 1
-            # Post Link
+            post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number])
-            post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number].to_i)
+            return link if post.nil? || post.hidden || !Guardian.new.can_see?(post)
-            return linked unless post
-            return linked if post.hidden
-            return linked unless Guardian.new.can_see?(post)
 
             topic = post.topic
             slug = Slug.for(topic.title)
-
             excerpt = post.excerpt(SiteSetting.post_onebox_maxlength)
-            excerpt.gsub!("\n"," ")
+            excerpt.gsub!(/[\r\n]+/, " ")
-            # hack to make it render for now
+            excerpt.gsub!("[/quote]", "[quote]") # don't break my quote
-            excerpt.gsub!("[/quote]", "[quote]")
+
             quote = "[quote=\"#{post.user.username}, topic:#{topic.id}, slug:#{slug}, post:#{post.post_number}\"]#{excerpt}[/quote]"
 
             args = {}
             args[:topic_id] = source_topic_id if source_topic_id > 0
-            cooked = PrettyText.cook(quote, args)
-            return cooked
 
+            PrettyText.cook(quote, args)
           else
-            # Topic Link
+            topic = Topic.find_by(id: route[:topic_id])
-            topic = Topic.where(id: route[:topic_id].to_i).includes(:user).first
+            return link if topic.nil? || !Guardian.new.can_see?(topic)
-            return linked unless topic
-            return linked unless Guardian.new.can_see?(topic)
 
-            post = topic.posts.first
+            first_post = topic.ordered_posts.first
 
-            posters = topic.posters_summary.map do |p|
+            args = {
-              {
+              topic: topic.id,
-                username: p[:user].username,
+              avatar: PrettyText.avatar_img(topic.user.avatar_template, "tiny"),
-                avatar: PrettyText.avatar_img(p[:user].avatar_template, 'tiny'),
+              original_url: @url,
-                description: p[:description],
+              title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)),
-                extras: p[:extras]
+              category_html: CategoryBadge.html_for(topic.category),
-              }
+              quote: first_post.excerpt(SiteSetting.post_onebox_maxlength),
-            end
+            }
 
-            quote = post.excerpt(SiteSetting.post_onebox_maxlength)
+            template = File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs")
-            args = { original_url: url,
+            Mustache.render(template, args)
-                     title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)),
-                     avatar: PrettyText.avatar_img(topic.user.avatar_template, 'tiny'),
-                     posts_count: topic.posts_count,
-                     last_post: FreedomPatches::Rails4.time_ago_in_words(topic.last_posted_at, false, scope: :'datetime.distance_in_words_verbose'),
-                     age: FreedomPatches::Rails4.time_ago_in_words(topic.created_at, false, scope: :'datetime.distance_in_words_verbose'),
-                     views: topic.views,
-                     posters: posters,
-                     quote: quote,
-                     category_html: CategoryBadge.html_for(topic.category),
-                     topic: topic.id }
-
-            return Mustache.render(File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs"), args)
           end
         end
 
-      rescue ActionController::RoutingError
-        nil
-      end
-
     end
   end
 end

lib/onebox/templates/discourse_topic_onebox.hbs

@@ -1,11 +1,9 @@
 <aside class='quote' data-post="1" data-topic="{{topic}}">
   <div class='title'>
-    <div class='quote-controls'></div>
     {{{avatar}}}
     <a href="{{original_url}}">{{{title}}}</a> {{{category_html}}}
   </div>
-  <blockquote>{{{quote}}}
+  <blockquote>
-    <div class='topic-info'>
+    {{{quote}}}
-    </div>
   </blockquote>
 </aside>

lib/oneboxer.rb

@@ -62,9 +62,7 @@ module Oneboxer
     onebox_links = doc.search("a.onebox")
     if onebox_links.present?
       onebox_links.each do |link|
-        if link['href'].present?
+        yield(link['href'], link) if link['href'].present?
-          yield link['href'], link
-        end
       end
     end
 

spec/components/onebox/engine/discourse_local_onebox_spec.rb

@@ -1,17 +1,8 @@
 require 'rails_helper'
 
 describe Onebox::Engine::DiscourseLocalOnebox do
-  it "matches for a topic url" do
-    url = "#{Discourse.base_url}/t/hot-topic"
-    expect(Onebox.has_matcher?(url)).to eq(true)
-    expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class)
-  end
 
-  it "matches for a post url" do
+  before { SiteSetting.external_system_avatars_enabled = false }
-    url = "#{Discourse.base_url}/t/hot-topic/23/2"
-    expect(Onebox.has_matcher?(url)).to eq(true)
-    expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class)
-  end
 
   context "for a link to a post" do
     let(:post)  { Fabricate(:post) }
@@ -36,13 +27,11 @@ describe Onebox::Engine::DiscourseLocalOnebox do
 
     it "returns some onebox goodness if post exists and can be seen" do
       url = "#{Discourse.base_url}#{post2.url}?source_topic_id=#{post2.topic_id+1}"
-      Guardian.any_instance.stubs(:can_see?).returns(true)
       html = Onebox.preview(url).to_s
       expect(html).to include(post2.excerpt)
       expect(html).to include(post2.topic.title)
 
-      url = "#{Discourse.base_url}#{post2.url}"
+      html = Onebox.preview("#{Discourse.base_url}#{post2.url}").to_s
-      html = Onebox.preview(url).to_s
       expect(html).to include(post2.user.username)
       expect(html).to include(post2.excerpt)
     end
@@ -52,8 +41,6 @@ describe Onebox::Engine::DiscourseLocalOnebox do
     let(:post)  { Fabricate(:post) }
     let(:topic) { post.topic }
 
-    before { topic.last_posted_at = Time.zone.now; topic.save; } # otherwise errors
-
     it "returns a link if topic isn't found" do
       url = "#{Discourse.base_url}/t/not-found/123"
       expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>")
@@ -67,16 +54,14 @@ describe Onebox::Engine::DiscourseLocalOnebox do
 
     it "replaces emoji in the title" do
       topic.update_column(:title, "Who wants to eat a :hamburger:")
-      expect(Onebox.preview(topic.url).to_s).to match(/hamburger.png/)
+      expect(Onebox.preview(topic.url).to_s).to match(/hamburger\.png/)
     end
 
     it "returns some onebox goodness if post exists and can be seen" do
-      SiteSetting.external_system_avatars_enabled = false
-      url = "#{topic.url}"
       Guardian.any_instance.stubs(:can_see?).returns(true)
-      html = Onebox.preview(url).to_s
+      html = Onebox.preview(topic.url).to_s
-      expect(html).to include(topic.posts.first.user.username)
+      expect(html).to include(topic.ordered_posts.first.user.username)
-      expect(html).to include("topic-info")
+      expect(html).to include("<blockquote>")
     end
   end
 
@@ -102,59 +87,42 @@ describe Onebox::Engine::DiscourseLocalOnebox do
   end
 
   context "When deployed to a subfolder" do
-    let(:base_url) { "http://test.localhost/subfolder" }
     let(:base_uri) { "/subfolder" }
+    let(:base_url) { "http://test.localhost#{base_uri}" }
 
     before do
       Discourse.stubs(:base_url).returns(base_url)
       Discourse.stubs(:base_uri).returns(base_uri)
     end
 
-    it "matches for a topic url" do
-      url = "#{Discourse.base_url}/t/hot-topic"
-      expect(Onebox.has_matcher?(url)).to eq(true)
-      expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class)
-    end
-
-    it "matches for a post url" do
-      url = "#{Discourse.base_url}/t/hot-topic/23/2"
-      expect(Onebox.has_matcher?(url)).to eq(true)
-      expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class)
-    end
-
     context "for a link to a post" do
       let(:post)  { Fabricate(:post) }
       let(:post2) { Fabricate(:post, topic: post.topic, post_number: 2) }
 
-      it "returns a link if post isn't found" do
-        url = "#{Discourse.base_url}/t/not-exist/3/2"
-        expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>")
-      end
-
-      it "returns a link if not allowed to see the post" do
-        url = "#{Discourse.base_url}#{post2.url}"
-        Guardian.any_instance.stubs(:can_see?).returns(false)
-        expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>")
-      end
-
-      it "returns a link if post is hidden" do
-        hidden_post = Fabricate(:post, topic: post.topic, post_number: 2, hidden: true, hidden_reason_id: Post.hidden_reasons[:flag_threshold_reached])
-        url = "#{Discourse.base_url}#{hidden_post.url}"
-        expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>")
-      end
-
       it "returns some onebox goodness if post exists and can be seen" do
         url = "#{Discourse.base_url}#{post2.url}?source_topic_id=#{post2.topic_id+1}"
         Guardian.any_instance.stubs(:can_see?).returns(true)
         html = Onebox.preview(url).to_s
         expect(html).to include(post2.excerpt)
         expect(html).to include(post2.topic.title)
-
-        url = "#{Discourse.base_url}#{post2.url}"
-        html = Onebox.preview(url).to_s
-        expect(html).to include(post2.user.username)
-        expect(html).to include(post2.excerpt)
       end
     end
   end
+
+  context "When login_required is enabled" do
+    before { SiteSetting.login_required = true }
+
+    context "for a link to a topic" do
+      let(:post)  { Fabricate(:post) }
+      let(:topic) { post.topic }
+
+      it "returns some onebox goodness if post exists and can be seen" do
+        html = Onebox.preview(topic.url).to_s
+        expect(html).to include(topic.ordered_posts.first.user.username)
+        expect(html).to include("<blockquote>")
+      end
+    end
+
+  end
+
 end