#4691 closed defect (bug) (fixed)
Break Password Hash when user is blocked
Reported by: | Ipstenu | Owned by: | dd32 |
---|---|---|---|
Milestone: | Priority: | highest omg bbq | |
Component: | Support Forums | Keywords: | |
Cc: |
Description
On reviewing the uptick in malicious trac attacks, it was pointed out that by blocking a user, we prevent them from being able to leave tickets. That is, since cookies are shared across all .org sites (including Trac and the forums), blocking a user in the forums PROPERLY stops them from being able to login on Trac.
But due to a change from BBpress 1 to 2, it _no longer_ invalidates cookies.
Per Nacin:
At least in bbPress 1.x, cookies got invalidated when a user is blocked (by mucking with their password hash).
While this can be achieved by manually by editing the user password, that comes with two concerns:
- You have to remember to edit the password
- The user would be emailed that the password is changed
If a user is a spammer/hacker and blocked, they generally use a throwaway email, so a message is sent for no benefit. However when a bad-actor is banned, they WOULD receive the email, which alerts them to the unrequested change.
Since we do inform users why they got banned before doing so, the extra notification is unnecessary.
However. As Otto said:
Any normal action in WordPress which changes the password causes it to send the email, so invalidating the hash without causing that isn't the simplest thing to do
Therefore I suggest we start small.
- When an account its blocked, the email is changed.
By automating this, we reduce the burden on forum support volunteers and anyone who doesn't happen to know this is necessary.
Further work would be to make this do so without emailing the user.
Change History (28)
#2
@
5 years ago
Maybe instead of tying it to blocked, we could give a "Clear Password" button, so that these two actions are separate?
#3
@
5 years ago
Copying my comment from slack to here:
FWIW this has been something that’s been on my radar for awhile.
The increase in Trac spam/pentesting hasn’t made me “fix” it though since they generally switch accounts before you block them anyway. As soon as you delete the ticket, they generally switch to a new account anyway.
If you change their password we sent out a “password changed” email too AFAIK, which is annoying..
#4
@
5 years ago
Maybe instead of tying it to blocked, we could give a "Clear Password" button, so that these two actions are separate?
My only reason I'm against that is making it two steps instead of one means people will forget. If we can reduce the mistakes, we'll save everyone a lot of headaches.
The increase in Trac spam/pentesting hasn’t made me “fix” it though since they generally switch accounts before you block them anyway.
Generally yes :) But at the same time, they're not the only kind of recidivist we get :D
This ticket was mentioned in Slack in #meta by tellyworth. View the logs.
5 years ago
#6
@
5 years ago
How about creating a new user role called "Blocked", or a new capability to block users? When setting enabling it, it would clear all of the user's sessions and prevent them from being able to login.
#7
follow-up:
↓ 8
@
5 years ago
How about creating a new user role called "Blocked",
Hi @pierlo,
bbPress includes exactly this already, but it was originally designed to only block authenticated users from reading or writing to the forums, not to prevent them from logging back in, or to block the rest of the site, other sites, other platforms (Trac, Slack, etc...)
What @Ipstenu is proposing in this issue (as an extension of that Blocked role) is to (paraphrasing) perform more blocking across more areas across WordPress.org, when that role gets applied, by doing something like mangling user data (like email addresses & passwords) so that the people using the offending the account can no longer use it anywhere.
So, you're on the right track, just a little bit behind. 👍
#8
in reply to:
↑ 7
@
5 years ago
Replying to johnjamesjacoby:
I know bbPress already has that role, but as you say its for blocking access to the forums. As mucking with the user data will cause an email to be sent, I think adding a new role (or modifying the user's capabilites) will be a better option, and would net result in an email being sent.
#9
follow-up:
↓ 10
@
5 years ago
mucking with the user data will cause an email to be sent
It will only trigger an email if the WordPress Core functions are used.
New functions can be written (like @Ipstenu linked to above in old bbPress) that can alter user data without sending emails or notifications, and without creating a new "Blocked" role like you recommended be done above.
(A new role by itself does not effectively prevent an account from being accessed. The user of the account can still perform a password reset, log into it, and easily recover the account - they'll just have whatever capabilities the Role does or does not provide.)
The goal in this issue (paraphrasing) is to invent a way to lock an account permanently. A new role on a single site is not enough here. WordPress.org is comprised of many sites, many multisite networks, and several non WordPress platforms.
#10
in reply to:
↑ 9
@
5 years ago
Replying to johnjamesjacoby:
Oh OK. My understanding of it was to block accesss for all the sites connected via SSO. So the easiest fix right now is to re-implement something similar to what was done in bbPress 1.X.
(PS: The role I mentioned could prevent access by removing all the user's capabilities, clearing all sessions, and checking for that role upon login).
#11
follow-up:
↓ 12
@
5 years ago
My understanding of it was to block accesss for all the sites connected via SSO
It is.
The role I mentioned could prevent access by removing all the user's capabilities, clearing all sessions, and checking for that role upon login
It is not that simple. All of those things are possible (and will likely happen) but the role is mostly irrelevant.
WordPress Roles are per-site. There is no such thing as an "installation wide" user role. There is no code to check if a user is blocked on any of dozens of sites that currently exist.
(For example, if I block a user from bbPress.org, they are not automatically blocked on WordPress.org.)
Code needs to written to prevent access globally, across everything, no matter what WordPress Site they are misbehaving on, and a single Role on a single Site cannot do that by itself.
#12
in reply to:
↑ 11
@
5 years ago
Replying to johnjamesjacoby:
Thanks for the explanation. I was assuming they were controlled from one source based on wporg-sso.
#13
@
5 years ago
I've opened a ticket on the bbPress Trac with a patch: https://bbpress.trac.wordpress.org/ticket/3262
This ticket was mentioned in Slack in #meta by pierlo. View the logs.
5 years ago
#15
follow-up:
↓ 16
@
5 years ago
Looking at the post, I said the wrong thing.
When an account its blocked, the email is changed.
That should be
When an account its blocked, the PASSWORD is changed.
:facepalm:
Mangling the password would be all that's needed. If we trash the email, then they can make a new account with the same email. Changing the passwords will effectively break sessions, so that's why we would want that.
The goal in this issue (paraphrasing) is to invent a way to lock an account permanently.
The goal in this issue was to make it easier for moderators to ban problematic people without having to remember the extra step of "Oh and ALSO do this to the passwords." Changing a user to blocked is sufficient, because we wrote code the other way in many places. That is, we check on the role for the forums. So an example is plugins. If your account is blocked on the forums, you can't be added to a plugin's committer list and you can't submit new plugins. This is because we know that if someone's blocked on forums, there's a reason.
Locking permanently is a larger issue, but if we can ensure a blocked user is logged out and can't log back in as THAT account, then it minimizes human error on our end and prevents the problematic user from reusing THAT account.
Can they make a new one? Of course. But that should never be an excuse to not do something :) People who are going to make multiple accounts would be a problem anyway, and that needs a totally separate kind of solution.
Perma-sitewide locking would be cool. It would need to loop in a lot of things like auto-closing all plugins/themes and revoking SVN access. :)
#16
in reply to:
↑ 15
@
5 years ago
Replying to Ipstenu:
That's exactly what my patch does, feel free to try it :)
#17
@
5 years ago
Thanks for the extra context & clarification @Ipstenu 🤝
Thanks @pierlo for the bbPress patch. I will be making time to review it sometime this (holiday) weekend. 👍
This ticket was mentioned in Slack in #forums by clorith. View the logs.
4 years ago
#21
@
4 years ago
- Priority changed from normal to highest omg bbq
- Type changed from enhancement to defect
#25
@
4 years ago
Okay, user sessions should no longer work when a user is blocked.
I've also added an automated user log entry when a user is blocked, it's not as great as it should be, it's purely Forum role changed to bbp_blocked.
See this test user dd256.
I'd like to enable adding a user note from the edit page, or to append that to a note made in the last 5 minutes, but didn't quite get there.
Here's how 1.x did it:
https://bbpress.trac.wordpress.org/browser/branches/1.2/bb-includes/functions.bb-pluggable.php#L537
(Thank you Nacin)