Merge pull request #4252 from patbolo/cross-device-remember-me

NEW Cross device "Remember Me" feature

docs/en/02_Developer_Guides/09_Security/00_Member.md

@@ -127,6 +127,13 @@ things, you should add appropriate `[api:Permission::checkMember()]` calls to th
 	  }
 	}
 
+## Saved User Logins ##
+
+Logins can be "remembered" across multiple devices when user checks the "Remember Me" box. By default, a new login token
+will be created and associated with the device used during authentication. When user logs out, all previously saved tokens
+for all devices will be revoked, unless `[api:RememberLoginHash::$logout_across_devices] is set to false. For extra security,
+single tokens can be enforced by setting `[api:RememberLoginHash::$force_single_token] to true.
+
 
 ## API Documentation
 

docs/en/04_Changelogs/4.0.0.md

@@ -63,6 +63,10 @@ or developing your own website. These improvements are mainly geared at CMS core
 
 See notes below on upgrading extensions to the ErrorPage class
 
+### Member
+
+ * `Member` Field 'RememberLoginToken' removed, replaced with 'RememberLoginHashes' has_many relationship
+
 ### Assets and Filesystem
 
 The following image manipulations previously deprecated has been removed:

lang/en.yml

@@ -383,13 +383,14 @@ en:
     FIRSTNAME: 'First Name'
     INTERFACELANG: 'Interface Language'
     INVALIDNEWPASSWORD: 'We couldn''t accept that password: {password}'
+    KEEPMESIGNEDIN: 'Keep me signed in'
     LOGGEDINAS: 'You''re logged in as {name}.'
     NEWPASSWORD: 'New Password'
     NoPassword: 'There is no password on this member.'
     PASSWORD: Password
     PASSWORDEXPIRED: 'Your password has expired. Please choose a new one.'
     PLURALNAME: Members
-    REMEMBERME: 'Remember me next time?'
+    REMEMBERME: 'Remember me next time? (for %d days on this device)'
     SINGULARNAME: Member
     SUBJECTPASSWORDCHANGED: 'Your password has been changed'
     SUBJECTPASSWORDRESET: 'Your password reset link'

security/Member.php

@@ -9,7 +9,6 @@
  * @property string $Surname
  * @property string $Email
  * @property string $Password
- * @property string $RememberLoginToken
  * @property string $TempIDHash
  * @property string $TempIDExpired
  * @property string $AutoLoginHash
@@ -32,7 +31,6 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		'TempIDHash' => 'Varchar(160)', // Temporary id used for cms re-authentication
 		'TempIDExpired' => 'SS_Datetime', // Expiry of temp login
 		'Password' => 'Varchar(160)',
-		'RememberLoginToken' => 'Varchar(160)', // Note: this currently holds a hash, not a token.
 		'AutoLoginHash' => 'Varchar(160)', // Used to auto-login the user on password reset
 		'AutoLoginExpired' => 'SS_Datetime',
 		// This is an arbitrary code pointing to a PasswordEncryptor instance,
