diff --git a/security/Security.php b/security/Security.php
index 97edc0f5c..77f6ed4c7 100644
--- a/security/Security.php
+++ b/security/Security.php
@@ -413,6 +413,20 @@ class Security extends Controller implements TemplateGlobalProvider {
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.
$customisedController = $controller->customise(array(
+ "Content" => $message,
"Message" => $message,
"MessageType" => $messageType,
"Form" => $content,
@@ -572,7 +587,8 @@ class Security extends Controller implements TemplateGlobalProvider {
* @return Form Returns the lost password form
*/
public function LostPasswordForm() {
- return MemberLoginForm::create( $this,
+ return MemberLoginForm::create(
+ $this,
'LostPasswordForm',
new FieldList(
new EmailField('Email', _t('Member.EMAIL', 'Email'))
diff --git a/tests/security/MemberTest.yml b/tests/security/MemberTest.yml
index 3ea9a90f1..223dba5c2 100644
--- a/tests/security/MemberTest.yml
+++ b/tests/security/MemberTest.yml
@@ -1,78 +1,78 @@
Permission:
- admin:
- Code: ADMIN
- security-admin:
- Code: CMS_ACCESS_SecurityAdmin
+ admin:
+ Code: ADMIN
+ security-admin:
+ Code: CMS_ACCESS_SecurityAdmin
Group:
- admingroup:
- Title: Admin
- Code: admin
- Permissions: =>Permission.admin
- securityadminsgroup:
- Title: securityadminsgroup
- Code: securityadminsgroup
- Permissions: =>Permission.security-admin
- staffgroup:
- Title: staffgroup
- Code: staffgroup
- managementgroup:
- Title: managementgroup
- Code: managementgroup
- Parent: =>Group.staffgroup
- accountinggroup:
- Title: accountinggroup
- Code: accountinggroup
- Parent: =>Group.staffgroup
- ceogroup:
- Title: ceogroup
- Code: ceogroup
- Parent: =>Group.managementgroup
- memberlessgroup:
- Title: Memberless Group
- code: memberless
+ admingroup:
+ Title: Admin
+ Code: admin
+ Permissions: =>Permission.admin
+ securityadminsgroup:
+ Title: securityadminsgroup
+ Code: securityadminsgroup
+ Permissions: =>Permission.security-admin
+ staffgroup:
+ Title: staffgroup
+ Code: staffgroup
+ managementgroup:
+ Title: managementgroup
+ Code: managementgroup
+ Parent: =>Group.staffgroup
+ accountinggroup:
+ Title: accountinggroup
+ Code: accountinggroup
+ Parent: =>Group.staffgroup
+ ceogroup:
+ Title: ceogroup
+ Code: ceogroup
+ Parent: =>Group.managementgroup
+ memberlessgroup:
+ Title: Memberless Group
+ code: memberless
Member:
- admin:
- FirstName: Admin
- Email: admin@silverstripe.com
- Groups: =>Group.admingroup
- other-admin:
- FirstName: OtherAdmin
- Email: other-admin@silverstripe.com
- Groups: =>Group.admingroup
- test:
- FirstName: Test
- Surname: User
- Email: sam@silverstripe.com
- Password: 1nitialPassword
- PasswordExpiry: 2030-01-01
- Groups: =>Group.securityadminsgroup
- expiredpassword:
- FirstName: Test
- Surname: User
- Email: expired@silverstripe.com
- Password: 1nitialPassword
- PasswordExpiry: 2006-01-01
- noexpiry:
- FirstName: Test
- Surname: User
- Email: noexpiry@silverstripe.com
- Password: 1nitialPassword
- staffmember:
- Email: staffmember@test.com
- Groups: =>Group.staffgroup
- managementmember:
- Email: managementmember@test.com
- Groups: =>Group.managementgroup
- accountingmember:
- Email: accountingmember@test.com
- Groups: =>Group.accountinggroup
- ceomember:
- Email: ceomember@test.com
- Groups: =>Group.ceogroup
- grouplessmember:
- FirstName: Groupless Member
- noformatmember:
- Email: noformat@test.com
- delocalemember:
- Email: delocalemember@test.com
- Locale: de_DE
\ No newline at end of file
+ admin:
+ FirstName: Admin
+ Email: admin@silverstripe.com
+ Groups: =>Group.admingroup
+ other-admin:
+ FirstName: OtherAdmin
+ Email: other-admin@silverstripe.com
+ Groups: =>Group.admingroup
+ test:
+ FirstName: Test
+ Surname: User
+ Email: sam@silverstripe.com
+ Password: 1nitialPassword
+ PasswordExpiry: 2030-01-01
+ Groups: =>Group.securityadminsgroup
+ expiredpassword:
+ FirstName: Test
+ Surname: User
+ Email: expired@silverstripe.com
+ Password: 1nitialPassword
+ PasswordExpiry: 2006-01-01
+ noexpiry:
+ FirstName: Test
+ Surname: User
+ Email: noexpiry@silverstripe.com
+ Password: 1nitialPassword
+ staffmember:
+ Email: staffmember@test.com
+ Groups: =>Group.staffgroup
+ managementmember:
+ Email: managementmember@test.com
+ Groups: =>Group.managementgroup
+ accountingmember:
+ Email: accountingmember@test.com
+ Groups: =>Group.accountinggroup
+ ceomember:
+ Email: ceomember@test.com
+ Groups: =>Group.ceogroup
+ grouplessmember:
+ FirstName: Groupless Member
+ noformatmember:
+ Email: noformat@test.com
+ delocalemember:
+ Email: delocalemember@test.com
+ Locale: de_DE
diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php
index be163dadb..c44aefef7 100644
--- a/tests/security/SecurityTest.php
+++ b/tests/security/SecurityTest.php
@@ -114,7 +114,64 @@ class SecurityTest extends FunctionalTest {
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('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('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(
+ '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(
+ '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() {
$member = DataObject::get_one('Member');