SECURITY: correct local onebox category checks

Also removes ugly "source_topic_id" from cooked posts

Patch was authored by @zogstrip

Signed-off-by: Sam <sam.saffron@gmail.com>

app/assets/javascripts/discourse/components/composer-editor.js.es6

@@ -372,7 +372,7 @@ export default Ember.Component.extend({
       post.set('refreshedPost', true);
     }
 
-    $oneboxes.each((_, o) => load(o, refresh, ajax, this.currentUser.id));
+    $oneboxes.each((_, o) => load({ elem: o, refresh, ajax, categoryId: this.get('composer.category.id') }));
   },
 
   _warnMentionedGroups($preview) {

app/assets/javascripts/discourse/components/composer-title.js.es6

@@ -80,7 +80,13 @@ export default Ember.Component.extend({
       const link = document.createElement('a');
       link.href = this.get('composer.title');
 
-      let loadOnebox = load(link, false, ajax, this.currentUser.id, true);
+      const loadOnebox = load({
+        elem: link,
+        refresh: false,
+        ajax,
+        synchronous: true,
+        categoryId: this.get('composer.category.id'),
+      });
 
       if (loadOnebox && loadOnebox.then) {
         loadOnebox.then( () => {

app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6

@@ -105,7 +105,6 @@ const rule = {
 
           let title = topicInfo.title;
 
-
           if (options.enableEmoji) {
             title = performEmojiUnescape(topicInfo.title, {
               getURL: options.getURL, emojiSet: options.emojiSet

app/assets/javascripts/pretty-text/oneboxer.js.es6

@@ -43,16 +43,15 @@ function loadNext(ajax) {
 
   let timeoutMs = 150;
   let removeLoading = true;
-  const { url, refresh, $elem, userId } = loadingQueue.shift();
+  const { url, refresh, $elem, categoryId } = loadingQueue.shift();
 
   // Retrieve the onebox
   return ajax("/onebox", {
     dataType: 'html',
-    data: { url, refresh },
+    data: { url, refresh, category_id: categoryId },
     cache: true
   }).then(html => {
     let $html = $(html);
-
     localCache[normalize(url)] = $html;
     $elem.replaceWith($html);
     applySquareGenericOnebox($html, normalize(url));
@@ -60,7 +59,7 @@ function loadNext(ajax) {
     if (result && result.jqXHR && result.jqXHR.status === 429) {
       timeoutMs = 2000;
       removeLoading = false;
-      loadingQueue.unshift({ url, refresh, $elem, userId });
+      loadingQueue.unshift({ url, refresh, $elem, categoryId });
     } else {
       failedCache[normalize(url)] = true;
     }
@@ -75,14 +74,14 @@ function loadNext(ajax) {
 
 // Perform a lookup of a onebox based an anchor $element.
 // It will insert a loading indicator and remove it when the loading is complete or fails.
-export function load(e, refresh, ajax, userId, synchronous) {
+export function load({ elem , refresh = true, ajax, synchronous = false, categoryId }) {
-  const $elem = $(e);
+  const $elem = $(elem);
 
   // If the onebox has loaded or is loading, return
   if ($elem.data('onebox-loaded')) return;
   if ($elem.hasClass('loading-onebox')) return;
 
-  const url = e.href;
+  const url = elem.href;
 
   // Unless we're forcing a refresh...
   if (!refresh) {
@@ -99,7 +98,7 @@ export function load(e, refresh, ajax, userId, synchronous) {
   $elem.addClass('loading-onebox');
 
   // Add to the loading queue
-  loadingQueue.push({ url, refresh, $elem, userId });
+  loadingQueue.push({ url, refresh, $elem, categoryId });
 
   // Load next url in queue
   if (synchronous) {

app/controllers/onebox_controller.rb

@@ -14,13 +14,19 @@ class OneboxController < ApplicationController
     return render(body: nil, status: 429) if Oneboxer.is_previewing?(current_user.id)
 
     user_id = current_user.id
+    category_id = params[:category_id].to_i
     invalidate = params[:refresh] == 'true'
     url = params[:url]
 
     hijack do
       Oneboxer.preview_onebox!(user_id)
 
-      preview = Oneboxer.preview(url, invalidate_oneboxes: invalidate)
+      preview = Oneboxer.preview(url,
+        invalidate_oneboxes: invalidate,
+        user_id: user_id,
+        category_id: category_id
+      )
+
       preview.strip! if preview.present?
 
       Oneboxer.onebox_previewed!(user_id)

app/models/post_analyzer.rb

@@ -31,7 +31,7 @@ class PostAnalyzer
       cooked = PrettyText.cook(raw, opts)
     end
 
-    result = Oneboxer.apply(cooked, topic_id: @topic_id) do |url, _|
+    result = Oneboxer.apply(cooked) do |url|
       @found_oneboxes = true
       Oneboxer.invalidate(url) if opts[:invalidate_oneboxes]
       Oneboxer.cached_onebox(url)

lib/cooked_post_processor.rb

@@ -368,15 +368,13 @@ class CookedPostProcessor
   end
 
   def post_process_oneboxes
-    args = {
+    Oneboxer.apply(@doc) do |url|
-      post_id: @post.id,
-      invalidate_oneboxes: !!@opts[:invalidate_oneboxes],
-    }
-
-    # apply oneboxes
-    Oneboxer.apply(@doc, topic_id: @post.topic_id) do |url|
       @has_oneboxes = true
-      Oneboxer.onebox(url, args)
+      Oneboxer.onebox(url,
+        invalidate_oneboxes: !!@opts[:invalidate_oneboxes],
+        user_id: @post&.user_id,
+        category_id: @post&.topic&.category_id
+      )
     end
 
     oneboxed_images.each do |img|

lib/discourse.rb

@@ -236,15 +236,11 @@ module Discourse
   end
 
   def self.route_for(uri)
-
+    uri = URI(uri) rescue nil unless uri.is_a?(URI)
-    uri = URI(uri) rescue nil unless (uri.is_a?(URI))
     return unless uri
 
     path = uri.path || ""
-    if (uri.host == Discourse.current_hostname &&
+    if !uri.host || (uri.host == Discourse.current_hostname && path.start_with?(Discourse.base_uri))
-      path.start_with?(Discourse.base_uri)) ||
-      !uri.host
-
       path.slice!(Discourse.base_uri)
       return Rails.application.routes.recognize_path(path)
     end

lib/onebox/engine/discourse_local_onebox.rb

@@ -1,129 +0,0 @@
-module Onebox
-  module Engine
-    class DiscourseLocalOnebox
-      include Engine
-
-      # Use this onebox before others
-      def self.priority
-        1
-      end
-
-      def self.===(other)
-        url = other.to_s
-        return false unless url[Discourse.base_url]
-
-        route = Discourse.route_for(url)
-
-        !!(route[:controller] =~ /topics|uploads|users/)
-      rescue ActionController::RoutingError
-        false
-      end
-
-      def to_html
-        uri = URI(@url)
-        path = uri.path || ""
-        route = Discourse.route_for(uri)
-
-        case route[:controller]
-        when "uploads" then upload_html(path)
-        when "topics"  then topic_html(route)
-        when "users"   then user_html(route)
-        end
-      end
-
-      private
-
-        def upload_html(path)
-          case File.extname(path)
-          when /^\.(mov|mp4|webm|ogv)$/i
-            "<video width='100%' height='100%' controls><source src='#{@url}'><a href='#{@url}'>#{@url}</a></video>"
-          when /^\.(mp3|ogg|wav|m4a)$/i
-            "<audio controls><source src='#{@url}'><a href='#{@url}'>#{@url}</a></audio>"
-          end
-        end
-
-        def topic_html(route)
-          link = "<a href='#{@url}'>#{@url}</a>"
-          source_topic_id = @url[/[&?]source_topic_id=(\d+)/, 1].to_i
-          source_topic = Topic.find_by(id: source_topic_id) if source_topic_id > 0
-
-          if route[:post_number].present? && route[:post_number].to_i > 1
-            post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number])
-            return link unless can_see_post?(post, source_topic)
-
-            topic = post.topic
-            slug = Slug.for(topic.title)
-            excerpt = post.excerpt(SiteSetting.post_onebox_maxlength)
-            excerpt.gsub!(/[\r\n]+/, " ")
-            excerpt.gsub!("[/quote]", "[quote]") # don't break my quote
-
-            quote = "[quote=\"#{post.user.username}, topic:#{topic.id}, slug:#{slug}, post:#{post.post_number}\"]\n#{excerpt}\n[/quote]"
-
-            args = {}
-            args[:topic_id] = source_topic_id if source_topic_id > 0
-
-            PrettyText.cook(quote, args)
-          else
-            topic = Topic.find_by(id: route[:topic_id])
-            return link unless can_see_topic?(topic, source_topic)
-
-            first_post = topic.ordered_posts.first
-
-            args = {
-              topic_id: topic.id,
-              avatar: PrettyText.avatar_img(topic.user.avatar_template, "tiny"),
-              original_url: @url,
-              title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)),
-              category_html: CategoryBadge.html_for(topic.category),
-              quote: first_post.excerpt(SiteSetting.post_onebox_maxlength),
-            }
-
-            template = File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs")
-            Mustache.render(template, args)
-          end
-        end
-
-        def user_html(route)
-          link = "<a href='#{@url}'>#{@url}</a>"
-          username = route[:username] || ''
-          user = User.find_by(username_lower: username.downcase)
-
-          if user
-            args = {
-              user_id: user.id,
-              username: user.username,
-              avatar: PrettyText.avatar_img(user.avatar_template, "extra_large"),
-              name: user.name,
-              bio: user.user_profile.bio_excerpt(230),
-              location: user.user_profile.location,
-              joined: I18n.t('joined'),
-              created_at: user.created_at.strftime(I18n.t('datetime_formats.formats.date_only')),
-              website: user.user_profile.website,
-              website_name: UserSerializer.new(user).website_name,
-              original_url: @url
-            }
-
-            template = File.read("#{Rails.root}/lib/onebox/templates/discourse_user_onebox.hbs")
-            Mustache.render(template, args)
-          else
-            return link
-          end
-        end
-
-        def can_see_post?(post, source_topic)
-          return false if post.nil? || post.hidden || post.trashed? || post.topic.nil?
-          Guardian.new.can_see_post?(post) || same_category?(post.topic.category, source_topic)
-        end
-
-        def can_see_topic?(topic, source_topic)
-          return false if topic.nil? || topic.trashed? || topic.private_message?
-          Guardian.new.can_see_topic?(topic) || same_category?(topic.category, source_topic)
-        end
-
-        def same_category?(category, source_topic)
-          source_topic.try(:category_id) == category.try(:id)
-        end
-
-    end
-  end
-end

lib/oneboxer.rb

@@ -28,13 +28,13 @@ module Oneboxer
   def self.preview(url, options = nil)
     options ||= {}
     invalidate(url) if options[:invalidate_oneboxes]
-    onebox_raw(url)[:preview]
+    onebox_raw(url, options)[:preview]
   end
 
   def self.onebox(url, options = nil)
     options ||= {}
     invalidate(url) if options[:invalidate_oneboxes]
-    onebox_raw(url)[:onebox]
+    onebox_raw(url, options)[:onebox]
   end
 
   def self.cached_onebox(url)
@@ -76,41 +76,22 @@ module Oneboxer
     doc
   end
 
-  def self.append_source_topic_id(url, topic_id)
-    # hack urls to create proper expansions
-    if url =~ Regexp.new("^#{Discourse.base_url.gsub(".", "\\.")}.*$", true)
-      uri = URI.parse(url) rescue nil
-      if uri && uri.path
-        route = Rails.application.routes.recognize_path(uri.path) rescue nil
-        if route && route[:controller] == 'topics'
-          url += (url =~ /\?/ ? "&" : "?") + "source_topic_id=#{topic_id}"
-        end
-      end
-    end
-    url
-  end
-
   def self.apply(string_or_doc, args = nil)
     doc = string_or_doc
     doc = Nokogiri::HTML::fragment(doc) if doc.is_a?(String)
     changed = false
 
     each_onebox_link(doc) do |url, element|
-      if args && args[:topic_id]
+      onebox, _ = yield(url, element)
-        url = append_source_topic_id(url, args[:topic_id])
-      end
-      onebox, _preview = yield(url, element)
       if onebox
         parsed_onebox = Nokogiri::HTML::fragment(onebox)
         next unless parsed_onebox.children.count > 0
 
         # special logic to strip empty p elements
-        if  element.parent &&
+        if element&.parent&.node_name&.downcase == "p" && element&.parent&.children&.count == 1
-            element.parent.node_name &&
-            element.parent.node_name.downcase == "p" &&
-            element.parent.children.count == 1
           element = element.parent
         end
+
         changed = true
         element.swap parsed_onebox.to_html
       end
@@ -149,7 +130,104 @@ module Oneboxer
       "onebox__#{url}"
     end
 
-    def self.onebox_raw(url)
+    def self.onebox_raw(url, opts = {})
+      local_onebox(url, opts) || external_onebox(url)
+    rescue => e
+      # no point warning here, just cause we have an issue oneboxing a url
+      # we can later hunt for failed oneboxes by searching logs if needed
+      Rails.logger.info("Failed to onebox #{url} #{e} #{e.backtrace}")
+      # return a blank hash, so rest of the code works
+      blank_onebox
+    end
+
+    def self.local_onebox(url, opts = {})
+      return unless route = Discourse.route_for(url)
+
+      html =
+        case route[:controller]
+        when "uploads" then local_upload_html(url)
+        when "topics"  then local_topic_html(url, route, opts)
+        when "users"   then local_user_html(url, route)
+        end
+
+      html = html.presence || "<a href='#{url}'>#{url}</a>"
+      { onebox: html, preview: html }
+    end
+
+    def self.local_upload_html(url)
+      case File.extname(URI(url).path || "")
+      when /^\.(mov|mp4|webm|ogv)$/i
+        "<video width='100%' height='100%' controls><source src='#{url}'><a href='#{url}'>#{url}</a></video>"
+      when /^\.(mp3|ogg|wav|m4a)$/i
+        "<audio controls><source src='#{url}'><a href='#{url}'>#{url}</a></audio>"
+      end
+    end
+
+    def self.local_topic_html(url, route, opts)
+      return unless current_user = User.find_by(id: opts[:user_id])
+      return unless current_category = Category.find_by(id: opts[:category_id])
+      return unless Guardian.new(current_user).can_see_category?(current_category)
+
+      if route[:post_number].to_i > 1
+        post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number])
+
+        return unless post.present? && !post.hidden
+        return unless current_category.id == post.topic.category_id || Guardian.new.can_see_post?(post)
+
+        topic = post.topic
+        excerpt = post.excerpt(SiteSetting.post_onebox_maxlength)
+        excerpt.gsub!(/[\r\n]+/, " ")
+        excerpt.gsub!("[/quote]", "[quote]") # don't break my quote
+
+        quote = "[quote=\"#{post.user.username}, topic:#{topic.id}, post:#{post.post_number}\"]\n#{excerpt}\n[/quote]"
+
+        PrettyText.cook(quote)
+      else
+        return unless topic = Topic.find_by(id: route[:topic_id])
+        return unless current_category.id == topic.category_id || Guardian.new.can_see_topic?(topic)
+
+        first_post = topic.ordered_posts.first
+
+        args = {
+          topic_id: topic.id,
+          avatar: PrettyText.avatar_img(topic.user.avatar_template, "tiny"),
+          original_url: url,
+          title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)),
+          category_html: CategoryBadge.html_for(topic.category),
+          quote: first_post.excerpt(SiteSetting.post_onebox_maxlength),
+        }
+
+        template = File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs")
+        Mustache.render(template, args)
+      end
+    end
+
+    def self.local_user_html(url, route)
+      username = route[:username] || ""
+
+      if user = User.find_by(username_lower: username.downcase)
+        args = {
+          user_id: user.id,
+          username: user.username,
+          avatar: PrettyText.avatar_img(user.avatar_template, "extra_large"),
+          name: user.name,
+          bio: user.user_profile.bio_excerpt(230),
+          location: user.user_profile.location,
+          joined: I18n.t('joined'),
+          created_at: user.created_at.strftime(I18n.t('datetime_formats.formats.date_only')),
+          website: user.user_profile.website,
+          website_name: UserSerializer.new(user).website_name,
+          original_url: url
+        }
+
+        template = File.read("#{Rails.root}/lib/onebox/templates/discourse_user_onebox.hbs")
+        Mustache.render(template, args)
+      else
+        nil
+      end
+    end
+
+    def self.external_onebox(url)
       Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do
         fd = FinalDestination.new(url, ignore_redirects: ignore_redirects, force_get_hosts: force_get_hosts)
         uri = fd.resolve
@@ -169,14 +247,8 @@ module Oneboxer
 
         r = Onebox.preview(uri.to_s, options)
 
-        { onebox: r.to_s, preview: r.try(:placeholder_html).to_s }
+        { onebox: r.to_s, preview: r&.placeholder_html.to_s }
       end
-    rescue => e
-      # no point warning here, just cause we have an issue oneboxing a url
-      # we can later hunt for failed oneboxes by searching logs if needed
-      Rails.logger.info("Failed to onebox #{url} #{e} #{e.backtrace}")
-      # return a blank hash, so rest of the code works
-      blank_onebox
     end
 
 end

spec/components/cooked_post_processor_spec.rb

@@ -479,10 +479,11 @@ describe CookedPostProcessor do
 
     before do
       Oneboxer.expects(:onebox)
-        .with("http://www.youtube.com/watch?v=9bZkp7q19f0", post_id: 123, invalidate_oneboxes: true)
+        .with("http://www.youtube.com/watch?v=9bZkp7q19f0", invalidate_oneboxes: true, user_id: nil, category_id: post.topic.category_id)
         .returns("<div>GANGNAM STYLE</div>")
       cpp.post_process_oneboxes
     end
+
     it "inserts the onebox without wrapping p" do
       expect(cpp).to be_dirty
       expect(cpp.html).to match_html "<div>GANGNAM STYLE</div>"

spec/components/onebox/engine/discourse_local_onebox_spec.rb

@@ -1,161 +0,0 @@
-require 'rails_helper'
-
-describe Onebox::Engine::DiscourseLocalOnebox do
-
-  before { SiteSetting.external_system_avatars_enabled = false }
-
-  def build_link(url)
-    %|<a href="#{url}" rel="nofollow noopener">#{url}</a>|
-  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(build_link(url))
-    end
-
-    it "returns a link if not allowed to see the post" do
-      url = "#{Discourse.base_url}#{post2.url}"
-      Guardian.any_instance.expects(:can_see_post?).returns(false)
-      expect(Onebox.preview(url).to_s).to eq(build_link(url))
-    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(build_link(url))
-    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}"
-      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}/?source_topic_id=#{post2.topic_id + 1}"
-      html = Onebox.preview(url).to_s
-      expect(html).to include(post2.excerpt)
-      expect(html).to include(post2.topic.title)
-
-      html = Onebox.preview("#{Discourse.base_url}#{post2.url}").to_s
-      expect(html).to include(post2.user.username)
-      expect(html).to include(post2.excerpt)
-    end
-  end
-
-  context "for a link to a topic" do
-    let(:post)  { Fabricate(:post) }
-    let(:topic) { post.topic }
-
-    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(build_link(url))
-    end
-
-    it "returns a link if not allowed to see the topic" do
-      url = topic.url
-      Guardian.any_instance.expects(:can_see_topic?).returns(false)
-      expect(Onebox.preview(url).to_s).to eq(build_link(url))
-    end
-
-    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/)
-    end
-
-    it "returns some onebox goodness if topic 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>")
-
-      html = Onebox.preview("#{topic.url}/?u=codinghorror").to_s
-      expect(html).to include(topic.ordered_posts.first.user.username)
-      expect(html).to include("<blockquote>")
-    end
-  end
-
-  context "for a link to a user profile" do
-    let(:user)   { Fabricate(:user) }
-
-    it "returns a link if user isn't found" do
-      url = "#{Discourse.base_url}/u/none"
-      expect(Onebox.preview(url).to_s).to eq(build_link(url))
-    end
-
-    it "returns some onebox goodness if user exists" do
-      html = Onebox.preview("#{Discourse.base_url}/u/#{user.username}").to_s
-      expect(html).to include(user.username)
-      expect(html).to include(user.name)
-      expect(html).to include(user.created_at.strftime("%B %-d, %Y"))
-      expect(html).to include('<aside class="onebox">')
-    end
-  end
-
-  context "for a link to an internal audio or video file" do
-
-    let(:sha) { Digest::SHA1.hexdigest("discourse") }
-    let(:path) { "/uploads/default/original/3X/5/c/#{sha}" }
-
-    it "returns nil if file type is not audio or video" do
-      url = "#{Discourse.base_url}#{path}.pdf"
-      stub_request(:get, url).to_return(body: '')
-      expect(Onebox.preview(url).to_s).to eq("")
-    end
-
-    it "returns some onebox goodness for audio file" do
-      url = "#{Discourse.base_url}#{path}.MP3"
-      html = Onebox.preview(url).to_s
-      # </source> will be removed by the browser
-      # need to fix https://github.com/rubys/nokogumbo/issues/14
-      expect(html).to eq(%|<audio controls=""><source src="#{url}"></source>#{build_link(url)}</audio>|)
-    end
-
-    it "returns some onebox goodness for video file" do
-      url = "#{Discourse.base_url}#{path}.mov"
-      html = Onebox.preview(url).to_s
-      expect(html).to eq(%|<video width="100%" height="100%" controls=""><source src="#{url}"></source>#{build_link(url)}</video>|)
-    end
-  end
-
-  context "When deployed to a subfolder" do
-    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
-
-    context "for a link to a post" do
-      let(:post)  { Fabricate(:post) }
-      let(:post2) { Fabricate(:post, topic: post.topic, post_number: 2) }
-
-      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}"
-        html = Onebox.preview(url).to_s
-        expect(html).to include(post2.excerpt)
-        expect(html).to include(post2.topic.title)
-      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

spec/components/oneboxer_spec.rb

@@ -11,4 +11,75 @@ describe Oneboxer do
     expect(Oneboxer.onebox("http://boom.com")).to eq("")
   end
 
+  context "local oneboxes" do
+
+    def link(url)
+      url = "#{Discourse.base_url}#{url}"
+      %{<a href="#{url}">#{url}</a>}
+    end
+
+    def preview(url, user, category = Category.first)
+      Oneboxer.preview("#{Discourse.base_url}#{url}", user_id: user.id, category_id: category.id).to_s
+    end
+
+    it "links to a topic/post" do
+      staff = Fabricate(:user)
+      Group[:staff].add(staff)
+
+      secured_category = Fabricate(:category)
+      secured_category.permissions = { staff: :full }
+      secured_category.save!
+
+      public_post   = Fabricate(:post)
+      public_topic  = public_post.topic
+      public_reply  = Fabricate(:post, topic: public_topic, post_number: 2)
+      public_hidden = Fabricate(:post, topic: public_topic, post_number: 3, hidden: true)
+
+      user = public_post.user
+      public_category = public_topic.category
+
+      secured_topic = Fabricate(:topic, user: staff, category: secured_category)
+      secured_post  = Fabricate(:post, user: staff, topic: secured_topic)
+      secured_reply = Fabricate(:post, user: staff, topic: secured_topic, post_number: 2)
+
+      expect(preview(public_topic.relative_url, user, public_category)).to include(public_topic.title)
+      expect(preview(public_post.url, user, public_category)).to include(public_topic.title)
+      expect(preview(public_reply.url, user, public_category)).to include(public_reply.cooked)
+      expect(preview(public_hidden.url, user, public_category)).to match_html(link(public_hidden.url))
+      expect(preview(secured_topic.relative_url, user, public_category)).to match_html(link(secured_topic.relative_url))
+      expect(preview(secured_post.url, user, public_category)).to match_html(link(secured_post.url))
+      expect(preview(secured_reply.url, user, public_category)).to match_html(link(secured_reply.url))
+
+      expect(preview(public_topic.relative_url, user, secured_category)).to match_html(link(public_topic.relative_url))
+      expect(preview(public_reply.url, user, secured_category)).to match_html(link(public_reply.url))
+      expect(preview(secured_post.url, user, secured_category)).to match_html(link(secured_post.url))
+      expect(preview(secured_reply.url, user, secured_category)).to match_html(link(secured_reply.url))
+
+      expect(preview(public_topic.relative_url, staff, secured_category)).to include(public_topic.title)
+      expect(preview(public_post.url, staff, secured_category)).to include(public_topic.title)
+      expect(preview(public_reply.url, staff, secured_category)).to include(public_reply.cooked)
+      expect(preview(public_hidden.url, staff, secured_category)).to match_html(link(public_hidden.url))
+      expect(preview(secured_topic.relative_url, staff, secured_category)).to include(secured_topic.title)
+      expect(preview(secured_post.url, staff, secured_category)).to include(secured_topic.title)
+      expect(preview(secured_reply.url, staff, secured_category)).to include(secured_reply.cooked)
+    end
+
+    it "links to an user profile" do
+      user = Fabricate(:user)
+
+      expect(preview("/u/does-not-exist", user)).to match_html(link("/u/does-not-exist"))
+      expect(preview("/u/#{user.username}", user)).to include(user.name)
+    end
+
+    it "links to an upload" do
+      user = Fabricate(:user)
+      path = "/uploads/default/original/3X/e/8/e8fcfa624e4fb6623eea57f54941a58ba797f14d"
+
+      expect(preview("#{path}.pdf", user)).to match_html(link("#{path}.pdf"))
+      expect(preview("#{path}.MP3", user)).to include("<audio ")
+      expect(preview("#{path}.mov", user)).to include("<video ")
+    end
+
+  end
+
 end

spec/controllers/onebox_controller_spec.rb

@@ -14,8 +14,8 @@ describe OneboxController do
     before { @user = log_in(:admin) }
 
     it 'invalidates the cache if refresh is passed' do
-      Oneboxer.expects(:preview).with(url, invalidate_oneboxes: true)
+      Oneboxer.expects(:preview).with(url, invalidate_oneboxes: true, user_id: @user.id, category_id: 0)
-      get :show, params: { url: url, refresh: 'true', user_id: @user.id }, format: :json
+      get :show, params: { url: url, refresh: 'true' }, format: :json
     end
 
     describe "cached onebox" do
@@ -41,13 +41,13 @@ describe OneboxController do
         stub_request(:get, url)
           .to_return(status: 200, headers: {}, body: onebox_html).then.to_raise
 
-        get :show, params: { url: url, user_id: @user.id, refresh: "true" }, format: :json
+        get :show, params: { url: url, refresh: "true" }, format: :json
 
         expect(response).to be_success
         expect(response.body).to include('Fred')
         expect(response.body).to include('bodycontent')
 
-        get :show, params: { url: url, user_id: @user.id }, format: :json
+        get :show, params: { url: url }, format: :json
         expect(response).to be_success
         expect(response.body).to include('Fred')
         expect(response.body).to include('bodycontent')
@@ -59,7 +59,7 @@ describe OneboxController do
 
       it "returns 429" do
         Oneboxer.expects(:is_previewing?).returns(true)
-        get :show, params: { url: url, user_id: @user.id }, format: :json
+        get :show, params: { url: url }, format: :json
         expect(response.status).to eq(429)
       end
 
@@ -70,8 +70,8 @@ describe OneboxController do
       let(:body) { "this is the onebox body" }
 
       before do
-        Oneboxer.expects(:preview).with(url, invalidate_oneboxes: false).returns(body)
+        Oneboxer.expects(:preview).returns(body)
-        get :show, params: { url: url, user_id: @user.id }, format: :json
+        get :show, params: { url: url }, format: :json
       end
 
       it 'returns the onebox response in the body' do
@@ -84,19 +84,51 @@ describe OneboxController do
     describe "missing onebox" do
 
       it "returns 404 if the onebox is nil" do
-        Oneboxer.expects(:preview).with(url, invalidate_oneboxes: false).returns(nil)
+        Oneboxer.expects(:preview).returns(nil)
-        get :show, params: { url: url, user_id: @user.id }, format: :json
+        get :show, params: { url: url }, format: :json
         expect(response.response_code).to eq(404)
       end
 
       it "returns 404 if the onebox is an empty string" do
-        Oneboxer.expects(:preview).with(url, invalidate_oneboxes: false).returns(" \t ")
+        Oneboxer.expects(:preview).returns(" \t ")
-        get :show, params: { url: url, user_id: @user.id }, format: :json
+        get :show, params: { url: url }, format: :json
         expect(response.response_code).to eq(404)
       end
 
     end
 
+    describe "local onebox" do
+
+      it 'does not cache local oneboxes' do
+        post1 = create_post
+        url = Discourse.base_url + post1.url
+
+        get :show, params: { url: url, category_id: post1.topic.category_id }, format: :json
+        expect(response.body).to include('blockquote')
+
+        post1.trash!
+
+        get :show, params: { url: url, category_id: post1.topic.category_id }, format: :json
+        expect(response.body).not_to include('blockquote')
+      end
+    end
+
+  end
+
+  it 'does not onebox when you have no permission on category' do
+    log_in
+
+    post1 = create_post
+    url = Discourse.base_url + post1.url
+
+    get :show, params: { url: url, category_id: post1.topic.category_id }, format: :json
+    expect(response.body).to include('blockquote')
+
+    post1.topic.category.set_permissions(staff: :full)
+    post1.topic.category.save
+
+    get :show, params: { url: url, category_id: post1.topic.category_id }, format: :json
+    expect(response.body).not_to include('blockquote')
   end
 
 end