We are using a mail server for authentication, which is not under our control. Once a while, it has a technical issue and users can't log in. The outages do not usually last long but are confusing to users. I thought to display an additional msg when this happens. I mean something like this: when let's say 10 subsequent users fail to log in, an error message states that there seems to be a problem with authentication server which prevents logins.
A simple scheme would be to track the number of failed logins. I mean having a persistent variable which is reset by each successful login and contains count or list of sequential failed logins. The trick is to count only unique users not login failures in general. Anyone has done that or has an idea how to program the latter easily and effectively in PHP? What I am stuck with is keeping a list of user ids, ensuring that they are unique.
I don't think there is an "easy" way.
What I would do is what you've done. Build a list of failed logins, create a uniqid(), and after 10 set some sort of global variable to express the error.
What isn't working with that?
Well, php is the latest language in my repertoir, and I am still learning it. In my most commonly used language, I would say
if ($userid is not among the lines of $failedloginids) then
put $userid & cr after $failedloginids
...
if (the number of lines of $failedloginids > 10) then
put get_string('invalidlogins') into $errormsg
else
put get_string('invalidlogin') into $errormsg
end if
In php, I guess, I could try to keep concatenating userids with space in-between and use strpos function to check if new current userid is already there.
if (strpos($user->id,$failedloginids) === 0 {
failedloginids .= ' '.$user->id;
}
and get word count through:
$failedloginidsarray = explode(" ", $failedloginids);
$failedlogincount = count($failedloginidsarray);
if ($failedlogincount > 10) {
$errormsg = get_string('invalidlogins');
} else {
$errormsg = get_string('invalidlogin');
}
Is there a smarter way?
if ($userid is not among the lines of $failedloginids) then
put $userid & cr after $failedloginids
...
if (the number of lines of $failedloginids > 10) then
put get_string('invalidlogins') into $errormsg
else
put get_string('invalidlogin') into $errormsg
end if
In php, I guess, I could try to keep concatenating userids with space in-between and use strpos function to check if new current userid is already there.
if (strpos($user->id,$failedloginids) === 0 {
failedloginids .= ' '.$user->id;
}
and get word count through:
$failedloginidsarray = explode(" ", $failedloginids);
$failedlogincount = count($failedloginidsarray);
if ($failedlogincount > 10) {
$errormsg = get_string('invalidlogins');
} else {
$errormsg = get_string('invalidlogin');
}
Is there a smarter way?
Hmm, I thought about this more after posting and realized that the above php code will fail miserably since userid 12 is a substring of userid 124, so I would have to include space before and after userid before doing match with strpos (and init the failedloginsid not with empty but with a space and append not prepend space when concatenating).
There must be a better way to do it in PHP...
There must be a better way to do it in PHP...
In PHP, variables are not maintained between page loads, so when one user loads a page, $failedloginids will be uninitialized. But it should be fairly simple to do it using the database. Just create a table with two columns: the regular id column, and a userid column. (You may also want to include a timestamp column, and expire rows when they're 'too old'.) When a user fails to log in, add a row to the table (if the user isn't already there). Then $failedlogincount is just the number of rows in the table.
I don't like the idea of hitting database with that, not to mention that it complicates coding. Keeping the variables persistent is not a problem. I showed only the code fragments that I have issues with.
Information about failed logins is also stored in the moodle log table, so you could just query this:
$sql = "SELECT DISTINCT COUNT('x') FROM {log} WHERE time > EXTRACT(epoch FROM NOW() - INTERVAL '10 minutes') AND module = 'login' AND action = 'error'"; $failedcount = $DB->count_records_sql($sql); if ($failedcount > $threshold) { $errormsg = get_string('invalidlogins'); } else { $errormsg = get_string('invalidlogin'); }
This requires one database call and relies on usernames rather than ids, but requires the fewest changes and least complications.
Andrew
Warning: postgres (and perhaps others) are likely to ignore the 'time' index with the above formulation as the result of that extract operation is a float and the column is int. At a minimum it will need a cast, or, in a more moodle-ish style:
$sql = "SELECT COUNT(*) FROM {log} WHERE time > :time AND module = :mod AND action = :act";
$count = $DB->count_records_sql($sql, array('time'=>time()-600, 'mod'=>'login', 'act'=>'error'));
edit: You might also find this is too expensive/contended to run on every login page - it could be computed in cron instead and a config value set which determines if the warning is displayed.
Thanks Tony,
The above does work on Postgres, but probably wouldn't on Oracle or MSSQL.
As I understood it, it was only intended to run on a failed login so it shouldn't be run on every page. I realise that the log table is not a great one to query and it would probably be worth applying a bit of thought as to the order of the WHERE clauses.
Andrew
In any sane planner, just the order of the where clause should make no difference and /ideally/ it should be able to recast types where it is safe. Your query does run without /error/ on postgres but for large log tables may be grossly inefficient due to use of scan by module/action rather than time:
=> explain SELECT DISTINCT COUNT('x') FROM mdl_log WHERE time > EXTRACT(epoch FROM NOW() - INTERVAL '10 minutes') AND module = 'login' AND action = 'error';
QUERY PLAN
---------------------------------------------------------------------------------------------------------------
HashAggregate (cost=2375679.89..2375679.90 rows=1 width=0)
-> Aggregate (cost=2375679.88..2375679.89 rows=1 width=0)
-> Index Scan using mdl_log_coumodact_ix on mdl_log (cost=0.00..2375679.87 rows=4 width=0)
Index Cond: (((module)::text = 'login'::text) AND ((action)::text = 'error'::text))
Filter: (("time")::double precision > date_part('epoch'::text, (now() - '00:10:00'::interval)))
(5 rows)
=> explain SELECT DISTINCT COUNT('x') FROM mdl_log WHERE time > EXTRACT(epoch FROM NOW() - INTERVAL '10 minutes')::integer AND module = 'login' AND action = 'error';
QUERY PLAN
----------------------------------------------------------------------------------------------------------
HashAggregate (cost=708.97..708.98 rows=1 width=0)
-> Aggregate (cost=708.96..708.97 rows=1 width=0)
-> Index Scan using mdl_log_tim_ix on mdl_log (cost=0.01..708.95 rows=1 width=0)
Index Cond: ("time" > (date_part('epoch'::text, (now() - '00:10:00'::interval)))::integer)
Filter: (((module)::text = 'login'::text) AND ((action)::text = 'error'::text))
(5 rows)
Thanks for idea. Unfortunately, this does not properly address the problem at hand. First, I can't rely of having enough logins in whatever time span I set to search db for failures. Second, there may be enough failed logins for a number of reasons but with successful logins in between (I gather the query could be amended to consider that but this increases the load). Third, the message should go right away if one login succeeds.
The symptom of authentication server not working is that all logins are failing regardless of the time passed.
The symptom of authentication server not working is that all logins are failing regardless of the time passed.
Maybe instead of looking for login failures you need to be actively monitoring the remote service for availability and display the message dependent on that. This would let you pre-emptively display the warning on the login page or whatever other action.
If you're running cron every minute you could even do that as a local plugin cron. But there are other pieces of software around that do this kind of thing better...
edit: and not prone to false positives from forgotten passwords, intrusion attempts etc...
We just had another authentication server glitch, which reminded me of this old thread. Here is an update.
I have implemented my original idea using an additional database table with a single record. The hack has been working nicely for a year now (Moodle 1.9) without a visible impact on performance and real benefit for users and myself (no more dozens of "I can't login" emails).
Basically, I keep track of consecutive login failures (different users only) and if their count is more than preset threshold, users get a different error message indicating that there may be problem with the central server and they should come back later.
I have implemented my original idea using an additional database table with a single record. The hack has been working nicely for a year now (Moodle 1.9) without a visible impact on performance and real benefit for users and myself (no more dozens of "I can't login" emails).
Basically, I keep track of consecutive login failures (different users only) and if their count is more than preset threshold, users get a different error message indicating that there may be problem with the central server and they should come back later.