login rate limiting sends local time to database (postgres) which causes undesired effects

[scott323]

My postgres db is set to use UTC as default timezone.
My system (and PHP) is set to use Europe/Bratislava as default timezone (currently it's UTC +01:00).
Column definition for users.failed_login is "timestamp with time zone", but roundcube omits timezone when setting it's value, which results in incorrect calculations in rate limiting function in rcube_user->failed_login().

Proposed (and tested) solution is simple patch:

--- roundcubemail/program/lib/Roundcube/rcube_db_pgsql.php.orig 2017-03-25 17:43:01.713984686 +0100
+++ roundcubemail/program/lib/Roundcube/rcube_db_pgsql.php      2017-03-25 17:43:39.654363537 +0100
@@ -231,4 +231,12 @@
         return $sql;
     }
 
+    /**
+     * Use ISO 8601 date (includes timezone info)
+     */
+    public function fromunixtime($timestamp)
+    {
+        return date("'c'", $timestamp);
+    }
+
 }

[alecpl]

I'm afraid that's only one of many issues that you can hit when database uses different timezone than Roundcube. We do have a check for this in the installer. So, probably not something that we can fix easily.

[alecpl]

For MySQL we use datetime type which ignores timezone. So, I think the best solution here would be to convert all columns to timestamp without time zone.

[thomascube]

AFAIK the datetime types in MySQL are not timezone-aware and therefore we require the database to use the same timezone as the Roundcube webserver. For other database backends which are timezone-aware, we should produce datetime values that include timezone information of the webserver. I suppose that's what @scott323 suggests here and I think that's the right approach.

[alecpl]

@thomascube If we set timezone on input to db we'll need to make sure to handle it correctly on output. I tried to skip this requirement if possible and find a "global" solution. I was afraid we'll have to modify some more code, but maybe it's not that bad.

[alecpl]

While it could be fixed this way:

--- a/program/lib/Roundcube/rcube_db_pgsql.php
+++ b/program/lib/Roundcube/rcube_db_pgsql.php
@@ -37,6 +52,7 @@ class rcube_db_pgsql extends rcube_db
     protected function conn_configure($dsn, $dbh)
     {
         $dbh->query("SET NAMES 'utf8'");
+        $dbh->query("SET TIME ZONE '" . date_default_timezone_get() . "'");
     }
.
     /**

I have a feeling this may be fragile. So, I'm going to fix this in a proposed way, though slightly improved.

[alecpl]

Fixed. Though I wouldn't apply the fix to a stable branch yet.