0013328: protect applications with second factor
authorPhilipp Schüle <p.schuele@metaways.de>
Mon, 10 Jul 2017 13:43:06 +0000 (15:43 +0200)
committerPhilipp Schüle <p.schuele@metaways.de>
Thu, 13 Jul 2017 12:45:29 +0000 (14:45 +0200)
* check second factor for configured apps in all
 controller actions with _checkRight()
* deactivate/mask application if pin was wrong

https://forge.tine20.org/view.php?id=13328

Change-Id: I56c982651c3c4d0014a586204cbfea886a50a4ce
Reviewed-on: http://gerrit.tine20.com/customers/5077
Tested-by: Philipp Schüle <p.schuele@metaways.de>
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
25 files changed:
tests/tine20/Tinebase/AuthTest.php
tine20/Admin/Controller/Container.php
tine20/Admin/Controller/Tags.php
tine20/Calendar/Controller/Resource.php
tine20/Felamimail/Controller/Account.php
tine20/Filemanager/Controller/DownloadLink.php
tine20/Sales/Controller/Customer.php
tine20/Sales/Controller/Invoice.php
tine20/Sales/Controller/Product.php
tine20/Sales/Controller/PurchaseInvoice.php
tine20/Sales/Controller/Supplier.php
tine20/Timetracker/Controller/Timeaccount.php
tine20/Tinebase/Auth.php
tine20/Tinebase/Auth/SecondFactor/Abstract.php
tine20/Tinebase/Config.php
tine20/Tinebase/Controller/Record/Abstract.php
tine20/Tinebase/Exception/SecondFactorRequired.php [new file with mode: 0644]
tine20/Tinebase/Frontend/Json.php
tine20/Tinebase/Tinebase.jsb2
tine20/Tinebase/js/ExceptionHandler.js
tine20/Tinebase/js/tineInit.js
tine20/Tinebase/js/ux/WindowFactory.js
tine20/Tinebase/js/widgets/MainScreen.js
tine20/Tinebase/js/widgets/dialog/PasswordDialog.js
tine20/Tinebase/js/widgets/dialog/SecondFactorDialog.js [new file with mode: 0644]

index 299cf0a..00a7a8c 100644 (file)
@@ -234,4 +234,40 @@ class Tinebase_AuthTest extends TestCase
         ));
         $this->assertEquals(Tinebase_Auth::SUCCESS, $result);
     }
+
+    /**
+     * @see 0013328: protect applications with second factor
+     */
+    public function testSecondFactorAppProtection()
+    {
+        // set configs
+        Tinebase_Config::getInstance()->set(Tinebase_Config::SECONDFACTORPROTECTEDAPPS, array(
+            'Tasks'
+        ));
+        Tinebase_Config::getInstance()->set(Tinebase_Config::AUTHENTICATIONSECONDFACTOR, array(
+            'active' => true,
+            'provider' => 'Tine20',
+        ));
+
+        // set pin
+        $user = Tinebase_Core::getUser();
+        Tinebase_User::getInstance()->setPin($user, '1234');
+
+        // try to access app
+        try {
+            $tasks = Tasks_Controller_Task::getInstance()->getAll();
+            self::fail('it should not be possible to access app without PIN');
+        } catch (Tinebase_Exception $te) {
+            // check exception
+            self::assertTrue($te instanceof Tinebase_Exception_SecondFactorRequired);
+        }
+
+        // validate pin
+        $json = new Tinebase_Frontend_Json();
+        $json->validateSecondFactor('1234');
+
+        // try to access app again
+        $result = Tasks_Controller_Task::getInstance()->getAll();
+        self::assertGreaterThanOrEqual(0, count($result));
+    }
 }
index c944eb9..dcf1a94 100644 (file)
@@ -328,5 +328,7 @@ class Admin_Controller_Container extends Tinebase_Controller_Record_Abstract
             default;
                break;
         }
+
+        parent::_checkRight($_action);
     }
 }
index c3abaad..6e22c0d 100644 (file)
@@ -202,5 +202,7 @@ class Admin_Controller_Tags extends Tinebase_Controller_Record_Abstract
             default;
                break;
         }
+
+        parent::_checkRight($_action);
     }
 }
