From 4d1b10425fe5f5afac54784a9a1d05f3a833ac29 Mon Sep 17 00:00:00 2001
From: Julian Seidenberg <julian@silverstripe.com>
Date: Wed, 23 Dec 2015 11:14:36 +1300
Subject: [PATCH 1/2] BUG fixing bug that prevents non-admin users from editing
 user defined form fields.

---
 code/model/EditableCustomRule.php             |  80 +++++++++++++
 .../editableformfields/EditableFormField.php  | 106 +++++++++++++++---
 .../editableformfields/EditableOption.php     |  68 ++++++++++-
 .../UserDefinedForm_EmailRecipient.php        |  41 ++++++-
 ...serDefinedForm_EmailRecipientCondition.php |  69 +++++++++++-
 5 files changed, 338 insertions(+), 26 deletions(-)

diff --git a/code/model/EditableCustomRule.php b/code/model/EditableCustomRule.php
index d99d767..c31df5c 100644
--- a/code/model/EditableCustomRule.php
+++ b/code/model/EditableCustomRule.php
@@ -4,6 +4,7 @@
  * A custom rule for showing / hiding an EditableFormField
  * based the value of another EditableFormField.
  *
+ * @method EditableFormField Parent()
  * @package userforms
  */
 class EditableCustomRule extends DataObject {
@@ -57,4 +58,83 @@ class EditableCustomRule extends DataObject {
 	public function doDeleteFromStage($stage) {
 		$this->deleteFromStage($stage);
 	}
+
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+	public function canDelete($member = null) {
+		return $this->canEdit($member);
+	}
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+	public function canEdit($member = null) {
+        return $this->Parent()->canEdit($member);
+	}
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+	public function canView($member = null) {
+		return $this->Parent()->canView($member);
+	}
+
+	/**
+	 * Return whether a user can create an object of this type
+	 *
+     * @param Member $member
+     * @param array $context Virtual parameter to allow context to be passed in to check
+	 * @return bool
+	 */
+	public function canCreate($member = null) {
+		// Check parent page
+        $parent = $this->getCanCreateContext(func_get_args());
+        if($parent) {
+            return $parent->canEdit($member);
+        }
+
+        // Fall back to secure admin permissions
+        return parent::canCreate($member);
+	}
+
+    /**
+     * Helper method to check the parent for this object
+     *
+     * @param array $args List of arguments passed to canCreate
+     * @return DataObject Some parent dataobject to inherit permissions from
+     */
+    protected function getCanCreateContext($args) {
+        // Inspect second parameter to canCreate for a 'Parent' context
+        if(isset($args[1]['Parent'])) {
+            return $args[1]['Parent'];
+        }
+        // Hack in currently edited page if context is missing
+        if(Controller::has_curr() && Controller::curr() instanceof CMSMain) {
+            return Controller::curr()->currentPage();
+        }
+
+        // No page being edited
+        return null;
+    }
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+    public function canPublish($member = null) {
+        return $this->canEdit($member);
+    }
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+    public function canUnpublish($member = null) {
+        return $this->canDelete($member);
+    }
 }
diff --git a/code/model/editableformfields/EditableFormField.php b/code/model/editableformfields/EditableFormField.php
index 8d464b6..40c6095 100755
--- a/code/model/editableformfields/EditableFormField.php
+++ b/code/model/editableformfields/EditableFormField.php
@@ -9,7 +9,7 @@ use SilverStripe\Forms\SegmentField;
  * @package userforms
  *
  * @property string Name
- *
+ * @method UserDefinedForm Parent() Parent page
  * @method DataList DisplayRules() List of EditableCustomRule objects
  */
 class EditableFormField extends DataObject {
@@ -320,34 +320,108 @@ class EditableFormField extends DataObject {
 		return false;
 	}
 
-	/**
-	 * Return whether a user can delete this form field
-	 * based on whether they can edit the page
-	 *
-	 * @return bool
-	 */
+    /**
+     * Return whether a user can delete this form field
+     * based on whether they can edit the page
+     *
+     * @param Member $member
+     * @return bool
+     */
 	public function canDelete($member = null) {
-		if($this->Parent()) {
-			return $this->Parent()->canEdit($member) && !$this->isReadonly();
+		return $this->canEdit($member);
+	}
+
+    /**
+     * Return whether a user can edit this form field
+     * based on whether they can edit the page
+     *
+     * @param Member $member
+     * @return bool
+     */
+	public function canEdit($member = null) {
+        $parent = $this->Parent();
+		if($parent && $parent->exists()) {
+			return $parent->canEdit($member) && !$this->isReadonly();
+		}
+
+        // Fallback to secure admin permissions
+		return parent::canEdit($member);
+	}
+
+    /**
+     * Return whether a user can view this form field
+     * based on whether they can view the page, regardless of the ReadOnly status of the field
+     *
+     * @param Member $member
+     * @return bool
+     */
+	public function canView($member = null) {
+		$parent = $this->Parent();
+		if($parent && $parent->exists()) {
+			return $parent->canView($member);
 		}
 
 		return true;
 	}
 
 	/**
-	 * Return whether a user can edit this form field
-	 * based on whether they can edit the page
+	 * Return whether a user can create an object of this type
 	 *
+     * @param Member $member
+     * @param array $context Virtual parameter to allow context to be passed in to check
 	 * @return bool
 	 */
-	public function canEdit($member = null) {
-		if($this->Parent()) {
-			return $this->Parent()->canEdit($member) && !$this->isReadonly();
-		}
+	public function canCreate($member = null) {
+		// Check parent page
+        $parent = $this->getCanCreateContext(func_get_args());
+        if($parent) {
+            return $parent->canEdit($member);
+        }
 
-		return true;
+        // Fall back to secure admin permissions
+        return parent::canCreate($member);
 	}
 
+    /**
+     * Helper method to check the parent for this object
+     *
+     * @param array $args List of arguments passed to canCreate
+     * @return SiteTree Parent page instance
+     */
+    protected function getCanCreateContext($args) {
+        // Inspect second parameter to canCreate for a 'Parent' context
+        if(isset($args[1]['Parent'])) {
+            return $args[1]['Parent'];
+        }
+        // Hack in currently edited page if context is missing
+        if(Controller::has_curr() && Controller::curr() instanceof CMSMain) {
+            return Controller::curr()->currentPage();
+        }
+
+        // No page being edited
+        return null;
+    }
+
+    /**
+     * Check if can publish
+     *
+     * @param Member $member
+     * @return bool
+     */
+    public function canPublish($member = null) {
+        return $this->canEdit($member);
+    }
+
+    /**
+     * Check if can unpublish
+     *
+     * @param Member $member
+     * @return bool
+     */
+    public function canUnpublish($member = null) {
+        return $this->canDelete($member);
+    }
+
 	/**
 	 * Publish this Form Field to the live site
 	 *
diff --git a/code/model/editableformfields/EditableOption.php b/code/model/editableformfields/EditableOption.php
index f95e9ec..16812f5 100644
--- a/code/model/editableformfields/EditableOption.php
+++ b/code/model/editableformfields/EditableOption.php
@@ -4,9 +4,9 @@
  * Base Class for EditableOption Fields such as the ones used in
  * dropdown fields and in radio check box groups
  *
+ * @method EditableMultipleOptionField Parent()
  * @package userforms
  */
-
 class EditableOption extends DataObject {
 
 	private static $default_sort = "Sort";
@@ -37,7 +37,7 @@ class EditableOption extends DataObject {
 	 * @return boolean
 	 */
 	public function canEdit($member = null) {
-		return ($this->Parent()->canEdit($member));
+		return $this->Parent()->canEdit($member);
 	}
 
 	/**
@@ -46,10 +46,72 @@ class EditableOption extends DataObject {
 	 * @return boolean
 	 */
 	public function canDelete($member = null) {
-		return ($this->Parent()->canDelete($member));
+		return $this->canEdit($member);
 	}
 
 	public function getEscapedTitle() {
 		return Convert::raw2att($this->Title);
 	}
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+	public function canView($member = null) {
+		return $this->Parent()->canView($member);
+	}
+
+	/**
+	 * Return whether a user can create an object of this type
+	 *
+     * @param Member $member
+     * @param array $context Virtual parameter to allow context to be passed in to check
+	 * @return bool
+	 */
+	public function canCreate($member = null) {
+		// Check parent page
+        $parent = $this->getCanCreateContext(func_get_args());
+        if($parent) {
+            return $parent->canEdit($member);
+        }
+
+        // Fall back to secure admin permissions
+        return parent::canCreate($member);
+	}
+
+    /**
+     * Helper method to check the parent for this object
+     *
+     * @param array $args List of arguments passed to canCreate
+     * @return DataObject Some parent dataobject to inherit permissions from
+     */
+    protected function getCanCreateContext($args) {
+        // Inspect second parameter to canCreate for a 'Parent' context
+        if(isset($args[1]['Parent'])) {
+            return $args[1]['Parent'];
+        }
+        // Hack in currently edited page if context is missing
+        if(Controller::has_curr() && Controller::curr() instanceof CMSMain) {
+            return Controller::curr()->currentPage();
+        }
+
+        // No page being edited
+        return null;
+    }
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+    public function canPublish($member = null) {
+        return $this->canEdit($member);
+    }
+
+    /**
+     * @param Member $member
+     * @return bool
+     */
+    public function canUnpublish($member = null) {
+        return $this->canDelete($member);
+    }
 }
diff --git a/code/model/recipients/UserDefinedForm_EmailRecipient.php b/code/model/recipients/UserDefinedForm_EmailRecipient.php
index 52db1c6..8c6d3db 100644
--- a/code/model/recipients/UserDefinedForm_EmailRecipient.php
+++ b/code/model/recipients/UserDefinedForm_EmailRecipient.php
@@ -265,21 +265,50 @@ class UserDefinedForm_EmailRecipient extends DataObject {
 	}
 
 	/**
-	 * @param Member
+	 * Return whether a user can create an object of this type
 	 *
-	 * @return boolean
+     * @param Member $member
+     * @param array $context Virtual parameter to allow context to be passed in to check
+	 * @return bool
 	 */
 	public function canCreate($member = null) {
-		return $this->Form()->canCreate();
+		// Check parent page
+        $parent = $this->getCanCreateContext(func_get_args());
+        if($parent) {
+            return $parent->canEdit($member);
+        }
+
+        // Fall back to secure admin permissions
+        return parent::canCreate($member);
 	}
 
+    /**
+     * Helper method to check the parent for this object
+     *
+     * @param array $args List of arguments passed to canCreate
+     * @return SiteTree Parent page instance
+     */
+    protected function getCanCreateContext($args) {
+        // Inspect second parameter to canCreate for a 'Parent' context
+        if(isset($args[1]['Form'])) {
+            return $args[1]['Form'];
+        }
+        // Hack in currently edited page if context is missing
+        if(Controller::has_curr() && Controller::curr() instanceof CMSMain) {
+            return Controller::curr()->currentPage();
+        }
+
+        // No page being edited
+        return null;
+    }
+
 	/**
 	 * @param Member
 	 *
 	 * @return boolean
 	 */
 	public function canView($member = null) {
-		return $this->Form()->canView();
+		return $this->Form()->canView($member);
 	}
 
 	/**
@@ -288,7 +317,7 @@ class UserDefinedForm_EmailRecipient extends DataObject {
 	 * @return boolean
 	 */
 	public function canEdit($member = null) {
-		return $this->Form()->canEdit();
+		return $this->Form()->canEdit($member);
 	}
 
 	/**
@@ -297,7 +326,7 @@ class UserDefinedForm_EmailRecipient extends DataObject {
 	 * @return boolean
 	 */
 	public function canDelete($member = null) {
-		return $this->Form()->canDelete();
+		return $this->canEdit($member);
 	}
 
 	/*
diff --git a/code/model/recipients/UserDefinedForm_EmailRecipientCondition.php b/code/model/recipients/UserDefinedForm_EmailRecipientCondition.php
index 0d752ee..424aae0 100644
--- a/code/model/recipients/UserDefinedForm_EmailRecipientCondition.php
+++ b/code/model/recipients/UserDefinedForm_EmailRecipientCondition.php
@@ -3,6 +3,8 @@
 
 /**
  * Declares a condition that determines whether an email can be sent to a given recipient
+ *
+ * @method UserDefinedForm_EmailRecipient Parent()
  */
 class UserDefinedForm_EmailRecipientCondition extends DataObject {
 
@@ -51,4 +53,69 @@ class UserDefinedForm_EmailRecipientCondition extends DataObject {
 				return ($this->ConditionOption === 'Equals') === (bool)$matches;
 		}
 	}
-}
\ No newline at end of file
+
+    	/**
+	 * Return whether a user can create an object of this type
+	 *
+     * @param Member $member
+     * @param array $context Virtual parameter to allow context to be passed in to check
+	 * @return bool
+	 */
+	public function canCreate($member = null) {
+		// Check parent page
+        $parent = $this->getCanCreateContext(func_get_args());
+        if($parent) {
+            return $parent->canEdit($member);
+        }
+
+        // Fall back to secure admin permissions
+        return parent::canCreate($member);
+	}
+
+    /**
+     * Helper method to check the parent for this object
+     *
+     * @param array $args List of arguments passed to canCreate
+     * @return SiteTree Parent page instance
+     */
+    protected function getCanCreateContext($args) {
+        // Inspect second parameter to canCreate for a 'Parent' context
+        if(isset($args[1]['Parent'])) {
+            return $args[1]['Parent'];
+        }
+        // Hack in currently edited page if context is missing
+        if(Controller::has_curr() && Controller::curr() instanceof CMSMain) {
+            return Controller::curr()->currentPage();
+        }
+
+        // No page being edited
+        return null;
+    }
+
+	/**
+	 * @param Member
+	 *
+	 * @return boolean
+	 */
+	public function canView($member = null) {
+		return $this->Parent()->canView($member);
+	}
+
+	/**
+	 * @param Member
+	 *
+	 * @return boolean
+	 */
+	public function canEdit($member = null) {
+		return $this->Parent()->canEdit($member);
+	}
+
+	/**
+	 * @param Member
+	 *
+	 * @return boolean
+	 */
+	public function canDelete($member = null) {
+		return $this->canEdit($member);
+	}
+}

From ac4a6df7e95257b429b2c76417e57b677f831fbb Mon Sep 17 00:00:00 2001
From: Chris Joe <flamer.ohr@gmail.com>
Date: Mon, 14 Mar 2016 13:53:25 +1300
Subject: [PATCH 2/2] Add permission tests for editing fix

---
 tests/EditableFormFieldTest.php | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/EditableFormFieldTest.php b/tests/EditableFormFieldTest.php
index 1258b20..5033e63 100644
--- a/tests/EditableFormFieldTest.php
+++ b/tests/EditableFormFieldTest.php
@@ -12,14 +12,18 @@ class EditableFormFieldTest extends FunctionalTest {
 		$text = $this->objFromFixture('EditableTextField', 'basic-text');
 
 		$this->logInWithPermission('ADMIN');
+		$this->assertTrue($text->canCreate());
+		$this->assertTrue($text->canView());
 		$this->assertTrue($text->canEdit());
 		$this->assertTrue($text->canDelete());
 
 		$text->setReadonly(true);
+		$this->assertTrue($text->canView());
 		$this->assertFalse($text->canEdit());
 		$this->assertFalse($text->canDelete());
 
 		$text->setReadonly(false);
+		$this->assertTrue($text->canView());
 		$this->assertTrue($text->canEdit());
 		$this->assertTrue($text->canDelete());
 
@@ -27,11 +31,15 @@ class EditableFormFieldTest extends FunctionalTest {
 		$member->logout();
 
 		$this->logInWithPermission('SITETREE_VIEW_ALL');
+		$this->assertFalse($text->canCreate());
+
 		$text->setReadonly(false);
+		$this->assertTrue($text->canView());
 		$this->assertFalse($text->canEdit());
 		$this->assertFalse($text->canDelete());
 
 		$text->setReadonly(true);
+		$this->assertTrue($text->canView());
 		$this->assertFalse($text->canEdit());
 		$this->assertFalse($text->canDelete());
 	}