Page MenuHomePhabricator

Uncheck "Automatically block the last IP address" by default on Special:Block on officewiki
Open, Needs TriagePublic

Description

Hello - not sure who to direct this request to.

May it be possible to change the default settings for this page on the office wiki - https://office.wikimedia.org/wiki/Special:Block

Can the setting "Automatically block the last IP..." - be unchecked rather than checked?
screenshot attached.

Event Timeline

eliza created this task.Sep 3 2019, 9:18 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 3 2019, 9:18 PM
Peachey88 removed ops-monitoring-bot as the assignee of this task.Sep 4 2019, 8:08 AM
Peachey88 added a project: Office-IT.
Peachey88 edited subscribers, added: ops-monitoring-bot; removed: Office-IT.
Peachey88 added a subscriber: Peachey88.

It doesn't appear we have a setting to change that (https://www.mediawiki.org/wiki/Manual:Block_and_unblock#Configuration_settings_related_to_blocking).

If it is causing a issue on OfficeWiki, we could probably change $wgAutoblockExpiry to a short few second range, and possibly disable $wgCookieSetOnAutoblock if its been enabled as well.

It might be possible to do some site JS customisations to try disable it.

Urbanecm changed the task status from Open to Stalled.Sep 4 2019, 8:28 AM
Urbanecm claimed this task.
Urbanecm edited subscribers, added: Urbanecm; removed: Peachey88.

Stalled, pending clarification, Tagging with Office-IT and Wikimedia-Site-requests, since it is related to OIT and site requests.

@eliza: Requests about changing wiki configuration should follow guidelines at https://meta.wikimedia.org/wiki/Requesting_wiki_configuration_changes. Configuration change requests are anything that changes user rights (what group can what, but not which users are in that group), or anything similar. If you're not sure, treat it as a configuration change and follow the guidelines. You will at the very least get info what to do next. Site requests (aka configuration change requests) should have project tag #wikimedia-site-request. It can have also Office-IT, if your request is relevant to OIT, and OIT wants to track it somewhere.

Generally, do not set any of the following fields: a) assignee b) priority c) anything you're not sure you don't need to set. Even if you fill description and title only, it is fine, someone will read that sometime, and advise. The used method of informing people is project tags, and you can tag with Wikimedia-Site-requests tag to get the attention of someone who processes site requests, usually, one who has sysadmin privilege.

Restricted Application added a project: User-Urbanecm. · View Herald TranscriptSep 4 2019, 8:28 AM
Urbanecm added a subscriber: Peachey88.EditedSep 4 2019, 8:29 AM

Sorry for unsubscribing, a conflict.

An easy way would be to grant IPBE to everyone. However, no one should notice an autoblock IMO. It's triggered on edit, not on login, and officewiki won't let you log in if you're blocked, as it's private wiki. Would need to test that.

Why do you need the change?

Aklapper renamed this task from Office Wiki : "block user" default settings to Uncheck "Automatically block the last IP address" by default on Special:Block on officewiki.Sep 4 2019, 8:35 AM
eliza added a comment.Sep 6 2019, 4:58 PM

What sometimes happens when we block accounts of departed staff is that the last IP used is often times the office network, blocking everyone in the office to access Office Wiki.

Keeping that checkbox unchecked by default, reduces the possibility of blocking the entire office.

DannyS712 added a subscriber: DannyS712.

What sometimes happens when we block accounts of departed staff is that the last IP used is often times the office network, blocking everyone in the office to access Office Wiki.
Keeping that checkbox unchecked by default, reduces the possibility of blocking the entire office.

Clear use case. Currently, the check box is defined as

AutoBlock
$a['AutoBlock'] = [
	'type' => 'check',
	'label-message' => 'ipbenableautoblock',
	'default' => true,
	'section' => 'options',
];

Changing the default can be accomplished by adding a new global variable, $wgAutoblockByDefault, which by default would be true, and then for officewiki can be configured to false. Tagging MediaWiki-Special-pages and MediaWiki-User-management due to change required in core; once the option is available it can be added to the WMF config. @Urbanecm is it okay if I work on this? (If yes, just assign the task to me)

Urbanecm reassigned this task from Urbanecm to DannyS712.Sep 7 2019, 4:53 PM

Sure, thanks!

