Those should probably be edited out.
eh, I'd still say security bug, while yes you could intercept them with shell access it's more difficult and in general the system is designed to try and make it as close to impossible as possible to figure out a users password. It is both relatively difficult (even most people with shell access likely don't know how to do the interception, maybe not on WMF servers but almost certainly for reusers who have a wider range of tech skills) to intercept and interception requires 'real time' (basically) compared to log entries which, more similar to the database where we hash them with a salt, can be looked at retroactively "at their leisure" and so makes it easier to target specific users and hide your access.
Once AuthManager comes out, passwords are likely to also be in a parameter named "retype". But the AuthenticationRequest might use any parameter name it wants, if some extension decides to do something particularly weird with it.
It looks for password and lgpassword; I "amended" it by editing the comment (probably a bad habit) and Phabricator does not send any email about that.
For AuthManager we should just redact fields with type=password.
The hard part there is in (1) knowing that the request is for an AuthManager-using module in the first place and then (2) actually determining the AuthenticationRequests that were involved. And asking the module itself could be problematic since things might have errored out before even getting to that point.
Is there an easy way to filter edit tokens here too? That's less important, but would be nice.
For centralauth tokens and OATH tokens, they expire when used, so having those logged isn't a problem.
@Anomie, for other AuthManger providers, I guess letting them append to some list of blacklisted api parameter names would be nice. I don't think we need to necessarily know that it's an authentication request in order to redact the log.
That's probably the thing to do.
I just now decided to do that for a different purpose (I saw T143285 and thought that that should be an error, eventually), and then I was reminded of this bug.
Hmm. So on my supporting backport of https://gerrit.wikimedia.org/r/#/c/336838/
If we don't merge that, we can't really put the fix for this into REL1_23... Anyone got any strong opinions either way?
For most client libraries.... Moving passwords/tokens from GET into POST (if they've not already done so for support of newer MW versions) should be trivial