Merge pull request #3020 from tractorcow/pulls/3.1-autocomplete-username

API Security.remember_username to disable login form autocompletion

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());

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.

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
 

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", 

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
 	 *

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');