WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#20201 closed defect (bug) (fixed)

wp.Options lies about the updatability of options.

Reported by: westi Owned by: westi
Milestone: 3.6 Priority: low
Severity: normal Version: 3.4
Component: XML-RPC Keywords: westi-likes has-patch dev-feedback commit
Focuses: Cc:

Description

When an XMLRPC client calls this API we give it a readonly status for each of the options returned which lets it know whether or not the options are writable or not.

This status is hardcoded in the code and not capability aware so a user with only subscriber level access is given the impression that they can update options they can't.

Attachments (1)

wp.getOptions.capfix.diff (483 bytes) - added by westi 9 years ago.
The simple fix for this bug.

Download all attachments as: .zip

Change History (11)

#1 @westi
9 years ago

A tangentially related issue was raised in #16986.

@westi
9 years ago

The simple fix for this bug.

#2 @westi
9 years ago

The test cases for this are in [UT567]

#3 @westi
9 years ago

  • Keywords 3.5-early westi-likes added

#4 follow-up: @nacin
9 years ago

I think this makes sense, but it also makes me wonder what the definition of 'readonly' is. There are two ways to look at it: A) the option is generally writable, B) the current user can write to a writable option.

Currently, we're dealing with method A. This patch will change it to method B.

I doubt many applications are using wp.getOptions to determine generally writable fields, but I could understand how the UI could be implemented differently, based on the context. Say, text for a read-only option, versus a disabled field if they don't have the ability to edit that option.

#5 in reply to: ↑ 4 @westi
9 years ago

Replying to nacin:

I think this makes sense, but it also makes me wonder what the definition of 'readonly' is. There are two ways to look at it: A) the option is generally writable, B) the current user can write to a writable option.

Currently, we're dealing with method A. This patch will change it to method B.

I doubt many applications are using wp.getOptions to determine generally writable fields, but I could understand how the UI could be implemented differently, based on the context. Say, text for a read-only option, versus a disabled field if they don't have the ability to edit that option.

Agreed, I think method A is wrong here and method B is right.

As an application developer I would want to be able to write an intelligent UI which used the read-only status to determine how my ui presented options to the user.

Switching to B allows us to also change our view on whether or not things are read-only in future more simply too because the API gives a application a true view of the current read/write status of the options.

#6 @markoheijnen
8 years ago

  • Keywords 3.5-early removed
  • Milestone changed from Future Release to 3.6

I would love to see this in 3.6 because I also agree that method B is right

#7 @markjaquith
8 years ago

  • Keywords has-patch dev-feedback added

Patch still applies, and I think it's the right fix.

#8 @markoheijnen
8 years ago

Can we commit this?

#9 @SergeyBiryukov
8 years ago

  • Keywords commit added

#10 @nacin
8 years ago

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

In 24597:

XML-RPC: For wp.getOptions, set readonly to true for writable options that the user does not have permission to edit.

props westi.
fixes #20201.

Note: See TracTickets for help on using tickets.