From f5b5d0def711f970d226024fd4e6c53e3ad50842 Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Fri, 8 Nov 2019 11:07:02 +0100
Subject: [PATCH] MVC: refactor CertificateField and add unit test for
 https://github.com/opnsense/core/issues/3252

---
 .../Base/FieldTypes/CertificateField.php      |  89 ++---------
 .../Base/FieldTypes/CertificateFieldTest.php  | 148 ++++++++++++++++++
 .../CertificateFieldTest/config.xml           |  19 +++
 3 files changed, 176 insertions(+), 80 deletions(-)
 create mode 100644 src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php
 create mode 100644 src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest/config.xml

diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php
index e64a481ac..9932ec85d 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php
@@ -1,7 +1,7 @@
 <?php
 
 /**
- *    Copyright (C) 2015-2016 Deciso B.V.
+ *    Copyright (C) 2015-2019 Deciso B.V.
  *
  *    All rights reserved.
  *
@@ -38,12 +38,8 @@ use OPNsense\Core\Config;
  * package to glue legacy certificates into the model.
  * @package OPNsense\Base\FieldTypes
  */
-class CertificateField extends BaseField
+class CertificateField extends BaseListField
 {
-    /**
-     * @var bool marks if this is a data node or a container
-     */
-    protected $internalIsContainer = false;
 
     /**
      * @var string certificate type cert/ca, reflects config section to use as source
@@ -51,19 +47,9 @@ class CertificateField extends BaseField
     private $certificateType = "cert";
 
     /**
-     * @var bool field may contain multiple certs at once
+     * @var array cached collected certs
      */
-    private $internalMultiSelect = false;
-
-    /**
-     * @var string default validation message string
-     */
-    protected $internalValidationMessage = "option not in list";
-
-    /**
-     * @var array collected options
-     */
-    private static $internalOptionList = array();
+    private static $internalStaticOptionList = array();
 
     /**
      * set certificate type (cert/ca)
@@ -80,77 +66,20 @@ class CertificateField extends BaseField
         }
     }
 
-    /**
-     * select if multiple certs may be selected at once
-     * @param $value boolean value Y/N
-     */
-    public function setMultiple($value)
-    {
-        if (trim(strtoupper($value)) == "Y") {
-            $this->internalMultiSelect = true;
-        } else {
-            $this->internalMultiSelect = false;
-        }
-    }
-
     /**
      * generate validation data (list of certificates)
      */
     protected function actionPostLoadingEvent()
     {
-        if (!isset(self::$internalOptionList[$this->certificateType])) {
-            self::$internalOptionList[$this->certificateType] = array();
+        if (!isset(self::$internalStaticOptionList[$this->certificateType])) {
+            self::$internalStaticOptionList[$this->certificateType] = array();
             $configObj = Config::getInstance()->object();
             foreach ($configObj->{$this->certificateType} as $cert) {
-                self::$internalOptionList[$this->certificateType][(string)$cert->refid] = (string)$cert->descr;
+                self::$internalStaticOptionList[$this->certificateType][(string)$cert->refid] = (string)$cert->descr;
             }
-            natcasesort(self::$internalOptionList[$this->certificateType]);
+            natcasesort(self::$internalStaticOptionList[$this->certificateType]);
         }
+        $this->internalOptionList = self::$internalStaticOptionList[$this->certificateType];
     }
 
-    /**
-     * get valid options, descriptions and selected value
-     * @return array
-     */
-    public function getNodeData()
-    {
-        $result = array ();
-        // if certificate is not required, add empty option
-        if (!$this->internalIsRequired) {
-            $result[""] = array("value" => gettext("none"), "selected" => 0);
-        }
-
-        $certs = explode(',', $this->internalValue);
-        foreach (self::$internalOptionList[$this->certificateType] as $optKey => $optValue) {
-            if (in_array($optKey, $certs)) {
-                $selected = 1;
-            } else {
-                $selected = 0;
-            }
-            $result[$optKey] = array("value" => $optValue, "selected" => $selected);
-        }
-
-        return $result;
-    }
-
-    /**
-     * retrieve field validators for this field type
-     * @return array returns InclusionIn validator
-     */
-    public function getValidators()
-    {
-        $validators = parent::getValidators();
-        if ($this->internalValue != null) {
-            if ($this->internalMultiSelect) {
-                // field may contain more than one cert
-                $validators[] = new CsvListValidator(array('message' => $this->internalValidationMessage,
-                    'domain'=>array_keys(self::$internalOptionList[$this->certificateType])));
-            } else {
-                // single cert selection
-                $validators[] = new InclusionIn(array('message' => $this->internalValidationMessage,
-                    'domain'=>array_keys(self::$internalOptionList[$this->certificateType])));
-            }
-        }
-        return $validators;
-    }
 }
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php
new file mode 100644
index 000000000..18adb317c
--- /dev/null
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php
@@ -0,0 +1,148 @@
+<?php
+/**
+ *    Copyright (C) 2019 Deciso B.V.
+ *
+ *    All rights reserved.
+ *
+ *    Redistribution and use in source and binary forms, with or without
+ *    modification, are permitted provided that the following conditions are met:
+ *
+ *    1. Redistributions of source code must retain the above copyright notice,
+ *       this list of conditions and the following disclaimer.
+ *
+ *    2. Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *
+ *    THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ *    INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ *    AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ *    AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ *    OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ *    SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ *    INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *    CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ *    ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *    POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+namespace tests\OPNsense\Base\FieldTypes;
+
+// @CodingStandardsIgnoreStart
+require_once 'Field_Framework_TestCase.php';
+// @CodingStandardsIgnoreEnd
+
+use \OPNsense\Base\FieldTypes\CertificateField;
+use \Phalcon\DI\FactoryDefault;
+use OPNsense\Core\Config;
+
+class CertificateFieldTest extends Field_Framework_TestCase
+{
+
+    /**
+     * test construct
+     */
+    public function testCanBeCreated()
+    {
+        $this->assertInstanceOf('\OPNsense\Base\FieldTypes\CertificateField', new CertificateField());
+        // switch config to test set for this type
+        FactoryDefault::getDefault()->get('config')->globals->config_path = __DIR__ .'/CertificateFieldTest/';
+        Config::getInstance()->forceReload();
+    }
+
+    /**
+     * Local database should always be there
+     * @depends testCanBeCreated
+     */
+    public function testDefaultCertExists()
+    {
+        // init field
+        $field = new CertificateField();
+        $field->eventPostLoading();
+        print_r(array_keys($field->getNodeData()));
+        $this->assertContains('5537f364ba123', array_keys($field->getNodeData()));
+    }
+
+    /**
+     *
+     * @depends testCanBeCreated
+     */
+    public function testConfigItemsExists()
+    {
+        // init field
+        $field = new CertificateField();
+        $field->setType("ca");
+        $field->eventPostLoading();
+
+        $this->assertContains('1537f364ba123', array_keys($field->getNodeData()));
+        $this->assertContains('15c382ee6020f', array_keys($field->getNodeData()));
+    }
+
+    /**
+     * @depends testCanBeCreated
+     * @expectedException \Phalcon\Validation\Exception
+     * @expectedExceptionMessage CsvListValidator
+     */
+    public function testSelectSetWithUnknownValue()
+    {
+        // init field
+        $field = new CertificateField();
+        $field->eventPostLoading();
+        $field->setMultiple("Y");
+        $field->setValue('testcase 1,testcase 2,testcase X');
+        $this->validateThrow($field);
+    }
+
+    /**
+     *
+     * @depends testCanBeCreated
+     */
+    public function testSelectSetWithoutUnknownValue()
+    {
+        // init field
+        $field = new CertificateField();
+        $field->eventPostLoading();
+        $field->setMultiple("Y");
+        $field->setValue('5537f364ba123,55c382ee6020f');
+        $this->assertEmpty($this->validate($field));
+    }
+
+    /**
+     * @depends testCanBeCreated
+     * @expectedException \Phalcon\Validation\Exception
+     * @expectedExceptionMessage InclusionIn
+     */
+    public function testSelectSetOnSingleValue()
+    {
+        // init field
+        $field = new CertificateField();
+        $field->eventPostLoading();
+        $field->setMultiple("N");
+        $field->setValue('5537f364ba123,55c382ee6020f');
+        $this->validateThrow($field);
+    }
+
+    /**
+     * @depends testCanBeCreated
+     */
+    public function testSelectSingleValue()
+    {
+        // init field
+        $field = new CertificateField();
+        $field->eventPostLoading();
+        $field->setMultiple("N");
+        $field->setValue('5537f364ba123');
+        $this->assertEmpty($this->validate($field));
+    }
+
+
+    /**
+     * type is not a container
+     */
+    public function testIsContainer()
+    {
+        $field = new CertificateField();
+        $this->assertFalse($field->isContainer());
+    }
+}
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest/config.xml b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest/config.xml
new file mode 100644
index 000000000..60edf4774
--- /dev/null
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest/config.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0"?>
+<opnsense>
+    <cert>
+        <refid>5537f364ba123</refid>
+        <descr>cert 1</descr>
+    </cert>
+    <cert>
+        <refid>55c382ee6020f</refid>
+        <descr>cert 2</descr>
+    </cert>
+    <ca>
+        <refid>1537f364ba123</refid>
+        <descr>ca 1</descr>
+    </ca>
+    <ca>
+        <refid>15c382ee6020f</refid>
+        <descr>ca 2</descr>
+    </ca>
+</opnsense>