Restricted Application added a project: User-DannyS712. · View Herald TranscriptSep 7 2019, 4:53 PM
DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.Sep 7 2019, 6:38 PM

Change 534936 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Introduce global setting wgAutoblockByDefault

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

Change 534937 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] Don't enable autoblock by default on officewiki

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

Change 534940 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] If $wgBlockDisablesLogin is true, don't enable autoblocks by default

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

Change 534937 abandoned by DannyS712:
Don't enable autoblock by default on officewiki

Reason:
Iccb99465f64fd55e10a9929e8e568d11c4c94ddc

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

Change 534936 abandoned by DannyS712:
Introduce global setting wgAutoblockByDefault

Reason:
Iccb99465f64fd55e10a9929e8e568d11c4c94ddc

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

eliza added a comment.Sep 18 2019, 1:17 PM

Following up on this. I noticed this change was not yet made and yet the project removed. Can you all confirm why.

Sure - I don't need to pay much attention to this change, so I removed my personal project from it. @DannyS712 uploaded relevant MediaWiki change that allows the change to be made. We need to wait for the change to be reviewed and deployed. Then, we would be able to process with the configuration change. I expect the whole process to take about one month.

eliza added a comment.Sep 18 2019, 2:01 PM

@Urbanecm - thank you for the clarification.

Urbanecm changed the task status from Stalled to Open.Sep 18 2019, 2:16 PM

You're welcome. As soon as the task would be deemed to be resolved, it will be "closed as resolved" by either Danny or me.

Restricted Application added a subscriber: MGChecker. · View Herald TranscriptSep 19 2019, 1:10 AM

I actually don't see why this option should even be availble if you have $wgBlockDisablesLogin enabled.... I propose we disable the field completely... unless @Tchanders or @dmaza can think of a use case where autoblocks would be used under that condition?

Mooeypoo added a subscriber: Mooeypoo.EditedSep 19 2019, 1:45 AM

First off, thank you @DannyS712 for picking this up :)

There's a bigger question here; the patch makes an assumption that the state of that checkbox (default value or, to your point, @dbarratt, whether it's enabled or not) is based on the $wgBlockDisablesLogin variable.

Incidentally, in production, at the moment, $wgBlockDisablesLogin only enabled for private wikis -- but that is not necessarily always the case. If we ever change this variable, the checkbox also changes states, and the connection between those two isn't really obvious beyond being active on private wikis, and especially in 3rd party wikis. A wiki may want to have that checkbox enabled (and default) even with $wgBlockDisablesLogin and there's a number of use cases for that.

I wouldn't assume tht it should be disabled necessarily, since I can see cases where that might not be the case for non private wikis.

@DannyS712 I don't mean to complicate your work here (and again -- thank you for picking this up!) but I think it would be better to at least add a check for private wiki alongside the conditional, so, something like

$autoblockByDefault = !($conf->get( 'BlockDisablesLogin' ) && !MediaWikiServices::getInstance()->getPermissionManager()->isEveryoneAllowed('read'));

First off, thank you @DannyS712 for picking this up :)
There's a bigger question here; the patch makes an assumption that the state of that checkbox (default value or, to your point, @dbarratt, whether it's enabled or not) is based on the $wgBlockDisablesLogin variable.
Incidentally, in production, at the moment, $wgBlockDisablesLogin only enabled for private wikis -- but that is not necessarily always the case. If we ever change this variable, the checkbox also changes states, and the connection between those two isn't really obvious beyond being active on private wikis, and especially in 3rd party wikis. A wiki may want to have that checkbox enabled (and default) even with $wgBlockDisablesLogin and there's a number of use cases for that.
I wouldn't assume tht it should be disabled necessarily, since I can see cases where that might not be the case for non private wikis.
@DannyS712 I don't mean to complicate your work here (and again -- thank you for picking this up!) but I think it would be better to at least add a check for private wiki alongside the conditional, so, something like

$autoblockByDefault = !($conf->get( 'BlockDisablesLogin' ) && !MediaWikiServices::getInstance()->getPermissionManager()->isEveryoneAllowed('read'));

This is why, originally, I had introduced a separate config variable to control this. However, @Krinkle objected on the patch (https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/534936/) to the addition of a new global setting, instead suggesting that $wgBlockDisablesLogin be used. I have no strong preference either way, but given that just checking BlockDisablesLogin wouldn't be enough, I again suggest that a new config value be added.

