[Forums] Clean up permission checks (#674)

app/controllers/forum_posts_controller.rb

@@ -20,9 +20,8 @@ class ForumPostsController < ApplicationController
   end
 
   def index
-    @query = ForumPost.permitted.active.search(search_params)
-    @query = ForumPost.permitted.search(search_params) if CurrentUser.is_moderator?
-    @forum_posts = @query.includes(:topic).paginate(params[:page], :limit => params[:limit], :search_count => params[:search])
+    @query = ForumPost.visible(CurrentUser.user).search(search_params)
+    @forum_posts = @query.includes(:topic).paginate(params[:page], limit: params[:limit], search_count: params[:search])
     respond_with(@forum_posts)
   end
 

app/controllers/forum_topics_controller.rb

@@ -25,9 +25,8 @@ class ForumTopicsController < ApplicationController
     params[:search] ||= {}
     params[:search][:order] ||= "sticky" if request.format == Mime::Type.lookup("text/html")
 
-    @query = ForumTopic.permitted.active.search(search_params)
-    @query = ForumTopic.permitted.search(search_params) if CurrentUser.is_moderator?
-    @forum_topics = @query.paginate(params[:page], :limit => per_page, :search_count => params[:search])
+    @query = ForumTopic.visible(CurrentUser.user).search(search_params)
+    @forum_topics = @query.paginate(params[:page], limit: per_page, search_count: params[:search])
 
     respond_with(@forum_topics) do |format|
       format.html do

app/models/forum_post.rb

@@ -46,19 +46,18 @@ class ForumPost < ApplicationRecord
       where("forum_posts.creator_id = ?", user_id)
     end
 
-    def active
-      where("(forum_posts.is_hidden = false or forum_posts.creator_id = ?)", CurrentUser.id)
-    end
-
-    def permitted
-      q = joins(:topic)
-      q = q.where("(forum_topics.is_hidden = false or forum_posts.creator_id = ?)", CurrentUser.id) unless CurrentUser.is_moderator?
+    def visible(user)
+      q = joins(topic: :category).where("forum_categories.can_view <= ?", user.level)
+      unless user.is_moderator?
+        q = q.where("forum_topics.is_hidden = FALSE OR forum_topics.creator_id = ?", user.id)
+        q = q.where("forum_posts.is_hidden = FALSE OR forum_posts.creator_id = ?", user.id)
+      end
       q
     end
 
     def search(params)
       q = super
-      q = q.permitted
+      q = q.visible(CurrentUser.user)
 
       q = q.where_user(:creator_id, :creator, params)
 

app/models/forum_topic.rb

@@ -53,12 +53,10 @@ class ForumTopic < ApplicationRecord
   end
 
   module SearchMethods
-    def active
-      where("(forum_topics.is_hidden = false or forum_topics.creator_id = ?)", CurrentUser.id)
-    end
-
-    def permitted
-      joins(:category).where('forum_categories.can_view <= ?', CurrentUser.level)
+    def visible(user)
+      q = joins(:category).where("forum_categories.can_view <= ?", user.level)
+      q = q.where("forum_topics.is_hidden = FALSE OR forum_topics.creator_id = ?", user.id) unless user.is_moderator?
+      q
     end
 
     def sticky_first
@@ -71,7 +69,7 @@ class ForumTopic < ApplicationRecord
 
     def search(params)
       q = super
-      q = q.permitted
+      q = q.visible(CurrentUser.user)
 
       q = q.attribute_matches(:title, params[:title_matches])
 
@@ -119,7 +117,7 @@ class ForumTopic < ApplicationRecord
         ForumTopicVisit.create(:user_id => user.id, :forum_topic_id => id, :last_read_at => updated_at)
       end
 
-      has_unread_topics = ForumTopic.permitted.active.where("forum_topics.updated_at >= ?", user.last_forum_read_at)
+      has_unread_topics = ForumTopic.visible(user).where("forum_topics.updated_at >= ?", user.last_forum_read_at)
       .joins("left join forum_topic_visits on (forum_topic_visits.forum_topic_id = forum_topics.id and forum_topic_visits.user_id = #{user.id})")
       .where("(forum_topic_visits.id is null or forum_topic_visits.last_read_at < forum_topics.updated_at)")
       .exists?

app/models/user.rb

@@ -385,7 +385,7 @@ class User < ApplicationRecord
   module ForumMethods
     def has_forum_been_updated?
       return false unless is_member?
-      max_updated_at = ForumTopic.permitted.active.order(updated_at: :desc).first&.updated_at
+      max_updated_at = ForumTopic.visible(self).order(updated_at: :desc).first&.updated_at
       return false if max_updated_at.nil?
       return true if last_forum_read_at.nil?
       return max_updated_at > last_forum_read_at

app/views/forum_posts/index.html.erb

@@ -12,18 +12,16 @@
       </thead>
       <tbody>
         <% @forum_posts.each do |forum_post| %>
-          <% if forum_post.visible?(CurrentUser.user) %>
-            <tr id="forum-post-<%= forum_post.id %>" data-topic-is-hidden="<%= forum_post.topic.is_hidden? %>" data-is-hidden="<%= forum_post.is_hidden? %>">
-              <td class="forum-post-topic-title">
-                <%= link_to forum_post.topic.title, forum_topic_path(forum_post.topic) %>
-              </td>
-              <td class="forum-post-excerpt">
-                <%= link_to truncate(forum_post.body, :length => 50), forum_post_path(forum_post) %>
-              </td>
-              <td><%= link_to_user forum_post.creator %></td>
-              <td><%= time_ago_in_words_tagged forum_post.created_at %></td>
-            </tr>
-          <% end %>
+          <tr id="forum-post-<%= forum_post.id %>" data-topic-is-hidden="<%= forum_post.topic.is_hidden? %>" data-is-hidden="<%= forum_post.is_hidden? %>">
+            <td class="forum-post-topic-title">
+              <%= link_to forum_post.topic.title, forum_topic_path(forum_post.topic) %>
+            </td>
+            <td class="forum-post-excerpt">
+              <%= link_to truncate(forum_post.body, :length => 50), forum_post_path(forum_post) %>
+            </td>
+            <td><%= link_to_user forum_post.creator %></td>
+            <td><%= time_ago_in_words_tagged forum_post.created_at %></td>
+          </tr>
         <% end %>
       </tbody>
     </table>

test/functional/forum_posts_controller_test.rb

@@ -62,6 +62,27 @@ class ForumPostsControllerTest < ActionDispatch::IntegrationTest
         assert_response :success
       end
 
+      context "with posts in a hidden category" do
+        setup do
+          as(@mod) do
+            @category2 = ForumCategory.create!(name: "test", can_view: @mod.level)
+            @forum_topic = create(:forum_topic, category: @category2, title: "test", original_post_attributes: { body: "test" })
+            @forum_post2 = @forum_topic.original_post
+          end
+        end
+
+        should "only list visible posts" do
+          get forum_posts_path
+          assert_response :success
+          assert_select "#forum-post-#{@forum_post.id}", true
+          assert_select "#forum-post-#{@forum_post2.id}", false
+
+          get forum_posts_path(format: :json)
+          assert_response :success
+          assert_equal([@forum_post.id], @response.parsed_body.pluck("id"))
+        end
+      end
+
       context "with search conditions" do
         should "list all matching forum posts" do
           get forum_posts_path, params: {:search => {:body_matches => "xxx"}}