index cd389f1..475f314 100644 (file)
@@ -207,6 +207,8 @@ class Calendar_Controller_Resource extends Tinebase_Controller_Record_Abstract
             default;
                break;
         }
+
+        parent::_checkRight($_action);
     }
     
     /**
index e140646..9512d86 100644 (file)
@@ -483,6 +483,8 @@ class Felamimail_Controller_Account extends Tinebase_Controller_Record_Abstract
             default;
                break;
         }
+
+        parent::_checkRight($_action);
     }
     
     /**
index 7f9a977..d2c0a49 100644 (file)
@@ -116,6 +116,8 @@ class Filemanager_Controller_DownloadLink extends Tinebase_Controller_Record_Abs
             default;
             break;
         }
+
+        parent::_checkRight($_action);
     }
     
     /**
index cc7a8a9..84a6eef 100644 (file)
@@ -215,5 +215,7 @@ class Sales_Controller_Customer extends Sales_Controller_NumberableAbstract
             default;
             break;
         }
+
+        parent::_checkRight($_action);
     }
 }
index 736fa39..0ff35ad 100644 (file)
@@ -1604,6 +1604,8 @@ class Sales_Controller_Invoice extends Sales_Controller_NumberableAbstract
             default;
             break;
         }
+
+        parent::_checkRight($_action);
     }
 
     /**
index 90c450e..a4d21ea 100644 (file)
@@ -132,6 +132,8 @@ class Sales_Controller_Product extends Sales_Controller_NumberableAbstract
             default;
                break;
         }
+
+        parent::_checkRight($_action);
     }
 
     /**
index f11bd3d..d0f94e1 100644 (file)
@@ -339,5 +339,7 @@ class Sales_Controller_PurchaseInvoice extends Sales_Controller_NumberableAbstra
             default;
             break;
         }
+
+        parent::_checkRight($_action);
     }
 }
index cb617c4..0b55e33 100644 (file)
@@ -192,5 +192,7 @@ class Sales_Controller_Supplier extends Sales_Controller_NumberableAbstract
             default;
             break;
         }
+
+        parent::_checkRight($_action);
     }
 }
index 1877502..ae9dd2e 100644 (file)
@@ -202,8 +202,8 @@ class Timetracker_Controller_Timeaccount extends Tinebase_Controller_Record_Abst
         if (! $hasRight) {
             throw new Tinebase_Exception_AccessDenied('You are not allowed to ' . $_action . ' timeaccounts.');
         }
-         
-        return;
+
+        return parent::_checkRight($_action);
     }
     
     public function doGrantChecks()
index ebdeff0..422c0c9 100755 (executable)
@@ -444,8 +444,15 @@ class Tinebase_Auth
      * @return int
      * @throws Tinebase_Exception_Backend
      */