In response to your suggested change, what happens if everyone can read, everyone can create an account, but only logged in users can edit? Then, by default, autoblock is disabled, but it would still be helpful in case vandals created extra accounts.

Mooeypoo added a comment.EditedSep 19 2019, 3:11 AM

This is why, originally, I had introduced a separate config variable to control this. However, @Krinkle objected on the patch (https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/534936/) to the addition of a new global setting, instead suggesting that $wgBlockDisablesLogin be used. I have no strong preference either way, but given that just checking BlockDisablesLogin wouldn't be enough, I again suggest that a new config value be added.

Ahha, okay, so my first instinct was also to suggest you use a new config variable, but I wasn't sure how warranted it is just for the sake of a default value for a checkbox, so I see @Krinkle's point here. That said, tacking this behavior onto wgBlockDisablesLogin seems wrong to me; it expands the scope and purpose of the variable to something that has other impact, and without having completely direct reason.

Between the available options -- a new config variable, using (and expanding the scope of) $wgBlockDisablesLogin or checking both $wgBlockDisablesLogin and isEveryoneAllowed('read') I think the most straightforward *while* least hacky is the third way (checking both).

@Krinkle since you've provided initial feedback, is there anything I'm missing here? Adding this functionality to the config var feels to me kinda the same as reusing an i18n message for a different purpose just because the message is incidentally the same. It works, but it's not really the right thing to do, and it opens the door for issues with having config vars that are impossible to adjust because they have impact broader than what they were intended for.

In response to your suggested change, what happens if everyone can read, everyone can create an account, but only logged in users can edit? Then, by default, autoblock is disabled, but it would still be helpful in case vandals created extra accounts.

So this is partly why I was a bit nervous of merging the code to begin with even if the config variable was very specific -- there are some implications to the toggling default in a lot of cases.

That said, the impact here is relatively low (worse case, people toggle the checkbox) and the cases might be more expansive. However, since the default'ness of the checkbox will be separate than strictly that config var, we will be able to adjust it in the future without having to change any other definitions.

So, my recommendation is to take the case that answers the most relevant of our expanded use case, and go with that; the double-check will ensure that it only impacts very specific wikis, under that specific use case, which means the impact is not as broad and we can get a bit more flexibility in iterating if needed. This functionality doesn't *disable* the checkbox. It just dictates whether it's checked by default.

I agree that it feels a bit problematic to make assumptions about autoblocks based on the $wgBlockDisablesLogin config. I'm not sure I agree the checking of $wgBlockDisablesLogin and isEveryoneAllowed('read') is much better - it's less impactful, but the principle is the same. (If a wiki with these settings wanted the checkbox checked by default, what would we do?)

Forms that pre-populate fields by default will just get it wrong in some cases... Could we get round the problem another way, as suggested above? E.g. setting $wgAutoblockExpiry to 0 or granting the permission 'ipblock-exempt' to all users?

I agree that it feels a bit problematic to make assumptions about autoblocks based on the $wgBlockDisablesLogin config. I'm not sure I agree the checking of $wgBlockDisablesLogin and isEveryoneAllowed('read') is much better - it's less impactful, but the principle is the same. (If a wiki with these settings wanted the checkbox checked by default, what would we do?)
Forms that pre-populate fields by default will just get it wrong in some cases... Could we get round the problem another way, as suggested above? E.g. setting $wgAutoblockExpiry to 0 or granting the permission 'ipblock-exempt' to all users?

The biggest issue I have, honestly, is that this changes the *meaning* of a somewhat unrelated config variable just to get this behavior, and you can see that by the fact the patch changes the documentation of that config variable.

Either we get another config var (which is an overkill) or we, at least, get that extra check in there so that the config var retains its original meaning, but the specific use case is represented.

Mooeypoo added a comment.EditedSep 19 2019, 10:48 PM

Forms that pre-populate fields by default will just get it wrong in some cases... Could we get round the problem another way, as suggested above? E.g. setting $wgAutoblockExpiry to 0 or granting the permission 'ipblock-exempt' to all users?

