Notifications for login attempts (successful or unsuccessful) from unrecognized computers/IPs is pretty standard. I know that Twitter, Facebook and Gmail definitely do this.

Danny suggests the following thresholds for failed notifications:
You get a message if there are 10 tries in 1 week from a known IP/device, or 2 tries in 2 weeks from a new IP/device.

At last meeting, Danny, Niharika, and I reached consensus to set LoginNotifyAttemptsKnownIP to 10 and LoginNotifyAttemptsNewIP to 2. Any other opinions are welcome.

We also decided to set LoginNotifyEnableOnSuccess to false for now, until a system is developed for allowing the user to force all sessions to log-out via an email link.

I know I'm not the brightest juicebox in the fridge, but when I searched the repo the only wgLoginNotify variables I see are $wgLoginNotifyEnableOnSuccess and $wgLoginNotifyEnableForPriv. Where are the other variables being used?

@MusikAnimal: Use of $wgVariable is slowly being deprecated in favor of $this->config->get( 'Variable' ), so you'll want to search for "LoginNotifyEnableOnSuccess" and "LoginNotifyEnableForPriv", etc.

The bot again failed to comment here, what am I doing wrong? I put Bug: T160094. Anyway here it tis, for updating the README and extension.json: https://gerrit.wikimedia.org/r/#/c/344661/

LoginNotifyCheckKnownIPs is whether to trigger a notification at all after failed logins from known IPs. We'll want this to be true.

LoginNotifyMaxCookieRecords refers to the number of users that are tracked as having logged in on a particular device. We can't store every user login forever (e.g. shared computers at libraries), so there is both a TTL and a max number of records to keep track of on a particular device. That is my understanding after looking at the code. I'm not sure what a good setting would be, but 6 users seems like a sensible default, with 6 months as the TTL.

So here's what I got for our config settings based on the above:

"LoginNotifyAttemptsKnownIP": 10,
"LoginNotifyExpiryKnownIP": 604800,
"LoginNotifyAttemptsNewIP": 2,
"LoginNotifyExpiryNewIP": 1209600,
"LoginNotifyCheckKnownIPs": true,
"LoginNotifyEnableOnSuccess": false,
"LoginNotifyEnableForPriv": [ "editinterface", "userrights" ],
"LoginNotifySecretKey": null,
"LoginNotifyCookieExpire": 15552000,
"LoginNotifyCookieDomain": null,
"LoginNotifyMaxCookieRecords": 6,
"LoginNotifyCacheLoginIPExpiry": 5184000

@Bawolff: Would love to get your assent on the settings above before moving forward with this in production.

Change 344661 merged by jenkins-bot:
[mediawiki/extensions/LoginNotify@master] Complete documentation of config variables

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

I know this has been signed off on, but I'd like to point out that 10 ($wgLoginNotifyAttemptsKnownIP) is an absurdly high number for someone to mess up their password that many times before it being reported. You need to fill a captcha after the 3rd/4th try so the chances of 10 password misses is very slim,

Aside this makes testing a real pain because you have to fill out a captcha every time after the 3rd try and you're blocked from attempting to login after the 6th/7th try and need to wait 5 minutes before trying again.

I encourage a rethink on this setting.

Also, interestingly, I did just test this out in the current state and made ~15 failed login attempts but it did not give any alerts when I did actually login. I'm wondering if somehow the "count" of failed login attempts is lost when the person is blocked from attempting to login for 5 minutes after too many failed attempts.
There could be a couple more reasons for it though. I'm looking.

I would be OK with lowering it. @DannyH, @Niharika: What do you think of lowering it from 10 to 6?

Change 346464 had a related patch set uploaded (by Niharika29):
[operations/mediawiki-config@master] Update $wgLoginNotifyAttemptsKnownIP in Labs to make testing easier

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

No problem with lowering it, sounds good. @Niharika @kaldari

Change 346464 merged by jenkins-bot:
[operations/mediawiki-config@master] Update $wgLoginNotifyAttemptsKnownIP in Labs to make testing easier

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

@MusikAnimal: I'm not sure I understand why we want to set LoginNotifyEnableForPriv to [ "editinterface", "userrights" ]. Can you elaborate on that one?

