Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 5244: Removed Email Subclasses #5271

Merged
merged 1 commit into from
Apr 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions dev/SapphireTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public function setUp() {
self::$is_running_test = true;

// i18n needs to be set to the defaults or tests fail
i18n::set_locale(i18n::default_locale());
i18n::set_locale(Config::inst()->get('i18n', 'default_locale'));
i18n::config()->date_format = null;
i18n::config()->time_format = null;

Expand Down Expand Up @@ -223,12 +223,6 @@ public function setUp() {

$prefix = defined('SS_DATABASE_PREFIX') ? SS_DATABASE_PREFIX : 'ss_';

// Set up email
$this->originalMailer = Email::mailer();
$this->mailer = new TestMailer();
Injector::inst()->registerService($this->mailer, 'Mailer');
Config::inst()->remove('Email', 'send_all_emails_to');

// Todo: this could be a special test model
$this->model = DataModel::inst();

Expand Down Expand Up @@ -289,6 +283,12 @@ public function setUp() {

// Clear requirements
Requirements::clear();

// Set up email
$this->originalMailer = Email::mailer();
$this->mailer = new TestMailer();
Injector::inst()->registerService($this->mailer, 'Mailer');
Config::inst()->remove('Email', 'send_all_emails_to');
}

/**
Expand Down
45 changes: 4 additions & 41 deletions security/Member.php
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,10 @@ public function onBeforeWrite() {
&& $this->record['Password']
&& $this->config()->notify_password_change
) {
$e = Member_ChangePasswordEmail::create();
/** @var Email $e */
$e = Email::create();
$e->setSubject(_t('Member.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject'));
$e->setTemplate('ChangePasswordEmail');
$e->populateTemplate($this);
$e->setTo($this->Email);
$e->send();
Expand Down Expand Up @@ -1762,46 +1765,6 @@ protected function getMember() {
}
}

/**
* Class used as template to send an email saying that the password has been
* changed.
*
* @package framework
* @subpackage security
*/
class Member_ChangePasswordEmail extends Email {

protected $from = ''; // setting a blank from address uses the site's default administrator email
protected $subject = '';
protected $ss_template = 'ChangePasswordEmail';

public function __construct() {
parent::__construct();

$this->subject = _t('Member.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject');
}
}



/**
* Class used as template to send the forgot password email
*
* @package framework
* @subpackage security
*/
class Member_ForgotPasswordEmail extends Email {
protected $from = ''; // setting a blank from address uses the site's default administrator email
protected $subject = '';
protected $ss_template = 'ForgotPasswordEmail';

public function __construct() {
parent::__construct();

$this->subject = _t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject');
}
}

/**
* Member Validator
*
Expand Down
5 changes: 4 additions & 1 deletion security/MemberLoginForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,10 @@ public function forgotPassword($data) {
if($member) {
$token = $member->generateAutologinTokenAndStoreHash();

$e = Member_ForgotPasswordEmail::create();
/** @var Email $e */
$e = Email::create();
$e->setSubject(_t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject'));
$e->setTemplate('ForgotPasswordEmail');
$e->populateTemplate($member);
$e->populateTemplate(array(
'PasswordResetLink' => Security::getPasswordResetLink($member, $token)
Expand Down
2 changes: 1 addition & 1 deletion templates/email/ChangePasswordEmail.ss
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

<p>
<%t ChangePasswordEmail_ss.CHANGEPASSWORDTEXT1 'You changed your password for' is 'for a url' %> $AbsoluteBaseURL.<br />
<%t ChangePasswordEmail_ss.CHANGEPASSWORDFOREMAIL 'The password for account with email address {email} has been changed. If you didn't change your password please change your password using the link below' email=$Email %><br />
<%t ChangePasswordEmail_ss.CHANGEPASSWORDFOREMAIL 'The password for account with email address {email} has been changed. If you didn\'t change your password please change your password using the link below' email=$Email %><br />
<a href="Security/changepassword"><%t ChangePasswordEmail_ss.CHANGEPASSWORDTEXT3 'Change password' %></a>
</p>
36 changes: 32 additions & 4 deletions tests/security/MemberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,44 @@ public function testPasswordChangeLogging() {
* Test that changed passwords will send an email
*/
public function testChangedPasswordEmaling() {
$oldSetting = Config::inst()->get('Member', 'notify_password_change');
Config::inst()->update('Member', 'notify_password_change', true);

$this->clearEmails();

$member = $this->objFromFixture('Member', 'test');
$this->assertNotNull($member);
$valid = $member->changePassword('32asDF##$$%%');
$this->assertTrue($valid->valid());
/*
$this->assertEmailSent("sam@silverstripe.com", null, "/changed password/",
'/sam@silverstripe\.com.*32asDF##\$\$%%/');
*/

$this->assertEmailSent("dummy@silverstripe.tld", null, "Your password has been changed",
'/dummy@silverstripe\.tld/');

Config::inst()->update('Member', 'notify_password_change', $oldSetting);
}

/**
* Test that triggering "forgotPassword" sends an Email with a reset link
*/
public function testForgotPasswordEmaling() {
$this->clearEmails();
$this->autoFollowRedirection = false;

$member = $this->objFromFixture('Member', 'test');
$this->assertNotNull($member);

// Initiate a password-reset
$response = $this->post('Security/LostPasswordForm', array('Email' => $member->Email));

$this->assertEquals($response->getStatusCode(), 302);

// We should get redirected to Security/passwordsent
$this->assertContains('Security/passwordsent/dummy@silverstripe.tld',
urldecode($response->getHeader('Location')));

// Check existance of reset link
$this->assertEmailSent("dummy@silverstripe.tld", null, 'Your password reset link',
'/Security\/changepassword\?m='.$member->ID.'&t=[^"]+/');
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/security/MemberTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Member:
test:
FirstName: Test
Surname: User
Email: sam@silverstripe.com
Email: dummy@silverstripe.tld
Password: 1nitialPassword
PasswordExpiry: 2030-01-01
Groups: =>Group.securityadminsgroup
Expand Down
46 changes: 23 additions & 23 deletions tests/security/SecurityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,13 @@ public function testExternalBackUrlRedirectionDisallowed() {
*/
public function testExpiredPassword() {
/* BAD PASSWORDS ARE LOCKED OUT */
$badResponse = $this->doTestLoginForm('sam@silverstripe.com' , 'badpassword');
$badResponse = $this->doTestLoginForm('dummy@silverstripe.tld' , 'badpassword');
$this->assertEquals(302, $badResponse->getStatusCode());
$this->assertRegExp('/Security\/login/', $badResponse->getHeader('Location'));
$this->assertNull($this->session()->inst_get('loggedInAs'));

/* UNEXPIRED PASSWORD GO THROUGH WITHOUT A HITCH */
$goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$goodResponse = $this->doTestLoginForm('dummy@silverstripe.tld' , '1nitialPassword');
$this->assertEquals(302, $goodResponse->getStatusCode());
$this->assertEquals(
Controller::join_links(Director::absoluteBaseURL(), 'test/link'),
Expand Down Expand Up @@ -340,7 +340,7 @@ public function testExpiredPassword() {
}

public function testChangePasswordForLoggedInUsers() {
$goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$goodResponse = $this->doTestLoginForm('dummy@silverstripe.tld' , '1nitialPassword');

// Change the password
$this->get('Security/changepassword?BackURL=test/back');
Expand All @@ -353,7 +353,7 @@ public function testChangePasswordForLoggedInUsers() {
$this->assertEquals($this->idFromFixture('Member', 'test'), $this->session()->inst_get('loggedInAs'));

// Check if we can login with the new password
$goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , 'changedPassword');
$goodResponse = $this->doTestLoginForm('dummy@silverstripe.tld' , 'changedPassword');
$this->assertEquals(302, $goodResponse->getStatusCode());
$this->assertEquals(
Controller::join_links(Director::absoluteBaseURL(), 'test/link'),
Expand All @@ -372,9 +372,9 @@ public function testChangePasswordFromLostPassword() {

// Request new password by email
$response = $this->get('Security/lostpassword');
$response = $this->post('Security/LostPasswordForm', array('Email' => 'sam@silverstripe.com'));
$response = $this->post('Security/LostPasswordForm', array('Email' => 'dummy@silverstripe.tld'));

$this->assertEmailSent('sam@silverstripe.com');
$this->assertEmailSent('dummy@silverstripe.tld');

// Load password link from email
$admin = DataObject::get_by_id('Member', $admin->ID);
Expand All @@ -394,7 +394,7 @@ public function testChangePasswordFromLostPassword() {
$this->assertEquals($this->idFromFixture('Member', 'test'), $this->session()->inst_get('loggedInAs'));

// Check if we can login with the new password
$goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , 'changedPassword');
$goodResponse = $this->doTestLoginForm('dummy@silverstripe.tld' , 'changedPassword');
$this->assertEquals(302, $goodResponse->getStatusCode());
$this->assertEquals($this->idFromFixture('Member', 'test'), $this->session()->inst_get('loggedInAs'));

Expand All @@ -412,7 +412,7 @@ public function testRepeatedLoginAttemptsLockingPeopleOut() {

// Login with a wrong password for more than the defined threshold
for($i = 1; $i <= Member::config()->lock_out_after_incorrect_logins+1; $i++) {
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , 'incorrectpassword');
$member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));

if($i < Member::config()->lock_out_after_incorrect_logins) {
Expand Down Expand Up @@ -442,7 +442,7 @@ public function testRepeatedLoginAttemptsLockingPeopleOut() {
}
}

$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , '1nitialPassword');
$this->assertNull(
$this->session()->inst_get('loggedInAs'),
'The user can\'t log in after being locked out, even with the right password'
Expand All @@ -452,7 +452,7 @@ public function testRepeatedLoginAttemptsLockingPeopleOut() {
$member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
$member->LockedOutUntil = date('Y-m-d H:i:s', time() - 30);
$member->write();
$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , '1nitialPassword');
$this->assertEquals(
$this->session()->inst_get('loggedInAs'),
$member->ID,
Expand All @@ -464,7 +464,7 @@ public function testRepeatedLoginAttemptsLockingPeopleOut() {

// Login again with wrong password, but less attempts than threshold
for($i = 1; $i < Member::config()->lock_out_after_incorrect_logins; $i++) {
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , 'incorrectpassword');
}
$this->assertNull($this->session()->inst_get('loggedInAs'));
$this->assertContains(
Expand All @@ -473,7 +473,7 @@ public function testRepeatedLoginAttemptsLockingPeopleOut() {
'The user can retry with a wrong password after the lockout expires'
);

$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , '1nitialPassword');
$this->assertEquals(
$this->session()->inst_get('loggedInAs'),
$member->ID,
Expand All @@ -488,8 +488,8 @@ public function testAlternatingRepeatedLoginAttempts() {

// ATTEMPTING LOG-IN TWICE WITH ONE ACCOUNT AND TWICE WITH ANOTHER SHOULDN'T LOCK ANYBODY OUT

$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , 'incorrectpassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , 'incorrectpassword');

$this->doTestLoginForm('noexpiry@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('noexpiry@silverstripe.com' , 'incorrectpassword');
Expand All @@ -503,7 +503,7 @@ public function testAlternatingRepeatedLoginAttempts() {
// BUT, DOING AN ADDITIONAL LOG-IN WITH EITHER OF THEM WILL LOCK OUT, SINCE THAT IS THE 3RD FAILURE IN
// THIS SESSION

$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('dummy@silverstripe.tld' , 'incorrectpassword');
$member1 = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
$this->assertNotNull($member1->LockedOutUntil);

Expand All @@ -516,16 +516,16 @@ public function testUnsuccessfulLoginAttempts() {
Security::config()->login_recording = true;

/* UNSUCCESSFUL ATTEMPTS WITH WRONG PASSWORD FOR EXISTING USER ARE LOGGED */
$this->doTestLoginForm('sam@silverstripe.com', 'wrongpassword');
$this->doTestLoginForm('dummy@silverstripe.tld', 'wrongpassword');
$attempt = DataObject::get_one('LoginAttempt', array(
'"LoginAttempt"."Email"' => 'sam@silverstripe.com'
'"LoginAttempt"."Email"' => 'dummy@silverstripe.tld'
));
$this->assertTrue(is_object($attempt));
$member = DataObject::get_one('Member', array(
'"Member"."Email"' => 'sam@silverstripe.com'
'"Member"."Email"' => 'dummy@silverstripe.tld'
));
$this->assertEquals($attempt->Status, 'Failure');
$this->assertEquals($attempt->Email, 'sam@silverstripe.com');
$this->assertEquals($attempt->Email, 'dummy@silverstripe.tld');
$this->assertEquals($attempt->MemberID, $member->ID);

/* UNSUCCESSFUL ATTEMPTS WITH NONEXISTING USER ARE LOGGED */
Expand All @@ -545,16 +545,16 @@ public function testSuccessfulLoginAttempts() {
Security::config()->login_recording = true;

/* SUCCESSFUL ATTEMPTS ARE LOGGED */
$this->doTestLoginForm('sam@silverstripe.com', '1nitialPassword');
$this->doTestLoginForm('dummy@silverstripe.tld', '1nitialPassword');
$attempt = DataObject::get_one('LoginAttempt', array(
'"LoginAttempt"."Email"' => 'sam@silverstripe.com'
'"LoginAttempt"."Email"' => 'dummy@silverstripe.tld'
));
$member = DataObject::get_one('Member', array(
'"Member"."Email"' => 'sam@silverstripe.com'
'"Member"."Email"' => 'dummy@silverstripe.tld'
));
$this->assertTrue(is_object($attempt));
$this->assertEquals($attempt->Status, 'Success');
$this->assertEquals($attempt->Email, 'sam@silverstripe.com');
$this->assertEquals($attempt->Email, 'dummy@silverstripe.tld');
$this->assertEquals($attempt->MemberID, $member->ID);
}

Expand Down