Fixing a bug and adding support for custom save device days and latest RC elastic skin.

[InputOutputZ]

This is a PR for following proposed changes:-

Adding support for custom save device with X days
Adding support for recent Roundcube elastic skin, i.e. tested on 1.6.
Fixed the bug when activated already TFA is not being reflected in the settings.
Adding support for customised QR image colour in config options.

[Sp1l]

Doesn't work for me. I get an error on the plugin settings page

Oops... something went wrong!

An internal error has occurred. Your request cannot be processed at this time.
For administrators: Please check the application and/or server error logs for more information.

Merged config changes in config.inc.php

//$rcmail_config['allow_save_device_30days'] = true;
$rcmail_config['allow_save_device_xdays'] = true;
$rcmail_config['save_device_xdays'] = 30;
$rcmail_config['qr_image_colour'] = "#000000";

FreeBSD 13.1 / PHP 8.1.9 / Roundcube 1.6.0

Can't find errors in the error.log or php-fpm logs

[InputOutputZ]

I have tested on php 8.1.3 and RC 1.6 and it works fine, anyhow I will carry more tests tomorrow and will keep you updated.

For the time being, I recommend to debug with following method, try to use log_error(print_r($anyvariable,true)) and figure out at which line the plugin breaks, also, try to check web server logs and RC logs/errors.log and I would appreciate if you can post any found errors, with thanks.

Zakaria.

[InputOutputZ]

Doesn't work for me. I get an error on the plugin settings page

Oops... something went wrong!

An internal error has occurred. Your request cannot be processed at this time. For administrators: Please check the application and/or server error logs for more information.

Merged config changes in config.inc.php

//$rcmail_config['allow_save_device_30days'] = true;
$rcmail_config['allow_save_device_xdays'] = true;
$rcmail_config['save_device_xdays'] = 30;
$rcmail_config['qr_image_colour'] = "#000000";

FreeBSD 13.1 / PHP 8.1.9 / Roundcube 1.6.0

Can't find errors in the error.log or php-fpm logs

I made few changes, not sure if they will fix the bug, but I recommend to give it a go 👍 .

Also, make sure to set $rcmail_config['enable_ua2fa'] to false, if you are not using users_allowed_2fa option, or comment it out its enabling configuration. Also, make sure to adjust the language you use to reflect the following modified label:- $labels['dont_ask_me_xdays'] = 'Don't ask me codes again on this computer for % days'. Also, when cloning the repo, make sure to run git checkout patch-1.

Good luck.

Zakaria.

[Sp1l]

I made few changes, not sure if they will fix the bug, but I recommend to give it a go 👍 .

Thanks! Testing!

Also, make sure to set $rcmail_config['enable_ua2fa'] to false, if you are not using users_allowed_2fa option, or comment it out its enabling configuration. Also, make sure to adjust the language you use to reflect the following modified label:- $labels['dont_ask_me_xdays'] = 'Don't ask me codes again on this computer for % days'. Also, when cloning the repo, make sure to run git checkout patch-1.

Naturally added your repo to the remotes and pulled the patch-1 branch. Using that to generate diffs I can add to the FreeBSD port.

No luck with this yet, still get that same fatal error message. Any clue how I can enable more logs (without resorting to PHP xdebug)?

Whilst you're at it: $config['log_dir'] = '/var/log/roundcube/';

Should be able to reuse that as prefix in

private $_logs_file = '/logs/log_errors_2FA.txt';
...
        // log error into $_logs_file directory
        private function __logError() {
                file_put_contents(realpath(".").$this->_logs_file, date("Y-m-d H:i:s")."|".$_SERVER['HTTP_X_FORWARDED_FOR']."|".$_SERVER['REMOTE_ADDR']."\n", FILE_APPEND);
        }

[Sp1l]

Found and enabled the debug_logger plugin. Not much more info to be found.

[29-Aug-2022 17:26:35 +0200] start: Action: plugin.twofactor_gauthenticator. Task: settings.
[29-Aug-2022 17:26:35 +0200] end: Action: plugin.twofactor_gauthenticator. Task: settings.  - 0.0071 seconds

