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