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.
This commit is contained in:
evazion
2021-01-17 00:41:09 -06:00
parent 6671711784
commit 054ac51d47
13 changed files with 24 additions and 23 deletions

View File

@@ -7,7 +7,7 @@ class CommentComponent < ApplicationComponent
def self.with_collection(comments, current_user:, **options) def self.with_collection(comments, current_user:, **options)
dtext_data = DText.preprocess(comments.map(&:body)) dtext_data = DText.preprocess(comments.map(&:body))
# XXX # 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) super(comments, current_user: current_user, dtext_data: dtext_data, **options)
end end

View File

@@ -11,7 +11,7 @@ class ForumPostComponent < ApplicationComponent
original_forum_post_id = forum_topic.original_post&.id original_forum_post_id = forum_topic.original_post&.id
forum_posts = forum_posts.includes(:creator, :bulk_update_request) 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) super(forum_posts, dtext_data: dtext_data, original_forum_post_id: original_forum_post_id, current_user: current_user)
end end

View File

@@ -182,7 +182,7 @@ class ApplicationController < ActionController::Base
end end
def pundit_user def pundit_user
[CurrentUser.user, request] CurrentUser.user
end end
def pundit_params_for(record) def pundit_params_for(record)

View File

@@ -392,7 +392,7 @@ class PostQueryBuilder
def favorites_include(username) def favorites_include(username)
favuser = User.find_by_name(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) favorites = Favorite.from("favorites_#{favuser.id % 100} AS favorites").where(user: favuser)
Post.where(id: favorites.select(:post_id)) Post.where(id: favorites.select(:post_id))
else else
@@ -403,7 +403,7 @@ class PostQueryBuilder
def ordfav_matches(username) def ordfav_matches(username)
user = User.find_by_name(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") Post.joins(:favorites).merge(Favorite.for_user(user.id)).order("favorites.id DESC")
else else
Post.none Post.none

View File

@@ -54,7 +54,7 @@ module RecommenderService
end end
if user.present? 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) 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) recs = RecommenderService.recommend_for_user(user, tags: params[:post_tags_match], limit: max_recommendations)
elsif post.present? elsif post.present?

View File

@@ -57,13 +57,13 @@ class ApplicationRecord < ActiveRecord::Base
# XXX deprecated, shouldn't expose this as an instance method. # XXX deprecated, shouldn't expose this as an instance method.
def api_attributes(user: CurrentUser.user) 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 policy.api_attributes
end end
# XXX deprecated, shouldn't expose this as an instance method. # XXX deprecated, shouldn't expose this as an instance method.
def html_data_attributes(user: CurrentUser.user) 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 policy.html_data_attributes
end end

View File

@@ -144,7 +144,7 @@ class Pool < ApplicationRecord
end end
def updater_can_edit_deleted 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") errors.add(:base, "You cannot update pools that are deleted")
end end
end end

View File

@@ -668,12 +668,12 @@ class Post < ApplicationRecord
when /^-favgroup:(.+)$/i when /^-favgroup:(.+)$/i
favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user) 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) favgroup&.remove!(self)
when /^favgroup:(.+)$/i when /^favgroup:(.+)$/i
favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user) 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) favgroup&.add!(self)
end end
@@ -779,7 +779,7 @@ class Post < ApplicationRecord
def add_favorite!(user) def add_favorite!(user)
Favorite.add(post: self, user: 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 rescue PostVote::Error
end end
@@ -789,7 +789,7 @@ class Post < ApplicationRecord
def remove_favorite!(user) def remove_favorite!(user)
Favorite.remove(post: self, user: 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 rescue PostVote::Error
end end
@@ -803,7 +803,7 @@ class Post < ApplicationRecord
# Users who publicly favorited this post, ordered by time of favorite. # Users who publicly favorited this post, ordered by time of favorite.
def visible_favorited_users(viewer) def visible_favorited_users(viewer)
favorited_users.order("favorites.id DESC").select do |fav_user| 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
end end
@@ -876,7 +876,7 @@ class Post < ApplicationRecord
end end
def vote!(vote, voter = CurrentUser.user) 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") raise PostVote::Error.new("You do not have permission to vote")
end end

View File

@@ -56,7 +56,7 @@ class PostEvent
true true
when PostFlag when PostFlag
flag = event flag = event
Pundit.policy!([user, nil], flag).can_view_flagger? Pundit.policy!(user, flag).can_view_flagger?
end end
end end

View File

@@ -31,7 +31,7 @@ class PostFlag < ApplicationRecord
def creator_matches(creator, searcher) def creator_matches(creator, searcher)
return none if creator.nil? 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? if policy.can_view_flagger?
where(creator: creator).where.not(post: searcher.posts) where(creator: creator).where.not(post: searcher.posts)

View File

@@ -204,7 +204,7 @@ class Tag < ApplicationRecord
# next few lines if the category is changed. # next few lines if the category is changed.
tag.update_category_cache 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) tag.update(category: category_id)
end end
end end

View File

@@ -1,8 +1,8 @@
class ApplicationPolicy class ApplicationPolicy
attr_reader :user, :request, :record attr_reader :user, :record
def initialize(context, record) def initialize(user, record)
@user, @request = context @user = user
@record = record @record = record
end end
@@ -43,7 +43,7 @@ class ApplicationPolicy
end end
def policy(object) def policy(object)
Pundit.policy!([user, request], object) Pundit.policy!(user, object)
end end
def permitted_attributes def permitted_attributes
@@ -68,6 +68,7 @@ class ApplicationPolicy
# The list of attributes that are permitted to be returned by the API. # The list of attributes that are permitted to be returned by the API.
def api_attributes def api_attributes
# XXX allow inet
record.class.attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym) record.class.attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym)
end end

View File

@@ -24,7 +24,7 @@
<%= time_ago_in_words_tagged(job.run_at) %> <%= time_ago_in_words_tagged(job.run_at) %>
<% end %> <% end %>
<% t.column column: "control" do |job| %> <% 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? %> <% if job.locked_at? %>
Running Running
<% elsif job.failed? %> <% elsif job.failed? %>