Page MenuHomePhabricator

Edit protection should still allow users to move the page
Open, MediumPublic

Description

Try to edit protect a page indef and move protect for 3 hours. The result will be indef edit/move protect.


See Also:
T37238: The createpage right should allow creating pages even without the edit right

Details

Reference
bz38365

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:09 AM
bzimport set Reference to bz38365.
bzimport added a subscriber: Unknown Object (MLST).

Bug found while trying to protect https://en.wikipedia.org/wiki/Friday_Night_Dinner from New Users/IPs (level 2 protection)

"Setting a different move protection expiry time from the edit protection results in the move protection taking the value of the edit protection"

What is "indef" here? Could you provide steps to reproduce (click by click) please?

Protect -> In Edit: Choose "Block new and unregistered users" (leave Expires: Indefinite untouched) -> Enable "Unlock further protect options" -> In Move: Choose "Block all non-admin users" and Expires: 3 hours -> Choose a reason and save.

The result will be

  • Edit "Block all non-admin users" - Expires: Indefnite
  • Move "Block all non-admin users" - Expires: 3 hours

Note the difference in the level of protection for editing.

I've determined this is actually unrelated to expiration issues. The root cause of this is that a page with edit protection blocks moving of a page, even if move protection is not enabled.

Change 105103 had a related patch set uploaded by Jackmcbarn:
Allow moving pages with edit-protection

https://gerrit.wikimedia.org/r/105103

(In reply to comment #5)

I've determined this is actually unrelated to expiration issues. The root
cause of this is that a page with edit protection blocks moving of a page,
even if move protection is not enabled.

I've rephrased this bug to be more explicit in its request.

I am not a sure whether this change is wanted, it might be intentional that edit protection also prevents moving. I'm not sure, but either way this doesn't look like a mistake, this would be a proper change away from a past decision.

I don't know what the rationale for it was, but considering moving a page is generally considered more harmful than simply editing it. Firtstly, it causes a history entry, changes its URL adn watchlist entry. Secondly, moving can be worked around by copying and pasting the page contents and turning it into a redirect manually.

From Code Review:

(Legoktm Jan 3 10:20)

I always viewed the 'move' permission as an extension of the 'edit' permission and saw this as a feature instead of a bug.

(Jackmcbarn Jan 3 16:53)

If you didn't want a page to be moved, you'd just move-protect it along with it (which happens by default). If we don't want this to work, then the software shouldn't allow move-protection to be set to a lower level than edit-protection at all.

There's two issues with this:

  1. All existing protections.
  1. What would be an example use case where one needs to prevent editing by certain users, but are totally fine with them renaming the page? It seems like a more impactful action, not less.
  1. All existing protections already have move-protection set alongside edit-protection, unless the protecting admin went out of their way to make this not the case.
  1. I'll admit don't see one. Right now, our setup is misleading, since it gives the impression that it's possible that such protection can be configured, when in fact it cannot be. If we don't want it to be possible, we should change the protection interface to force the move-protection level to always be at least the edit-protection level.

It certainly looks like checking both the move and edit protections for a page move is intentional: this has been part of the code ever since move protection was correctly added in r6827.

I think the UI fix suggested in comment 9 would be the way to go. Although the definition of "at least" may be problematic, since protection levels aren't necessarily hierarchical.

(In reply to Brad Jorsch from comment #10)

It certainly looks like checking both the move and edit protections for a
page move is intentional: this has been part of the code ever since move
protection was correctly added in r6827.

I think the UI fix suggested in comment 9 would be the way to go. Although
the definition of "at least" may be problematic, since protection levels
aren't necessarily hierarchical.

Indeed, they are not hierarchical.

However, as in many places, changing a page requires the ability to edit plus any additional rights. Though this principle could use much better documentation, I don't think it is a bug.

We use this in quite a few places. This, for example, allows user groups to be configured with relative ease. There is no need to go out of one's way and figure out all the possible actions a user can take (not to mention actions provided by extensions, it simply doesn't scale). Instead we provide lower-level actions like "edit" and "read" that are intended to cascade through to more impactful actions. Meaning if a page is protected from editing and your user group is not allowed to edit, the fact that your group has the 'move' right in general doesn't matter.