From 997077ae83a5a315f62df167e7a4b3f9f59a52fe Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 9 Apr 2014 11:41:25 +1200 Subject: [PATCH] API Security.remember_username to disable login form autocompletion --- dev/FunctionalTest.php | 4 ++++ docs/en/changelogs/3.1.5.md | 9 +++++++++ docs/en/topics/security.md | 1 + security/MemberLoginForm.php | 9 ++++++++- security/Security.php | 9 +++++++++ tests/security/SecurityTest.php | 30 ++++++++++++++++++++++++++++++ 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 docs/en/changelogs/3.1.5.md diff --git a/dev/FunctionalTest.php b/dev/FunctionalTest.php index 675e767a6..61df9e9be 100644 --- a/dev/FunctionalTest.php +++ b/dev/FunctionalTest.php @@ -39,6 +39,8 @@ class FunctionalTest extends SapphireTest { /** * CSSContentParser for the most recently requested page. + * + * @var CSSContentParser */ protected $cssParser = null; @@ -176,6 +178,8 @@ class FunctionalTest extends SapphireTest { /** * Return a CSSContentParser for the most recent content. + * + * @return CSSContentParser */ public function cssParser() { if(!$this->cssParser) $this->cssParser = new CSSContentParser($this->mainSession->lastContent()); diff --git a/docs/en/changelogs/3.1.5.md b/docs/en/changelogs/3.1.5.md new file mode 100644 index 000000000..54647c1e8 --- /dev/null +++ b/docs/en/changelogs/3.1.5.md @@ -0,0 +1,9 @@ +# 3.1.5 + +## Upgrading + + * If running an application in an environment where user security is critical, it may be necessary to + assign the config value `Security.remember_username` to false. This will disable persistence of + user login name between sessions, and disable browser auto-completion on the username field. + Note that users of certain browsers who have previously autofilled and saved login credentials + will need to clear their password autofill history before this setting is properly respected. diff --git a/docs/en/topics/security.md b/docs/en/topics/security.md index be5386d16..44493a9f0 100644 --- a/docs/en/topics/security.md +++ b/docs/en/topics/security.md @@ -445,6 +445,7 @@ In addition, you can tighten password security with the following configuration the user is blocked from further attempts for the timespan defined in `$lock_out_delay_mins` * `Member.lock_out_delay_mins`: Minutes of enforced lockout after incorrect password attempts. Only applies if `lock_out_after_incorrect_logins` is greater than 0. + * `Security.remember_username`: Set to false to disable autocomplete on login form ## Clickjacking: Prevent iframe Inclusion diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php index c95216b42..bb250115c 100644 --- a/security/MemberLoginForm.php +++ b/security/MemberLoginForm.php @@ -80,9 +80,16 @@ class MemberLoginForm extends LoginForm { new HiddenField("AuthenticationMethod", null, $this->authenticator_class, $this), // Regardless of what the unique identifer field is (usually 'Email'), it will be held in the // 'Email' value, below: - new TextField("Email", $label, Session::get('SessionForms.MemberLoginForm.Email'), null, $this), + $emailField = new TextField("Email", $label, null, null, $this), new PasswordField("Password", _t('Member.PASSWORD', 'Password')) ); + if(Security::config()->remember_username) { + $emailField->setValue(Session::get('SessionForms.MemberLoginForm.Email')); + } else { + // Some browsers won't respect this attribute unless it's added to the form + $this->setAttribute('autocomplete', 'off'); + $emailField->setAttribute('autocomplete', 'off'); + } if(Security::config()->autologin_enabled) { $fields->push(new CheckboxField( "Remember", diff --git a/security/Security.php b/security/Security.php index 26f1536be..315fda7f6 100644 --- a/security/Security.php +++ b/security/Security.php @@ -63,6 +63,15 @@ class Security extends Controller { */ private static $autologin_enabled = true; + /** + * Determine if login username may be remembered between login sessions + * If set to false this will disable autocomplete and prevent username persisting in the session + * + * @config + * @var bool + */ + private static $remember_username = true; + /** * Location of word list to use for generating passwords * diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 90d1cb617..3d3ce1066 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -15,6 +15,8 @@ class SecurityTest extends FunctionalTest { protected $priorDefaultAuthenticator = null; protected $priorUniqueIdentifierField = null; + + protected $priorRememberUsername = null; public function setUp() { // This test assumes that MemberAuthenticator is present and the default @@ -29,6 +31,7 @@ class SecurityTest extends FunctionalTest { // And that the unique identified field is 'Email' $this->priorUniqueIdentifierField = Member::config()->unique_identifier_field; + $this->priorRememberUsername = Security::config()->remember_username; Member::config()->unique_identifier_field = 'Email'; parent::setUp(); @@ -48,6 +51,7 @@ class SecurityTest extends FunctionalTest { // Restore unique identifier field Member::config()->unique_identifier_field = $this->priorUniqueIdentifierField; + Security::config()->remember_username = $this->priorRememberUsername; parent::tearDown(); } @@ -124,6 +128,32 @@ class SecurityTest extends FunctionalTest { $this->session()->inst_set('loggedInAs', null); } + public function testLoginUsernamePersists() { + // Test that username does not persist + $this->session()->inst_set('SessionForms.MemberLoginForm.Email', 'myuser@silverstripe.com'); + Security::config()->remember_username = false; + $this->get(Config::inst()->get('Security', 'login_url')); + $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm #Email input.text'); + $this->assertEquals(1, count($items)); + $this->assertEmpty((string)$items[0]->attributes()->value); + $this->assertEquals('off', (string)$items[0]->attributes()->autocomplete); + $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm'); + $this->assertEquals(1, count($form)); + $this->assertEquals('off', (string)$form[0]->attributes()->autocomplete); + + // Test that username does persist when necessary + $this->session()->inst_set('SessionForms.MemberLoginForm.Email', 'myuser@silverstripe.com'); + Security::config()->remember_username = true; + $this->get(Config::inst()->get('Security', 'login_url')); + $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm #Email input.text'); + $this->assertEquals(1, count($items)); + $this->assertEquals('myuser@silverstripe.com', (string)$items[0]->attributes()->value); + $this->assertNotEquals('off', (string)$items[0]->attributes()->autocomplete); + $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm'); + $this->assertEquals(1, count($form)); + $this->assertNotEquals('off', (string)$form[0]->attributes()->autocomplete); + } + public function testExternalBackUrlRedirectionDisallowed() { // Test internal relative redirect $response = $this->doTestLoginForm('noexpiry@silverstripe.com', '1nitialPassword', 'testpage');