WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#14365 closed enhancement (fixed)

Admin custom option screen not saved unless user manage_options capability

Reported by: markauk Owned by: westi
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.0
Component: Administration Keywords: has-patch close
Focuses: Cc:

Description

I have a custom options screen. The menu and sub-menu pages for that screen are set to show only if a user has a custom capability ('be_super_editor') in this case.

The options screen shows or not as expected if a user has or does not have the 'be_super_editor' capability.

However, the options cannot be updated unless the user has 'manage_options' capability as well. This seems to be wrong for two reasons:-

(1) if a capability allows an options screen to be accessed, it is reasonable to assume that the user should be able to make changes to that screen.

(2) giving these users 'manage_options' capability is not a good idea as that allows them to do other things that they shouldn't be able to do (e.g. access wp-admin/options.php directly).

Changing line 30 of wp-admin/options.php from:-

if ( !current_user_can('manage_options') )

to:-

if ( !current_user_can('manage_options') && 'update' != $action )

fixes the problem, though I don't know enough about the inner workings of WP security to say if this creates any further security/permissions issues.

Attachments (2)

14365.diff (2.1 KB) - added by nacin 4 years ago.
Implements a filter and leverages it in Twenty Eleven.
14365.api.diff (4.5 KB) - added by westi 4 years ago.
An alternative way using api additions

Download all attachments as: .zip

Change History (26)

comment:1 @nacin5 years ago

That would definitely create a hole, but you bring up an interesting point. I'd think we'd want to want the settings API leveraged where possible, including say theme options pages that may only be governed by edit_themes or edit_theme_options for cap checks.

comment:2 @nacin5 years ago

  • Milestone changed from Awaiting Review to 3.1
  • Type changed from defect (bug) to feature request

Someone is welcome to run with a patch for this, otherwise I'll circle back.

comment:3 @nacin5 years ago

I was thinking that capabilities should be assigned to the corresponding settings page, with a default of manage_options, not individual settings.

comment:4 @nacin5 years ago

  • Type changed from feature request to enhancement

comment:5 @nacin5 years ago

set_settings_group_capabilty() was the function I had tried to implement.

The problem is that options.php needs quite a bit of refactoring, because we can't make the manage_options check too early now as it depends on the requested page, while at the same time, so much goes through options.php that we still need to check for that in quite a few places.

comment:6 @jane5 years ago

  • Keywords needs-patch added

If anyone wants to get this in for 3.1, a patch needs to be posted now, as freeze is coming up very soon.

comment:7 @ryan5 years ago

  • Milestone changed from 3.1 to Future Release

We'll be cleaning up the options API in 3.2.

comment:8 @nacin5 years ago

  • Keywords 3.2-early added

comment:9 @maorb4 years ago

  • Cc maor@… added

++ for this. This is a very needed enhancement.
It will be great if this would be implemented in 3.2.
Thanks

comment:10 @scribu4 years ago

  • Keywords 3.2-early removed

Seems 3.2 is not be the right time for this.

comment:11 @maorb4 years ago

:(
Why not? Is it too complicated to achieve for the 3.2 timeline?

comment:12 @scribu4 years ago

It because we're focusing on something else for this release:

http://wpdevel.wordpress.com/2011/03/18/wordpress-3-2-the-plan-faster-lighter/

Also, as ryan hinted at, a larger cleanup is required.

Version 0, edited 4 years ago by scribu (next)

comment:13 @nacin4 years ago

A few of us have expressed interest in this ticket (for 3.2), but I don't think we know how we want to implement it.

@nacin4 years ago

Implements a filter and leverages it in Twenty Eleven.

comment:14 @nacin4 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 3.2

Attached patch implements a filter called "option_page_capability_{$option_page}", which can be used to tweak the capability required when using the settings API. The filter is untouched if you're not using the whitelist.

It's not pretty, but this should suffice until we rework the settings API, and would not be difficult to maintain backwards compatibility.

comment:15 @johnjamesjacoby4 years ago

Tested 14365.diff.

Works as expected.

comment:16 @nacin4 years ago

In [17986]:

Add option_page_capability_$option_page filter. see #14365.

comment:17 @nacin4 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [17987]:

Leverage option_page_capability_* filter in Twenty Eleven. fixes #14365.

comment:18 @westi4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I don't think we should implement this as a filter.

Adding some forward compatible api changes to support this should be relatively easy.

Alternative patch incoming.

@westi4 years ago

An alternative way using api additions

comment:19 @westi4 years ago

Another viable alternative is to allow for the registration of an alternative capability for an options group via an api call - that is effectively all the filter or my patch do.

comment:20 @nacin4 years ago

  • Keywords close added

Suggest closed as fixed. Waiting for westi on the call.

It is seriously lame, but maybe it'll encourage us to do a better settings API overall in 3.3 or 3.4.

comment:21 @nacin4 years ago

  • Owner changed from nacin to westi
  • Status changed from reopened to reviewing

comment:22 @ryan4 years ago

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

comment:23 @ctsttom3 years ago

  • Cc me@… added
  • Keywords settings-3.6 added

comment:24 @ctsttom3 years ago

  • Cc me@… removed
  • Keywords settings-3.6 removed
Note: See TracTickets for help on using tickets.