Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#38705 closed defect (bug) (fixed)

Store user ID with setting change written into changeset and restore user when setting is saved

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

In researching how best to accommodate Jetpack (#38672) being able to allow users who cannot unfiltered_css to be able to edit Custom CSS by adding additional sanitizing filters, I realized a deficiency in changesets (#30937). A supderadmin user user may write a change into a changeset, such as post_content, and this may include unsafe HTML as intended by the superadmin user. If non-super admin user comes along and makes a change to a different setting and updates the changeset, then if that non-super admin publishes the changes, the kses filters will unexpectedly strip out the unsafe markup content that the superadmin had written in their changeset update. This likewise will happen when a superadmin schedules their changeset for publishing later: when WP Cron runs there is no current user, and so the kses filters would apply then as well.

What is needed is for the user who modified a setting to have their user_id associated with the setting in the changeset. Then when the changeset is published with the settings being saved/persisted to the DB, then the associated user should be logged-in temporarily so that the setting will save with the expected user context. For normal user updates to changesets, this means that settings would no longer need to have their associated capability temporarily overridden to be exist.

Attachments (1)

38705.0.diff (20.2 KB) - added by westonruter 8 years ago.
https://github.com/xwp/wordpress-develop/pull/196

Download all attachments as: .zip

Change History (6)

#1 @westonruter
8 years ago

  • Keywords has-patch added

#2 @westonruter
8 years ago

  • Keywords has-unit-tests added
  • Owner set to westonruter
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core by helen. View the logs.


8 years ago

#4 @westonruter
8 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 39181:

Customize: Store modifying user ID with setting change written into changeset and restore current user when setting is being saved.

Restoring the current user context when saving a setting ensures filters apply as expected, such as Kses. When a user is not associated with a given setting change, continue to override capability to be exist when saving. Skip overwriting setting values in a changeset that have not changed, facilitating concurrent editing and amending a changeset by a user with fewer privileges.

See #30937.
Fixes #38705.

#5 @westonruter
8 years ago

In 39320:

Customize: Ensure that WP_Customize_Manager::save_changeset_post() returns setting_validities even for supplied values that are unchanged from values in changeset.

Check setting existence and authorization via WP_Customize_Manager::validate_setting_values() even for null values to account for custom params being added to settings, preventing failures from being silently ignored.

See #38705, #30937.
Fixes #38865.

Note: See TracTickets for help on using tickets.