pundit: convert favorite groups to pundit.

app/controllers/favorite_group_orders_controller.rb

@@ -1,9 +1,8 @@
 class FavoriteGroupOrdersController < ApplicationController
   respond_to :html, :xml, :json, :js
-  before_action :member_only
 
   def edit
-    @favorite_group = FavoriteGroup.find(params[:favorite_group_id])
+    @favorite_group = authorize FavoriteGroup.find(params[:favorite_group_id])
     respond_with(@favorite_group)
   end
 end

app/controllers/favorite_groups_controller.rb

@@ -1,10 +1,9 @@
 class FavoriteGroupsController < ApplicationController
-  before_action :member_only, :except => [:index, :show]
   respond_to :html, :xml, :json, :js
 
   def index
     params[:search][:creator_id] ||= params[:user_id]
-    @favorite_groups = FavoriteGroup.visible(CurrentUser.user).paginated_search(params)
+    @favorite_groups = authorize FavoriteGroup.visible(CurrentUser.user).paginated_search(params)
     @favorite_groups = @favorite_groups.includes(:creator) if request.format.html?
 
     respond_with(@favorite_groups)
@@ -13,33 +12,31 @@ class FavoriteGroupsController < ApplicationController
   def show
     limit = params[:limit].presence || CurrentUser.user.per_page
 
-    @favorite_group = FavoriteGroup.find(params[:id])
+    @favorite_group = authorize FavoriteGroup.find(params[:id])
-    check_read_privilege(@favorite_group)
     @posts = @favorite_group.posts.paginate(params[:page], limit: limit, count: @favorite_group.post_count)
 
     respond_with(@favorite_group)
   end
 
   def new
-    @favorite_group = FavoriteGroup.new
+    @favorite_group = authorize FavoriteGroup.new
     respond_with(@favorite_group)
   end
 
   def create
-    @favorite_group = CurrentUser.favorite_groups.create(favgroup_params)
+    @favorite_group = authorize FavoriteGroup.new(creator: CurrentUser.user, **permitted_attributes(FavoriteGroup))
+    @favorite_group.save
     respond_with(@favorite_group)
   end
 
   def edit
-    @favorite_group = FavoriteGroup.find(params[:id])
+    @favorite_group = authorize FavoriteGroup.find(params[:id])
-    check_write_privilege(@favorite_group)
     respond_with(@favorite_group)
   end
 
   def update
-    @favorite_group = FavoriteGroup.find(params[:id])
+    @favorite_group = authorize FavoriteGroup.find(params[:id])
-    check_write_privilege(@favorite_group)
+    @favorite_group.update(permitted_attributes(@favorite_group))
-    @favorite_group.update(favgroup_params)
     unless @favorite_group.errors.any?
       flash[:notice] = "Favorite group updated"
     end
@@ -47,31 +44,15 @@ class FavoriteGroupsController < ApplicationController
   end
 
   def destroy
-    @favorite_group = FavoriteGroup.find(params[:id])
+    @favorite_group = authorize FavoriteGroup.find(params[:id])
-    check_write_privilege(@favorite_group)
     @favorite_group.destroy!
     flash[:notice] = "Favorite group deleted" if request.format.html?
     respond_with(@favorite_group, location: favorite_groups_path(search: { creator_name: CurrentUser.name }))
   end
 
   def add_post
-    @favorite_group = FavoriteGroup.find(params[:id])
+    @favorite_group = authorize FavoriteGroup.find(params[:id])
-    check_write_privilege(@favorite_group)
     @post = Post.find(params[:post_id])
     @favorite_group.add!(@post)
   end
-
-  private
-
-  def check_write_privilege(favgroup)
-    raise User::PrivilegeError unless favgroup.editable_by?(CurrentUser.user)
-  end
-
-  def check_read_privilege(favgroup)
-    raise User::PrivilegeError unless favgroup.viewable_by?(CurrentUser.user)
-  end
-
-  def favgroup_params
-    params.fetch(:favorite_group, {}).permit(%i[name post_ids post_ids_string is_public], post_ids: [])
-  end
 end

app/logical/post_query_builder.rb

@@ -842,14 +842,14 @@ class PostQueryBuilder
 
             when "-favgroup"
               favgroup = FavoriteGroup.find_by_name_or_id!(g2, CurrentUser.user)
-              raise User::PrivilegeError unless favgroup.viewable_by?(CurrentUser.user)
+              raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], favgroup).show?
 
               q[:favgroups_neg] ||= []
               q[:favgroups_neg] << favgroup
 
             when "favgroup"
               favgroup = FavoriteGroup.find_by_name_or_id!(g2, CurrentUser.user)
-              raise User::PrivilegeError unless favgroup.viewable_by?(CurrentUser.user)
+              raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], favgroup).show?
 
               q[:favgroups] ||= []
               q[:favgroups] << favgroup

app/models/favorite_group.rb

@@ -160,14 +160,6 @@ class FavoriteGroup < ApplicationRecord
     post_ids.include?(post_id)
   end
 
-  def editable_by?(user)
-    creator_id == user.id
-  end
-
-  def viewable_by?(user)
-    creator_id == user.id || is_public
-  end
-
   def self.available_includes
     [:creator]
   end

app/models/post.rb

@@ -827,12 +827,12 @@ class Post < ApplicationRecord
 
         when /^-favgroup:(.+)$/i
           favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user)
