From 7f7fe73b772ebf652217c64d0767c66740f39709 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Mon, 25 Sep 2017 13:55:33 +0100 Subject: [PATCH 01/14] Reduce dependence on session state for accessing draft stages --- src/Dev/FunctionalTest.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index 7e14e944c..add41d955 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -4,6 +4,7 @@ namespace SilverStripe\Dev; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; +use SilverStripe\Control\HTTP; use SilverStripe\Control\Session; use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Config\Config; @@ -12,6 +13,7 @@ use SilverStripe\Security\BasicAuth; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\Security\SecurityToken; +use SilverStripe\Versioned\Versioned; use SilverStripe\View\SSViewer; use PHPUnit_Framework_AssertionFailedError; use SimpleXMLElement; @@ -161,6 +163,9 @@ class FunctionalTest extends SapphireTest implements TestOnly public function get($url, $session = null, $headers = null, $cookies = null) { $this->cssParser = null; + if (Versioned::get_stage() === Versioned::DRAFT) { + $url = HTTP::setGetVar('stage', Versioned::DRAFT, $url); + } $response = $this->mainSession->get($url, $session, $headers, $cookies); if ($this->autoFollowRedirection && is_object($response) && $response->getHeader('Location')) { $response = $this->mainSession->followRedirection(); @@ -411,11 +416,9 @@ class FunctionalTest extends SapphireTest implements TestOnly public function useDraftSite($enabled = true) { if ($enabled) { - $this->session()->set('readingMode', 'Stage.Stage'); - $this->session()->set('unsecuredDraftSite', true); + Versioned::set_stage(Versioned::DRAFT); } else { - $this->session()->set('readingMode', 'Stage.Live'); - $this->session()->set('unsecuredDraftSite', false); + Versioned::set_stage(Versioned::LIVE); } } From 2c121e8a07297baa89c0bedad086b1a62de2ba0a Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 26 Sep 2017 12:24:09 +0100 Subject: [PATCH 02/14] new approach --- src/Dev/FunctionalTest.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index add41d955..7df93552f 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -105,11 +105,6 @@ class FunctionalTest extends SapphireTest implements TestOnly // Flush user $this->logOut(); - // Switch to draft site, if necessary - if (static::get_use_draft_site()) { - $this->useDraftSite(); - } - // Unprotect the site, tests are running with the assumption it's off. They will enable it on a case-by-case // basis. BasicAuth::protect_entire_site(false); @@ -163,7 +158,7 @@ class FunctionalTest extends SapphireTest implements TestOnly public function get($url, $session = null, $headers = null, $cookies = null) { $this->cssParser = null; - if (Versioned::get_stage() === Versioned::DRAFT) { + if (self::get_use_draft_site()) { $url = HTTP::setGetVar('stage', Versioned::DRAFT, $url); } $response = $this->mainSession->get($url, $session, $headers, $cookies); From 0fe56732af2e26699a103b67536fb6c19c7f4a14 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 20 Mar 2018 15:08:29 +1300 Subject: [PATCH 03/14] RequestHandler updateLink() extension point --- src/Control/RequestHandler.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index a8f1cfa69..9627be71a 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -560,6 +560,8 @@ class RequestHandler extends ViewableData // Check configured url_segment $url = $this->config()->get('url_segment'); if ($url) { + // Give extensions the chance to modify by reference + $this->extend('updateLink', $link, $action); return Controller::join_links($url, $action, '/'); } From e6ff8bc6814ad645ea8798118e33806300162629 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 20 Mar 2018 16:48:36 +1300 Subject: [PATCH 04/14] Docs for new versioned get param behaviour See https://github.com/silverstripe/silverstripe-cms/issues/1578 --- .../00_Model/10_Versioning.md | 91 ++++++++++++++++++- .../02_Controllers/01_Introduction.md | 17 ++-- docs/en/04_Changelogs/4.2.0.md | 37 ++++++++ 3 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 docs/en/04_Changelogs/4.2.0.md 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 ac824e28c..18207bbf2 100644 --- a/docs/en/02_Developer_Guides/00_Model/10_Versioning.md +++ b/docs/en/02_Developer_Guides/00_Model/10_Versioning.md @@ -554,10 +554,93 @@ public function init() ### Controllers -The current stage for each request is determined by `VersionedRequestFilter` before any controllers initialize, through -`Versioned::choose_site_stage()`. It checks for a `Stage` GET parameter, so you can force a draft stage by appending -`?stage=Stage` to your request. The setting is "sticky" in the PHP session, so any subsequent requests will also be in -draft stage. +The current stage for each request is determined by `VersionedHTTPMiddleware` before any controllers initialize, through +`Versioned::choose_site_stage()`. It checks for a `stage` GET parameter, so you can force a draft stage by appending +`?stage=Stage` to your request. + +Since SilverStripe 4.2, the current stage setting is no longer "sticky" in the session. +Any links presented on the view produced with `?stage=Stage` need to have the same GET parameters in order +to retain the stage. If you are using the `SiteTree->Link()` and `Controller->Link()` methods, +this is automatically the case for page links, controller links and form actions. + +You can opt for a session base stage setting through the `Versioned.use_session` setting. +Warning: This can lead to leaking of unpublished information, if a live URL is viewed in draft mode, +and the result is cached due to agressive cache settings (not varying on cookie values). + +*mysite/src/MyObject.php* + +```php +use SilverStripe\Core\Injector\Injector; +use SilverStripe\ORM\DataObject; +use SilverStripe\Versioned\Versioned; +use SilverStripe\Control\Controller; + +class MyObject extends DataObject { + + private static $extensions = [ + Versioned::class + ]; + + public function Link($action = null) + { + return Injector::inst()->get(MyObjectController::class)->Link($this->ID); + } + + public function CustomLink($action = null) + { + $link = Controller::join_links('custom-route', $this->ID, '?rand=' . rand()); + $this->extend('updateLink', $link, $action); // updates $link by reference + return $link; + } +} +``` + +*mysite/src/MyObjectController.php* + +```php +use SilverStripe\Control\Controller; +use SilverStripe\Control\HTTPRequest; +use SilverStripe\Versioned\VersionedRequestHandlerExtension; + +class MyObjectController extends Controller +{ + private static $extensions = [ + VersionedRequestHandlerExtension::class + ]; + + public function index(HTTPRequest $request) + { + $obj = MyObject::get()->byID($request->param('ID')); + if (!$obj) { + return $this->httpError(404); + } + + // Construct view + $html = sprintf('%s', $obj->Link(), $obj->ID); + + return $html; + } + + public function Link($action = null) + { + // Construct link with graceful handling of GET parameters + $link = Controller::join_links('my-objects', $action, '?rand=' . rand()); + + // Allow Versioned and other extension to update $link by reference + $this->extend('updateLink', $link, $action); + + return $link; + } +} +``` + +*mysite/_config/routes.yml* + +```yml +SilverStripe\Control\Director: + rules: + 'my-objects/$ID': 'MyObjectController' +```
The `choose_site_stage()` call only deals with setting the default stage, and doesn't check if the user is diff --git a/docs/en/02_Developer_Guides/02_Controllers/01_Introduction.md b/docs/en/02_Developer_Guides/02_Controllers/01_Introduction.md index 44c6810b4..c44f532dc 100644 --- a/docs/en/02_Developer_Guides/02_Controllers/01_Introduction.md +++ b/docs/en/02_Developer_Guides/02_Controllers/01_Introduction.md @@ -159,20 +159,23 @@ For more information about templates, inheritance and how to render into views, ## Link -Each controller should define a `Link()` method. This should be used to avoid hard coding your routing in views etc. +Each controller should define a `Link()` method. This should be used to avoid hard coding your routing in views, +as well as give other features in SilverStripe the ability to influence link behaviour. **mysite/code/controllers/TeamController.php** ```php public function Link($action = null) { - return Controller::join_links('teams', $action); + // Construct link with graceful handling of GET parameters + $link = Controller::join_links('teams', $ction); + + // Allow Versioned and other extension to update $link by reference. + $this->extend('updateLink', $link, $action); + + return $link; } -``` - -
-The [Controller::join_links()](api:SilverStripe\Control\Controller::join_links()) is optional, but makes `Link()` more flexible by allowing an `$action` argument, and concatenates the path segments with slashes. The action should map to a method on your controller. -
+``` ## Related Lessons * [Controller actions/DataObjects as pages](https://www.silverstripe.org/learn/lessons/v4/controller-actions-dataobjects-as-pages-1) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md new file mode 100644 index 000000000..5f92ae438 --- /dev/null +++ b/docs/en/04_Changelogs/4.2.0.md @@ -0,0 +1,37 @@ +# 4.2.0 + +## Overview {#overview} + + * Disable session-based stage setting in `Versioned` (see [#1578](https://github.com/silverstripe/silverstripe-cms/issues/1578)) + +## Upgrading {#upgrading} + +### Disable session-based stage setting + +When viewing a versioned record (usually pages) in "draft" mode, +SilverStripe used to record this mode in the session for further requests. +This has the advantage of transparently working on XHR and API requests, +as well as authenticated users navigating through other views. + +These subsequent requests no longer carried an explicit `stage` query parameter, +which meant the same URL might show draft or live content depending on your session state. +While most HTTP caching layers deal gracefully with this variation by disabling +any caching when a session cookie is present, there is a small chance +that draft content is exposed to unauthenticated users for the lifetime of the cache. + +Due to this potential risk for information leakage, +we have decided to only rely on the `stage` query parameter. +If you are consistently using the built-in `SiteTree->Link()` +and `Controller->Link()` methods to get URLs, this change likely won't affect you. + +If you are manually concatenating URLs to SilverStripe controllers +rather than through their `Link()` methods (in custom PHP or JavaScript), +or have implemented your own `Link()` methods on controllers exposing +versioned objects, you'll need to check your business logic. + +Alternatively, you can opt-out of this security feature via YAML configuration: + +```yml +SilverStripe\Versioned\Versioned: + use_session: true +``` \ No newline at end of file From 26402f3bb52b2effa4218ada4b06ce9199216190 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Mar 2018 15:50:31 +1300 Subject: [PATCH 05/14] ENHANCEMENT Enable request handlers to be extended --- src/Control/RequestHandler.php | 4 +++- src/Core/Extensible.php | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index 9627be71a..fc880def7 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -560,9 +560,11 @@ class RequestHandler extends ViewableData // Check configured url_segment $url = $this->config()->get('url_segment'); if ($url) { + $link = Controller::join_links($url, $action, '/'); + // Give extensions the chance to modify by reference $this->extend('updateLink', $link, $action); - return Controller::join_links($url, $action, '/'); + return $link; } // no link defined by default diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index da338e204..0723f189b 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -48,7 +48,6 @@ trait Extensible */ private static $unextendable_classes = array( ViewableData::class, - RequestHandler::class, ); /** From ba94e020e7deadd5deb9b4c6744f72bec37453b3 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Mar 2018 16:07:31 +1300 Subject: [PATCH 06/14] Fix FunctionalTest not setting persistent versioned mode --- src/Dev/FunctionalTest.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index 7df93552f..fdf1894bd 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -406,15 +406,13 @@ class FunctionalTest extends SapphireTest implements TestOnly * This is helpful if you're not testing publication functionality and don't want "stage management" cluttering * your test. * - * @param bool $enabled toggle the use of the draft site + * @param bool $draft toggle the use of the draft site */ - public function useDraftSite($enabled = true) + public function useDraftSite($draft = true) { - if ($enabled) { - Versioned::set_stage(Versioned::DRAFT); - } else { - Versioned::set_stage(Versioned::LIVE); - } + $stage = $draft ? Versioned::DRAFT : Versioned::LIVE; + Versioned::set_stage($stage); + Versioned::set_default_reading_mode(Versioned::get_reading_mode()); } /** From 5a069e2e7584dda00722b5d0034d9f20efdf5f57 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Mar 2018 16:44:14 +1300 Subject: [PATCH 07/14] remove unnecessary use --- src/Dev/State/SapphireTestState.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Dev/State/SapphireTestState.php b/src/Dev/State/SapphireTestState.php index ba2bd2556..3571a8006 100644 --- a/src/Dev/State/SapphireTestState.php +++ b/src/Dev/State/SapphireTestState.php @@ -3,7 +3,6 @@ namespace SilverStripe\Dev\State; use SilverStripe\Core\Injector\Injectable; -use SilverStripe\Dev\Debug; use SilverStripe\Dev\SapphireTest; class SapphireTestState implements TestState From 6fef72062b3732d1d7c95406297df2feb2795200 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Mar 2018 16:56:40 +1300 Subject: [PATCH 08/14] Restore old functionaltest behaviour --- src/Dev/FunctionalTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index fdf1894bd..2f3d6a6dc 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -2,20 +2,15 @@ namespace SilverStripe\Dev; -use SilverStripe\Control\Controller; +use PHPUnit_Framework_AssertionFailedError; use SilverStripe\Control\Director; -use SilverStripe\Control\HTTP; -use SilverStripe\Control\Session; use SilverStripe\Control\HTTPResponse; +use SilverStripe\Control\Session; use SilverStripe\Core\Config\Config; -use SilverStripe\ORM\DataObject; use SilverStripe\Security\BasicAuth; -use SilverStripe\Security\Member; -use SilverStripe\Security\Security; use SilverStripe\Security\SecurityToken; use SilverStripe\Versioned\Versioned; use SilverStripe\View\SSViewer; -use PHPUnit_Framework_AssertionFailedError; use SimpleXMLElement; /** @@ -105,6 +100,11 @@ class FunctionalTest extends SapphireTest implements TestOnly // Flush user $this->logOut(); + // Switch to draft site, if necessary + if (static::get_use_draft_site()) { + $this->useDraftSite(); + } + // Unprotect the site, tests are running with the assumption it's off. They will enable it on a case-by-case // basis. BasicAuth::protect_entire_site(false); @@ -158,9 +158,6 @@ class FunctionalTest extends SapphireTest implements TestOnly public function get($url, $session = null, $headers = null, $cookies = null) { $this->cssParser = null; - if (self::get_use_draft_site()) { - $url = HTTP::setGetVar('stage', Versioned::DRAFT, $url); - } $response = $this->mainSession->get($url, $session, $headers, $cookies); if ($this->autoFollowRedirection && is_object($response) && $response->getHeader('Location')) { $response = $this->mainSession->followRedirection(); @@ -410,6 +407,9 @@ class FunctionalTest extends SapphireTest implements TestOnly */ public function useDraftSite($draft = true) { + if (!class_exists(Versioned::class)) { + return; + } $stage = $draft ? Versioned::DRAFT : Versioned::LIVE; Versioned::set_stage($stage); Versioned::set_default_reading_mode(Versioned::get_reading_mode()); From 7e73ad2101957d06f84fd88ebee6173c4cd15e8e Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Mar 2018 17:28:02 +1300 Subject: [PATCH 09/14] Extensible test session --- src/Dev/FunctionalTest.php | 15 ++++++++------- src/Dev/TestSession.php | 12 ++++++++---- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index 2f3d6a6dc..45f0ce020 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -403,16 +403,17 @@ class FunctionalTest extends SapphireTest implements TestOnly * This is helpful if you're not testing publication functionality and don't want "stage management" cluttering * your test. * - * @param bool $draft toggle the use of the draft site + * @param bool $enabled toggle the use of the draft site */ - public function useDraftSite($draft = true) + public function useDraftSite($enabled = true) { - if (!class_exists(Versioned::class)) { - return; + if ($enabled) { + $this->session()->set('readingMode', 'Stage.Stage'); + $this->session()->set('unsecuredDraftSite', true); + } else { + $this->session()->set('readingMode', 'Stage.Live'); + $this->session()->set('unsecuredDraftSite', false); } - $stage = $draft ? Versioned::DRAFT : Versioned::LIVE; - Versioned::set_stage($stage); - Versioned::set_default_reading_mode(Versioned::get_reading_mode()); } /** diff --git a/src/Dev/TestSession.php b/src/Dev/TestSession.php index 171ec509f..a27142632 100644 --- a/src/Dev/TestSession.php +++ b/src/Dev/TestSession.php @@ -2,15 +2,16 @@ namespace SilverStripe\Dev; -use SilverStripe\Control\Cookie_Backend; -use SilverStripe\Control\HTTPRequest; -use SilverStripe\Control\Session; +use Exception; use SilverStripe\Control\Controller; +use SilverStripe\Control\Cookie_Backend; use SilverStripe\Control\Director; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; +use SilverStripe\Control\Session; +use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injector; use SimpleByName; -use Exception; use SimplePage; use SimplePageBuilder; @@ -20,6 +21,7 @@ use SimplePageBuilder; */ class TestSession { + use Extensible; /** * @var Session @@ -89,6 +91,7 @@ class TestSession */ public function get($url, $session = null, $headers = null, $cookies = null) { + $this->extend('updateGetURL', $url, $session, $headers, $cookies); $headers = (array) $headers; if ($this->lastUrl && !isset($headers['Referer'])) { $headers['Referer'] = $this->lastUrl; @@ -123,6 +126,7 @@ class TestSession */ public function post($url, $data, $headers = null, $session = null, $body = null, $cookies = null) { + $this->extend('updatePostURL', $url, $data, $headers, $session, $body, $cookies); $headers = (array) $headers; if ($this->lastUrl && !isset($headers['Referer'])) { $headers['Referer'] = $this->lastUrl; From 92f5147e3660345fb35182a72cd626be1a8cb559 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 22 Mar 2018 11:27:28 +1300 Subject: [PATCH 10/14] Updated versioned examples to changed API --- .../00_Model/10_Versioning.md | 28 +++++++++++-------- docs/en/04_Changelogs/4.2.0.md | 5 +++- 2 files changed, 20 insertions(+), 13 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 18207bbf2..12f46464e 100644 --- a/docs/en/02_Developer_Guides/00_Model/10_Versioning.md +++ b/docs/en/02_Developer_Guides/00_Model/10_Versioning.md @@ -552,7 +552,7 @@ public function init() } ``` -### Controllers +### Controllers {#controllers} The current stage for each request is determined by `VersionedHTTPMiddleware` before any controllers initialize, through `Versioned::choose_site_stage()`. It checks for a `stage` GET parameter, so you can force a draft stage by appending @@ -561,7 +561,9 @@ The current stage for each request is determined by `VersionedHTTPMiddleware` be Since SilverStripe 4.2, the current stage setting is no longer "sticky" in the session. Any links presented on the view produced with `?stage=Stage` need to have the same GET parameters in order to retain the stage. If you are using the `SiteTree->Link()` and `Controller->Link()` methods, -this is automatically the case for page links, controller links and form actions. +this is automatically the case for `DataObject` links, controller links and form actions. +Note that this behaviour applies for unversioned objects as well, since the views +these are presented in might still contain dependant objects that are versioned. You can opt for a session base stage setting through the `Versioned.use_session` setting. Warning: This can lead to leaking of unpublished information, if a live URL is viewed in draft mode, @@ -581,17 +583,23 @@ class MyObject extends DataObject { Versioned::class ]; - public function Link($action = null) + public function Link() { return Injector::inst()->get(MyObjectController::class)->Link($this->ID); } - public function CustomLink($action = null) + public function CustomLink() { $link = Controller::join_links('custom-route', $this->ID, '?rand=' . rand()); - $this->extend('updateLink', $link, $action); // updates $link by reference + $this->extend('updateLink', $link); // updates $link by reference return $link; } + + public function LiveLink() + { + // Force live link even when current view is in draft mode + return Controller::join_links(Injector::inst()->get(MyObjectController::class)->Link($this->ID), '?stage=Live'); + } } ``` @@ -600,14 +608,9 @@ class MyObject extends DataObject { ```php use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; -use SilverStripe\Versioned\VersionedRequestHandlerExtension; class MyObjectController extends Controller { - private static $extensions = [ - VersionedRequestHandlerExtension::class - ]; - public function index(HTTPRequest $request) { $obj = MyObject::get()->byID($request->param('ID')); @@ -624,9 +627,10 @@ class MyObjectController extends Controller public function Link($action = null) { // Construct link with graceful handling of GET parameters - $link = Controller::join_links('my-objects', $action, '?rand=' . rand()); + $link = Controller::join_links('my-objects', $action); - // Allow Versioned and other extension to update $link by reference + // Allow Versioned and other extension to update $link by reference. + // Calls VersionedStateExtension->updateLink(). $this->extend('updateLink', $link, $action); return $link; diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index 5f92ae438..361ecd98b 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -34,4 +34,7 @@ Alternatively, you can opt-out of this security feature via YAML configuration: ```yml SilverStripe\Versioned\Versioned: use_session: true -``` \ No newline at end of file +``` + +Check our [versioning docs](/developer_guides/model/versioning#controllers) +for more details. \ No newline at end of file From df9e0e40d351a6cad390939f2bcf5e931d4a03e2 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 22 Mar 2018 17:20:27 +1300 Subject: [PATCH 11/14] Deprecate useDraftStage --- src/Dev/FunctionalTest.php | 10 +++++++--- tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php | 9 ++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index 45f0ce020..29f787ebc 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -9,7 +9,6 @@ use SilverStripe\Control\Session; use SilverStripe\Core\Config\Config; use SilverStripe\Security\BasicAuth; use SilverStripe\Security\SecurityToken; -use SilverStripe\Versioned\Versioned; use SilverStripe\View\SSViewer; use SimpleXMLElement; @@ -46,6 +45,7 @@ class FunctionalTest extends SapphireTest implements TestOnly /** * Set this to true on your sub-class to use the draft site by default for every test in this class. * + * @deprecated 4.2..5.0 Use ?stage=Stage in your ->get() querystring requests instead * @var bool */ protected static $use_draft_site = false; @@ -101,6 +101,7 @@ class FunctionalTest extends SapphireTest implements TestOnly $this->logOut(); // Switch to draft site, if necessary + // If you rely on this you should be crafting stage-specific urls instead though. if (static::get_use_draft_site()) { $this->useDraftSite(); } @@ -403,16 +404,18 @@ class FunctionalTest extends SapphireTest implements TestOnly * This is helpful if you're not testing publication functionality and don't want "stage management" cluttering * your test. * + * @deprecated 4.2..5.0 Use ?stage=Stage querystring arguments instead of useDraftSite * @param bool $enabled toggle the use of the draft site */ public function useDraftSite($enabled = true) { + Deprecation::notice('5.0', 'Use ?stage=Stage querystring arguments instead of useDraftSite'); if ($enabled) { $this->session()->set('readingMode', 'Stage.Stage'); $this->session()->set('unsecuredDraftSite', true); } else { - $this->session()->set('readingMode', 'Stage.Live'); - $this->session()->set('unsecuredDraftSite', false); + $this->session()->clear('readingMode'); + $this->session()->clear('unsecuredDraftSite'); } } @@ -425,6 +428,7 @@ class FunctionalTest extends SapphireTest implements TestOnly } /** + * @deprecated 4.2..5.0 Use ?stage=Stage in your querystring arguments instead * @return bool */ public static function get_use_draft_site() diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php index b986dadd6..76aed937c 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php +++ b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php @@ -2,30 +2,25 @@ namespace SilverStripe\Forms\Tests\HTMLEditor; +use Silverstripe\Assets\Dev\TestAssetStore; use SilverStripe\Assets\File; use SilverStripe\Assets\FileNameFilter; use SilverStripe\Assets\Filesystem; use SilverStripe\Assets\Folder; use SilverStripe\Assets\Image; -use Silverstripe\Assets\Dev\TestAssetStore; use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Manifest\ModuleLoader; -use SilverStripe\Core\Manifest\ModuleManifest; -use SilverStripe\Core\Manifest\ModuleResourceLoader; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\FunctionalTest; use SilverStripe\Forms\HTMLEditor\HTMLEditorField; +use SilverStripe\Forms\HTMLEditor\TinyMCEConfig; use SilverStripe\Forms\HTMLReadonlyField; use SilverStripe\Forms\Tests\HTMLEditor\HTMLEditorFieldTest\TestObject; use SilverStripe\ORM\FieldType\DBHTMLText; -use SilverStripe\Forms\HTMLEditor\TinyMCEConfig; class HTMLEditorFieldTest extends FunctionalTest { protected static $fixture_file = 'HTMLEditorFieldTest.yml'; - protected static $use_draft_site = true; - protected static $extra_dataobjects = [ TestObject::class, ]; From 7351caf48702558d20a93bd83fdc4c36d4be50c3 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 23 Mar 2018 12:22:27 +1300 Subject: [PATCH 12/14] API Allow non-DataExtension Extensions to decorate dataobject --- src/Core/Extensible.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index 0723f189b..e7e9597aa 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -211,13 +211,6 @@ trait Extensible )); Injector::inst()->unregisterNamedObject($class); - - // load statics now for DataObject classes - if (is_subclass_of($class, DataObject::class)) { - if (!is_subclass_of($extensionClass, DataExtension::class)) { - user_error("$extensionClass cannot be applied to $class without being a DataExtension", E_USER_ERROR); - } - } return true; } From 28255b79eb2c70571215cc63ae29467c03e89d86 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 23 Mar 2018 12:47:07 +1300 Subject: [PATCH 13/14] Upgrading docs on new APIs in Versioned --- docs/en/04_Changelogs/4.2.0.md | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index 361ecd98b..dddf7738b 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -3,6 +3,7 @@ ## Overview {#overview} * Disable session-based stage setting in `Versioned` (see [#1578](https://github.com/silverstripe/silverstripe-cms/issues/1578)) + * Deprecated `FunctionalTest::useDraftSite()`. You should use querystring args instead for setting stage. ## Upgrading {#upgrading} @@ -37,4 +38,25 @@ SilverStripe\Versioned\Versioned: ``` Check our [versioning docs](/developer_guides/model/versioning#controllers) -for more details. \ No newline at end of file +for more details. + +### New Versioned API + +The following methods have been added to [api:SilverStripe\Versioned\Versioned] class: + + * `withVersionedMode()` Allows users to execute a closure which may internally modify + the current stage, but will guarantee these changes are reverted safely on return. + Helpful when temporarily performing a task in another stage or view mode. + * `get_draft_site_secured()` / `set_draft_site_secured()` Enables the explicit toggle + of draft site security. By setting this to false, you can expose a draft mode to + unauthenticated users. Replaces `unsecuredDraftSite` session var. + * `get_default_reading_mode()` / `set_default_reading_mode()` The default reading + mode is now configurable. Any non-default reading mode must have querystring args + to be visible. This will be the mode choosen for requests that do not have these args. + Note that the default mode for CMS is now draft, but is live on the frontend. + +A new class [api:SilverStripe\Versioned\ReadingMode] has also been added to assist with +conversion of the reading mode between: + - Reading mode string + - DataQuery parameters + - Querystring parameters From 386ef27f655d4470d6f74f2d10b0017be409f104 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 23 Mar 2018 15:28:00 +1300 Subject: [PATCH 14/14] Update requesthandlers with missing extension points --- src/Forms/FormField.php | 4 +++- src/Forms/FormRequestHandler.php | 14 ++++++------ .../ChangePasswordHandler.php | 22 +++++++++---------- .../MemberAuthenticator/LoginHandler.php | 19 ++++++++-------- .../LostPasswordHandler.php | 21 ++++++++++-------- src/Security/Security.php | 4 +++- 6 files changed, 46 insertions(+), 38 deletions(-) diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index 0c9108fcd..8b6c3a571 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -363,7 +363,9 @@ class FormField extends RequestHandler */ public function Link($action = null) { - return Controller::join_links($this->form->FormAction(), 'field/' . $this->name, $action); + $link = Controller::join_links($this->form->FormAction(), 'field/' . $this->name, $action); + $this->extend('updateLink', $link, $action); + return $link; } /** diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php index 0244a11f5..408f57d77 100644 --- a/src/Forms/FormRequestHandler.php +++ b/src/Forms/FormRequestHandler.php @@ -85,15 +85,15 @@ class FormRequestHandler extends RequestHandler // Respect FormObjectLink() method if ($controller->hasMethod("FormObjectLink")) { - return Controller::join_links( - $controller->FormObjectLink($this->form->getName()), - $action, - '/' - ); + $base = $controller->FormObjectLink($this->form->getName()); + } else { + $base = Controller::join_links($controller->Link(), $this->form->getName()); } - // Default form link - return Controller::join_links($controller->Link(), $this->form->getName(), $action, '/'); + // Join with action and decorate + $link = Controller::join_links($base, $action, '/'); + $this->extend('updateLink', $link, $action); + return $link; } /** diff --git a/src/Security/MemberAuthenticator/ChangePasswordHandler.php b/src/Security/MemberAuthenticator/ChangePasswordHandler.php index 3149f3404..74cc528fe 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordHandler.php +++ b/src/Security/MemberAuthenticator/ChangePasswordHandler.php @@ -7,7 +7,6 @@ use Psr\Container\NotFoundExceptionInterface; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\RequestHandler; -use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBField; @@ -25,9 +24,11 @@ class ChangePasswordHandler extends RequestHandler protected $authenticator; /** + * Link to this handler + * * @var string */ - protected $link; + protected $link = null; /** * @var array Allowed Actions @@ -123,8 +124,8 @@ class ChangePasswordHandler extends RequestHandler . '

You can request a new one here or change your password after' . ' you logged in.

', [ - 'link1' => $this->link('lostpassword'), - 'link2' => $this->link('login') + 'link1' => $this->Link('lostpassword'), + 'link2' => $this->Link('login') ] ) ); @@ -164,16 +165,15 @@ class ChangePasswordHandler extends RequestHandler /** * Return a link to this request handler. * The link returned is supplied in the constructor - * @param null $action + * + * @param string|null $action * @return string */ - public function link($action = null) + public function Link($action = null) { - if ($action) { - return Controller::join_links($this->link, $action); - } - - return $this->link; + $link = Controller::join_links($this->link, $action); + $this->extend('updateLink', $link, $action); + return $link; } /** diff --git a/src/Security/MemberAuthenticator/LoginHandler.php b/src/Security/MemberAuthenticator/LoginHandler.php index 2058c7864..f97538810 100644 --- a/src/Security/MemberAuthenticator/LoginHandler.php +++ b/src/Security/MemberAuthenticator/LoginHandler.php @@ -41,9 +41,11 @@ class LoginHandler extends RequestHandler ]; /** - * @var string Called link on this handler + * Link to this handler + * + * @var string */ - private $link; + protected $link = null; /** * @param string $link The URL to recreate this request handler @@ -59,16 +61,15 @@ class LoginHandler extends RequestHandler /** * Return a link to this request handler. * The link returned is supplied in the constructor + * * @param null|string $action * @return string */ - public function link($action = null) + public function Link($action = null) { - if ($action) { - return Controller::join_links($this->link, $action); - } - - return $this->link; + $link = Controller::join_links($this->link, $action); + $this->extend('updateLink', $link, $action); + return $link; } /** @@ -152,7 +153,7 @@ class LoginHandler extends RequestHandler public function getReturnReferer() { - return $this->link(); + return $this->Link(); } /** diff --git a/src/Security/MemberAuthenticator/LostPasswordHandler.php b/src/Security/MemberAuthenticator/LostPasswordHandler.php index d5d4b4e22..5086e525c 100644 --- a/src/Security/MemberAuthenticator/LostPasswordHandler.php +++ b/src/Security/MemberAuthenticator/LostPasswordHandler.php @@ -44,7 +44,12 @@ class LostPasswordHandler extends RequestHandler 'passwordsent', ]; - private $link = null; + /** + * Link to this handler + * + * @var string + */ + protected $link = null; /** * @param string $link The URL to recreate this request handler @@ -59,16 +64,14 @@ class LostPasswordHandler extends RequestHandler * Return a link to this request handler. * The link returned is supplied in the constructor * - * @param string $action + * @param string|null $action * @return string */ - public function link($action = null) + public function Link($action = null) { - if ($action) { - return Controller::join_links($this->link, $action); - } - - return $this->link; + $link = Controller::join_links($this->link, $action); + $this->extend('updateLink', $link, $action); + return $link; } /** @@ -261,7 +264,7 @@ class LostPasswordHandler extends RequestHandler protected function redirectToSuccess(array $data) { $link = Controller::join_links( - $this->link('passwordsent'), + $this->Link('passwordsent'), rawurlencode($data['Email']), '/' ); diff --git a/src/Security/Security.php b/src/Security/Security.php index 675ff0ba0..e3952c056 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -483,7 +483,9 @@ class Security extends Controller implements TemplateGlobalProvider public function Link($action = null) { /** @skipUpgrade */ - return Controller::join_links(Director::baseURL(), "Security", $action); + $link = Controller::join_links(Director::baseURL(), "Security", $action); + $this->extend('updateLink', $link, $action); + return $link; } /**