-    public static function validateSecondFactor($username, $password, $options)
+    public static function validateSecondFactor($username, $password, $options = null)
     {
+        if (! $options) {
+            $options = Tinebase_Config::getInstance()->get(
+                Tinebase_Config::AUTHENTICATIONSECONDFACTOR,
+                new Tinebase_Config_Struct()
+            )->toArray();
+        }
+
         if (isset($options['provider'])) {
             $authProviderClass = 'Tinebase_Auth_SecondFactor_' . $options['provider'];
             if (class_exists($authProviderClass)) {
index 2fab36a..7fdb67e 100644 (file)
@@ -5,7 +5,7 @@
  * @package     Tinebase
  * @subpackage  Auth
  * @license     http://www.gnu.org/licenses/agpl.html AGPL Version 3
- * @copyright   Copyright (c) 2016 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @copyright   Copyright (c) 2016-2017 Metaways Infosystems GmbH (http://www.metaways.de)
  * @author      Philipp Schüle <p.schuele@metaways.de>
  */
 abstract class Tinebase_Auth_SecondFactor_Abstract
@@ -16,6 +16,40 @@ abstract class Tinebase_Auth_SecondFactor_Abstract
     {
         $this->_options = $options;
     }
-    
+
+    /**
+     * validate second factor
+     *
+     * @param $username
+     * @param $password
+     * @return mixed
+     */
     abstract public function validate($username, $password);
+
+    /**
+     * @param int $lifetimeMinutes
+     * @throws Exception
+     * @throws Zend_Session_Exception
+     */
+    public static function saveValidSecondFactor($lifetimeMinutes = 15)
+    {
+        Tinebase_Session::getSessionNamespace()->secondFactorValidUntil =
+            Tinebase_DateTime::now()->addMinute($lifetimeMinutes)->toString();
+    }
+
+    /**
+     * @return bool
+     * @throws Exception
+     * @throws Zend_Session_Exception
+     */
+    public static function hasValidSecondFactor()
+    {
+        $currentValidUntil = Tinebase_Session::getSessionNamespace()->secondFactorValidUntil;
+        if ($currentValidUntil) {
+            $validUntil = new Tinebase_DateTime($currentValidUntil);
+            return Tinebase_DateTime::now()->isEarlier($validUntil);
+        } else {
+            return false;
+        }
+    }
 }
index 0ff7b78..84da8ae 100644 (file)
@@ -42,6 +42,13 @@ class Tinebase_Config extends Tinebase_Config_Abstract
     const AUTHENTICATIONSECONDFACTOR = 'Tinebase_Authentication_SecondFactor';
 
     /**
+     * authentication second factor protection for apps
+     *
+     * @var string
+     */
+    const SECONDFACTORPROTECTEDAPPS = 'Tinebase_Authentication_SecondFactor_Protected_Apps';
+
+    /**
      * save automatic alarms when creating new record
      *
      * @var string
@@ -696,6 +703,16 @@ class Tinebase_Config extends Tinebase_Config_Abstract
             'setByAdminModule'      => FALSE,
             'setBySetupModule'      => TRUE,
         ),
+        self::SECONDFACTORPROTECTEDAPPS => array(
+            //_('Second Factor Protected Applications')
+            'label'                 => 'Second Factor Protected Applications',
+            //_('Second Factor Authentication is needed to access these applications')
+            'description'           => 'Second Factor Authentication is needed to access these applications',
+            'type'                  => 'array',
+            'clientRegistryInclude' => true,
+            'setByAdminModule'      => false,
+            'setBySetupModule'      => true,
+        ),
         self::USERBACKENDTYPE => array(
                                    //_('User Backend')
             'label'                 => 'User Backend',
index 7440212..7f69cf3 100644 (file)
@@ -50,6 +50,13 @@ abstract class Tinebase_Controller_Record_Abstract
     protected $_doRightChecks = TRUE;
 
     /**
+     * only do second factor validation once
+     *
+     * @var boolean
+     */
+    protected $_secondFactorValidated = false;
+
+    /**
      * use notes - can be enabled/disabled by useNotes
      *
      * @var boolean
@@ -1823,15 +1830,45 @@ abstract class Tinebase_Controller_Record_Abstract
     }
 
     /**
-     * overwrite this function to check rights
+     * overwrite this function to check rights / don't forget to call parent
      *
      * @param string $_action {get|create|update|delete}
+     * @return void
      * @throws Tinebase_Exception_AccessDenied
+     * @throws Tinebase_Exception_SecondFactorRequired
      */
     protected function _checkRight(/** @noinspection PhpUnusedParameterInspection */
                                     $_action)
     {
-        return;
+        if (! $this->_doRightChecks) {
+            return;
+        }
+
+        $this->_checkSecondFactor();
+    }
+
+    /**
+     * check second factor
+     *
+     * @throws Tinebase_Exception_SecondFactorRequired
+     */
+    protected function _checkSecondFactor()
+    {
+        if ($this->_secondFactorValidated) {
+            return;
+        }
+
+        // TODO only check with json frontend? maybe we should enable this only from json frontends
+
+        $protectedApps = Tinebase_Config::getInstance()->get(Tinebase_Config::SECONDFACTORPROTECTEDAPPS, array());
+        if (in_array($this->_applicationName, $protectedApps)) {
+            if (! Tinebase_Auth_SecondFactor_Abstract::hasValidSecondFactor()) {
+                throw new Tinebase_Exception_SecondFactorRequired('Second Factor required for application '
+                    . $this->_applicationName);
+            } else {
+                $this->_secondFactorValidated = true;
+            }
+        }
     }
 
     /**
diff --git a/tine20/Tinebase/Exception/SecondFactorRequired.php b/tine20/Tinebase/Exception/SecondFactorRequired.php
new file mode 100644 (file)
index 0000000..65cb37a
--- /dev/null
@@ -0,0 +1,30 @@
+<?php
+/**
+ * Tine 2.0
+ * 
+ * @package     Tinebase
+ * @subpackage  Exception
+ * @license     http://www.gnu.org/licenses/agpl.html AGPL Version 3
+ * @copyright   Copyright (c) 2017 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @author      Philipp Schüle <p.schuele@metaways.de>
+ *
+ */
+
+/**
+ * SecondFactorRequired exception
+ * 
+ * @package     Tinebase
+ * @subpackage  Exception
+ */
+class Tinebase_Exception_SecondFactorRequired extends Tinebase_Exception_SystemGeneric
+{
+    /**
+     * @var string _('Second Factor Required')
+     */
+    protected $_title = 'Second Factor Required';
+
+    public function __construct($_message, $_code=630)
+    {
+        parent::__construct($_message, $_code);
+    }
+}
index 8041e0b..9ab0d25 100644 (file)
@@ -205,6 +205,28 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
     }
 
     /**
+     * validate second factor
+     *
+     * @param $password
+     * @return array
+     * @throws Tinebase_Exception_Backend
+     */
+    public function validateSecondFactor($password)
+    {
+        $user = Tinebase_Core::getUser();
+        $result = Tinebase_Auth::validateSecondFactor($user->accountLoginName, $password);
+        $success = Tinebase_Auth::SUCCESS === $result;
+
+        if ($success) {
+            Tinebase_Auth_SecondFactor_Abstract::saveValidSecondFactor();
+        }
+
+        return array(
+            'success' => $success
+        );
+    }
+
+    /**
      * change pin of user
      *
      * @param  string $oldPassword the old password
index c8ab84e..2f5b809 100644 (file)
           "path": "js/widgets/dialog/"
         },
         {
+          "text": "SecondFactorDialog.js",
+          "path": "js/widgets/dialog/"
+        },
+        {
           "text": "RecordForm.js",
           "path": "js/widgets/form/"
         },
index 5ed964d..4c05972 100644 (file)
@@ -279,7 +279,16 @@ Tine.Tinebase.ExceptionHandler = function() {
                     msg: i18n._('Your user account has no role memberships. Please contact your administrator.')
                 }));
                 break;
-                
+
+            // second factor validation required
+            case 630:
+                // TODO add more actions depending on request?
+                window.postal.publish({
+                    channel: "messagebus",
+                    topic: 'secondfactor.invalid'
+                });
+                break;
+
             // Tinebase_Exception_InvalidRelationConstraints
             case 912: 
                 Ext.MessageBox.show(Ext.apply(defaults, {
index c59b847..37b5a9b 100644 (file)
@@ -174,7 +174,8 @@ Tine.Tinebase.tineInit = {
         };
         postal.fedx.addFilter( [
             { channel: 'thirdparty', topic: '#', direction: 'both' },
-            { channel: 'recordchange', topic: '#', direction: 'both' }
+            { channel: 'recordchange', topic: '#', direction: 'both' },
+            { channel: 'messagebus', topic: '#', direction: 'both' }
             //{ channel: 'postal.request-response', topic: '#', direction: 'both' }
         ] );
         postal.fedx.signalReady();
index 5a52030..10f6804 100644 (file)
@@ -96,10 +96,9 @@ Ext.ux.WindowFactory.prototype = {
                 items: [this.getCenterPanel(c)]
             };
 
-            // we can only handle one window yet
+            // NOTE: is this still true ?? -> we can only handle one window yet
             c.modal = true;
 
-
             win = new Ext.Window(c);
             c.items.items[0].window = win;
         }
@@ -186,7 +185,7 @@ Ext.ux.WindowFactory.prototype = {
      * brings window to front
      */
     getWindow: function (config) {
-        var windowType = (config.modal) ? 'Ext' : this.windowType;
+        var windowType = config.windowType || ((config.modal) ? 'Ext' : this.windowType);
         
         // Names are only allowed to be alnum
         config.name = Ext.isString(config.name) ? config.name.replace(/[^a-zA-Z0-9_]/g, '') : config.name;
index ebe3927..a7a31c7 100644 (file)
@@ -3,7 +3,7 @@
  * 
  * @license     http://www.gnu.org/licenses/agpl.html AGPL Version 3
  * @author      Cornelius Weiss <c.weiss@metaways.de>
- * @copyright   Copyright (c) 2007-2016 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @copyright   Copyright (c) 2007-2017 Metaways Infosystems GmbH (http://www.metaways.de)
  */
 Ext.ns('Tine.widgets');
 
@@ -45,10 +45,78 @@ Tine.widgets.MainScreen = Ext.extend(Ext.Panel, {
     initComponent: function() {
         this.useModuleTreePanel = Ext.isArray(this.contentTypes) && this.contentTypes.length > 1;
         this.initLayout();
+        this.initMessageBus();
 
         Tine.widgets.MainScreen.superclass.initComponent.apply(this, arguments);
     },
 
+    initMessageBus: function() {
+        // check config, only subscribe in protected apps
+        var protectedApps = Tine.Tinebase.configManager.get('Tinebase_Authentication_SecondFactor_Protected_Apps');
+        if (protectedApps && protectedApps.indexOf(this.app.appName) !== -1) {
+            postal.subscribe({
+                channel: "messagebus",
+                topic: 'secondfactor.*',
+                callback: this.onSecondFactorValidation.createDelegate(this)
+            });
+        }
+    },
+
+    /**
+     * validate second factor
+     *
+     * @param data
+     * @param e
+     *
+     * @refactor use/create layer helper
+     */
+    onSecondFactorValidation: function(data, e) {
+        if (! this.rendered) {
+            return;
+        }
+        switch (e.topic) {
+            case 'secondfactor.invalid':
+                // recycle old layer + dialog
+                if (! this.secondfactorDialogLayer) {
+                    this.getEl().mask();
+
+                    this.secondfactorDialog = new Tine.Tinebase.widgets.dialog.SecondFactorDialog();
+                    this.secondfactorDialogLayer = new Ext.Layer({
+                        items: [this.secondfactorDialog]
+                    });
+
+                    // create layer for sf dialog
+                    var layerParent = this.getEl(); // Ext.getDom(this.getLayerParent() || Ext.getBody());
+                    this.secondfactorDialogLayer = new Ext.Layer({
+                        parentEl: layerParent,
+                        shadow: true,
+                        constrain: false
+                    });
+
+                    this.innerLayer = this.secondfactorDialogLayer.createChild({});
+                    this.innerLayer.setWidth(400);
+
+                    this.secondfactorDialog.render(this.innerLayer);
+
+                    var height = 120;
+                    this.innerLayer.dom.style.height = '';
+                    this.innerLayer.setHeight(height);
+
+                    this.secondfactorDialogLayer.beginUpdate();
+                    this.secondfactorDialogLayer.setHeight(height);
+                    this.secondfactorDialogLayer.alignTo(this.getEl(), 'c', [-100, -50]);
+                    this.secondfactorDialogLayer.endUpdate();
+                }
+                this.secondfactorDialogLayer.show();
+
+                break;
+            case 'secondfactor.valid':
+                this.getEl().unmask();
+                this.secondfactorDialogLayer.hide();
+                break;
+        }
+    },
+
     /**
      * returns canonical path part
      * @returns {string}
index ecc0dae..e6e9742 100644 (file)
@@ -24,6 +24,11 @@ Tine.Tinebase.widgets.dialog.PasswordDialog = Ext.extend(Ext.Panel, {
     allowEmptyPassword: false,
 
     /**
+     * dialog provides password generation action
+     */
+    hasPwGen: true,
+
+    /**
      * Password field
      */
     passwordField: null,
@@ -34,7 +39,7 @@ Tine.Tinebase.widgets.dialog.PasswordDialog = Ext.extend(Ext.Panel, {
     initComponent: function () {
         this.addEvents(
             /**
-             * If the dialog will close and a password were choisen
+             * If the dialog will close and a password was chosen
              * @param node
              */
             'passwordEntered'
@@ -91,16 +96,18 @@ Tine.Tinebase.widgets.dialog.PasswordDialog = Ext.extend(Ext.Panel, {
             scope: this
         });
 
-        this.tbar = [
-            this.pwgenAction
-        ];
+        if (this.hasPwGen) {
+            this.tbar = [
+                this.pwgenAction
+            ];
+        }
 
         this.bbar = [
             '->',
             this.okAction
         ];
 
-        Tine.Filemanager.FilePickerDialog.superclass.initComponent.call(this);
+        Tine.Tinebase.widgets.dialog.PasswordDialog.superclass.initComponent.call(this);
     },
 
     /**
@@ -137,7 +144,9 @@ Tine.Tinebase.widgets.dialog.PasswordDialog = Ext.extend(Ext.Panel, {
      */
     onOk: function () {
         this.fireEvent('passwordEntered', this.passwordField.getValue());
-        this.window.close();
+        if (this.window) {
+            this.window.close();
+        }
     },
 
     /**
@@ -145,8 +154,9 @@ Tine.Tinebase.widgets.dialog.PasswordDialog = Ext.extend(Ext.Panel, {
      *
      * @returns {null}
      */
-    openWindow: function () {
-        this.window = Tine.WindowFactory.getWindow({
+    openWindow: function (config) {
+        config = config || {};
+        this.window = Tine.WindowFactory.getWindow(Ext.apply({
             title: i18n._('Set password'),
             closeAction: 'close',
             modal: true,
@@ -159,7 +169,7 @@ Tine.Tinebase.widgets.dialog.PasswordDialog = Ext.extend(Ext.Panel, {
             items: [
                 this
             ]
-        });
+        }, config));
 
         return this.window;
     }
diff --git a/tine20/Tinebase/js/widgets/dialog/SecondFactorDialog.js b/tine20/Tinebase/js/widgets/dialog/SecondFactorDialog.js
new file mode 100644 (file)
index 0000000..7c7e779
--- /dev/null
@@ -0,0 +1,73 @@
+/*
+ * Tine 2.0
+ *
+ * @license     http://www.gnu.org/licenses/agpl.html AGPL Version 3
+ * @author      Philipp Schüle <p.schuele@metaways.de>
+ * @copyright   Copyright (c) 2017 Metaways Infosystems GmbH (http://www.metaways.de)
+ */
+
+Ext.ns('Tine.Tinebase.widgets.dialog');
+
+Tine.Tinebase.widgets.dialog.SecondFactorDialog = Ext.extend(Tine.Tinebase.widgets.dialog.PasswordDialog, {
+    hasPwGen: false,
+    allowEmptyPassword: true,
+    height: 120,
+
+    initComponent: function() {
+
+        this.title = i18n._('Second Factor Password Required');
+
+        Tine.Tinebase.widgets.dialog.SecondFactorDialog.superclass.initComponent.call(this);
+
+        this.on('passwordEntered', function (password) {
+
+            window.postal.publish({
+                channel: "messagebus",
+                topic: 'secondfactor.check'
+            });
+
+            this.loadMask = new Ext.LoadMask(this.getEl(), {
+                msg: i18n._('Validating password...')
+            });
+            this.loadMask.show();
+
+            Ext.Ajax.request({
+                params: {
+                    method: 'Tinebase.validateSecondFactor',
+                    password: password,
+                },
+                success: function (_result, _request) {
+                    this.loadMask.hide();
+                    var response = Ext.util.JSON.decode(_result.responseText);
+                    // TODO add data to messagebus events?
+
+                    if (response.success) {
+                        window.postal.publish({
+                            channel: "messagebus",
+                            topic: 'secondfactor.valid'
+                        });
+                    } else {
+                        window.postal.publish({
+                            channel: "messagebus",
+                            topic: 'secondfactor.invalid'
+                        });
+
+                        this.passwordField.setValue('');
+
+                        // TODO fix z-index?
+                        //Ext.MessageBox.show({
+                        //    title: i18n._('Wrong Password'),
+                        //    msg: i18n._('Invalid second factor password given'),
+                        //    buttons: Ext.MessageBox.OK
+                        //});
+                    }
+                },
+                failure: function() {
+                    // TODO do some more?
+                    this.loadMask.hide();
+                },
+                scope: this
+            });
+        }, this);
+    }
+});