In T160094#3158376, @kaldari wrote:

@MusikAnimal: I'm not sure I understand why we want to set LoginNotifyEnableForPriv to [ "editinterface", "userrights" ]. Can you elaborate on that one?

Yeah that should probably be set to false, which I think makes it work for all users? I went by the defaults

I don't really understand what LoginNotifyEnableForPriv does. Whether it's set to false or an array of rights doesn't seem to make any difference. The notifications still work for all users. @Bawolff: Could you shed some light here?

@MusikAnimal: If there's no response from Bawolff, can you dig into the code and see what it's supposed to do?

Also https://github.com/wikimedia/mediawiki-extensions-LoginNotify/blob/master/extension.json#L18 should probably be set to true.

@DannyH: Any opinion on the setting for echo-subscriptions-email-login-fail? In other words, should the notification for failed logins be web-only by default or web and email by default?

In T160094#3158715, @kaldari wrote:

I don't really understand what LoginNotifyEnableForPriv does. Whether it's set to false or an array of rights doesn't seem to make any difference. The notifications still work for all users.

I looks like it allows you set different defaults for users with certain rights, see onUserLoadOptions. I think this is configured using a combination of getOverridenOptions and these options in extension.json. So all you're doing is setting a default that echo and/or email for "Failed login attempts" or "Login from new computer" are turned on at all, not other options like AttemptsKnownIP, etc. Maybe we'll want to set the email options to true for users with admin-like permissions?

@MusikAnimal: Can you verify this and update the documentation at https://www.mediawiki.org/wiki/Extension:LoginNotify and in the extension.json and README files?

In T160094#3175840, @kaldari wrote:

@MusikAnimal: Can you verify this and update the documentation at https://www.mediawiki.org/wiki/Extension:LoginNotify and in the extension.json and README files?

So with two fresh new accounts, one an admin (has userrights permission), and another a non-admin account, the difference I see is that "Login from new computer" is defaulted on for the admin. It seems then that the feature works, whereby any options placed here are automatically treated as being true for the configured "privileged" users. This apparently means the defaults for privileged users are not configurable.

Before we update the documentation, I think we should first answer this question. I do like that admins by default get the "Someone has successfully logged into your account from a computer which you have not edited from recently" notification, but this is not without caveats and "icky" and "hacky" code. Maybe it's not worth it, or we should refactor this to make it not-so-hacky. I will write about these findings at T162750 too.

Are the settings mentioned in the task description accurate? I recall we decided to make "LoginNotifyAttemptsNewIP" 2 in our meeting.

Yeah, the idea for that setting is that high priv users should in theory default enabled since their account security can affect other users, but we shouldnt auto-annoy normal users who dont matter unless they opt in

"wgLoginNotifyAttemptsNewIP" has now been set to 1 in Labs and it solves all of the issues surrounding number mismatch between icon and attempts and bundling works very nicely with it.

"wgLoginNotifyAttemptsKnownIP" is still at 10, which seems like a safe limit. There will be a mismatch between the number showing up on the icon and the number of bundles and attempts if someone does try 10 times but it's pretty hard to reach that count, given that the captcha kicks in after 3 attempts and you're blocked from attempting to login for 5 minutes after 5 attempts. I'm gonna try to replicate this and come up with a screenshot.

Let's set the default for LoginNotifyAttemptsNewIP to 1 in the extension.json as well since it solves the bundling issues.

Change 350089 had a related patch set uploaded (by Niharika29):
[mediawiki/extensions/LoginNotify@master] Change default configs for LoginNotify production

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

@Niharika: Merged. The only thing left to do on this task is to correct the documentation for LoginNotifyEnableForPriv (see discussion above).

Change 350089 merged by jenkins-bot:
[mediawiki/extensions/LoginNotify@master] Change default configs for LoginNotify production

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

Change 350665 had a related patch set uploaded (by Niharika29; owner: Niharika29):
[mediawiki/extensions/LoginNotify@master] Update documentation for "LoginNotifyEnableForPriv"

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

Change 350665 merged by jenkins-bot:
[mediawiki/extensions/LoginNotify@master] Update documentation for "LoginNotifyEnableForPriv"

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