Sorry, as was pointed out to me justifiably IRL, I misunderstood this part. @Tchanders you're making a good point about this; however, this will only fix the underlying ask tangentially. It would mean that the checkbox being selected won't matter, but it won't actually unselect that checkbox. After discussing things with @eliza they might actually have specific cases where they *intentionally want* to set the checkbox to "true" and have that behavior consistent, but since that is an edge case, they want the default to be off.

Since that's the case, I would suggest we take a step back here. All the solutions so far have assumed we can somewhat generalize this request to fit multiple cases or wikis or needs. This caused us to go on a bit of a loop, finding the right way to generalize it while also finding all the implications that generalizations created -- then causing us to try and find solutions for those. I fear we've dug ourselves into a pit of our own making :)

So, here's my suggestion. Let's take a step back, and fix the actual need: The need is to be able to set that specific checkbox (and potentially another) to a different default state on load. We can do that as an quick gadget, and all we're missing is a hook that allows us to manipulate the ooui widgets only after they're actually loaded and done. We can add that hook, then (see patch) which is pretty trivial.

Then, individual wikis or even admins, if they so wish, can add a quick script that changes their defaults to whatever they want -- while not touching or harming the underlying behavior of the system. There has also been an idea (ticket upcoming) to allow for changing of checkbox states based on the selection of "block reason". This, as a feature, would be hard to implement because it would require a pretty big generalization to allow any wiki to set customized behavior... but if it's a gadget, it's pretty trivial.

I think this is the best and most direct way to solve this specific problem, solve some of the upcoming issues, and dig ourselves out of the pit of circular generalizations we've gotten into in the ticket :)

To utilize the hook in the patch below, one can add this to MediaWiki:common.js:

mw.hook( 'special.block.ready' ).add( function () {
	var checkWidget,
		$checkbox = $( '#mw-input-wpAutoBlock' );
	if ( $checkbox ) {
		try {
			checkWidget = OO.ui.infuse( $checkbox );
			// Make the checkbox default false
			checkWidget.setSelected( false );
		} catch ( e ) {
			// Debugging purposes only
			console.log( 'Special:Block: Could not infuse wpAutoBlock checkbox' );
		}
	}
} );

Change 538125 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[mediawiki/core@master] Add mw.hook: special.block.ready when Special:Blocks finishes loading

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

@Tchanders you're making a good point about this; however, this will only fix the underlying ask tangentially. It would mean that the checkbox being selected won't matter, but it won't actually unselect that checkbox.

Just to help us understand this correctly - are there cases where a logged-in user on office wiki should be blocked if the IP they are editing from is blocked? If not, then we could use the 'ipblock-exempt' option.

We can add that hook, then (see patch) which is pretty trivial.

Adding the hook can be trivial, but maintaining hooks and the code they affect isn't necessarily. I'd be wary of adding them unless they are the only feasible option.

Mooeypoo added a comment.EditedSep 20 2019, 1:12 AM

@Tchanders you're making a good point about this; however, this will only fix the underlying ask tangentially. It would mean that the checkbox being selected won't matter, but it won't actually unselect that checkbox.

Just to help us understand this correctly - are there cases where a logged-in user on office wiki should be blocked if the IP they are editing from is blocked? If not, then we could use the 'ipblock-exempt' option.

The main problem that was described in this ticket is when blocking a user that is in the office; that would mean that subsequently blocking used IPs would block the office's IP. However, that's not true if blocking users that are not in the office. That use case may warrant, in some cases, using this option.

So, either way, this option isn't obviously a "always on" or "always off". There are cases where it should be toggled. The vast majority of cases, however, seem to be "unchecked" case, which is why the request is to make that the default.

That also means that we shouldn't rush to set this as an irrelevant checkbox, though. It might be useful in some of the cases.

We can add that hook, then (see patch) which is pretty trivial.

Adding the hook can be trivial, but maintaining hooks and the code they affect isn't necessarily. I'd be wary of adding them unless they are the only feasible option.

I don't think there's as much future maintenance for this as it may sound.

Javascript hooks "announcer" hooks, more similar to event emitted than the manipulation-driven/allowing hooks that PHP has. We have multiple hooks in production, in multiple locations, whose entire and only purpose is simply to announce "this section is ready for whatever you want to do", and we've added a few of those with other products to allow gadgets and user scripts to be developed.

