tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 12:05:37 +00:00
Code Issues Projects Releases Wiki Activity

API Security better respects BackURL on login

Browse Source 
BUG Restore missing authentication message not appearing in the login form $Content area (regression from #1807)
This commit is contained in:
Damian Mooyman 2015-03-31 18:06:00 +13:00
parent 43f49e8434
commit 95c162ef0d
3 changed files with 149 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
security/Security.php
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'))

150
tests/security/MemberTest.yml
View File

@ -1,78 +1,78 @@
Permission: Permission:
admin: admin:
Code: ADMIN Code: ADMIN
security-admin: security-admin:
Code: CMS_ACCESS_SecurityAdmin Code: CMS_ACCESS_SecurityAdmin
Group: Group:
admingroup: admingroup:
Title: Admin Title: Admin
Code: admin Code: admin
Permissions: =>Permission.admin Permissions: =>Permission.admin
securityadminsgroup: securityadminsgroup:
Title: securityadminsgroup Title: securityadminsgroup
Code: securityadminsgroup Code: securityadminsgroup
Permissions: =>Permission.security-admin Permissions: =>Permission.security-admin
staffgroup: staffgroup:
Title: staffgroup Title: staffgroup
Code: staffgroup Code: staffgroup
managementgroup: managementgroup:
Title: managementgroup Title: managementgroup
Code: managementgroup Code: managementgroup
Parent: =>Group.staffgroup Parent: =>Group.staffgroup
accountinggroup: accountinggroup:
Title: accountinggroup Title: accountinggroup
Code: accountinggroup Code: accountinggroup
Parent: =>Group.staffgroup Parent: =>Group.staffgroup
ceogroup: ceogroup:
Title: ceogroup Title: ceogroup
Code: ceogroup Code: ceogroup
Parent: =>Group.managementgroup Parent: =>Group.managementgroup
memberlessgroup: memberlessgroup:
Title: Memberless Group Title: Memberless Group
code: memberless code: memberless
Member: Member:
admin: admin:
FirstName: Admin FirstName: Admin
Email: admin@silverstripe.com Email: admin@silverstripe.com
Groups: =>Group.admingroup Groups: =>Group.admingroup
other-admin: other-admin:
FirstName: OtherAdmin FirstName: OtherAdmin
Email: other-admin@silverstripe.com Email: other-admin@silverstripe.com
Groups: =>Group.admingroup Groups: =>Group.admingroup
test: test:
FirstName: Test FirstName: Test
Surname: User Surname: User
Email: sam@silverstripe.com Email: sam@silverstripe.com
Password: 1nitialPassword Password: 1nitialPassword
PasswordExpiry: 2030-01-01 PasswordExpiry: 2030-01-01
Groups: =>Group.securityadminsgroup Groups: =>Group.securityadminsgroup
expiredpassword: expiredpassword:
FirstName: Test FirstName: Test
Surname: User Surname: User
Email: expired@silverstripe.com Email: expired@silverstripe.com
Password: 1nitialPassword Password: 1nitialPassword
PasswordExpiry: 2006-01-01 PasswordExpiry: 2006-01-01
noexpiry: noexpiry:
FirstName: Test FirstName: Test
Surname: User Surname: User
Email: noexpiry@silverstripe.com Email: noexpiry@silverstripe.com
Password: 1nitialPassword Password: 1nitialPassword
staffmember: staffmember:
Email: staffmember@test.com Email: staffmember@test.com
Groups: =>Group.staffgroup Groups: =>Group.staffgroup
managementmember: managementmember:
Email: managementmember@test.com Email: managementmember@test.com
Groups: =>Group.managementgroup Groups: =>Group.managementgroup
accountingmember: accountingmember:
Email: accountingmember@test.com Email: accountingmember@test.com
Groups: =>Group.accountinggroup Groups: =>Group.accountinggroup
ceomember: ceomember:
Email: ceomember@test.com Email: ceomember@test.com
Groups: =>Group.ceogroup Groups: =>Group.ceogroup
grouplessmember: grouplessmember:
FirstName: Groupless Member FirstName: Groupless Member
noformatmember: noformatmember:
Email: noformat@test.com Email: noformat@test.com
delocalemember: delocalemember:
Email: delocalemember@test.com Email: delocalemember@test.com
Locale: de_DE Locale: de_DE

57
tests/security/SecurityTest.php
View File

@ -114,7 +114,64 @@ 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');