-          raise User::PrivilegeError unless favgroup.editable_by?(CurrentUser.user)
+          raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], favgroup).update?
           favgroup&.remove!(self)
 
         when /^favgroup:(.+)$/i
           favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user)
-          raise User::PrivilegeError unless favgroup.editable_by?(CurrentUser.user)
+          raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], favgroup).update?
           favgroup&.add!(self)
 
         end

app/policies/favorite_group_policy.rb

@@ -0,0 +1,21 @@
+class FavoriteGroupPolicy < ApplicationPolicy
+  def show?
+    record.creator_id == user.id || record.is_public
+  end
+
+  def create?
+    user.is_member?
+  end
+
+  def update?
+    record.creator_id == user.id
+  end
+
+  def add_post?
+    update?
+  end
+
+  def permitted_attributes
+    [:name, :post_ids_string, :is_public, :post_ids, post_ids: []]
+  end
+end

app/views/favorite_groups/_secondary_links.html.erb

@@ -1,6 +1,6 @@
 <% content_for(:secondary_links) do %>
   <%= subnav_link_to "Listing", favorite_groups_path %>
-  <% if CurrentUser.is_member? %>
+  <% if policy(FavoriteGroup).create? %>
     <%= subnav_link_to "New", new_favorite_group_path %>
   <% end %>
   <%= subnav_link_to "Help", wiki_page_path("help:favorite_groups") %>
@@ -9,7 +9,7 @@
     <li>|</li>
     <%= subnav_link_to "Show", favorite_group_path(@favorite_group) %>
     <%= subnav_link_to "Posts", posts_path(:tags => "favgroup:#{@favorite_group.id}") %>
-    <% if @favorite_group.editable_by?(CurrentUser.user) %>
+    <% if policy(@favorite_group).update? %>
       <%= subnav_link_to "Edit", edit_favorite_group_path(@favorite_group) %>
       <%= subnav_link_to "Delete", favorite_group_path(@favorite_group), :method => :delete, :data => {:confirm => "Are you sure you want to delete this favorite group?"} %>
       <% if @favorite_group.post_count <= 100 %>

app/views/favorite_groups/index.html.erb

@@ -26,7 +26,7 @@
         <%= time_ago_in_words_tagged(favgroup.updated_at) %>
       <% end %>
       <% t.column column: "control" do |favgroup| %>
-        <% if favgroup.editable_by?(CurrentUser.user) %>
+        <% if policy(favgroup).update? %>
           <%= link_to "Edit", edit_favorite_group_path(favgroup) %> |
           <%= link_to "Delete", favorite_group_path(favgroup), method: :delete, remote: true, "data-confirm": "Are you sure you want to delete this favgroup? This cannot be undone." %>
         <% end %>

test/functional/favorite_groups_controller_test.rb

@@ -15,10 +15,22 @@ class FavoriteGroupsControllerTest < ActionDispatch::IntegrationTest
     end
 
     context "show action" do
-      should "render" do
+      should "show public favgroups to anonymous users" do
         get favorite_group_path(@favgroup)
         assert_response :success
       end
+
+      should "show private favgroups to the creator" do
+        @favgroup.update!(is_public: false)
+        get_auth favorite_group_path(@favgroup), @user
+        assert_response :success
+      end
+
+      should "not show private favgroups to other users" do
+        @favgroup = create(:favorite_group, is_public: false)
+        get_auth favorite_group_path(@favgroup), create(:user)
+        assert_response 403
+      end
     end
 
     context "new action" do
@@ -51,25 +63,46 @@ class FavoriteGroupsControllerTest < ActionDispatch::IntegrationTest
         assert_equal("foo", @favgroup.reload.name)
         assert_equal(@posts.map(&:id), @favgroup.post_ids)
       end
+
+      should "not allow users to update favgroups belonging to other users" do
+        put_auth favorite_group_path(@favgroup), create(:user), params: { favorite_group: { name: "foo" } }
+
+        assert_response 403
+        assert_not_equal("foo", @favgroup.reload.name)
+      end
     end
 
     context "destroy action" do
       should "render" do
         delete_auth favorite_group_path(@favgroup), @user
-        assert_redirected_to favorite_groups_path
+        assert_redirected_to favorite_groups_path(search: { creator_name: @user.name })
+      end
+
+      should "not destroy favgroups belonging to other users" do
+        delete_auth favorite_group_path(@favgroup), create(:user)
+        assert_response 403
       end
     end
 
     context "add_post action" do
       should "render" do
-        as_user do
+        @post = create(:post)
-          @post = FactoryBot.create(:post)
-        end
-
         put_auth add_post_favorite_group_path(@favgroup), @user, params: {post_id: @post.id, format: "js"}
         assert_response :success
-        @favgroup.reload
+        assert_equal([@post.id], @favgroup.reload.post_ids)
-        assert_equal([@post.id], @favgroup.post_ids)
+      end
+
+      should "not add posts to favgroups belonging to other users" do
+        @post = create(:post)
+        put_auth add_post_favorite_group_path(@favgroup), create(:user), params: {post_id: @post.id, format: "js"}
+        assert_response 403
+      end
+    end
+
+    context "edit order action" do
+      should "render" do
+        get_auth edit_favorite_group_order_path(@favgroup), @user
+        assert_response :success
       end
     end
   end