[InputOutputZ]

With that being said, I recommend this hacky way, open twofactor_gauthenticator.php and in each method at the beginning add

error_log(print_r("The plugin works fine so far ".$anyvariable,true))

Keep moving logging call down the lines until you find at which point the plugin breaks i.e. you wont find any new logs. I would appreciate if you can send over the results.

With thanks.

Zakaria.

[Sp1l]

Got something, added

ini_set('display_errors', 1);

early in the plugin, now I have something useful

Warning: Undefined array key "twofactor_gauthenticator" in /usr/local/www/roundcube/plugins/twofactor_gauthenticator/twofactor_gauthenticator.php on line 451

Fatal error: Uncaught TypeError: array_key_exists(): Argument #2 ($array) must be of type array, null given in /usr/local/www/roundcube/plugins/twofactor_gauthenticator/twofactor_gauthenticator.php:291 Stack trace: #0 /usr/local/www/roundcube/program/include/rcmail_output_html.php(1484): twofactor_gauthenticator->twofactor_gauthenticator_form(Array) #1 [internal function]: rcmail_output_html->xml_command(Array) #2 /usr/local/www/roundcube/program/include/rcmail_output_html.php(1322): preg_replace_callback('/<roundcube:([-...', Array, '<roundcube:incl...') #3 /usr/local/www/roundcube/program/include/rcmail_output_html.php(825): rcmail_output_html->parse_xml('<roundcube:incl...') #4 /usr/local/www/roundcube/program/include/rcmail_output_html.php(654): rcmail_output_html->parse('plugin', false) #5 /usr/local/www/roundcube/plugins/twofactor_gauthenticator/twofactor_gauthenticator.php(232): rcmail_output_html->send('plugin') #6 /usr/local/www/roundcube/program/lib/Roundcube/rcube_plugin_api.php(575): twofactor_gauthenticator->twofactor_gauthenticator_init() #7 /usr/local/www/roundcube/program/include/rcmail.php(248): rcube_plugin_api->exec_action('plugin.twofacto...') #8 /usr/local/www/roundcube/index.php(278): rcmail->action_handler() #9 {main} thrown in /usr/local/www/roundcube/plugins/twofactor_gauthenticator/twofactor_gauthenticator.php on line 291

[Sp1l]

There you go! Fixes the error if no twofactor_gauthenticator element is found in the user's prefs.

diff --git a/twofactor_gauthenticator.php b/twofactor_gauthenticator.php
index 20fa225..978284c 100644
--- a/twofactor_gauthenticator.php
+++ b/twofactor_gauthenticator.php
@@ -445,7 +445,11 @@ class twofactor_gauthenticator extends rcube_plugin
                $user = $rcmail->user;

                $arr_prefs = $user->get_prefs();
-               return $arr_prefs['twofactor_gauthenticator'];
+               if ( array_key_exists('twofactor_gauthenticator') ) {
+                       return $arr_prefs['twofactor_gauthenticator'];
+               } else {
+                       return [];
+               }
        }

        // we can set array to NULL to remove

[Sp1l]

To update all localizations in one go, you can run

