Make WordPress Core

Opened 14 years ago

Closed 13 years ago

Last modified 11 years ago

#14365 closed enhancement (fixed)

Admin custom option screen not saved unless user manage_options capability

Reported by: markauk's profile markauk Owned by: westi's profile 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 13 years ago.
Implements a filter and leverages it in Twenty Eleven.
14365.api.diff (4.5 KB) - added by westi 13 years ago.
An alternative way using api additions

Download all attachments as: .zip

Change History (26)

#1 @nacin
14 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.

#2 @nacin
14 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.

#3 @nacin
14 years ago

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

#4 @nacin
14 years ago

  • Type changed from feature request to enhancement

#5 @nacin
14 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.

#6 @jane
13 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.

#7 @ryan
13 years ago

  • Milestone changed from 3.1 to Future Release

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

#8 @nacin
13 years ago

  • Keywords 3.2-early added

#9 @maorb
13 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

#10 @scribu
13 years ago

  • Keywords 3.2-early removed

Seems 3.2 is not be the right time for this.

#11 @maorb
13 years ago

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

#12 @scribu
13 years ago

It's 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.

Last edited 13 years ago by scribu (previous) (diff)

#13 @nacin
13 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.

@nacin
13 years ago

Implements a filter and leverages it in Twenty Eleven.

#14 @nacin
13 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.

#15 @johnjamesjacoby
13 years ago

Tested 14365.diff.

Works as expected.

#16 @nacin
13 years ago

In [17986]:

Add option_page_capability_$option_page filter. see #14365.

#17 @nacin
13 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.

#18 @westi
13 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.

@westi
13 years ago

An alternative way using api additions

#19 @westi
13 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.

#20 @nacin
13 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.

#21 @nacin
13 years ago

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

#22 @ryan
13 years ago

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

#23 @ctsttom
11 years ago

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

#24 @ctsttom
11 years ago

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