From af0036db6bdb2287766590a1cf76e7afb9dbc51b Mon Sep 17 00:00:00 2001 From: Donovan Daniels Date: Sun, 14 Jul 2024 15:14:09 -0500 Subject: [PATCH] [Forums] Clean up permission checks (#674) --- app/controllers/forum_posts_controller.rb | 5 ++--- app/controllers/forum_topics_controller.rb | 5 ++--- app/models/forum_post.rb | 15 ++++++------- app/models/forum_topic.rb | 14 +++++------- app/models/user.rb | 2 +- app/views/forum_posts/index.html.erb | 22 +++++++++---------- .../functional/forum_posts_controller_test.rb | 21 ++++++++++++++++++ 7 files changed, 49 insertions(+), 35 deletions(-) diff --git a/app/controllers/forum_posts_controller.rb b/app/controllers/forum_posts_controller.rb index c87eb5638..24b774c80 100644 --- a/app/controllers/forum_posts_controller.rb +++ b/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 diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index a1402455e..706e08883 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/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 diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 06082f9f7..5939b100d 100644 --- a/app/models/forum_post.rb +++ b/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) diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index 504530c42..5df769b3e 100644 --- a/app/models/forum_topic.rb +++ b/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? diff --git a/app/models/user.rb b/app/models/user.rb index 3444b3cbc..6c72e9f4f 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/views/forum_posts/index.html.erb b/app/views/forum_posts/index.html.erb index 160e76cf6..8c753d78f 100644 --- a/app/views/forum_posts/index.html.erb +++ b/app/views/forum_posts/index.html.erb @@ -12,18 +12,16 @@ <% @forum_posts.each do |forum_post| %> - <% if forum_post.visible?(CurrentUser.user) %> - - - <%= link_to forum_post.topic.title, forum_topic_path(forum_post.topic) %> - - - <%= link_to truncate(forum_post.body, :length => 50), forum_post_path(forum_post) %> - - <%= link_to_user forum_post.creator %> - <%= time_ago_in_words_tagged forum_post.created_at %> - - <% end %> + + + <%= link_to forum_post.topic.title, forum_topic_path(forum_post.topic) %> + + + <%= link_to truncate(forum_post.body, :length => 50), forum_post_path(forum_post) %> + + <%= link_to_user forum_post.creator %> + <%= time_ago_in_words_tagged forum_post.created_at %> + <% end %> diff --git a/test/functional/forum_posts_controller_test.rb b/test/functional/forum_posts_controller_test.rb index dd1aa425a..a3ebc11d8 100644 --- a/test/functional/forum_posts_controller_test.rb +++ b/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"}}