sed -i .orig 's/30days/xdays/;s/30/%/' localization/*.inc

[InputOutputZ]

There you go! Fixes the error if no twofactor_gauthenticator element is found in the user's prefs.

diff --git a/twofactor_gauthenticator.php b/twofactor_gauthenticator.php
index 20fa225..978284c 100644
--- a/twofactor_gauthenticator.php
+++ b/twofactor_gauthenticator.php
@@ -445,7 +445,11 @@ class twofactor_gauthenticator extends rcube_plugin
                $user = $rcmail->user;

                $arr_prefs = $user->get_prefs();
-               return $arr_prefs['twofactor_gauthenticator'];
+               if ( array_key_exists('twofactor_gauthenticator') ) {
+                       return $arr_prefs['twofactor_gauthenticator'];
+               } else {
+                       return [];
+               }
        }

        // we can set array to NULL to remove

I think you've missed the array in array_key_exists parameter, given this is its format.

Anyhow, I've just submitted this fix along in patch-1, and thanks for it as well as the sed tip, its very much appreciated.

[InputOutputZ]

By the way, this is how it looked like before:-

Web Mode

Mobile Mode

After:-

Web Mode

Mobile Mode

With thanks.

Zakaria.

[Sp1l]

I think you've missed the array in array_key_exists parameter, given this is its format.

Anyhow, I've just submitted this fix along in patch-1, and thanks for it as well as the sed tip, its very much appreciated.

Yep, was monkey-patching that in live env and replicating back to the git repo.

Thanks for bringing this plugin up-to-date!!

[Sp1l]

The recovery codes don't show for me...

Improved logging a bit

diff --git a/twofactor_gauthenticator.php b/twofactor_gauthenticator.php
index 83f102d..6f3742b 100644
--- a/twofactor_gauthenticator.php
+++ b/twofactor_gauthenticator.php
@@ -19,8 +19,9 @@ class twofactor_gauthenticator extends rcube_plugin
 {
        private $_number_recovery_codes = 4;

-        // relative from RC home dir, not plugin directory
-        private $_logs_file = '/logs/log_errors_2FA.txt';
+        // relative to $config['log_dir'], not plugin directory
+        private $_log_dir = '.';
+        private $_log_file = 'twofactor_gauthenticator.txt';

     function init()
     {
@@ -32,7 +33,8 @@ class twofactor_gauthenticator extends rcube_plugin
        $this->add_hook('render_page', array($this, 'popup_msg_enrollment'));

        $this->load_config();
-
+       $this->_log_dir = realpath($rcmail->config->get('log_dir','.')).DIRECTORY_SEPARATOR;
+
                $allowedPlugin = $this->__pluginAllowedByConfig();

                // skipping all logic and plugin not appears
@@ -573,8 +575,8 @@ class twofactor_gauthenticator extends rcube_plugin


         // log error into $_logs_file directory
-        private function __logError() {
-                file_put_contents(realpath(".").$this->_logs_file, date("Y-m-d H:i:s")."|".$_SERVER['HTTP_X_FORWARDED_FOR']."|".$_SERVER['REMOTE_ADDR']."\n", FILE_APPEND);
+        private function __logError($msg = '') {
+                file_put_contents($this->_log_dir.$this->_log_file, date("Y-m-d H:i:s")."|".$_SERVER['HTTP_X_FORWARDED_FOR']."|".$_SERVER['REMOTE_ADDR']."|".$msg."\n", FILE_APPEND);
         }

 }

[InputOutputZ]

I recommend to check browser console, it could be a Javascript issue or another array_key_exists missing, anyhow it works for me just fine.

[alexandregz]

I try to check with your code but code is incorrect every time I try to config with all fields.

Do you have test this?

[InputOutputZ]

I try to check with your code but code is incorrect every time I try to config with all fields.

Do you have test this?

Yes, I did test this on RC 1.6 and it works. Please try after recent commit. Since I made several changes from first commit.

[InputOutputZ]

An update, I just fixed one tiny bug in X days.

[Sp1l]

Hi,

@InputOutputZ Can you please fix the extraneous ( in https://github.com/InputOutputZ/twofactor_gauthenticator/blob/patch-1/twofactor_gauthenticator.php#L242?
and also complete the localization fix sed -i .orig 's/30days/xdays/;s/30/%/' localization/*.inc?

@alexandregz Saw your comment on your failing test(s), can you elaborate? Really would like to see a new release for this.

[InputOutputZ]

Hi,

@InputOutputZ Can you please fix the extraneous ( in https://github.com/InputOutputZ/twofactor_gauthenticator/blob/patch-1/twofactor_gauthenticator.php#L242? and also complete the localization fix sed -i .orig 's/30days/xdays/;s/30/%/' localization/*.inc?

@alexandregz Saw your comment on your failing test(s), can you elaborate? Really would like to see a new release for this.

Just fixed it. Mind me, for localisation, I will leave it for @alexandregz to look after, since the code you provided works only for English localisation.

Zakaria.

[InputOutputZ]

Tiny bug fix. Email address is set to undefined in the authenticator app.