Merge pull request #4051 from tractorcow/pulls/3/fix-security-returnurl

API Security better respects BackURL on login
This commit is contained in:
Daniel Hensby 2015-03-31 21:44:04 +01:00
commit aea3512e61
3 changed files with 149 additions and 76 deletions

View File

@ -413,6 +413,20 @@ class Security extends Controller implements TemplateGlobalProvider {
if($result instanceof SS_HTTPResponse) return $result; if($result instanceof SS_HTTPResponse) return $result;
} }
} }
// If arriving on the login page already logged in, with no security error, and a ReturnURL then redirect
// back. The login message check is neccesary to prevent infinite loops where BackURL links to
// an action that triggers Security::permissionFailure.
// This step is necessary in cases such as automatic redirection where a user is authenticated
// upon landing on an SSL secured site and is automatically logged in, or some other case
// where the user has permissions to continue but is not given the option.
if($this->request->requestVar('BackURL')
&& !$this->getLoginMessage()
&& ($member = Member::currentUser())
&& $member->exists()
) {
return $this->redirectBack();
}
} }
/** /**
@ -523,6 +537,7 @@ class Security extends Controller implements TemplateGlobalProvider {
// Finally, customise the controller to add any form messages and the form. // Finally, customise the controller to add any form messages and the form.
$customisedController = $controller->customise(array( $customisedController = $controller->customise(array(
"Content" => $message,
"Message" => $message, "Message" => $message,
"MessageType" => $messageType, "MessageType" => $messageType,
"Form" => $content, "Form" => $content,
@ -572,7 +587,8 @@ class Security extends Controller implements TemplateGlobalProvider {
* @return Form Returns the lost password form * @return Form Returns the lost password form
*/ */
public function LostPasswordForm() { public function LostPasswordForm() {
return MemberLoginForm::create( $this, return MemberLoginForm::create(
$this,
'LostPasswordForm', 'LostPasswordForm',
new FieldList( new FieldList(
new EmailField('Email', _t('Member.EMAIL', 'Email')) new EmailField('Email', _t('Member.EMAIL', 'Email'))

View File

@ -115,6 +115,63 @@ class SecurityTest extends FunctionalTest {
Config::unnest(); Config::unnest();
} }
/**
* Follow all redirects recursively
*
* @param string $url
* @param int $limit Max number of requests
* @return SS_HTTPResponse
*/
protected function getRecursive($url, $limit = 10) {
$this->cssParser = null;
$response = $this->mainSession->get($url);
while(--$limit > 0 && $response instanceof SS_HTTPResponse && $response->getHeader('Location')) {
$response = $this->mainSession->followRedirection();
}
return $response;
}
public function testAutomaticRedirectionOnLogin() {
// BackURL with permission error (not authenticated) should not redirect
if($member = Member::currentUser()) $member->logOut();
$response = $this->getRecursive('SecurityTest_SecuredController');
$this->assertContains(Convert::raw2xml("That page is secured."), $response->getBody());
$this->assertContains('<input type="submit" name="action_dologin"', $response->getBody());
// Non-logged in user should not be redirected, but instead shown the login form
// No message/context is available as the user has not attempted to view the secured controller
$response = $this->getRecursive('Security/login?BackURL=SecurityTest_SecuredController/');
$this->assertNotContains(Convert::raw2xml("That page is secured."), $response->getBody());
$this->assertNotContains(Convert::raw2xml("You don't have access to this page"), $response->getBody());
$this->assertContains('<input type="submit" name="action_dologin"', $response->getBody());
// BackURL with permission error (wrong permissions) should not redirect
$this->logInAs('grouplessmember');
$response = $this->getRecursive('SecurityTest_SecuredController');
$this->assertContains(Convert::raw2xml("You don't have access to this page"), $response->getBody());
$this->assertContains(
'<input type="submit" name="action_logout" value="Log in as someone else"',
$response->getBody()
);
// Directly accessing this page should attempt to follow the BackURL, but stop when it encounters the error
$response = $this->getRecursive('Security/login?BackURL=SecurityTest_SecuredController/');
$this->assertContains(Convert::raw2xml("You don't have access to this page"), $response->getBody());
$this->assertContains(
'<input type="submit" name="action_logout" value="Log in as someone else"',
$response->getBody()
);
// Check correctly logged in admin doesn't generate the same errors
$this->logInAs('admin');
$response = $this->getRecursive('SecurityTest_SecuredController');
$this->assertContains(Convert::raw2xml("Success"), $response->getBody());
// Directly accessing this page should attempt to follow the BackURL and succeed
$response = $this->getRecursive('Security/login?BackURL=SecurityTest_SecuredController/');
$this->assertContains(Convert::raw2xml("Success"), $response->getBody());
}
public function testLogInAsSomeoneElse() { public function testLogInAsSomeoneElse() {
$member = DataObject::get_one('Member'); $member = DataObject::get_one('Member');