Change 268972 had a related patch set uploaded (by Gergő Tisza):
[WIP] Log multiple IPs using the same session or the same user account

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

Change 268972 merged by jenkins-bot:
Log multiple IPs using the same session or the same user account

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

Tested in beta, works as expected.

Change 270240 had a related patch set uploaded (by BryanDavis):
Log multiple IPs using the same session or the same user account

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

Change 270240 merged by jenkins-bot:
Log multiple IPs using the same session or the same user account

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

Mentioned in SAL [2016-02-12T19:15:12Z] <bd808> Synced files for T125455 in wrong order; broke all wikis

Mentioned in SAL [2016-02-12T19:15:44Z] <bd808@mira> Synchronized php-1.27.0-wmf.13/includes/session/SessionManager.php: Log multiple IPs using the same session or the same user account (rMW4d8b8ca9b497) (T125455) (duration: 01m 17s)

Change 270359 had a related patch set uploaded (by Gergő Tisza):
Send log messages from session-ip channel to logstash

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

Change 270359 merged by jenkins-bot:
Send log messages from session-ip channel to logstash

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

Mentioned in SAL [2016-02-12T21:30:42Z] <tgr@mira> Synchronized wmf-config/InitialiseSettings.php: T125455: log session-ip channel to logstash (duration: 01m 17s)

In T125455#2023620, @Stashbot wrote:

Mentioned in SAL [2016-02-12T19:15:12Z] <bd808> Synced files for T125455 in wrong order; broke all wikis

The incident report for this mistake is at https://wikitech.wikimedia.org/wiki/Incident_documentation/20160212-AllWikisOutage

Discussed this briefly with @Tgr. I don't think we should keep the current implementation. It won't help us spot accounts that have been compromised, because an attacker is not likely to use enough IPs to set off the warning. When one of my e-mail accounts was compromised last year, the attacker used Tor, which caused my account to be accessed from a half-dozen IPs over the course of several days. This rate of change is equivalent to what you would see if a person was traveling.

The logging added in f22549a605 would be primarily useful for detecting a major failure of the authentication system. If that were to happen again -- which I think is unlikely -- we would hear about it.

I do think it would be useful to provide users with a dashboard which shows them recent IPs that had been used to access their account, but I think it would be easier to implement something like that from scratch than adapt the code in f22549a605 for that purpose.

So: it's a lot of complicated logic and a round-trip to Redis on every request. I propose reverting it, now that the session pollution issue has been resolved.

In T125455#2054194, @ori wrote:

The logging added in f22549a605 would be primarily useful for detecting a major failure of the authentication system. If that were to happen again -- which I think is unlikely -- we would hear about it.

Indeed, the main purpose was detecting something like a cached authentication cookie that affects lots of people. We were unsure how likely it would be that we hear about it, given that the past two records were from super power users - that might indicate it's a frequent issue but only reported by people with a good understand of tech issues and how they are handled. (Or it could indicate that there is something in the power user or global usage pattern that triggers the bug.)

I do think it would be useful to provide users with a dashboard which shows them recent IPs that had been used to access their account, but I think it would be easier to implement something like that from scratch than adapt the code in f22549a605 for that purpose.

That's T107707 (sort of) but that would require very different implementation. The IP logging code was always meant to be temporary.

So: it's a lot of complicated logic and a round-trip to Redis on every request. I propose reverting it, now that the session pollution issue has been resolved.

Apparently not :( T126069#2054327

In T125455#2054820, @Tgr wrote:

In T125455#2054194, @ori wrote:

The logging added in f22549a605 would be primarily useful for detecting a major failure of the authentication system. If that were to happen again -- which I think is unlikely -- we would hear about it.

Indeed, the main purpose was detecting something like a cached authentication cookie that affects lots of people. We were unsure how likely it would be that we hear about it, given that the past two records were from super power users - that might indicate it's a frequent issue but only reported by people with a good understand of tech issues and how they are handled. (Or it could indicate that there is something in the power user or global usage pattern that triggers the bug.)

I do think it would be useful to provide users with a dashboard which shows them recent IPs that had been used to access their account, but I think it would be easier to implement something like that from scratch than adapt the code in f22549a605 for that purpose.

That's T107707 (sort of) but that would require very different implementation. The IP logging code was always meant to be temporary.

So: it's a lot of complicated logic and a round-trip to Redis on every request. I propose reverting it, now that the session pollution issue has been resolved.

Apparently not :( T126069#2054327

Well, the logging did not help us in this case, right? Is there even an indication that the account had been accessed from enough IPs to hit the threshold?

Change 272795 had a related patch set uploaded (by Ori.livneh):
Revert "Log multiple IPs using the same session or the same user account"

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

Change 272795 merged by jenkins-bot:
Revert "Log multiple IPs using the same session or the same user account"

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

Logging IP changes is being done via T264799: Log when a request with the same user session comes from a different IP