API Disable get parameter access to site stage mode

BUG Fix missing and undocumented response from Security::permissionFailure()
This commit is contained in:
Damian Mooyman 2015-12-07 17:25:27 +13:00
parent 1bc7297c8f
commit 38e154af0a
4 changed files with 127 additions and 72 deletions

View File

@ -1,4 +1,5 @@
<?php <?php
/** /**
* Initialises the versioned stage when a request is made. * Initialises the versioned stage when a request is made.
* *
@ -8,7 +9,37 @@
class VersionedRequestFilter implements RequestFilter { class VersionedRequestFilter implements RequestFilter {
public function preRequest(SS_HTTPRequest $request, Session $session, DataModel $model) { public function preRequest(SS_HTTPRequest $request, Session $session, DataModel $model) {
Versioned::choose_site_stage($session); // Bootstrap session so that Session::get() accesses the right instance
$dummyController = new Controller();
$dummyController->setSession($session);
$dummyController->setRequest($request);
$dummyController->pushCurrent();
// Block non-authenticated users from setting the stage mode
if(!Versioned::can_choose_site_stage($request)) {
$permissionMessage = sprintf(
_t(
"ContentController.DRAFT_SITE_ACCESS_RESTRICTION",
'You must log in with your CMS password in order to view the draft or archived content. '.
'<a href="%s">Click here to go back to the published site.</a>'
),
Controller::join_links(Director::baseURL(), $request->getURL(), "?stage=Live")
);
// Force output since RequestFilter::preRequest doesn't support response overriding
$response = Security::permissionFailure($dummyController, $permissionMessage);
$session->inst_save();
$dummyController->popCurrent();
// Prevent output in testing
if(class_exists('SapphireTest', false) && SapphireTest::is_running_test()) {
return false;
}
$response->output();
die;
}
Versioned::choose_site_stage();
$dummyController->popCurrent();
return true; return true;
} }

View File

@ -991,6 +991,26 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
//-----------------------------------------------------------------------------------------------// //-----------------------------------------------------------------------------------------------//
/**
* Determine if the current user is able to set the given site stage / archive
*
* @param SS_HTTPRequest $request
* @return bool
*/
public static function can_choose_site_stage($request) {
// Request is allowed if stage isn't being modified
if((!$request->getVar('stage') || $request->getVar('stage') === static::get_live_stage())
&& !$request->getVar('archiveDate')
) {
return true;
}
// Check permissions with member ID in session.
$member = Member::currentUser();
$permissions = Config::inst()->get(get_called_class(), 'non_live_permissions');
return $member && Permission::checkMember($member, $permissions);
}
/** /**
* Choose the stage the site is currently on. * Choose the stage the site is currently on.
* *
@ -1002,14 +1022,10 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
* *
* If neither of these are set, it checks the session, otherwise the stage * If neither of these are set, it checks the session, otherwise the stage
* is set to 'Live'. * is set to 'Live'.
*
* @param Session $session Optional session within which to store the resulting stage
*/ */
public static function choose_site_stage($session = null) { public static function choose_site_stage() {
// Check any pre-existing session mode // Check any pre-existing session mode
$preexistingMode = $session $preexistingMode = Session::get('readingMode');
? $session->inst_get('readingMode')
: Session::get('readingMode');
// Determine the reading mode // Determine the reading mode
if(isset($_GET['stage'])) { if(isset($_GET['stage'])) {
@ -1031,11 +1047,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
if(($preexistingMode && $preexistingMode !== $mode) if(($preexistingMode && $preexistingMode !== $mode)
|| (!$preexistingMode && $mode !== self::DEFAULT_MODE) || (!$preexistingMode && $mode !== self::DEFAULT_MODE)
) { ) {
if($session) { Session::set('readingMode', $mode);
$session->inst_set('readingMode', $mode);
} else {
Session::set('readingMode', $mode);
}
} }
if(!headers_sent() && !Director::is_cli()) { if(!headers_sent() && !Director::is_cli()) {

View File

@ -208,6 +208,7 @@ class Security extends Controller implements TemplateGlobalProvider {
* *
* The alreadyLoggedIn value can contain a '%s' placeholder that will be replaced with a link * The alreadyLoggedIn value can contain a '%s' placeholder that will be replaced with a link
* to log in. * to log in.
* @return SS_HTTPResponse
*/ */
public static function permissionFailure($controller = null, $messageSet = null) { public static function permissionFailure($controller = null, $messageSet = null) {
self::set_ignore_disallowed_actions(true); self::set_ignore_disallowed_actions(true);
@ -226,79 +227,78 @@ class Security extends Controller implements TemplateGlobalProvider {
} }
} }
return $response; return $response;
} else { }
// Prepare the messageSet provided
if(!$messageSet) {
if($configMessageSet = static::config()->get('default_message_set')) {
$messageSet = $configMessageSet;
} else {
$messageSet = array(
'default' => _t(
'Security.NOTEPAGESECURED',
"That page is secured. Enter your credentials below and we will send "
. "you right along."
),
'alreadyLoggedIn' => _t(
'Security.ALREADYLOGGEDIN',
"You don't have access to this page. If you have another account that "
. "can access that page, you can log in again below.",
"%s will be replaced with a link to log in." // Prepare the messageSet provided
) if(!$messageSet) {
); if($configMessageSet = static::config()->get('default_message_set')) {
} $messageSet = $configMessageSet;
} else {
$messageSet = array(
'default' => _t(
'Security.NOTEPAGESECURED',
"That page is secured. Enter your credentials below and we will send "
. "you right along."
),
'alreadyLoggedIn' => _t(
'Security.ALREADYLOGGEDIN',
"You don't have access to this page. If you have another account that "
. "can access that page, you can log in again below.",
"%s will be replaced with a link to log in."
)
);
} }
}
if(!is_array($messageSet)) { if(!is_array($messageSet)) {
$messageSet = array('default' => $messageSet); $messageSet = array('default' => $messageSet);
} }
$member = Member::currentUser(); $member = Member::currentUser();
// Work out the right message to show // Work out the right message to show
if($member && $member->exists()) { if($member && $member->exists()) {
$response = ($controller) ? $controller->getResponse() : new SS_HTTPResponse(); $response = ($controller) ? $controller->getResponse() : new SS_HTTPResponse();
$response->setStatusCode(403); $response->setStatusCode(403);
//If 'alreadyLoggedIn' is not specified in the array, then use the default //If 'alreadyLoggedIn' is not specified in the array, then use the default
//which should have been specified in the lines above //which should have been specified in the lines above
if(isset($messageSet['alreadyLoggedIn'])) { if(isset($messageSet['alreadyLoggedIn'])) {
$message = $messageSet['alreadyLoggedIn']; $message = $messageSet['alreadyLoggedIn'];
} else {
$message = $messageSet['default'];
}
// Somewhat hackish way to render a login form with an error message.
$me = new Security();
$form = $me->LoginForm();
$form->sessionMessage($message, 'warning');
Session::set('MemberLoginForm.force_message',1);
$formText = $me->login();
$response->setBody($formText);
$controller->extend('permissionDenied', $member);
return $response;
} else { } else {
$message = $messageSet['default']; $message = $messageSet['default'];
} }
Session::set("Security.Message.message", $message); // Somewhat hackish way to render a login form with an error message.
Session::set("Security.Message.type", 'warning'); $me = new Security();
$form = $me->LoginForm();
$form->sessionMessage($message, 'warning');
Session::set('MemberLoginForm.force_message',1);
$formText = $me->login();
Session::set("BackURL", $_SERVER['REQUEST_URI']); $response->setBody($formText);
// TODO AccessLogEntry needs an extension to handle permission denied errors
// Audit logging hook
$controller->extend('permissionDenied', $member); $controller->extend('permissionDenied', $member);
$controller->redirect( return $response;
Config::inst()->get('Security', 'login_url') } else {
. "?BackURL=" . urlencode($_SERVER['REQUEST_URI']) $message = $messageSet['default'];
);
} }
return;
Session::set("Security.Message.message", $message);
Session::set("Security.Message.type", 'warning');
Session::set("BackURL", $_SERVER['REQUEST_URI']);
// TODO AccessLogEntry needs an extension to handle permission denied errors
// Audit logging hook
$controller->extend('permissionDenied', $member);
return $controller->redirect(
Config::inst()->get('Security', 'login_url')
. "?BackURL=" . urlencode($_SERVER['REQUEST_URI'])
);
} }
public function init() { public function init() {

View File

@ -597,6 +597,8 @@ class VersionedTest extends SapphireTest {
*/ */
public function testReadingPersistent() { public function testReadingPersistent() {
$session = Injector::inst()->create('Session', array()); $session = Injector::inst()->create('Session', array());
$adminID = $this->logInWithPermission('ADMIN');
$session->inst_set('loggedInAs', $adminID);
// Set to stage // Set to stage
Director::test('/?stage=Stage', null, $session); Director::test('/?stage=Stage', null, $session);
@ -628,10 +630,11 @@ class VersionedTest extends SapphireTest {
// Test that session doesn't redundantly store the default stage if it doesn't need to // Test that session doesn't redundantly store the default stage if it doesn't need to
$session2 = Injector::inst()->create('Session', array()); $session2 = Injector::inst()->create('Session', array());
$session2->inst_set('loggedInAs', $adminID);
Director::test('/', null, $session2); Director::test('/', null, $session2);
$this->assertEmpty($session2->inst_changedData()); $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData());
Director::test('/?stage=Live', null, $session2); Director::test('/?stage=Live', null, $session2);
$this->assertEmpty($session2->inst_changedData()); $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData());
// Test choose_site_stage // Test choose_site_stage
Session::set('readingMode', 'Stage.Stage'); Session::set('readingMode', 'Stage.Stage');
@ -645,6 +648,15 @@ class VersionedTest extends SapphireTest {
$this->assertEquals('Stage.Live', Versioned::get_reading_mode()); $this->assertEquals('Stage.Live', Versioned::get_reading_mode());
} }
/**
* Test that stage parameter is blocked by non-administrative users
*/
public function testReadingModeSecurity() {
$this->setExpectedException('SS_HTTPResponse_Exception', 'Invalid request');
$session = Injector::inst()->create('Session', array());
$result = Director::test('/?stage=Stage', null, $session);
}
/** /**
* Ensures that the latest version of a record is the expected value * Ensures that the latest version of a record is the expected value
* *