Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

API CHANGE Checking for SiteTree::$allowed_children in SiteTree->vali…

…date() (was only checked via JavaScript before).

BUGFIX Ensure that VirtualPage $allowed_children are checked on original classes to avoid allowing more than necessary (AIR-38)
  • Loading branch information...
commit 2dd96a4050c74554c82b3ee42a3ee65a3eeea602 1 parent d03724e
@chillu chillu authored
View
37 code/model/SiteTree.php
@@ -16,6 +16,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
* If a classname is prefixed by "*", such as "*Page", then only that
* class is allowed - no subclasses. Otherwise, the class and all its
* subclasses are allowed.
+ * To control allowed children on root level (no parent), use {@link $can_be_root}.
*
* @var array
*/
@@ -1479,6 +1480,36 @@ function onAfterDelete() {
parent::onAfterDelete();
}
+ function validate() {
+ $result = parent::validate();
+
+ // Allowed children validation
+ $parent = $this->getParent();
+ if($parent && $parent->exists()) {
+ // No need to check for subclasses or instanceof, as allowedChildren() already
+ // deconstructs any inheritance trees already.
+ $allowed = $parent->allowedChildren();
+ $subject = ($this instanceof VirtualPage) ? $this->CopyContentFrom() : $this;
+ if($subject->ID && !in_array($subject->ClassName, $allowed)) {
+
+ $result->error(
+ sprintf(
+ _t(
+ 'SiteTree.PageTypeNotAllowed',
+ 'Page type "%s" not allowed as child of this parent page',
+ PR_MEDIUM,
+ 'First argument is a class name'
+ ),
+ $subject->i18n_singular_name()
+ ),
+ 'ALLOWED_CHILDREN'
+ );
+ }
+ }
+
+ return $result;
+ }
+
/**
* Returns TRUE if this object has a URLSegment value that does not conflict with any other objects. This methods
* checks for:
@@ -2329,9 +2360,12 @@ protected function getClassDropdown() {
* @return array
*/
function allowedChildren() {
+ $allowedChildren = array();
$candidates = $this->stat('allowed_children');
if($candidates && $candidates != "none" && $candidates != "SiteTree_root") {
foreach($candidates as $candidate) {
+ // If a classname is prefixed by "*", such as "*Page", then only that
+ // class is allowed - no subclasses. Otherwise, the class and all its subclasses are allowed.
if(substr($candidate,0,1) == '*') {
$allowedChildren[] = substr($candidate,1);
} else {
@@ -2341,8 +2375,9 @@ function allowedChildren() {
}
}
}
- return $allowedChildren;
}
+
+ return $allowedChildren;
}
View
109 tests/model/SiteTreeTest.php
@@ -10,6 +10,14 @@ class SiteTreeTest extends SapphireTest {
'SiteTree' => array('SiteTreeSubsites')
);
+ protected $extraDataObjects = array(
+ 'SiteTreeTest_ClassA',
+ 'SiteTreeTest_ClassB',
+ 'SiteTreeTest_ClassC',
+ 'SiteTreeTest_ClassD',
+ 'SiteTreeTest_ClassCext'
+ );
+
/**
* @todo Necessary because of monolithic Translatable design
*/
@@ -742,6 +750,81 @@ function testPageTypeClasses() {
$this->assertContains('Page', $classes, 'Page types do contain subclasses');
}
+ function testAllowedChildren() {
+ $page = new SiteTree();
+ $this->assertContains(
+ 'VirtualPage',
+ $page->allowedChildren(),
+ 'Includes core subclasses by default'
+ );
+
+ $classA = new SiteTreeTest_ClassA();
+ $this->assertEquals(
+ array('SiteTreeTest_ClassB'),
+ $classA->allowedChildren(),
+ 'Direct setting of allowed children'
+ );
+
+ $classB = new SiteTreeTest_ClassB();
+ $this->assertEquals(
+ array('SiteTreeTest_ClassC', 'SiteTreeTest_ClassCext'),
+ $classB->allowedChildren(),
+ 'Includes subclasses'
+ );
+
+ $classD = new SiteTreeTest_ClassD();
+ $this->assertEquals(
+ array('SiteTreeTest_ClassC'),
+ $classD->allowedChildren(),
+ 'Excludes subclasses if class is prefixed by an asterisk'
+ );
+
+ $classC = new SiteTreeTest_ClassC();
+ $this->assertEquals(
+ array(),
+ $classC->allowedChildren(),
+ 'Null setting'
+ );
+ }
+
+ function testAllowedChildrenValidation() {
+ $page = new SiteTree();
+ $page->write();
+ $classA = new SiteTreeTest_ClassA();
+ $classA->write();
+ $classB = new SiteTreeTest_ClassB();
+ $classB->write();
+ $classC = new SiteTreeTest_ClassC();
+ $classC->write();
+ $classD = new SiteTreeTest_ClassD();
+ $classD->write();
+ $classCext = new SiteTreeTest_ClassCext();
+ $classCext->write();
+
+ $classB->ParentID = $page->ID;
+ $valid = $classB->validate();
+ $this->assertTrue($valid->valid(), "Does allow children on unrestricted parent");
+
+ $classB->ParentID = $classA->ID;
+ $valid = $classB->validate();
+ $this->assertTrue($valid->valid(), "Does allow child specifically allowed by parent");
+
+ $classC->ParentID = $classA->ID;
+ $valid = $classC->validate();
+ $this->assertFalse($valid->valid(), "Doesnt allow child on parents specifically restricting children");
+
+ $classB->ParentID = $classC->ID;
+ $valid = $classB->validate();
+ $this->assertFalse($valid->valid(), "Doesnt allow child on parents disallowing all children");
+
+ $classB->ParentID = $classC->ID;
+ $valid = $classB->validate();
+ $this->assertFalse($valid->valid(), "Doesnt allow child on parents disallowing all children");
+
+ $classCext->ParentID = $classD->ID;
+ $valid = $classCext->validate();
+ $this->assertFalse($valid->valid(), "Doesnt allow child where only parent class is allowed on parent node, and asterisk prefixing is used");
+ }
}
/**#@+
@@ -773,4 +856,30 @@ class SiteTreeTest_NullHtmlCleaner extends HTMLCleaner {
function cleanHTML($html) {
return $html;
}
+}
+
+class SiteTreeTest_ClassA extends Page implements TestOnly {
+
+ static $need_permission = array('ADMIN', 'CMS_ACCESS_CMSMain');
+
+ static $allowed_children = array('SiteTreeTest_ClassB');
+}
+
+class SiteTreeTest_ClassB extends Page implements TestOnly {
+ // Also allowed subclasses
+ static $allowed_children = array('SiteTreeTest_ClassC');
+}
+
+class SiteTreeTest_ClassC extends Page implements TestOnly {
+ static $allowed_children = array();
+}
+
+class SiteTreeTest_ClassD extends Page implements TestOnly {
+ // Only allows this class, no children classes
+ static $allowed_children = array('*SiteTreeTest_ClassC');
+}
+
+class SiteTreeTest_ClassCext extends SiteTreeTest_ClassC implements TestOnly {
+ // Override SiteTreeTest_ClassC definitions
+ static $allowed_children = array('SiteTreeTest_ClassB');
}
View
50 tests/model/VirtualPageTest.php
@@ -3,6 +3,11 @@
class VirtualPageTest extends SapphireTest {
static $fixture_file = 'VirtualPageTest.yml';
+ protected $extraDataObjects = array(
+ 'VirtualPageTest_ClassA',
+ 'VirtualPageTest_ClassB',
+ );
+
/**
* Test that, after you update the source page of a virtual page, all the virtual pages
* are updated
@@ -321,4 +326,49 @@ function testDeletingFromLiveSourcePageOfAVirtualPageAlsoUnpublishesVirtualPage(
$vp = DataObject::get_by_id('SiteTree', $vp->ID);
$this->assertEquals(1, $vp->HasBrokenLink);
}
+
+ /**
+ * Base functionality tested in {@link SiteTreeTest->testAllowedChildrenValidation()}.
+ */
+ function testAllowedChildrenLimitedOnVirtualPages() {
+ $classA = new SiteTreeTest_ClassA();
+ $classA->write();
+ $classB = new SiteTreeTest_ClassB();
+ $classB->write();
+ $classBVirtual = new VirtualPage();
+ $classBVirtual->CopyContentFromID = $classB->ID;
+ $classBVirtual->write();
+ $classC = new SiteTreeTest_ClassC();
+ $classC->write();
+ $classCVirtual = new VirtualPage();
+ $classCVirtual->CopyContentFromID = $classC->ID;
+ $classCVirtual->write();
+
+ $classBVirtual->ParentID = $classA->ID;
+ $valid = $classBVirtual->validate();
+ $this->assertTrue($valid->valid(), "Does allow child linked to virtual page type allowed by parent");
+
+ $classCVirtual->ParentID = $classA->ID;
+ $valid = $classCVirtual->validate();
+ $this->assertFalse($valid->valid(), "Doesn't allow child linked to virtual page type disallowed by parent");
+ }
+}
+
+class VirtualPageTest_ClassA extends Page implements TestOnly {
+
+ static $db = array(
+ 'MyInitiallyCopiedField' => 'Text',
+ 'MyVirtualField' => 'Text',
+ 'MyNonVirtualField' => 'Text',
+ );
+
+ static $allowed_children = array('VirtualPageTest_ClassB');
+}
+
+class VirtualPageTest_ClassB extends Page implements TestOnly {
+ static $allowed_children = array('VirtualPageTest_ClassC');
+}
+
+class VirtualPageTest_ClassC extends Page implements TestOnly {
+ static $allowed_children = array();
}
Please sign in to comment.
Something went wrong with that request. Please try again.