From 6089a7c5bd25d6591deb154f1a34908fa91ac198 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 2 Dec 2015 17:33:25 +1300 Subject: [PATCH] API Create default security permission model for versioned data objects --- .../00_Model/10_Versioning.md | 72 ++++++++- model/Versioned.php | 93 ++++++++++++ tests/model/DataObjectLazyLoadingTest.php | 17 ++- tests/model/VersionedTest.php | 140 ++++++++++++++++-- tests/model/VersionedTest.yml | 42 +++--- 5 files changed, 330 insertions(+), 34 deletions(-) diff --git a/docs/en/02_Developer_Guides/00_Model/10_Versioning.md b/docs/en/02_Developer_Guides/00_Model/10_Versioning.md index b344df6d2..01b149489 100644 --- a/docs/en/02_Developer_Guides/00_Model/10_Versioning.md +++ b/docs/en/02_Developer_Guides/00_Model/10_Versioning.md @@ -137,8 +137,76 @@ Example: Get the first 10 live records, filtered by creation date: ### Permissions -The `Versioned` extension doesn't provide any permissions on its own, but you can have a look at the `SiteTree` class -for implementation samples, specifically `canPublish()` and `canDeleteFromStage()`. +By default, `Versioned` will come out of the box with security extensions which restrict +the visibility of objects in Draft (stage) or Archive viewing mode. + +
+As is standard practice, user code should always invoke `canView()` on any object before +rendering it. DataLists do not filter on `canView()` automatically, so this must be +done via user code. This be be achieved either by wrapping `<% if $canView %>` in +your template, or by implementing your visibility check in PHP. +
+ +Versioned object visibility can be customised in one of the following ways by editing your user code: + + * Override the `canViewVersioned` method in your code. Make sure that this returns true or + false if the user is not allowed to view this object in the current viewing mode. + * Override the `canView` method to override the method visibility completely. + +E.g. + + :::php + class MyObject extends DataObject { + private static $extensions = array( + 'Versioned' + ); + + public function canViewVersioned($member = null) { + // Check if site is live + $mode = $this->getSourceQueryParam("Versioned.mode"); + $stage = $this->getSourceQueryParam("Versioned.stage"); + if ($mode === 'Stage' && $stage === 'Live') { + return true; + } + + // Only admins can view non-live objects + return Permission::checkMember($member, 'ADMIN'); + } + } + +If you want to control permissions of an object in an extension, you can also use +one of the below extension points in your `DataExtension` subclass: + + * `canView` to update the visibility of the object's `canView` + * `canViewNonLive` to update the visibility of this object only in non-live mode. + +Note that unlike canViewVersioned, the canViewNonLive method will +only be invoked if the object is in a non-published state. + +E.g. + + :::php + class MyObjectExtension extends DataExtension { + public function canViewNonLive($member = null) { + return Permission::check($member, 'DRAFT_STATUS'); + } + } + +If none of the above checks are overridden, visibility will be determined by the +permissions in the `TargetObject.non_live_permissions` config. + +E.g. + + :::php + class MyObject extends DataObject { + private static $extensions = array( + 'Versioned' + ); + private static $non_live_permissions = array('ADMIN'); + } + +Versioned applies no additional permissions to `canEdit` or `canCreate`, and such +these permissions should be implemented as per standard unversioned DataObjects. ### Page Specific Operations diff --git a/model/Versioned.php b/model/Versioned.php index e089d4feb..849e3056a 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -130,6 +130,14 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { */ protected static $versionableExtensions = array('Translatable' => 'lang'); + /** + * Permissions necessary to view records outside of the live stage (e.g. archive / draft stage). + * + * @config + * @var array + */ + private static $non_live_permissions = array('CMS_ACCESS_LeftAndMain', 'CMS_ACCESS_CMSMain', 'VIEW_DRAFT_CONTENT'); + /** * Reset static configuration variables to their default values. */ @@ -728,6 +736,91 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $this->migrateVersion(null); } + /** + * Extend permissions to include additional security for objects that are not published to live. + * + * @param Member $member + * @return bool|null + */ + public function canView($member = null) { + // Invoke default version-gnostic canView + if ($this->owner->canViewVersioned($member) === false) { + return false; + } + } + + /** + * Determine if there are any additional restrictions on this object for the given reading version. + * + * Override this in a subclass to customise any additional effect that Versioned applies to canView. + * + * This is expected to be called by canView, and thus is only responsible for denying access if + * the default canView would otherwise ALLOW access. Thus it should not be called in isolation + * as an authoritative permission check. + * + * This has the following extension points: + * - canViewDraft is invoked if Mode = stage and Stage = stage + * - canViewArchived is invoked if Mode = archive + * + * @param Member $member + * @return bool False is returned if the current viewing mode denies visibility + */ + public function canViewVersioned($member = null) { + // Bypass when live stage + $mode = $this->owner->getSourceQueryParam("Versioned.mode"); + $stage = $this->owner->getSourceQueryParam("Versioned.stage"); + if ($mode === 'stage' && $stage === static::get_live_stage()) { + return true; + } + + // Bypass if site is unsecured + if (Session::get('unsecuredDraftSite')) { + return true; + } + + // If we weren't definitely loaded from live, and we can't view non-live content, we need to + // check to make sure this version is the live version and so can be viewed + $latestVersion = Versioned::get_versionnumber_by_stage($this->owner->class, 'Live', $this->owner->ID); + if ($latestVersion == $this->owner->Version) { + // Even if this is loaded from a non-live stage, this is the live version + return true; + } + + // Extend versioned behaviour + $extended = $this->owner->extendedCan('canViewNonLive', $member); + if($extended !== null) { + return (bool)$extended; + } + + // Fall back to default permission check + $permissions = Config::inst()->get($this->owner->class, 'non_live_permissions', Config::FIRST_SET); + $check = Permission::checkMember($member, $permissions); + return (bool)$check; + } + + /** + * Determines canView permissions for the latest version of this object on a specific stage. + * Usually the stage is read from {@link Versioned::current_stage()}. + * + * This method should be invoked by user code to check if a record is visible in the given stage. + * + * This method should not be called via ->extend('canViewStage'), but rather should be + * overridden in the extended class. + * + * @param string $stage + * @param Member $member + * @return bool + */ + public function canViewStage($stage = 'Live', $member = null) { + $oldMode = Versioned::get_reading_mode(); + Versioned::reading_stage($stage); + + $versionFromStage = DataObject::get($this->class)->byID($this->ID); + + Versioned::set_reading_mode($oldMode); + return $versionFromStage ? $versionFromStage->canView($member) : false; + } + /** * Determine if a table is supporting the Versioned extensions (e.g. * $table_versions does exists). diff --git a/tests/model/DataObjectLazyLoadingTest.php b/tests/model/DataObjectLazyLoadingTest.php index 79db4d151..a3fab2cf6 100644 --- a/tests/model/DataObjectLazyLoadingTest.php +++ b/tests/model/DataObjectLazyLoadingTest.php @@ -24,10 +24,23 @@ class DataObjectLazyLoadingTest extends SapphireTest { 'DataObjectTest_TeamComment', 'DataObjectTest_EquipmentCompany', 'DataObjectTest_SubEquipmentCompany', - 'VersionedTest_DataObject', - 'VersionedTest_Subclass', + 'DataObjectTest\NamespacedClass', + 'DataObjectTest\RelationClass', + 'DataObjectTest_ExtendedTeamComment', + 'DataObjectTest_Company', + 'DataObjectTest_Staff', + 'DataObjectTest_CEO', + 'DataObjectTest_Fan', 'VersionedLazy_DataObject', 'VersionedLazySub_DataObject', + 'VersionedTest_DataObject', + 'VersionedTest_Subclass', + 'VersionedTest_AnotherSubclass', + 'VersionedTest_RelatedWithoutVersion', + 'VersionedTest_SingleStage', + 'VersionedTest_WithIndexes', + 'VersionedTest_PublicStage', + 'VersionedTest_PublicViaExtension', ); public function testQueriedColumnsID() { diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index 989b71be0..72fc3d312 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -15,6 +15,8 @@ class VersionedTest extends SapphireTest { 'VersionedTest_RelatedWithoutVersion', 'VersionedTest_SingleStage', 'VersionedTest_WithIndexes', + 'VersionedTest_PublicStage', + 'VersionedTest_PublicViaExtension', ); protected $requiredExtensions = array( @@ -601,29 +603,29 @@ class VersionedTest extends SapphireTest { // Set to stage Director::test('/?stage=Stage', null, $session); $this->assertEquals( - 'Stage.Stage', - $session->inst_get('readingMode'), - 'Check querystring changes reading mode to Stage' + 'Stage.Stage', + $session->inst_get('readingMode'), + 'Check querystring changes reading mode to Stage' ); Director::test('/', null, $session); $this->assertEquals( - 'Stage.Stage', - $session->inst_get('readingMode'), - 'Check that subsequent requests in the same session remain in Stage mode' + 'Stage.Stage', + $session->inst_get('readingMode'), + 'Check that subsequent requests in the same session remain in Stage mode' ); // Test live persists Director::test('/?stage=Live', null, $session); $this->assertEquals( - 'Stage.Live', - $session->inst_get('readingMode'), - 'Check querystring changes reading mode to Live' + 'Stage.Live', + $session->inst_get('readingMode'), + 'Check querystring changes reading mode to Live' ); Director::test('/', null, $session); $this->assertEquals( - 'Stage.Live', - $session->inst_get('readingMode'), - 'Check that subsequent requests in the same session remain in Live mode' + 'Stage.Live', + $session->inst_get('readingMode'), + 'Check that subsequent requests in the same session remain in Live mode' ); // Test that session doesn't redundantly store the default stage if it doesn't need to @@ -750,6 +752,54 @@ class VersionedTest extends SapphireTest { $testData->NewField = 'Test'; $testData->write(); } + + public function testCanView() { + $public1ID = $this->idFromFixture('VersionedTest_PublicStage', 'public1'); + $public2ID = $this->idFromFixture('VersionedTest_PublicViaExtension', 'public2'); + $privateID = $this->idFromFixture('VersionedTest_DataObject', 'page1'); + + // Test that all (and only) public pages are viewable in stage mode + Session::clear("loggedInAs"); + Versioned::reading_stage('Stage'); + $public1 = Versioned::get_one_by_stage('VersionedTest_PublicStage', 'Stage', array('"ID"' => $public1ID)); + $public2 = Versioned::get_one_by_stage('VersionedTest_PublicViaExtension', 'Stage', array('"ID"' => $public2ID)); + $private = Versioned::get_one_by_stage('VersionedTest_DataObject', 'Stage', array('"ID"' => $privateID)); + + $this->assertTrue($public1->canView()); + $this->assertTrue($public2->canView()); + $this->assertFalse($private->canView()); + + // Adjusting the current stage should not allow objects loaded in stage to be viewable + Versioned::reading_stage('Live'); + $this->assertTrue($public1->canView()); + $this->assertTrue($public2->canView()); + $this->assertFalse($private->canView()); + + // Writing the private page to live should be fine though + $private->publish("Stage", "Live"); + $privateLive = Versioned::get_one_by_stage('VersionedTest_DataObject', 'Live', array('"ID"' => $privateID)); + $this->assertTrue($private->canView()); + $this->assertTrue($privateLive->canView()); + + // But if the private version becomes different to the live version, it's once again disallowed + Versioned::reading_stage('Stage'); + $private->Title = 'Secret Title'; + $private->write(); + $this->assertFalse($private->canView()); + $this->assertTrue($privateLive->canView()); + + // And likewise, viewing a live page (when mode is draft) should be ok + Versioned::reading_stage('Stage'); + $this->assertFalse($private->canView()); + $this->assertTrue($privateLive->canView()); + + // Logging in as admin should allow all permissions + $this->logInWithPermission('ADMIN'); + Versioned::reading_stage('Stage'); + $this->assertTrue($public1->canView()); + $this->assertTrue($public2->canView()); + $this->assertTrue($private->canView()); + } } @@ -776,6 +826,14 @@ class VersionedTest_DataObject extends DataObject implements TestOnly { 'Related' => 'VersionedTest_RelatedWithoutVersion' ); + + public function canView($member = null) { + $extended = $this->extendedCan(__FUNCTION__, $member); + if($extended !== null) { + return $extended; + } + return true; + } } class VersionedTest_WithIndexes extends DataObject implements TestOnly { @@ -851,3 +909,61 @@ class VersionedTest_SingleStage extends DataObject implements TestOnly { 'Versioned("Stage")' ); } + +/** + * Versioned dataobject with public stage mode + */ +class VersionedTest_PublicStage extends DataObject implements TestOnly { + private static $db = array( + 'Title' => 'Varchar' + ); + + private static $extensions = array( + "Versioned('Stage', 'Live')" + ); + + public function canView($member = null) { + $extended = $this->extendedCan(__FUNCTION__, $member); + if($extended !== null) { + return $extended; + } + return true; + } + + public function canViewVersioned($member = null) { + // All non-live modes are public + return true; + } +} + +/** + * Public access is provided via extension rather than overriding canViewVersioned + */ +class VersionedTest_PublicViaExtension extends DataObject implements TestOnly { + + public function canView($member = null) { + $extended = $this->extendedCan(__FUNCTION__, $member); + if($extended !== null) { + return $extended; + } + return true; + } + + private static $db = array( + 'Title' => 'Varchar' + ); + + private static $extensions = array( + "Versioned('Stage', 'Live')", + "VersionedTest_PublicExtension" + ); +} + +/** + * Alters stage mode of extended object to be public + */ +class VersionedTest_PublicExtension extends DataExtension implements TestOnly { + public function canViewNonLive($member = null) { + return true; + } +} diff --git a/tests/model/VersionedTest.yml b/tests/model/VersionedTest.yml index 0d050c5a8..754f19636 100644 --- a/tests/model/VersionedTest.yml +++ b/tests/model/VersionedTest.yml @@ -1,19 +1,25 @@ VersionedTest_DataObject: - page1: - Title: Page 1 - page2: - Title: Page 2 - page3: - Title: Page 3 - page2a: - Parent: =>VersionedTest_DataObject.page2 - Title: Page 2a - page2b: - Parent: =>VersionedTest_DataObject.page2 - Title: Page 2b - page3a: - Parent: =>VersionedTest_DataObject.page3 - Title: Page 3a - page3b: - Parent: =>VersionedTest_DataObject.page3 - Title: Page 3b + page1: + Title: Page 1 + page2: + Title: Page 2 + page3: + Title: Page 3 + page2a: + Parent: =>VersionedTest_DataObject.page2 + Title: Page 2a + page2b: + Parent: =>VersionedTest_DataObject.page2 + Title: Page 2b + page3a: + Parent: =>VersionedTest_DataObject.page3 + Title: Page 3a + page3b: + Parent: =>VersionedTest_DataObject.page3 + Title: Page 3b +VersionedTest_PublicStage: + public1: + Title: 'Some page' +VersionedTest_PublicViaExtension: + public2: + Title: 'Another page' \ No newline at end of file