From 054ac51d471b192cdc5136b82bb1a5c939bd7332 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 17 Jan 2021 00:41:09 -0600 Subject: [PATCH] policies: remove current request from context. This refactors Pundit policies to only rely on the current user, not on the current user and the current HTTP request. In retrospect, it was a bad idea to include the current request in the Pundit context. It bleeds out everywhere and there are many contexts (in tests and models) where we only have the current user, not the current request. The previous commit got rid of the only two places where we used it. --- app/components/comment_component.rb | 2 +- app/components/forum_post_component.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/logical/post_query_builder.rb | 4 ++-- app/logical/recommender_service.rb | 2 +- app/models/application_record.rb | 4 ++-- app/models/pool.rb | 2 +- app/models/post.rb | 12 ++++++------ app/models/post_event.rb | 2 +- app/models/post_flag.rb | 2 +- app/models/tag.rb | 2 +- app/policies/application_policy.rb | 9 +++++---- app/views/delayed_jobs/index.html.erb | 2 +- 13 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/components/comment_component.rb b/app/components/comment_component.rb index 26b1afa31..537bf654f 100644 --- a/app/components/comment_component.rb +++ b/app/components/comment_component.rb @@ -7,7 +7,7 @@ class CommentComponent < ApplicationComponent def self.with_collection(comments, current_user:, **options) dtext_data = DText.preprocess(comments.map(&:body)) # XXX - #comments = comments.includes(:moderation_reports) if Pundit.policy!([current_user, nil], ModerationReport).show? + #comments = comments.includes(:moderation_reports) if Pundit.policy!(current_user, ModerationReport).show? super(comments, current_user: current_user, dtext_data: dtext_data, **options) end diff --git a/app/components/forum_post_component.rb b/app/components/forum_post_component.rb index 4118b6df6..47d4767b5 100644 --- a/app/components/forum_post_component.rb +++ b/app/components/forum_post_component.rb @@ -11,7 +11,7 @@ class ForumPostComponent < ApplicationComponent original_forum_post_id = forum_topic.original_post&.id forum_posts = forum_posts.includes(:creator, :bulk_update_request) - forum_posts = forum_posts.includes(:moderation_reports) if Pundit.policy!([current_user, nil], ModerationReport).show? + forum_posts = forum_posts.includes(:moderation_reports) if Pundit.policy!(current_user, ModerationReport).show? super(forum_posts, dtext_data: dtext_data, original_forum_post_id: original_forum_post_id, current_user: current_user) end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d64946ffb..27a4b3889 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -182,7 +182,7 @@ class ApplicationController < ActionController::Base end def pundit_user - [CurrentUser.user, request] + CurrentUser.user end def pundit_params_for(record) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 6ef425cf7..7c87a0e37 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -392,7 +392,7 @@ class PostQueryBuilder def favorites_include(username) favuser = User.find_by_name(username) - if favuser.present? && Pundit.policy!([current_user, nil], favuser).can_see_favorites? + if favuser.present? && Pundit.policy!(current_user, favuser).can_see_favorites? favorites = Favorite.from("favorites_#{favuser.id % 100} AS favorites").where(user: favuser) Post.where(id: favorites.select(:post_id)) else @@ -403,7 +403,7 @@ class PostQueryBuilder def ordfav_matches(username) user = User.find_by_name(username) - if user.present? && Pundit.policy!([current_user, nil], user).can_see_favorites? + if user.present? && Pundit.policy!(current_user, user).can_see_favorites? Post.joins(:favorites).merge(Favorite.for_user(user.id)).order("favorites.id DESC") else Post.none diff --git a/app/logical/recommender_service.rb b/app/logical/recommender_service.rb index 315292997..94c28f76f 100644 --- a/app/logical/recommender_service.rb +++ b/app/logical/recommender_service.rb @@ -54,7 +54,7 @@ module RecommenderService end if user.present? - raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], user).can_see_favorites? + raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, user).can_see_favorites? max_recommendations = params.fetch(:max_recommendations, user.favorite_count + 500).to_i.clamp(0, 50000) recs = RecommenderService.recommend_for_user(user, tags: params[:post_tags_match], limit: max_recommendations) elsif post.present? diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 67fab07f0..38c49a44d 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -57,13 +57,13 @@ class ApplicationRecord < ActiveRecord::Base # XXX deprecated, shouldn't expose this as an instance method. def api_attributes(user: CurrentUser.user) - policy = Pundit.policy([user, nil], self) || ApplicationPolicy.new([user, nil], self) + policy = Pundit.policy(user, self) || ApplicationPolicy.new(user, self) policy.api_attributes end # XXX deprecated, shouldn't expose this as an instance method. def html_data_attributes(user: CurrentUser.user) - policy = Pundit.policy([user, nil], self) || ApplicationPolicy.new([user, nil], self) + policy = Pundit.policy(user, self) || ApplicationPolicy.new(user, self) policy.html_data_attributes end diff --git a/app/models/pool.rb b/app/models/pool.rb index 547c8e295..935ad3d1a 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -144,7 +144,7 @@ class Pool < ApplicationRecord end def updater_can_edit_deleted - if is_deleted? && !Pundit.policy!([CurrentUser.user, nil], self).update? + if is_deleted? && !Pundit.policy!(CurrentUser.user, self).update? errors.add(:base, "You cannot update pools that are deleted") end end diff --git a/app/models/post.rb b/app/models/post.rb index 00295b051..53092b64f 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -668,12 +668,12 @@ class Post < ApplicationRecord when /^-favgroup:(.+)$/i favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user) - raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], favgroup).update? + raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, favgroup).update? favgroup&.remove!(self) when /^favgroup:(.+)$/i favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user) - raise User::PrivilegeError unless Pundit.policy!([CurrentUser.user, nil], favgroup).update? + raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, favgroup).update? favgroup&.add!(self) end @@ -779,7 +779,7 @@ class Post < ApplicationRecord def add_favorite!(user) Favorite.add(post: self, user: user) - vote!("up", user) if Pundit.policy!([user, nil], PostVote).create? + vote!("up", user) if Pundit.policy!(user, PostVote).create? rescue PostVote::Error end @@ -789,7 +789,7 @@ class Post < ApplicationRecord def remove_favorite!(user) Favorite.remove(post: self, user: user) - unvote!(user) if Pundit.policy!([user, nil], PostVote).create? + unvote!(user) if Pundit.policy!(user, PostVote).create? rescue PostVote::Error end @@ -803,7 +803,7 @@ class Post < ApplicationRecord # Users who publicly favorited this post, ordered by time of favorite. def visible_favorited_users(viewer) favorited_users.order("favorites.id DESC").select do |fav_user| - Pundit.policy!([viewer, nil], fav_user).can_see_favorites? + Pundit.policy!(viewer, fav_user).can_see_favorites? end end @@ -876,7 +876,7 @@ class Post < ApplicationRecord end def vote!(vote, voter = CurrentUser.user) - unless Pundit.policy!([voter, nil], PostVote).create? + unless Pundit.policy!(voter, PostVote).create? raise PostVote::Error.new("You do not have permission to vote") end diff --git a/app/models/post_event.rb b/app/models/post_event.rb index 7dbda3983..a5114a8d1 100644 --- a/app/models/post_event.rb +++ b/app/models/post_event.rb @@ -56,7 +56,7 @@ class PostEvent true when PostFlag flag = event - Pundit.policy!([user, nil], flag).can_view_flagger? + Pundit.policy!(user, flag).can_view_flagger? end end diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 647d852ed..c5303e0ea 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -31,7 +31,7 @@ class PostFlag < ApplicationRecord def creator_matches(creator, searcher) return none if creator.nil? - policy = Pundit.policy!([searcher, nil], PostFlag.new(creator: creator)) + policy = Pundit.policy!(searcher, PostFlag.new(creator: creator)) if policy.can_view_flagger? where(creator: creator).where.not(post: searcher.posts) diff --git a/app/models/tag.rb b/app/models/tag.rb index 25ab4e528..dee7997ae 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -204,7 +204,7 @@ class Tag < ApplicationRecord # next few lines if the category is changed. tag.update_category_cache - if Pundit.policy!([creator, nil], tag).can_change_category? + if Pundit.policy!(creator, tag).can_change_category? tag.update(category: category_id) end end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index fe035e33e..3449fee00 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -1,8 +1,8 @@ class ApplicationPolicy - attr_reader :user, :request, :record + attr_reader :user, :record - def initialize(context, record) - @user, @request = context + def initialize(user, record) + @user = user @record = record end @@ -43,7 +43,7 @@ class ApplicationPolicy end def policy(object) - Pundit.policy!([user, request], object) + Pundit.policy!(user, object) end def permitted_attributes @@ -68,6 +68,7 @@ class ApplicationPolicy # The list of attributes that are permitted to be returned by the API. def api_attributes + # XXX allow inet record.class.attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym) end diff --git a/app/views/delayed_jobs/index.html.erb b/app/views/delayed_jobs/index.html.erb index 6c5cab416..a6c2ef44d 100644 --- a/app/views/delayed_jobs/index.html.erb +++ b/app/views/delayed_jobs/index.html.erb @@ -24,7 +24,7 @@ <%= time_ago_in_words_tagged(job.run_at) %> <% end %> <% t.column column: "control" do |job| %> - <% if DelayedJobPolicy.new([CurrentUser.user, request], job).update? %> + <% if DelayedJobPolicy.new(CurrentUser.user, job).update? %> <% if job.locked_at? %> Running <% elsif job.failed? %>