From bab1f230bfccf8f7fcc66bc38630eb4b2c1c2c4a Mon Sep 17 00:00:00 2001 From: Jean-Fabien Barrois Date: Fri, 5 Feb 2016 11:21:06 +1300 Subject: [PATCH] NEW Cross device "Remember Me" feature At the moment, using the "Remember me" function on more than one device will only work with the last device used. Previous devices will not auto login. This PR introduces a new DataObject for storing hashed tokens against multiple devices. Developers can configure if logging out should discard all tokens, or only the one used on the device logging out; token expiry date is 90 days by default but configurable. For added security, the old behaviour can still be enforced if multiple tokens are not desired. See silverstripe#1574 for additional background --- .../09_Security/00_Member.md | 7 + docs/en/04_Changelogs/4.0.0.md | 4 + lang/en.yml | 3 +- security/Member.php | 105 +++++--- security/MemberLoginForm.php | 16 +- security/RememberLoginHash.php | 164 +++++++++++++ tests/security/MemberTest.php | 230 ++++++++++++++++++ 7 files changed, 488 insertions(+), 41 deletions(-) create mode 100644 security/RememberLoginHash.php diff --git a/docs/en/02_Developer_Guides/09_Security/00_Member.md b/docs/en/02_Developer_Guides/09_Security/00_Member.md index f84ebf425..9996bfe2f 100644 --- a/docs/en/02_Developer_Guides/09_Security/00_Member.md +++ b/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 diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index f1114ee34..35027e82a 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/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: diff --git a/lang/en.yml b/lang/en.yml index a15ae6005..1b504ed16 100644 --- a/lang/en.yml +++ b/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' diff --git a/security/Member.php b/security/Member.php index 7f4bfb5a7..6b9da5ff0 100644 --- a/security/Member.php +++ b/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. - $generator = new RandomGenerator(); - $token = $generator->randomToken('sha1'); - $hash = $this->encryptWithUserSettings($token); - $this->RememberLoginToken = $hash; - Cookie::set('alc_enc', $this->ID . ':' . $token, 90, null, null, null, true); - } else { - $this->RememberLoginToken = null; + $rememberLoginHash = RememberLoginHash::generate($this); + $tokenExpiryDays = Config::inst()->get('RememberLoginHash', 'token_expiry_days'); + $deviceExpiryDays = Config::inst()->get('RememberLoginHash', 'device_expiry_days'); + Cookie::set('alc_enc', $this->ID . ':' . $rememberLoginHash->getToken(), + $tokenExpiryDays, null, null, null, true); + Cookie::set('alc_device', $rememberLoginHash->DeviceID, $deviceExpiryDays, null, null, null, true); + } else { + 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 - if($member) { - $hash = $member->encryptWithUserSettings($token); - if(!$member->RememberLoginToken || $member->RememberLoginToken !== $hash) { - $member = null; - } - } + $rememberLoginHash = null; - if($member) { - self::session_regenerate_id(); - Session::set("loggedInAs", $member->ID); - // 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, null, null, false, true); + // check if autologin token matches + if($member) { + $hash = $member->encryptWithUserSettings($token); + $rememberLoginHash = RememberLoginHash::get() + ->filter(array( + '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(); - $token = $generator->randomToken('sha1'); - $hash = $member->encryptWithUserSettings($token); - $member->RememberLoginToken = $hash; - Cookie::set('alc_enc', $member->ID . ':' . $token, 90, null, null, false, true); + if($member) { + self::session_regenerate_id(); + Session::set("loggedInAs", $member->ID); + // 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, 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->extend('memberAutoLoggedIn'); + $member->write(); + + // 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) { diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php index 88b6a7e97..0169f130a 100644 --- a/security/MemberLoginForm.php +++ b/security/MemberLoginForm.php @@ -91,10 +91,18 @@ class MemberLoginForm extends LoginForm { $emailField->setAttribute('autocomplete', 'off'); } if(Security::config()->autologin_enabled) { - $fields->push(new CheckboxField( - "Remember", - _t('Member.REMEMBERME', "Remember me next time?") - )); + $fields->push( + CheckboxField::create( + "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) { diff --git a/security/RememberLoginHash.php b/security/RememberLoginHash.php new file mode 100644 index 000000000..4df4ff060 --- /dev/null +++ b/security/RememberLoginHash.php @@ -0,0 +1,164 @@ + '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(); + } + +} \ No newline at end of file diff --git a/tests/security/MemberTest.php b/tests/security/MemberTest.php index b9f5ff37d..9e7f978c0 100644 --- a/tests/security/MemberTest.php +++ b/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');