From 2c4c29b81a4d93f5c7be3423f3684bc861904ca3 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 19 Mar 2020 18:12:16 -0500 Subject: [PATCH] pundit: convert favorite groups to pundit. --- .../favorite_group_orders_controller.rb | 3 +- app/controllers/favorite_groups_controller.rb | 39 ++++----------- app/logical/post_query_builder.rb | 4 +- app/models/favorite_group.rb | 8 --- app/models/post.rb | 4 +- app/policies/favorite_group_policy.rb | 21 ++++++++ .../favorite_groups/_secondary_links.html.erb | 4 +- app/views/favorite_groups/index.html.erb | 2 +- .../favorite_groups_controller_test.rb | 49 ++++++++++++++++--- 9 files changed, 80 insertions(+), 54 deletions(-) create mode 100644 app/policies/favorite_group_policy.rb diff --git a/app/controllers/favorite_group_orders_controller.rb b/app/controllers/favorite_group_orders_controller.rb index 7e24f7e04..b8fe161dd 100644 --- a/app/controllers/favorite_group_orders_controller.rb +++ b/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 diff --git a/app/controllers/favorite_groups_controller.rb b/app/controllers/favorite_groups_controller.rb index ed3f98945..c3abe691f 100644 --- a/app/controllers/favorite_groups_controller.rb +++ b/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]) - check_read_privilege(@favorite_group) + @favorite_group = authorize FavoriteGroup.find(params[:id]) @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]) - check_write_privilege(@favorite_group) + @favorite_group = authorize FavoriteGroup.find(params[:id]) respond_with(@favorite_group) end def update - @favorite_group = FavoriteGroup.find(params[:id]) - check_write_privilege(@favorite_group) - @favorite_group.update(favgroup_params) + @favorite_group = authorize FavoriteGroup.find(params[:id]) + @favorite_group.update(permitted_attributes(@favorite_group)) 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]) - check_write_privilege(@favorite_group) + @favorite_group = authorize FavoriteGroup.find(params[:id]) @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]) - check_write_privilege(@favorite_group) + @favorite_group = authorize FavoriteGroup.find(params[:id]) @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 diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 2ed46a7c2..14af3523f 100644 --- a/app/logical/post_query_builder.rb +++ b/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 diff --git a/app/models/favorite_group.rb b/app/models/favorite_group.rb index 77accc492..1ccc7a4d8 100644 --- a/app/models/favorite_group.rb +++ b/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 diff --git a/app/models/post.rb b/app/models/post.rb index a0f0071d8..22b981935 100644 --- a/app/models/post.rb +++ b/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 diff --git a/app/policies/favorite_group_policy.rb b/app/policies/favorite_group_policy.rb new file mode 100644 index 000000000..784f8b33a --- /dev/null +++ b/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 diff --git a/app/views/favorite_groups/_secondary_links.html.erb b/app/views/favorite_groups/_secondary_links.html.erb index f7c167189..ff955e704 100644 --- a/app/views/favorite_groups/_secondary_links.html.erb +++ b/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 @@
  • |
    • <%= 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 %> diff --git a/app/views/favorite_groups/index.html.erb b/app/views/favorite_groups/index.html.erb index 69f127d98..c19d3ee2d 100644 --- a/app/views/favorite_groups/index.html.erb +++ b/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 %> diff --git a/test/functional/favorite_groups_controller_test.rb b/test/functional/favorite_groups_controller_test.rb index 284b49f47..1b853dd0e 100644 --- a/test/functional/favorite_groups_controller_test.rb +++ b/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 = FactoryBot.create(:post) - end - + @post = create(:post) 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.post_ids) + assert_equal([@post.id], @favgroup.reload.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