[ss-2018-005] Remove isDev / isTest querystring arguments

docs/en/00_Getting_Started/01_Installation/05_Common_Problems.md

@@ -8,13 +8,28 @@ This first and foremost means that your environment is set to "live mode" (see [
 detailed error messages for security reasons. You'll typically need to get your environment into "dev mode" to see more
 information.
 
-If you can log-in to the CMS as an administrator, append `?isDev=1` to any URL to temporarily set your browsing session into
-"dev mode". If you can't log-in in the first place because of the error, please
-configure an `SS_ENVIRONMENT_TYPE` through [environment-management] (don't forget to remove it afterwards!).
+It is highly recommended that you have separate environments for debugging issues, in order
+to prevent vulnerability disclosure on your live site.
+
+However, if you are unable to replicate your issue on a secure test environment, you can
+configure your live site to display verbose errors with the below YML.
+
+```yaml
+---
+Name: project-live-logging
+After: live-logging
+Except:
+  environment: dev
+---
+SilverStripe\Core\Injector\Injector:
+  Monolog\Handler\HandlerInterface:
+    properties:
+      DefaultFormatter: '%$Monolog\Formatter\FormatterInterface.detailed'
+```
 
 <div class="warning" markdown='1'>
-On "live" environments, the `?isDev=1` solution is preferred, as it means that your other visitors don't see ugly
-(and potentially security sensitive) PHP errors as well.
+Note: It is only recommended to enable this feature for short periods of time, and
+only if necessary to debug issues that cannot be safely reproduced in a separate environment.
 </div>
 
 ## mod_rewrite isn't working but it's installed (prior to SilverStripe 3.1.11)

docs/en/02_Developer_Guides/07_Debugging/02_URL_Variable_Tools.md

@@ -19,14 +19,12 @@ session variables, used templates and much more.
  | URL Variable | | Values | | Description                                                     | 
  | ------------ | | ------ | | -----------                                                     | 
  | flush        | | 1      | | Clears out all caches. Used mainly during development, e.g. when adding new classes or templates. Requires "dev" mode or ADMIN login |
- | showtemplate | | 1      | | Show the compiled version of all the templates used, including line numbers.  Good when you have a syntax error in a template. Cannot be used on a Live site without **isDev**. |
+ | showtemplate | | 1      | | Show the compiled version of all the templates used, including line numbers.  Good when you have a syntax error in a template. Cannot be used on a Live site. |
 
 ## General Testing
 
  | URL Variable  | | Values | | Description                                                | 
  | ------------  | | ------ | | -----------                                                | 
- | isDev         | | 1      | | Put the site into [development mode](../), enabling debugging messages to the browser on a live server.  For security, you'll be asked to log in with an administrator log-in. Will persist for the current browser session. | 
- | isTest        | | 1      | | See above. | 
  | debug         | | 1      | | Show a collection of debugging information about the director / controller operation        |
  | debug_request | | 1      | | Show all steps of the request from initial [HTTPRequest](api:SilverStripe\Control\HTTPRequest) to [Controller](api:SilverStripe\Control\Controller) to Template Rendering  | 
 

docs/en/04_Changelogs/5.0.0.md

@@ -30,6 +30,8 @@ guide developers in preparing existing 4.x code for compatibility with 5.0.
 * Some of the core SilverStripe methods have begun implementing PHP 7 scalar type hinting and return type
   hints. Wherever your project code overloads methods with new signatures you will need to update them to
   match.
+* `isDev` / `isTest` querystring arguments have been removed due to security concerns 
+  [ss-2018-005](https://www.silverstripe.org/download/security-releases/ss-2018-005/).
 
 ### Versioning {#overview-versioning}
 

src/Core/CoreKernel.php

@@ -133,12 +133,6 @@ public function getEnvironment()
             return $this->enviroment;
         }
 
-        // Check saved session
-        $env = $this->sessionEnvironment();
-        if ($env) {
-            return $env;
-        }
-
         // Check getenv
         if ($env = Environment::getEnv('SS_ENVIRONMENT_TYPE')) {
             return $env;
@@ -147,43 +141,6 @@ public function getEnvironment()
         return self::LIVE;
     }
 
-    /**
-     * Check or update any temporary environment specified in the session.
-     *
-     * @return null|string
-     */
-    protected function sessionEnvironment()
-    {
-        // Check isDev in querystring
-        if (isset($_GET['isDev'])) {
-            if (isset($_SESSION)) {
-                unset($_SESSION['isTest']); // In case we are changing from test mode
-                $_SESSION['isDev'] = $_GET['isDev'];
-            }
-            return self::DEV;
-        }
-
-        // Check isTest in querystring
-        if (isset($_GET['isTest'])) {
-            if (isset($_SESSION)) {
-                unset($_SESSION['isDev']); // In case we are changing from dev mode
-                $_SESSION['isTest'] = $_GET['isTest'];
-            }
-            return self::TEST;
-        }
-
-        // Check session
-        if (!empty($_SESSION['isDev'])) {
-            return self::DEV;
-        }
-        if (!empty($_SESSION['isTest'])) {
-            return self::TEST;
-        }
-
-        // no session environment
-        return null;
-    }
-
     public function boot($flush = false)
     {
         $this->bootPHP();

src/Core/Startup/ErrorControlChainMiddleware.php

@@ -36,7 +36,7 @@ public function process(HTTPRequest $request, callable $next)
 
         // Prepare tokens and execute chain
         $reloadToken = ParameterConfirmationToken::prepare_tokens(
-            ['isTest', 'isDev', 'flush'],
+            ['flush'],
             $request
         );
         $chain = new ErrorControlChain();

src/Core/Startup/ParameterConfirmationToken.php

@@ -214,7 +214,6 @@ public function reloadRequiredIfError()
      */
     public function suppress()
     {
-        unset($_GET[$this->parameterName]);
         $this->request->offsetUnset($this->parameterName);
     }
 

tests/php/Control/DirectorTest.php

@@ -395,49 +395,6 @@ public function testIsSiteUrl()
         $this->assertFalse(Director::is_site_url("//test.com?url=" . Director::absoluteBaseURL()));
     }
 
-    /**
-     * Tests isDev, isTest, isLive set from querystring
-     */
-    public function testQueryIsEnvironment()
-    {
-        if (!isset($_SESSION)) {
-            $_SESSION = [];
-        }
-        // Reset
-        unset($_SESSION['isDev']);
-        unset($_SESSION['isLive']);
-        unset($_GET['isTest']);
-        unset($_GET['isDev']);
-
-        /** @var Kernel $kernel */
-        $kernel = Injector::inst()->get(Kernel::class);
-        $kernel->setEnvironment(null);
-
-        // Test isDev=1
-        $_GET['isDev'] = '1';
-        $this->assertTrue(Director::isDev());
-        $this->assertFalse(Director::isTest());
-        $this->assertFalse(Director::isLive());
-
-        // Test persistence
-        unset($_GET['isDev']);
-        $this->assertTrue(Director::isDev());
-        $this->assertFalse(Director::isTest());
-        $this->assertFalse(Director::isLive());
-
-        // Test change to isTest
-        $_GET['isTest'] = '1';
-        $this->assertFalse(Director::isDev());
-        $this->assertTrue(Director::isTest());
-        $this->assertFalse(Director::isLive());
-
-        // Test persistence
-        unset($_GET['isTest']);
-        $this->assertFalse(Director::isDev());
-        $this->assertTrue(Director::isTest());
-        $this->assertFalse(Director::isLive());
-    }
-
     public function testResetGlobalsAfterTestRequest()
     {
         $_GET = array('somekey' => 'getvalue');

tests/php/Core/Startup/ParameterConfirmationTokenTest.php

@@ -20,17 +20,17 @@ class ParameterConfirmationTokenTest extends SapphireTest
     protected function setUp()
     {
         parent::setUp();
-        $_GET = [];
-        $_GET['parameterconfirmationtokentest_notoken'] = 'value';
-        $_GET['parameterconfirmationtokentest_empty'] = '';
-        $_GET['parameterconfirmationtokentest_withtoken'] = '1';
-        $_GET['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
-        $_GET['parameterconfirmationtokentest_nulltoken'] = '1';
-        $_GET['parameterconfirmationtokentest_nulltokentoken'] = null;
-        $_GET['parameterconfirmationtokentest_emptytoken'] = '1';
-        $_GET['parameterconfirmationtokentest_emptytokentoken'] = '';
-        $_GET['BackURL'] = 'page?parameterconfirmationtokentest_backtoken=1';
-        $this->request = new HTTPRequest('GET', 'anotherpage', $_GET);
+        $get = [];
+        $get['parameterconfirmationtokentest_notoken'] = 'value';
+        $get['parameterconfirmationtokentest_empty'] = '';
+        $get['parameterconfirmationtokentest_withtoken'] = '1';
+        $get['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
+        $get['parameterconfirmationtokentest_nulltoken'] = '1';
+        $get['parameterconfirmationtokentest_nulltokentoken'] = null;
+        $get['parameterconfirmationtokentest_emptytoken'] = '1';
+        $get['parameterconfirmationtokentest_emptytokentoken'] = '';
+        $get['BackURL'] = 'page?parameterconfirmationtokentest_backtoken=1';
+        $this->request = new HTTPRequest('GET', 'anotherpage', $get);
         $this->request->setSession(new Session([]));
     }
 
@@ -129,11 +129,6 @@ public function testPrepareTokens()
             $this->request
         );
         $this->assertEquals('parameterconfirmationtokentest_backtoken', $token->getName());
-
-        // Test prepare_tokens() unsets $_GET vars
-        $this->assertArrayNotHasKey('parameterconfirmationtokentest_notoken', $_GET);
-        $this->assertArrayNotHasKey('parameterconfirmationtokentest_empty', $_GET);
-        $this->assertArrayNotHasKey('parameterconfirmationtokentest_noparam', $_GET);
     }
 
     public function dataProviderURLs()