@@ -59,6 +57,7 @@ class Member extends DataObject implements TemplateGlobalProvider {
 
 	private static $has_many = array(
 		'LoggedPasswords' => 'MemberPassword',
+		'RememberLoginHashes' => 'RememberLoginHash'
 	);
 
 	private static $many_many = array();
@@ -109,7 +108,6 @@ class Member extends DataObject implements TemplateGlobalProvider {
 	 * @var array
 	 */
 	private static $hidden_fields = array(
-		'RememberLoginToken',
 		'AutoLoginHash',
 		'AutoLoginExpired',
 		'PasswordEncryption',
@@ -447,16 +445,22 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		// This lets apache rules detect whether the user has logged in
 		if(Member::config()->login_marker_cookie) Cookie::set(Member::config()->login_marker_cookie, 1, 0);
 
+		// Cleans up any potential previous hash for this member on this device
+		if ($alcDevice = Cookie::get('alc_device')) {
+			RememberLoginHash::get()->filter('DeviceID', $alcDevice)->removeAll();
+		}
 		if($remember) {
-			// Store the hash and give the client the cookie with the token.
+			$rememberLoginHash = RememberLoginHash::generate($this);
-			$generator = new RandomGenerator();
+			$tokenExpiryDays = Config::inst()->get('RememberLoginHash', 'token_expiry_days');
-			$token = $generator->randomToken('sha1');
+			$deviceExpiryDays = Config::inst()->get('RememberLoginHash', 'device_expiry_days');
-			$hash = $this->encryptWithUserSettings($token);
+			Cookie::set('alc_enc', $this->ID . ':' . $rememberLoginHash->getToken(), 
-			$this->RememberLoginToken = $hash;
+				$tokenExpiryDays, null, null, null, true);
-			Cookie::set('alc_enc', $this->ID . ':' . $token, 90, null, null, null, true);
+			Cookie::set('alc_device', $rememberLoginHash->DeviceID, $deviceExpiryDays, null, null, null, true);
-		} else {
+		} else {			
-			$this->RememberLoginToken = null;
+			Cookie::set('alc_enc', null);
+			Cookie::set('alc_device', null);
 			Cookie::force_expiry('alc_enc');
+			Cookie::force_expiry('alc_device');
 		}
 
 		// Clear the incorrect log-in count
@@ -515,7 +519,9 @@ class Member extends DataObject implements TemplateGlobalProvider {
 	 */
 	public static function autoLogin() {
 		// Don't bother trying this multiple times
-		self::$_already_tried_to_auto_log_in = true;
+		if (!class_exists('SapphireTest') || !SapphireTest::is_running_test()) {
+			self::$_already_tried_to_auto_log_in = true;
+		}
 
 		if(strpos(Cookie::get('alc_enc'), ':') === false
 			|| Session::get("loggedInAs")
@@ -524,36 +530,56 @@ class Member extends DataObject implements TemplateGlobalProvider {
 			return;
 		}
 
-		list($uid, $token) = explode(':', Cookie::get('alc_enc'), 2);
+		if(strpos(Cookie::get('alc_enc'), ':') && Cookie::get('alc_device') && !Session::get("loggedInAs")) {
+			list($uid, $token) = explode(':', Cookie::get('alc_enc'), 2);
+			$deviceID = Cookie::get('alc_device');
 
-		$member = DataObject::get_by_id("Member", $uid);
+			$member = Member::get()->byId($uid);
 
-		// check if autologin token matches
+			$rememberLoginHash = null;
-		if($member) {
-			$hash = $member->encryptWithUserSettings($token);
-			if(!$member->RememberLoginToken || $member->RememberLoginToken !== $hash) {
-				$member = null;
-			}
-		}
 
-		if($member) {
+			// check if autologin token matches
-			self::session_regenerate_id();
+			if($member) {
-			Session::set("loggedInAs", $member->ID);
+				$hash = $member->encryptWithUserSettings($token);
-			// This lets apache rules detect whether the user has logged in
+				$rememberLoginHash = RememberLoginHash::get()
-			if(Member::config()->login_marker_cookie) {
+					->filter(array(
-				Cookie::set(Member::config()->login_marker_cookie, 1, 0, null, null, false, true);
+						'MemberID' => $member->ID,
+						'DeviceID' => $deviceID,
+						'Hash' => $hash
+					))->First();
+				if(!$rememberLoginHash) {
+					$member = null;
+				} else {
+					// Check for expired token
+					$expiryDate = new DateTime($rememberLoginHash->ExpiryDate);
+					$now = SS_Datetime::now();
+					$now = new DateTime($now->Rfc2822());
+					if ($now > $expiryDate) {
+						$member = null;
+					}
+				}
 			}
 
-			$generator = new RandomGenerator();
+			if($member) {
-			$token = $generator->randomToken('sha1');
+				self::session_regenerate_id();
-			$hash = $member->encryptWithUserSettings($token);
+				Session::set("loggedInAs", $member->ID);
-			$member->RememberLoginToken = $hash;
+				// This lets apache rules detect whether the user has logged in
-			Cookie::set('alc_enc', $member->ID . ':' . $token, 90, null, null, false, true);
+				if(Member::config()->login_marker_cookie) {
+					Cookie::set(Member::config()->login_marker_cookie, 1, 0, null, null, false, true);
+				}
 
-			$member->write();
+				if ($rememberLoginHash) {
+					$rememberLoginHash->renew();
+					$tokenExpiryDays = Config::inst()->get('RememberLoginHash', 'token_expiry_days');
+					Cookie::set('alc_enc', $member->ID . ':' . $rememberLoginHash->getToken(), 
+						$tokenExpiryDays, null, null, false, true);
+				}
 
-			// Audit logging hook
+				$member->write();
-			$member->extend('memberAutoLoggedIn');
+
+				// Audit logging hook
+				$member->extend('memberAutoLoggedIn');
+			}
 		}
 	}
 
@@ -570,8 +596,13 @@ class Member extends DataObject implements TemplateGlobalProvider {
 
 		$this->extend('memberLoggedOut');
 
-		$this->RememberLoginToken = null;
+		// Clears any potential previous hashes for this member
+		RememberLoginHash::clear($this, Cookie::get('alc_device'));
+
+		Cookie::set('alc_enc', null); // // Clear the Remember Me cookie
 		Cookie::force_expiry('alc_enc');
+		Cookie::set('alc_device', null);
+		Cookie::force_expiry('alc_device');
 
 		// Switch back to live in order to avoid infinite loops when
 		// redirecting to the login screen (if this login screen is versioned)
@@ -1332,6 +1363,8 @@ class Member extends DataObject implements TemplateGlobalProvider {
 			// Members shouldn't be able to directly view/edit logged passwords
 			$fields->removeByName('LoggedPasswords');
 
+			$fields->removeByName('RememberLoginHashes');
+
 			if(Permission::check('EDIT_PERMISSIONS')) {
 				$groupsMap = array();
 				foreach(Group::get() as $group) {

security/MemberLoginForm.php

@@ -91,10 +91,18 @@ class MemberLoginForm extends LoginForm {
 					$emailField->setAttribute('autocomplete', 'off');
 				}
 				if(Security::config()->autologin_enabled) {
-					$fields->push(new CheckboxField(
+					$fields->push(
-						"Remember",
+						CheckboxField::create(
-						_t('Member.REMEMBERME', "Remember me next time?")
+							"Remember",
-					));
+							_t('Member.KEEPMESIGNEDIN', "Keep me signed in")
+						)->setAttribute(
+							'title',
+							sprintf(
+								_t('Member.REMEMBERME', "Remember me next time? (for %d days on this device)"),
+								Config::inst()->get('RememberLoginHash', 'token_expiry_days')
+							)
+						)
+					);
 				}
 			}
 			if(!$actions) {

security/RememberLoginHash.php

@@ -0,0 +1,164 @@
+<?php
+/**
+ * Persists a token associated with a device for users who opted for the "Remember Me"
+ * feature when logging in.
+ * By default, logging out will discard all existing tokens for this user
+ * The device ID is a temporary ID associated with the device when the user logged in
+ * and chose to get the login state remembered on this device. When logging out, the ID 
+ * is discarded as well.
+ *
+ * @package framework
+ * @subpackage security
+ *
+ * @property string $DeviceID
+ * @property string $RememberLoginHash
+ */
+
+class RememberLoginHash extends DataObject {
+
+	private static $db = array (
+		'DeviceID' => 'Varchar(40)',
+		'Hash' => 'Varchar(160)',
+		'ExpiryDate' => 'SS_Datetime'
+	);
+
+	private static $has_one = array (
+		'Member' => 'Member',
+	);
+
+	private static $indexes = array(
+		'DeviceID' => true,
+		'Hash' => true
+	);
+
+	/**
+	 * Determines if logging out on one device also clears existing login tokens
+	 * on all other devices owned by the member.
+	 * If set to false, there is no way for users to revoke a login, unless additional
+	 * code (custom or with a module) is provided by the developer
+	 *
+	 * @config
+	 * @var bool
+	 */
+	private static $logout_across_devices = true;
+
+	/**
+	 * Number of days the token will be valid for
+	 *
+	 * @config
+	 * @var int
+	 */
+	private static $token_expiry_days = 90;
+
+	/**
+	 * Number of days the device ID will be valid for
+	 *
+	 * @config
+	 * @var int
+	 */
+	private static $device_expiry_days = 365;
+
+	/**
+	 * If true, user can only use auto login on one device. A user can still login from multiple
+	 * devices, but previous tokens from other devices will become invalid.
+	 *
+	 * @config
+	 * @var bool
+	 */
+	private static $force_single_token = false;
+
+	/**
+	 * The token used for the hash
+	 */
+	private $token = null;
+
+	public function getToken() {
+		return $this->token;
+	}
+
+	public function setToken($token) {
+		$this->token = $token;
+	}
+
+	/**
+	 * Randomly generates a new ID used for the device
+	 * @return string A device ID
+	 */
+	protected function getNewDeviceID(){
+		$generator = new RandomGenerator();
+		return $generator->randomToken('sha1');
+	}
+
+	/**
+	 * Creates a new random token and hashes it using the 
+	 * member information
+	 * @param Member The logged in user 
+	 * @return string The hash to be stored in the database
+	 */
+	public function getNewHash(Member $member){
+		$generator = new RandomGenerator();
+		$this->setToken($generator->randomToken('sha1'));
+		return $member->encryptWithUserSettings($this->token);
+	}
+
+	/**
+	 * Generates a new login hash associated with a device
+	 * The device is assigned a globally unique device ID
+	 * The returned login hash stores the hashed token in the 
+	 * database, for this device and this member
+	 * @param Member The logged in user 
+	 * @return RememberLoginHash The generated login hash
+	 */
+	public static function generate(Member $member) {
+		if(!$member->exists()) { return; }
+		if (Config::inst()->get('RememberLoginHash', 'force_single_token') == true) {
+			$rememberLoginHash = RememberLoginHash::get()->filter('MemberID', $member->ID)->removeAll();
+		} 
+		$rememberLoginHash = RememberLoginHash::create();
+		do {
+			$deviceID = $rememberLoginHash->getNewDeviceID();
+		} while (RememberLoginHash::get()->filter('DeviceID', $deviceID)->Count());
+		
+		$rememberLoginHash->DeviceID = $deviceID;
+		$rememberLoginHash->Hash = $rememberLoginHash->getNewHash($member);
+		$rememberLoginHash->MemberID = $member->ID;
+		$now = SS_Datetime::now();
+		$expiryDate = new DateTime($now->Rfc2822());
+		$tokenExpiryDays = Config::inst()->get('RememberLoginHash', 'token_expiry_days');
+		$expiryDate->add(new DateInterval('P'.$tokenExpiryDays.'D'));
+		$rememberLoginHash->ExpiryDate = $expiryDate->format('Y-m-d H:i:s');
+		$rememberLoginHash->extend('onAfterGenerateToken');
+		$rememberLoginHash->write();
+		return $rememberLoginHash;
+	}
+
+	/**
+	 * Generates a new hash for this member but keeps the device ID intact
+	 * @param Member the logged in user
+	 * @return RememberLoginHash
+	 */
+	public function renew() {
+		$hash = $this->getNewHash($this->Member());
+		$this->Hash = $hash;
+		$this->extend('onAfterRenewToken');
+		$this->write();
+		return $this;
+	}
+
+	/**
+	 * Deletes existing tokens for this member
+	 * if logout_across_devices is true, all tokens are deleted, otherwise
+	 * only the token for the provided device ID will be removed
+	 */
+	public static function clear(Member $member, $alcDevice = null) {
+		if(!$member->exists()) { return; }
+		$filter = array('MemberID'=>$member->ID);
+		if ((Config::inst()->get('RememberLoginHash', 'logout_across_devices') == false) && $alcDevice) {
+			$filter['DeviceID'] = $alcDevice;
+		}		
+		RememberLoginHash::get()
+			->filter($filter)
+			->removeAll();
+	}
+	
+}

tests/security/MemberTest.php

@@ -789,6 +789,236 @@ class MemberTest extends FunctionalTest {
 		$this->assertFalse($m2->validateAutoLoginToken($m1Token), 'Fails token validity test against other member.');
 	}
 
+	public function testRememberMeHashGeneration() {
+		$m1 = $this->objFromFixture('Member', 'grouplessmember');
+
+		$m1->login(true);
+		$hashes = RememberLoginHash::get()->filter('MemberID', $m1->ID);
+		$this->assertEquals($hashes->Count(), 1);
+		$firstHash = $hashes->First();
+		$this->assertNotNull($firstHash->DeviceID);
+		$this->assertNotNull($firstHash->Hash);
+	}
+
+	public function testRememberMeHashAutologin() {
+		$m1 = $this->objFromFixture('Member', 'noexpiry');
+
+		$m1->login(true);
+		$firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->First();
+		$this->assertNotNull($firstHash);
+
+		// re-generates the hash so we can get the token
+		$firstHash->Hash = $firstHash->getNewHash($m1);
+		$token = $firstHash->getToken();
+		$firstHash->write();
+
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$token,
+				'alc_device' => $firstHash->DeviceID
+			)
+		);
+		$message = _t(
+			'Member.LOGGEDINAS',
+			"You're logged in as {name}.",
+			array('name' => $m1->FirstName)
+		);
+		$this->assertContains($message, $response->getBody());
+
+		$this->session()->inst_set('loggedInAs', null);
+
+		// A wrong token or a wrong device ID should not let us autologin 
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.str_rot13($token),
+				'alc_device' => $firstHash->DeviceID
+			)
+		);
+		$this->assertNotContains($message, $response->getBody());
+
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$token,
+				'alc_device' => str_rot13($firstHash->DeviceID)
+			)
+		);
+		$this->assertNotContains($message, $response->getBody());
+
+		// Re-logging (ie 'alc_enc' has expired), and not checking the "Remember Me" option 
+		// should remove all previous hashes for this device
+		$response = $this->post(
+			'Security/LoginForm', 
+			array(
+				'Email' => $m1->Email,
+				'Password' => '1nitialPassword',
+				'AuthenticationMethod' => 'MemberAuthenticator',
+				'action_dologin' => 'action_dologin'
+			),
+			null,
+			$this->session(),
+			null, 
+			array(
+				'alc_device' => $firstHash->DeviceID
+			)
+		);
+		$this->assertContains($message, $response->getBody());
+		$this->assertEquals(RememberLoginHash::get()->filter('MemberID', $m1->ID)->Count(), 0);
+	}
+
+	public function testExpiredRememberMeHashAutologin() {
+		$m1 = $this->objFromFixture('Member', 'noexpiry');
+
+		$m1->login(true);
+		$firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->First();
+		$this->assertNotNull($firstHash);
+
+		// re-generates the hash so we can get the token
+		$firstHash->Hash = $firstHash->getNewHash($m1);
+		$token = $firstHash->getToken();
+		$firstHash->ExpiryDate = '2000-01-01 00:00:00';
+		$firstHash->write();
+
+		SS_DateTime::set_mock_now('1999-12-31 23:59:59');
+
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$token,
+				'alc_device' => $firstHash->DeviceID
+			)
+		);
+		$message = _t(
+			'Member.LOGGEDINAS',
+			"You're logged in as {name}.",
+			array('name' => $m1->FirstName)
+		);
+		$this->assertContains($message, $response->getBody());
+
+		$this->session()->inst_set('loggedInAs', null);
+
+		// re-generates the hash so we can get the token
+		$firstHash->Hash = $firstHash->getNewHash($m1);
+		$token = $firstHash->getToken();
+		$firstHash->ExpiryDate = '2000-01-01 00:00:00';
+		$firstHash->write();
+
+		SS_DateTime::set_mock_now('2000-01-01 00:00:01');
+
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$token,
+				'alc_device' => $firstHash->DeviceID
+			)
+		);
+		$this->assertNotContains($message, $response->getBody());
+		$this->session()->inst_set('loggedInAs', null);
+		SS_Datetime::clear_mock_now();
+	}
+
+	public function testRememberMeMultipleDevices() {
+		$m1 = $this->objFromFixture('Member', 'noexpiry');
+
+		// First device
+		$m1->login(true);
+		Cookie::set('alc_device', null);
+		// Second device
+		$m1->login(true);
+
+		// Hash of first device
+		$firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->First();
+		$this->assertNotNull($firstHash);
+
+		// Hash of second device
+		$secondHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->Last();
+		$this->assertNotNull($secondHash);
+
+		// DeviceIDs are different
+		$this->assertNotEquals($firstHash->DeviceID, $secondHash->DeviceID);
+
+		// re-generates the hashes so we can get the tokens
+		$firstHash->Hash = $firstHash->getNewHash($m1);
+		$firstToken = $firstHash->getToken();
+		$firstHash->write();
+
+		$secondHash->Hash = $secondHash->getNewHash($m1);
+		$secondToken = $secondHash->getToken();
+		$secondHash->write();
+
+		// Accessing the login page should show the user's name straight away
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$firstToken,
+				'alc_device' => $firstHash->DeviceID
+			)
+		);
+		$message = _t(
+			'Member.LOGGEDINAS',
+			"You're logged in as {name}.",
+			array('name' => $m1->FirstName)
+		);
+		$this->assertContains($message, $response->getBody());
+
+		$this->session()->inst_set('loggedInAs', null);
+
+		// Accessing the login page from the second device
+		$response = $this->get(
+			'Security/login', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$secondToken,
+				'alc_device' => $secondHash->DeviceID
+			)
+		);
+		$this->assertContains($message, $response->getBody());
+
+		$logout_across_devices = Config::inst()->get('RememberLoginHash', 'logout_across_devices');
+
+		// Logging out from the second device - only one device being logged out
+		Config::inst()->update('RememberLoginHash', 'logout_across_devices', false);
+		$response = $this->get(
+			'Security/logout', 
+			$this->session(), 
+			null, 
+			array(
+				'alc_enc' => $m1->ID.':'.$secondToken,
+				'alc_device' => $secondHash->DeviceID
+			)
+		);
+		$this->assertEquals(
+			RememberLoginHash::get()->filter(array('MemberID'=>$m1->ID, 'DeviceID'=>$firstHash->DeviceID))->Count(), 
+			1
+		);
+
+		// Logging out from any device when all login hashes should be removed
+		Config::inst()->update('RememberLoginHash', 'logout_across_devices', true);
+		$m1->login(true);
+		$response = $this->get('Security/logout', $this->session());
+		$this->assertEquals(
+			RememberLoginHash::get()->filter('MemberID', $m1->ID)->Count(), 
+			0
+		);
+
+		Config::inst()->update('RememberLoginHash', 'logout_across_devices', $logout_across_devices);
+	}
+
 	public function testCanDelete() {
 		$admin1 = $this->objFromFixture('Member', 'admin');
 		$admin2 = $this->objFromFixture('Member', 'other-admin');