Hi,
I am working on a API built with Drupal 8, had very strange issue the other day which was quite hard to pin down. I deployed the API to our live server and a simple GET api call which works on staging, just returned empty message on live e.g.
http://mysite.com/get/status?_format=json
{
"message":""
}
I reviewed the logs and could see access denied, strange, since same db and code on both servers. After some investigation i realised that the flood table had > 5 entries for my servers IP address.
This is the troublesome code:
// Don't allow login if the limit for this user has been reached.
// Default is to allow 5 failed attempts every 6 hours.
if ($this->flood->isAllowed('basic_auth.failed_login_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) {
$uid = $this->userAuth->authenticate($username, $password);
if ($uid) {
$this->flood->clear('basic_auth.failed_login_user', $identifier);
return $this->entityManager->getStorage('user')->load($uid);
}
else {
// Register a per-user failed login event.
$this->flood->register('basic_auth.failed_login_user', $flood_config->get('user_window'), $identifier);
}
}The problem is that if I am not allowed due to entries in the flood table, there is no else statement to catch this and at least raise a decent entry in the log system. I would prefer it if there was something like this, drupal_set_message('Flood table has >5 entries', ERROR) so this is clear as very hard to figure this out.
Thanks
Comments
Comment #2
zarexogre commentedFYI, to fix the problem I just truncated the flood table and everything worked again, took me about a day to figure this out so be good to save others this time with a nice message!
Comment #3
cilefen commentedThat is a good explanation of the bug. I have looked this issue over and I feel it doesn't meet the requirements for a major priority issue based on the workaround.
Comment #13
longwaveDiscussed with @Lendude during #bugsmash triage and closed #2997847: Basic auth - flood filled, no error but perm issue as duplicate.
There is an underlying issue that the interface does not allow for a message on failure.
AuthenticationProviderInterface::authenticate()only says:So there is nowhere really to inject a suitable message. Maybe we need to extend this to allow exceptions as well? We could then distinguish between errors such as flood conditions, and "normal" authentication failures?
Comment #14
longwave#2431357: Extract a reusable UserAuthFlood service and reuse it in UserLoginForm and BasicAuth and UserAuthenticationController might also go towards solving this, or at least unifying this code.