make it harder to brute-force a password via blogapi and any other non-standard login routes

Makes a lot of sense.

The watchdog entry is awesome. I'd also like to suggest a new $op for hook_user(). Currently, there's no way for contrib modules to react to failures in user_authenticate(). This is a problem because while people have a hard time agreeing on the right response to brute-force attempts (tarpit? captcha? ban the IP? Ban the user? etc), lots of people want to do something about it.

A patch was posted in an old issue, see
http://drupal.org/node/117056#comment-1179721

Thoughts?

OK, fair enough, but shouldn't the watchdog message originate from user_authenticate()? blogapi isn't the only module that auths users without using the user_login form (services module, etc).

Also realized we don't really need another hook_user($op) anyway, once this gets in a contrib module that wanted to react to "login failed" could capture the message with hook_watchdog and use that as a trigger. It'd be a tad indirect but the possibility exists.

Committed this to Drupal 6. Looking at D7, while blogapi is not there, the latest patch has nothing to do with blogapi per say, better logging with blogapi is just a side-effect. I believe moving the logging to the more general place could be useful in D7 as well.

How about also sharing the flood control in user_login_authenticate_validate()?

Good question - should all authentication mechanisms have the same flood control - or do we leave it up to each method?

I'm inclined to agree with #13. Will work on a revised patch.

Here's a new patch that decouples the flood protection from the form API.

Because the flood check needs to return multiple values, it's not possible to roll all this into user_authenticate(). So there's a new function to test the flood limits. In a way this is beneficial as contrib authors aren't forced into adopting the flood control if they don't want it.

A contrib module can simply call:

if (!user_flood_control($name) && user_authenticate($name, $password)) { // Wheee! }

And get watchdog logging an flood protection without having to re-invent it all.

There are no tests in this patch however the login and flood system seems to have pretty thorough coverage in user.test. There are no functional changes, just re-organizing code.

I'd like to revive this... anyone up for a review? If the consensus is that #16 is too late this close to beta, #12 still applies and is a straight port of what was committed to D6.

Patches still apply with small offset.

re: #16, I think it needs work because the flood_clear_event should be outside the user_authenticate if the call to user_flood_control() is outside it.

Should this be a Major ticket because it affects security?

I don't mind if you change the priority - though a re-roll incorporating pwolanin's feedback would probably do more to move this forward.

Moving to 8.x-dev first.

I recently had issues with several sites (all completely different clients) coming under heavy bruteforce login attacks, and this was the resulting solution ;)

https://www.drupal.org/project/cbp
"What is Crowd Bruteforce Protection?
Well, it's exactly what it says, it uses the Crowd Sourcing mentality for protecting your website, wikipedia explains it best: http://en.wikipedia.org/wiki/Crowdsourcing Currently it protects against Bruteforce logins and bans any IP that tries to login to your website X times"

It does much more than that short blurb above of course, but if your interested in reading more, you can read the project page, it does use a crowdsourcing idea for banning beyond just your own website. It makes as much use as possible from drupal's built in flood control and ip blocking and does not implement its own so that it could work with other modules that also utilize these drupal features if there even are any. It might not be the right solution for others, but for me it was perfect. If people start to use it, it'll have other side effects that could only help I think.

@greggles - This is a Core issue, so D8 would be right. Maybe I'm totally missing something though.

I'm also unassigning @grendzy as it's been 4 years.

Right.... Thanks @greggles.