Wikimedia Bugzilla is closed!

Global groups are created by stewards, so i think your request should be on meta, not here.

(In reply to comment #1)
> Global groups are created by stewards, so i think your request should be on
> meta, not here.
> 

Yes, but they are not able to limit it per-namespace (that's another disadvantage of Werdna's system :()

Sorry, there appears to be some confusion as to where we should go next with this. Should a request be made to Stewards that a global usergroup be made for Commons admins? Are we waiting on anything on the tech side before this request can be made?

Thanks for your time.

[[User:Giggy]]

Reassigning to Gmaxwell who has The Patch that would allow per namespace settings.

Please implement this only *after* image oversighting. (And let the wikis some months time afterwards to oversight images that need to be.) Commons sysops are not globally elected people or trusted on any wiki or even known.

Th.

(In reply to comment #5)
> Please implement this only *after* image oversighting. (And let the wikis some
> months time afterwards to oversight images that need to be.) Commons sysops are
> not globally elected people or trusted on any wiki or even known.
> 
> Th.
> 

See the Meta poll for how this line of argument was dealt with.  As far as the users in the Meta discussion were aware, this feature would be implemented as soon as the poll was finished.  Please don't try to overturn the discussion at Meta by appealing to the devs.  

(In reply to comment #6)
> (In reply to comment #5)
> > Please implement this only *after* image oversighting. (And let the wikis some
> > months time afterwards to oversight images that need to be.) Commons sysops are
> > not globally elected people or trusted on any wiki or even known.
> > 
> > Th.
> > 
> 
> See the Meta poll for how this line of argument was dealt with.  As far as the
> users in the Meta discussion were aware, this feature would be implemented as
> soon as the poll was finished.  Please don't try to overturn the discussion at
> Meta by appealing to the devs.  
> 

Please explain where in the policy this is dealt with, I don't find anything about image oversighting there. Don't expect the devs to read all the megabytes of discussion to find that. Thanks.

Folks, this is for discussion of technical implementation - objections to the proposal should go on-wiki. Consensus to implement this has already been established; we await only implementation, which requires developer intervention to create a view-deleted-images-only right so Steward may assign it to Commons sysops. Unless your comment furthers that, it should go on-wiki.

(In reply to comment #8)
> Folks, this is for discussion of technical implementation - objections to the
> proposal should go on-wiki. Consensus to implement this has already been
> established; we await only implementation, which requires developer
> intervention to create a view-deleted-images-only right so Steward may assign
> it to Commons sysops. Unless your comment furthers that, it should go on-wiki.
> 

As per some of the above, Gmaxwell should be implementing it (soon).

Best system would be a deletion queue, such that things that don't *need* to be hidden from the whole universe forever don't have to be.

(In reply to comment #10)
> Best system would be a deletion queue, such that things that don't *need* to be
> hidden from the whole universe forever don't have to be.
> 

I may be misunderstanding you, but this wouldn't solve our problem. The problem is that Commons admins sometimes need to see deleted image-namespace content on other wikis. The point of deleting images is that they aren't viewable by those who shouldn't be viewing them - up to now, that has included sysops on the relevant wiki; we want to include Commons sysops in that group of "people who should be able to see deleted images."

I see the 'reviewed' keyword - is that accurate? If the patch has been reviewed, can't it be committed then?

Right, replaced this one by 'patch' as it has not yet been reviewed (I don't even know if the patch is ready). Something like 'community approved' would be more precise.
Anyway... Go for it!

Where's that patch?

(In reply to comment #14)
> Where's that patch?
> 

I guess it's trivial to "just do it", but Brion wants it done /properly/ so we wait. Gmaxwell has promised to do it though.

Is there a progress update?

Created attachment 5927 [details]
Basic patch by ChrisiPK

I've been playing around with the code and this is what I came up with. I'm not saying it's perfect, but it works fine for me. Maybe this will give people a few ideas or maybe someone can make this better/safe/whatever.

What it does: It adds the user rights viewdeletedarticle, viewdeleteduser, viewdeletedproject, viewdeletedfile, viewdeletedsysmsg, viewdeletedtemplate, viewdeletedhelp and viewdeletedcategory, which enable you to give out per-namespace rights to view deleted content.
What it does not (yet) do: It does not give you a restore link when you are allowed to view something based on these rights. It does not give you a search box when accessing Special:Undelete without a target parameter. To work it, you must access Special:Undelete?target=Page_name_here.

Regards,

ChrisiPK

I don't like that it created one permission per namespace, which then lead to those giantic switches.
What about $allowed = !$wgUser->isBlocked() && $wgUser->isAllowed( "viewdeletedns" . MWNamespace::getSubject( $target->getNamespace() ) . "andtalk" ); ?

Removed shell keyword as there's nothing to do on shell ATM - we're still waiting on Greg's patch.

Any update so far? 

(In reply to comment #18)
> I don't like that it created one permission per namespace, which then lead to
> those giantic switches.

Yes, I can't possibly see something like that getting applied to trunk.

> What about $allowed = !$wgUser->isBlocked() && $wgUser->isAllowed(
> "viewdeletedns" . MWNamespace::getSubject( $target->getNamespace() ) .
> "andtalk" ); ?

You're still having to add all those rights, even if you're not switch()ing over them.

What do you mean by 'adding all those rights'?

I just simplified ChrisiPK's switch with a one-liner, 
summarising his 8 rights with a templated permission 
viewdeletedns{$number}andtalk

It might be preferable to not join subject and talk view 
deleted permissions.

Any update on this one? Isn't this just the "browsearchive" right limited to ns 6 and maybe 7?

I think it will take some interface changes as well since access to Special:Undelete is needed to view anything, but that won't work if you don't have the rights to restore. At the very least the "Restore revisions" box (where you can add a comment when restoring) along with the checkboxes next to each revision needs to be hidden. The title of the page "View and restore deleted pages" would also have to change.

And what about "deletedhistory"? (Don't really know the differences between them or why they're separate.)

It seems to be already taken into account. Look at UndeleteForm::$mAllowed.
If you don't have the undelete right, you can view the deleted page but the 
restore options aren't shown.

Created attachment 6569 [details]
Patch for Special:Undelete

New patch for MediaWiki 1.15.1, now with the right "view-deleted-ns-<namespace number goes here>". This makes the switches from the old patch obsolete and allows users to add the right for non-default namespaces.

Please note: This is still only a patch for Special:Undelete. I still haven't found out where the view/undel links are added; pointers are appreciated.

1. The patch is reversed.
2. Use unified diff when possible.
3. By removing 'deletedhistory', and the part of "!$wgUser->isAllowed( 'deletedhistory' )" you're allowing everyone to see the deleted history.
4. Why are you removing the 1143,1155d1126 and 1195,1200d1165 chunks?

Created attachment 6576 [details]
Patch (unreversed, with unified diff) for Special:Undelete

(In reply to comment #27)
> 1. The patch is reversed.
> 2. Use unified diff when possible.
> 3. By removing 'deletedhistory', and the part of "!$wgUser->isAllowed(
> 'deletedhistory' )" you're allowing everyone to see the deleted history.
> 4. Why are you removing the 1143,1155d1126 and 1195,1200d1165 chunks?
> 

1. Indeed it is, sorry about that. I still haven't completely mastered the diff tool.
2. I think I found the relevant option, the new patch should be "unified".
3. As you already noted, the patch is reversed. I removed the need for the deletedhistory permission in SpecialPage.php as this prevents users from viewing the page unless they have this right (which not everyone with view-deleted-ns-# might). That is why I added a check in SpecialUndelete.php, see the new patch at @@ -625,6 +637,10 @@.
4. Once again, the patch is reversed, so I am not removing them, but adding them. They take care of the newly introduced variable mAllowedView, which determines whether you are allowed to view the revision (unlike mAllowed, which determines whether you are allowed to restore it).

Best Regards and sorry for the double patch.

Couple quick notes...

The patch doesn't apply cleanly against development trunk; lots of stuff has changed internally since 1.15, so this'll be a lot easier to get merged if it's rebuilt against trunk instead of a release branch.

I also see some code duplication, copying portions of the UI output between the can-restore and can't-restore cases. This is easy to do the first time around, but makes code maintenance harder -- the copies can get out of sync easily, and generally just clutter the codebase. I would recommend merging the two cases together; just don't output the form fields when they won't be needed.

Created attachment 6579 [details]
Patch for Special:Undelete built against trunk

New patch, built against current trunk. This is considerably smaller because the trunk already has a variable mCanView, which basically does what I was doing with mAllowedView.

Also note that this still does not provide the "undel/view" links for deleted revisions as I still have not found out where those are created.

Regards!

Created attachment 6580 [details]
Patch for Special:Undelete built against trunk (fixed)

Yet another patch (sorry for spamming you). This fixes a null pointer exception when accessing Special:Undelete without the undelete right and enables viewing of files. Note: This also enables viewing of files when user has the deletedcontent right - I assume this was intended anyway, right?

Ahm... it's been a while.

I often find myself in situations to wait hours or days for an admin at a local wiki.

What's the progress on this ?

I would prefer this to have this dependant of a Title->userCan(), and having a way to set per namespace $wgGroupPermissions in a sane way.

(In reply to comment #33)
> I would prefer this to have this dependant of a Title->userCan(), and having a
> way to set per namespace $wgGroupPermissions in a sane way.

We are talking about global groups, so $wgGroupPermissions seems irrelevant. Your suggestion though would imply implementing per-namespace group permission support in CentralAuth.

On an unrelated note a sane way for $wgGroupPermissions to support per-namespace permissions is to allow an array as argument, e.g.

 $wgGroupPermissions['sysop']['deletedhistory'] = array( NS_FILE => true );

In any case I think setting permissions per-namespace is the way to go, rather than creating per-namespace permissions.

*bump*

Commons is getting fuller and fuller, more wikis moving to commons, more work.

It's been 3 years since it was accepted through a global-vote on Meta.

Bryan's suggestion appears to be a plausible solution. Altough I don't know a lot about the permissions and authentication in core and CentralAuth, it sounds like something that shouldn't be too hard.

Whatever uses $wg*Permissions checks if either the value itself or the current namespace array item in that value is true.

Can someone write a patch for this ? Do we need to update most/all usages of User->isAllowed / userCan() for addition/replacement with Title->userCan, or can it be implemented in there ?

Even if you can't write a patch, adding some notes would be great so others can pick them up.

Only those in SpecialUndelete would need to be changed, but there are a number of them.

*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*

The backlogs in Commons are piling up as far back as 2004 with review of moves from other projects to Wikimedia Commons.

Local wikis are refusing to delete images because the moves haven't been reviewed, if they would delete them (clean up), it is likely a Commons sysop may have to request temporary undeletion in order to review. This is taking hours/days/weeks for a single images, simply unworkable.

with over 6 years of backlog and 3 years of waiting for an implementation I think it's time we get this under serious consideration and write out a road map of what needs to be done. An estimation at the very least because the work-arounds are killing at the very least and no where even near motivating or collaborative.

Needed end-result:
* There is a global user group which is allowed to view deleted files and their file-page history on all wmf-wikis.

Technical aspects:
* [x] Global groups
* [_] Per namespace rights
* [_] Per namespace rights support through global rights

Making for triage, waiting for assignee and review of patch.

Just a quick note, because you asked for patch review: That patch was more of a quick first attempt at hacking on my part, to give people an idea, where to put this. In the mean time, we also have Deleted Revisions, which would likely also need to be patched. That patch will definitely have to be re-written from scratch, as it was already insufficient at time of build and might now be useless, because the code base has changed.

(In reply to comment #34)
> (In reply to comment #33)
> > I would prefer this to have this dependant of a Title->userCan(), and having a
> > way to set per namespace $wgGroupPermissions in a sane way.
> 
> We are talking about global groups, so $wgGroupPermissions seems irrelevant.
> Your suggestion though would imply implementing per-namespace group permission
> support in CentralAuth.
> 
> On an unrelated note a sane way for $wgGroupPermissions to support
> per-namespace permissions is to allow an array as argument, e.g.
> 
>  $wgGroupPermissions['sysop']['deletedhistory'] = array( NS_FILE => true );
> 
> In any case I think setting permissions per-namespace is the way to go, rather
> than creating per-namespace permissions.

I agree. So the userright-key in the user-group array in $wgGroupPermissions is either boolean or an array of namespace-ids with booleans.

So in the userCan function needs the option (or requirement) to specify a Title object.

The Title-class currently has permission-related functions, how usable are they for this bug ?

> 
> The Title-class currently has permission-related functions, how usable are they
> for this bug ?

The userCan hook already gets passed a title. One could just simply make an extension using that hook. A really hacky implementation could be done in about ten minutes. (userCan hook that just checks local group membership at commons [hey we already do cross-db access for global usage], and lets people read if they are a sysop over there). Admittedly that'd be a bit hacky [Normally rights give people rights to do stuff, not group membership].

(In reply to comment #41)
> > 
> > The Title-class currently has permission-related functions, how usable are they
> > for this bug ?
> 
> The userCan hook already gets passed a title. One could just simply make an
> extension using that hook. A really hacky implementation could be done in about
> ten minutes. (userCan hook that just checks local group membership at commons

This bug (or rather part of it, we should split it) requests to:
1) Have the ability to set permissions per namespace
2) Have the globally
3) Set some in NS_FILE for commons admins. This last part could either be cross-wiki read, or as a global group. I don't think having to manually add as global group is that big a deal though. Could be done by a wiki bot, that's all very minor details. The problem is having these permissions work and being able to set them from LocalSettings and for global groups.

When I referred to the usefulness related to this bug I meant point 1 and 2.

Created attachment 8772 [details]
Per namespace permissions for User.php

Patch that adds support for per namespace permissions in User.php. Next step would be to patch User::isAllowed() and Title::getUserPermissionsErrors().

Comment on attachment 6580 [details]
Patch for Special:Undelete built against trunk (fixed)

marking patch obsolete as it no longer applies cleanly to trunk and is meant for bug 29780.

Comment on attachment 8772 [details]
Per namespace permissions for User.php

marking patch obsolete as it no longer applies cleanly to trunk and is meant for bug 29780.

It's /just/ four years since the bug was reported, and a community consensus was granted. Are there any updates on the status of this bug? Is it ever going to be fixed?

(In reply to comment #46)
> It's /just/ four years since the bug was reported, and a community consensus
> was granted. Are there any updates on the status of this bug? Is it ever going
> to be fixed?

I suspect that, however difficult it may be, it's actually easier to get consensus for a global group with deletedtext permission on all namespaces than to get this bug fixed. If there was consensus for that, stewards could easily create the group and add any Commons admin who needed it. Beware, this is not the place to discuss it, you have to change [[m:Global deleted image review]] (and get it approved).

I'll create a global group on my wiki and test this. I'll be back with answers.

(In reply to comment #48)
> I'll create a global group on my wiki and test this. I'll be back with answers.

Don't bother. That will work just fine. We create that kind of global groups all the time.

Moving this bug back to the Wikimedia product, because this is just a request for configuration, not MediaWiki software implementation. The implementation of features that make this possible are already tracked and set as blockers for this bug:
* bug 29780
* bug 29781

This isn't right. The original request asked for creating a global group for Commons administrators with the rights to see deleted content only from the File: and File talk: namespaces (numbers 6 and 7). 

Now you are changing this request into one that grants Commons sysops the rights to view deleted content from all namespaces -- and there wasn't community consensus for that.

Please revert or create & lead another RFC on Meta asking the community to create a group with such permissions.

(In reply to comment #50)

> Now you are changing this request into one that grants Commons sysops the
> rights to view deleted content from all namespaces

Where?

(In reply to comment #51)

> Where?

Here.

(In reply to comment #52)
> (In reply to comment #51)
> 
> > Where?
> 
> Here.

Please...

Bug 29780 and bug 29781 specifically said per-namespace permission, and Nemo_bis's comment #47 also meant that it is easier to get a '''new''' consensus onwiki. Maybe I missed out something but I don't see anyone changed the bug to a request of global deletetext.

So could you kindly point that out.

As I understand, this bug was originally asking (though not very clearly) to add per-namespace permissions to the core of MediaWiki, and then assign 'deletedhistory' and 'deletedtext' for namespaces 6 and 7 to a global group including Commons administators only. 

I also understood comment #47 to suggest that getting a new community consensus to assign 'deletedhistory' and 'deletedtext' for all namespaces to the global Commons administrators' group as the way that this bug should be resolved, and wanted to point out that the community consensus from 2008 was only to grant these rights on the two aforementioned namespaces.

I am sorry for not being clear enough; I now see that this is only waiting for bug 29780 to be fixed, and then it will truly be only a configuration matter. And thanks for your patience, Benjamin ;-))

-shellpolicy: This has already been agreed on, no policy.

-shell: This can't be acted upon yet, because it has not been implemented in mediawiki core (bug 29780) and in CentralAuth (bug 29781) yet.

Readding shellpolicy to make clear that this is on hold and waiting for (multiple) clarifications: *given* the single-namespace only requirement (not in discussion [yet]), several implementations have been proposed and it's not clear which one we're going to pursue.
If bug 29781 was the only way, this should be RESOLVED INVALID because bug 29781 would make it a matter of steward requests, not of site configuration.

Removing shellpolicy again. This is not in need of further clarification. The proposal was very clear and straight forward about how the implementation was to be done (deletedtext in the file namespace, globally).

Yes, once done it will probably be implementable by stewards. This is just a tracking bug because it is a major thing that needs software to be written. The actual resolution of it probably doesn't require wmf-config changes.

Lowering priority, as bug 29780 and bug 29781 need to get fixed first.

Change 154868 had a related patch set uploaded by Legoktm:
SpecialUndelete: Check permissions on a per-page basis

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

I think restructuring the entire permissions system would be nice, but I doubt anyone is planning to do that for the next few years.

So I think bawolff's comment 41 is the best and easiest way to move forward. I uploaded a patch which changes Special:Undelete to check per-page restrictions, allowing us to set per-namespace restrictions in the wmf-config:

$wgHooks['TitleQuickPermissions'][] = function ( Title $title, User $user, $action, &$errors, $doExpensiveQueries, $short ) {
	return ( !in_array( $action, array( 'deletedhistory', 'deletedtext' ) ) || !$title->inNamespaces( NS_FILE, NS_FILE_TALK ) || !$user->isAllowed( 'globalimagereview' ) );
};

Then we just create a global group that has the "globalimagereview" userright.

Change 154868 merged by jenkins-bot:
SpecialUndelete: Check permissions on a per-page basis

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

Change 162546 had a related patch set uploaded by Legoktm:
Add "viewdeletedfile" userright for global deleted image review

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

Change 162546 merged by jenkins-bot:
Add "viewdeletedfile" userright for global deleted image review

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

On a technical level this is now implemented, marking as fixed accordingly. The userright is now grantable by stewards, but it won't start working until 1.25wmf1 is deployed to all sites (it hit group0 today, will take a week).

See also https://meta.wikimedia.org/wiki/Requests_for_comment/Global_file_deletion_review