We do not support those external scripts, and we're not expected to. Unlike PHP hooks that may manipulate some variable that our own system then continues to handle -- which means we need to account for whatever an external system did with it, and if we want to change things we need to support that again -- in JS hooks, the hook is an event that delivers the state and that's it. It doesn't wait for anything else to complete, and it does not assume dependency or even cares about what the external script did.

Some examples that might demonstrate this are 'wikipage.content', structuredChangeFilters.ui.initialized and ext.echo.badge.countChange.

This style (unlike the PHP events) is more similar to OOjs' EventEmitter than it is to the PHP style hooks.

I think adding a hook is cheap in this case, requires very little maintainance (if at all) and allows not only for us to solve this *specific* case, but also opens the door for other tweaks to be made, independently, for this form.

To be fair, I think that there should be a general event that fires whenever core JS finishes loading *in general*; if we already allow anyone to be creative with gadgets and user scripts that make tools to help productivity, we might as well enable it better. That, however, is a much more massive and architectural ask from all of MW. So, for this case (and potentially future tweak requests for this page's form) I think a hook is fair.

It won't work for no-js implementation, but I think that's fine, especially since we're mostly concerned with the interactivity here.

I think the question of whether a logged-in user is ever expected to be blocked by an IP block on office wiki is relevant: if the answer is no, then the 'ipblock-exempt' right should be granted to all users, regardless of this task (and then the problem that this task is trying to fix would be solved).

Even though the hook is essentially just firing an event, I think there can be overheads of gadget maintenance, plus any overhead associated with preserving the order in which things load. The question of whether we should have the general JS-has-loaded hook is interesting; whether we should offer a way to tinker with form defaults does feel like a higher-level question about all forms, rather than just Special:Block.

To sum up we have one immediate use case, from a problem experienced and reported from a single wiki. Other use cases are (to my knowledge) theoretical. While they could turn real any minute, I think it's important we keep this in mind. As a general principle (not specific to this), I think that it's fine for any theoretical use case that becomes real to be taken seriously. But, while any of them can, not all do. I think it's worth optimising and tailoring our maintenance cost towards what we have at hand.

Some thoughts in reply to @Mooeypoo per my CR involvement. Didn't mean to block, sorry about that. Please consider it unblocked now :)

  1. Add a general-purpose configuration variable that determines the default state of one checkbox on one form.

This means a site admin has to discover something it (or its users) want or want not, and then discover this very specific option to change it. I've recommended against this because I don't think it's a sustainable approach for us as maintainers long-term, and also not a good site-admin experience.

Having said that, on any individual case I'm happy to set that aside. I just wanted to have asked the question whether this is deserving of being "special". If it is, go ahead :)

  1. Provide good defaults that make common cases easy, and avoid common mistakes.

This was my recommendation. The idea that this isn't enabling or disabling any fundamental ability. It's just the default state on a form. Having some basic heuristics that are consistent within a wiki (predicable by users; not vary by target user or time of day) to determine a good default. This is easier for us to maintain because we don't make any hard promise other than we'll do our best to provide the best default experience. We'd switch on a general "Intranet" mode, which is effectively what wgBlockDisablesLogin is. Perhaps we could introduce a new config var to represent that explicitly before, or after, this, if you go this route.

  1. Add a general-purpose JS hook.

This means we don't support the use case and let site admins implement it on their own with JS in Common.js. Relatively easy to maintain, assuming we make no promises of stability over the form layout and HTML attributes between releases. Enables many more use cases, but does come back to the site-admin UX/discoverability issue.

  1. Solve the underlying problem for the current use case (make intranet users immune to IP blocks).

Hard to argue with this one, seems worthwhile doing regardless of the outcome of this specific ticket. If we go this way, we might be able to shelve this ticket until and unless the use case comes up outside the Intranet/officewiki use case.

I think the question of whether a logged-in user is ever expected to be blocked by an IP block on office wiki is relevant: if the answer is no, then the 'ipblock-exempt' right should be granted to all users, regardless of this task (and then the problem that this task is trying to fix would be solved).

Except that may not solve the problem - see T233441: Cookie blocks apply regardless of ipblock-exempt right

@DannyS712 Thanks for pointing out that task. (See T233441#5513467 - the problem is with cookie blocks and won't affect users on different devices.)

As @Peachey88 mentions above, it might be that $wgCookieSetOnAutoblock should be set to false as well.