FIX: Remove `plugin.enabled?` checks at initialization time (#6166)

Checking `plugin.enabled?` while initializing plugins causes issues in two ways:
  - An application restart is required for changes to take effect. A load-balanced multi-server environment could behave very weirdly if containers restart at different times.
  - In a multisite environment, it takes the `enabled?` setting from the default site. Changes on that site affect all other sites in the cluster.

Instead, `plugin.enabled?` should be checked at runtime, in the context of a request. This commit removes `plugin.enabled?` from many `instance.rb` methods.

I have added a working `plugin.enabled?` implementation for methods that actually affect security/functionality:
  - `post_custom_fields_whitelist`
  - `whitelist_staff_user_custom_field`
  - `add_permitted_post_create_param`
This commit is contained in:
David Taylor 2018-07-25 16:44:09 +01:00 committed by GitHub
parent f38942d121
commit 0d0d78841b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 18 deletions

View File

@ -615,8 +615,8 @@ class PostsController < ApplicationController
:visible
]
if Post.permitted_create_params.present?
permitted.concat(Post.permitted_create_params.to_a)
Post.plugin_permitted_create_params.each do |key, plugin|
permitted << key if plugin.enabled?
end
# param munging for WordPress

View File

@ -19,8 +19,8 @@ class Post < ActiveRecord::Base
include HasCustomFields
include LimitedEdit
cattr_accessor :permitted_create_params
self.permitted_create_params = Set.new
cattr_accessor :plugin_permitted_create_params
self.plugin_permitted_create_params = {}
# increase this number to force a system wide post rebake
# Version 1, was the initial version

View File

@ -92,26 +92,26 @@ class Plugin::Instance
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def add_report(name, &block)
reloadable_patch do |plugin|
if plugin.enabled?
Report.add_report(name, &block)
end
Report.add_report(name, &block)
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def replace_flags
settings = ::FlagSettings.new
yield settings
reloadable_patch do |plugin|
::PostActionType.replace_flag_settings(settings) if plugin.enabled?
::PostActionType.replace_flag_settings(settings)
end
end
def whitelist_staff_user_custom_field(field)
reloadable_patch do |plugin|
::User.register_plugin_staff_custom_field(field, plugin) if plugin.enabled?
::User.register_plugin_staff_custom_field(field, plugin) # plugin.enabled? is checked at runtime
end
end
@ -122,9 +122,10 @@ class Plugin::Instance
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def add_body_class(class_name)
reloadable_patch do |plugin|
::ApplicationHelper.extra_body_classes << class_name if plugin.enabled?
::ApplicationHelper.extra_body_classes << class_name
end
end
@ -180,27 +181,34 @@ class Plugin::Instance
end
end
# Add a post_custom_fields_whitelister block to the TopicView, respecting if the plugin is enabled
def topic_view_post_custom_fields_whitelister(&block)
reloadable_patch do |plugin|
::TopicView.add_post_custom_fields_whitelister(&block) if plugin.enabled?
::TopicView.add_post_custom_fields_whitelister do |user|
return [] unless plugin.enabled?
block.call(user)
end
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def add_preloaded_group_custom_field(field)
reloadable_patch do |plugin|
::Group.preloaded_custom_field_names << field if plugin.enabled?
::Group.preloaded_custom_field_names << field
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def add_preloaded_topic_list_custom_field(field)
reloadable_patch do |plugin|
::TopicList.preloaded_custom_fields << field if plugin.enabled?
::TopicList.preloaded_custom_fields << field
end
end
# Add a permitted_create_param to Post, respecting if the plugin is enabled
def add_permitted_post_create_param(name)
reloadable_patch do |plugin|
::Post.permitted_create_params << name if plugin.enabled?
::Post.plugin_permitted_create_params[name] = plugin
end
end
@ -286,27 +294,31 @@ class Plugin::Instance
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def register_category_custom_field_type(name, type)
reloadable_patch do |plugin|
Category.register_custom_field_type(name, type) if plugin.enabled?
Category.register_custom_field_type(name, type)
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def register_topic_custom_field_type(name, type)
reloadable_patch do |plugin|
::Topic.register_custom_field_type(name, type) if plugin.enabled?
::Topic.register_custom_field_type(name, type)
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def register_post_custom_field_type(name, type)
reloadable_patch do |plugin|
::Post.register_custom_field_type(name, type) if plugin.enabled?
::Post.register_custom_field_type(name, type)
end
end
# Applies to all sites in a multisite environment. Ignores plugin.enabled?
def register_group_custom_field_type(name, type)
reloadable_patch do |plugin|
::Group.register_custom_field_type(name, type) if plugin.enabled?
::Group.register_custom_field_type(name, type)
end
end