Document Security - Coverage, Testing, and Bug-fixing

docs/topics/recipes.md

@@ -988,6 +988,9 @@ $security->setLockStructure(true);
 $security->setWorkbookPassword("PhpSpreadsheet");
 ```
 
+Note that there are additional methods setLockRevision and setRevisionsPassword
+which apply only to change tracking and history for shared workbooks.
+
 ### Worksheet
 
 An example on setting worksheet security:

src/PhpSpreadsheet/Document/Security.php

@@ -50,156 +50,87 @@ public function __construct()
 
     /**
      * Is some sort of document security enabled?
-     *
-     * @return bool
      */
-    public function isSecurityEnabled()
+    public function isSecurityEnabled(): bool
     {
         return  $this->lockRevision ||
                 $this->lockStructure ||
                 $this->lockWindows;
     }
 
-    /**
-     * Get LockRevision.
-     *
-     * @return bool
-     */
-    public function getLockRevision()
+    public function getLockRevision(): bool
     {
         return $this->lockRevision;
     }
 
-    /**
-     * Set LockRevision.
-     *
-     * @param bool $pValue
-     *
-     * @return $this
-     */
-    public function setLockRevision($pValue)
+    public function setLockRevision(?bool $pValue): self
     {
-        $this->lockRevision = $pValue;
+        if ($pValue !== null) {
+            $this->lockRevision = $pValue;
+        }
 
         return $this;
     }
 
-    /**
-     * Get LockStructure.
-     *
-     * @return bool
-     */
-    public function getLockStructure()
+    public function getLockStructure(): bool
     {
         return $this->lockStructure;
     }
 
-    /**
-     * Set LockStructure.
-     *
-     * @param bool $pValue
-     *
-     * @return $this
-     */
-    public function setLockStructure($pValue)
+    public function setLockStructure(?bool $pValue): self
     {
-        $this->lockStructure = $pValue;
+        if ($pValue !== null) {
+            $this->lockStructure = $pValue;
+        }
 
         return $this;
     }
 
-    /**
-     * Get LockWindows.
-     *
-     * @return bool
-     */
-    public function getLockWindows()
+    public function getLockWindows(): bool
     {
         return $this->lockWindows;
     }
 
-    /**
-     * Set LockWindows.
-     *
-     * @param bool $pValue
-     *
-     * @return $this
-     */
-    public function setLockWindows($pValue)
+    public function setLockWindows(?bool $pValue): self
     {
-        $this->lockWindows = $pValue;
+        if ($pValue !== null) {
+            $this->lockWindows = $pValue;
+        }
 
         return $this;
     }
 
-    /**
-     * Get RevisionsPassword (hashed).
-     *
-     * @return string
-     */
-    public function getRevisionsPassword()
+    public function getRevisionsPassword(): string
     {
         return $this->revisionsPassword;
     }
 
-    /**
-     * Set RevisionsPassword.
-     *
-     * @param string $pValue
-     * @param bool $pAlreadyHashed If the password has already been hashed, set this to true
-     *
-     * @return $this
-     */
-    public function setRevisionsPassword($pValue, $pAlreadyHashed = false)
+    public function setRevisionsPassword(?string $pValue, bool $pAlreadyHashed = false): self
     {
-        if (!$pAlreadyHashed) {
-            $pValue = PasswordHasher::hashPassword($pValue);
+        if ($pValue !== null) {
+            if (!$pAlreadyHashed) {
+                $pValue = PasswordHasher::hashPassword($pValue);
+            }
+            $this->revisionsPassword = $pValue;
         }
-        $this->revisionsPassword = $pValue;
 
         return $this;
     }
 
-    /**
-     * Get WorkbookPassword (hashed).
-     *
-     * @return string
-     */
-    public function getWorkbookPassword()
+    public function getWorkbookPassword(): string
     {
         return $this->workbookPassword;
     }
 
-    /**
-     * Set WorkbookPassword.
-     *
-     * @param string $pValue
-     * @param bool $pAlreadyHashed If the password has already been hashed, set this to true
-     *
-     * @return $this
-     */
-    public function setWorkbookPassword($pValue, $pAlreadyHashed = false)
+    public function setWorkbookPassword(?string $pValue, bool $pAlreadyHashed = false): self
     {
-        if (!$pAlreadyHashed) {
-            $pValue = PasswordHasher::hashPassword($pValue);
+        if ($pValue !== null) {
+            if (!$pAlreadyHashed) {
+                $pValue = PasswordHasher::hashPassword($pValue);
+            }
+            $this->workbookPassword = $pValue;
         }
-        $this->workbookPassword = $pValue;
 
         return $this;
     }
-
-    /**
-     * Implement PHP __clone to create a deep clone, not just a shallow copy.
-     */
-    public function __clone()
-    {
-        $vars = get_object_vars($this);
-        foreach ($vars as $key => $value) {
-            if (is_object($value)) {
-                $this->$key = clone $value;
-            } else {
-                $this->$key = $value;
-            }
-        }
-    }
 }

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -1808,17 +1808,9 @@ private function readProtection(Spreadsheet $excel, SimpleXMLElement $xmlWorkboo
             return;
         }
 
-        if ($xmlWorkbook->workbookProtection['lockRevision']) {
-            $excel->getSecurity()->setLockRevision((bool) $xmlWorkbook->workbookProtection['lockRevision']);
-        }
-
-        if ($xmlWorkbook->workbookProtection['lockStructure']) {
-            $excel->getSecurity()->setLockStructure((bool) $xmlWorkbook->workbookProtection['lockStructure']);
-        }
-
-        if ($xmlWorkbook->workbookProtection['lockWindows']) {
-            $excel->getSecurity()->setLockWindows((bool) $xmlWorkbook->workbookProtection['lockWindows']);
-        }
+        $excel->getSecurity()->setLockRevision(self::getLockValue($xmlWorkbook->workbookProtection, 'lockRevision'));
+        $excel->getSecurity()->setLockStructure(self::getLockValue($xmlWorkbook->workbookProtection, 'lockStructure'));
+        $excel->getSecurity()->setLockWindows(self::getLockValue($xmlWorkbook->workbookProtection, 'lockWindows'));
 
         if ($xmlWorkbook->workbookProtection['revisionsPassword']) {
             $excel->getSecurity()->setRevisionsPassword(
@@ -1835,6 +1827,18 @@ private function readProtection(Spreadsheet $excel, SimpleXMLElement $xmlWorkboo
         }
     }
 
+    private static function getLockValue(SimpleXmlElement $protection, string $key): ?bool
+    {
+        $returnValue = null;
+        $protectKey = $protection[$key];
+        if (!empty($protectKey)) {
+            $protectKey = (string) $protectKey;
+            $returnValue = $protectKey !== 'false' && (bool) $protectKey;
+        }
+
+        return $returnValue;
+    }
+
     private function readFormControlProperties(Spreadsheet $excel, ZipArchive $zip, $dir, $fileWorksheet, $docSheet, array &$unparsedLoadedData): void
     {
         if (!$zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels')) {

tests/PhpSpreadsheetTests/Document/SecurityTest.php

@@ -0,0 +1,75 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Document;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+class SecurityTest extends AbstractFunctional
+{
+    public function testSecurity(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Hello');
+        $security = $spreadsheet->getSecurity();
+        $security->setLockRevision(true);
+        $revisionsPassword = 'revpasswd';
+        $security->setRevisionsPassword($revisionsPassword);
+        $hashedRevisionsPassword = $security->getRevisionsPassword();
+        self::assertNotEquals($revisionsPassword, $hashedRevisionsPassword);
+        $security->setLockWindows(true);
+        $security->setLockStructure(true);
+        $workbookPassword = 'wbpasswd';
+        $security->setWorkbookPassword($workbookPassword);
+        $hashedWorkbookPassword = $security->getWorkbookPassword();
+        self::assertNotEquals($workbookPassword, $hashedWorkbookPassword);
+
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
+        $reloadedSecurity = $reloadedSpreadsheet->getSecurity();
+        self::assertTrue($reloadedSecurity->getLockRevision());
+        self::assertTrue($reloadedSecurity->getLockWindows());
+        self::assertTrue($reloadedSecurity->getLockStructure());
+        self::assertSame($hashedWorkbookPassword, $reloadedSecurity->getWorkbookPassword());
+        self::assertSame($hashedRevisionsPassword, $reloadedSecurity->getRevisionsPassword());
+
+        $reloadedSecurity->setRevisionsPassword($hashedWorkbookPassword, true);
+        self::assertSame($hashedWorkbookPassword, $reloadedSecurity->getRevisionsPassword());
+        $reloadedSecurity->setWorkbookPassword($hashedRevisionsPassword, true);
+        self::assertSame($hashedRevisionsPassword, $reloadedSecurity->getWorkbookPassword());
+    }
+
+    public function providerLocks(): array
+    {
+        return [
+            [false, false, false],
+            [false, false, true],
+            [false, true, false],
+            [false, true, true],
+            [true, false, false],
+            [true, false, true],
+            [true, true, false],
+            [true, true, true],
+        ];
+    }
+
+    /**
+     * @dataProvider providerLocks
+     */
+    public function testLocks(bool $revision, bool $windows, bool $structure): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Hello');
+        $security = $spreadsheet->getSecurity();
+        $security->setLockRevision($revision);
+        $security->setLockWindows($windows);
+        $security->setLockStructure($structure);
+        $enabled = $security->isSecurityEnabled();
+        self::assertSame($enabled, $revision || $windows || $structure);
+
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
+        $reloadedSecurity = $reloadedSpreadsheet->getSecurity();
+        self::assertSame($revision, $reloadedSecurity->getLockRevision());
+        self::assertSame($windows, $reloadedSecurity->getLockWindows());
+        self::assertSame($structure, $reloadedSecurity->getLockStructure());
+    }
+}