Page MenuHomePhabricator

Blocking/Permissions Rewrite
Closed, DuplicatePublic


Currently, our blocking/permission system is rather ad-hoc. In looking at some ways it could use improvement, it is becoming increasingly clear that a re-do is in order. To do this, I have composed several ideas:

  1. Blocking should be permission-based. If a user can upload, read, write, they should be able to be blocked from uploading but not writing. This allows for finer-grained blocks which allow users to still contribute yet be prohibited from doing something they're having problems with. The major hurdle is keeping current blocks in place (I would suggest making all current blocks a general "block from everything").
  1. Checking permission should be a 1-stop-shop. With the aforementioned grained blocks, we'll see an easier integration with the general permissions check. User::isAllowed() will be able to check if a user is _really_ allowed to do something. This eliminates the usual 2-step of "check if blocked," then, "check if has permission."

I'm keeping in mind that we'll need to preserve current abilities (rangeblocks, autoblocks, etc). Additionally, the Special:Blockip interface needs to remain as simple as possible. I'm planning to start a BlockingRewrite branch, but I'd like some input first. Thanks.

Version: unspecified
Severity: enhancement



Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 10:09 PM
bzimport set Reference to bz14636.
bzimport added a subscriber: Unknown Object (MLST).

I was going to do this at some stage, but I'm unlikely to get around to it until the end of the year due to real-life commitments. If you do go ahead before me, here are a few things to consider:

  1. All permissions checks can already done, in Title::getUserPermissionsErrors, which also returns the reason that the action can't be done.
  2. Performance - we can't scan the block table per-pageview, so any checks need to be done based on an index. My idea was to have a separate table for conditions which cause a block to occur, and use an index on that.
  3. It's possibly a good way to write per-page blocking (bug 870).

I'm sure there were others.

Also for the rewrite: determine a syntax to give the permissions per-namespace.

Bringing the current systems closer together would be great. There are not many changes needed to implement the current block options as rights functionality.

One step is to

  • add a 'can edit own user talk' permission
  • remove/deprecate $wgBlockAllowsUTEdit

Then the existing system can be replicated with an autopromote group 'blocked' that uses APCOND_BLOCKED (r52083), and revokes 'edit' and grants 'editowntalk' for wikis with $wgBlockAllowsUTEdit.

That would mean the three rights currently rights under admin control ('createaccount', 'sendemail', 'editowntalk') are proper permissions and the storage details can be abstracted under the permissions API, which may continue to access them from ipblocks table, or uses a new user_rights_override table which would require the block logic is updated as well.

prog4life wrote:

So if we work backwards, how would the new permissions be set via LocalSettings.php?
I thought of something like this:

$wgGroupPermissions['users']['createtalk'] = true; As normal: Permission granted
$wgGroupPermissions['somegroup']['createtalk'] = false;
As normal: This group does not have the permission to do so, but the permission maybe granted from another group the user is present in, otherwise of course performing the above action is denied.
$wgGroupPermissions['somegroup2']['createtalk'] = -1; //If a user is present in this group, then regardless of whether they have permission granted for the above action from another group, they would always be denied to perform the above action.

This would of course get rid of $wgRevokePermissions.

Any thoughts on the above?

prog4life wrote:

Basic work

OK. As I briefly mentioned on IRC, here is my contribution on an attempt to rewrite the permission system.
I know that this is nowhere near a solution, but I see it as a start and just would like some feedback on the work, and maybe I could build from there.

The system works just like I described it in Comment 5, it builds on top of the usual $wgGroupPermissions, but gets rid of $wgRevokePermissions.

For the following example, if the user is in group 'noupload', no matter whether he already has permission to upload from another group or not, he would always be denied to upload.

$wgGroupPermissions['noupload']['upload'] = -1;

So, any feedback is really appreciated! Thanks.

attachment bug14636-pre.patch ignored as obsolete

prog4life wrote:

Comment on attachment 10491
Basic work

Patch with several modifications submitted to Gerrit for review: (Change I0886845d)
Marking attached patch as obsolete.

sumanah wrote:

Thanks for putting it in Git/Gerrit!