Skip to content

Commit

Permalink
New password generation method: send user email (#16519)
Browse files Browse the repository at this point in the history
### What does it do?

This is a re-up of #15461 originally by @sdrenth back in 2021, which has
gone stale waiting for some minor changes. I've rebased it, tweaked it,
tested it, so we can include it in 3.1.

This adds a new option for setting the password when creating/updating a
user: send the user a link to set their password. That's more secure and
builds upon improvements to the password reset flow that was done in
3.0.

### Why is it needed?

Showing the password on screen or manually setting a password is kinda
outdated and insecure.

### How to test

Create and/or edit a user, and choose "Let the user choose their own
password via email" for the password method. Look for the email (make
sure you have email delivery set up beforehand) and attempt to set the
new password.

### Related issue(s)/PR(s)

This PR replaces the stale PR #15461

Sterc#22
#13973
Sterc#31

Co-authored-by: sander <[email protected]>
  • Loading branch information
Mark-H and sdrenth authored Aug 30, 2024
1 parent 23d8965 commit aab292e
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 14 deletions.
6 changes: 3 additions & 3 deletions core/lexicon/en/user.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
$_lang['password_gen_specify'] = 'Let me specify the password:';
$_lang['password_method'] = 'Password notification method';
$_lang['password_method_screen'] = 'Show the new password on screen.';
$_lang['password_gen_user_email_specify'] = 'Let the user choose their own password via email';
$_lang['notify_new_user'] = 'Email this user about their new login for this website.';
$_lang['password_new'] = 'New Password';
$_lang['password_notification'] = 'Password Notification';
Expand Down Expand Up @@ -197,6 +198,5 @@
$_lang['users'] = 'Users';
$_lang['user_createdon'] = 'Created On';
$_lang['user_createdon_desc'] = 'The date the user was created.';

// Renamed and/or deprecated as of 3.0.4; remove in 3.1.0
$_lang['ugc_mutate'] = 'User Group Access to Context'; // now in access.inc.php, access_context_create
$_lang['user_password_email_subject'] = 'Set up your password';
$_lang['user_password_email'] = '<h2>Set up your password</h2><p>We received a request to set up your MODX Revolution password. You can set up your password by clicking the button below and following the instructions on screen.</p><p class="center"><a href="[[+url_scheme]][[+http_host]][[+manager_url]]?modhash=[[+hash]]" class="btn">Set up my password</a></p><p class="small">If you did not send this request, please ignore this email.</p>';
51 changes: 51 additions & 0 deletions core/src/Revolution/Processors/Security/User/Create.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@


use Exception;
use MODX\Revolution\Hashing\modHashing;
use MODX\Revolution\Processors\Model\CreateProcessor;
use MODX\Revolution\Processors\Processor;
use MODX\Revolution\modUser;
use MODX\Revolution\modUserGroup;
use MODX\Revolution\modUserGroupMember;
use MODX\Revolution\modUserProfile;
use MODX\Revolution\modX;
use MODX\Revolution\Registry\modRegister;
use MODX\Revolution\Registry\modRegistry;
use MODX\Revolution\Smarty\modSmarty;

/**
Expand Down Expand Up @@ -232,6 +235,54 @@ public function sendNotificationEmail() {
'html' => true,
]);
}

if (
$this->getProperty('passwordgenmethod') === 'user_email_specify'
) {
$activationHash = bin2hex(random_bytes(32));

/** @var modRegistry $registry */
$registry = $this->modx->getService('registry', 'registry.modRegistry');
/** @var modRegister $register */
$register = $registry->getRegister('user', 'registry.modDbRegister');
$register->connect();
$register->subscribe('/pwd/change/');
$register->send('/pwd/change/', [$activationHash => $this->object->get('username')], ['ttl' => 86400]);

// Send activation email
$message = $this->modx->lexicon('user_password_email');
$placeholders = array_merge($this->modx->config, $this->object->toArray());
$placeholders['hash'] = $activationHash;

// Store previous placeholders
$ph = $this->modx->placeholders;
// now set those useful for modParser
$this->modx->setPlaceholders($placeholders);
$this->modx->getParser()->processElementTags('', $message, true, false, '[[', ']]', [], 10);
$this->modx->getParser()->processElementTags('', $message, true, true, '[[', ']]', [], 10);
// Then restore previous placeholders to prevent any breakage
$this->modx->placeholders = $ph;

$this->modx->getService('smarty', 'smarty.modSmarty', '', ['template_dir' => $this->modx->getOption('manager_path') . 'templates/default/']);

$this->modx->smarty->assign('_config', $this->modx->config);
$this->modx->smarty->assign('content', $message, true);

$sent = $this->object->sendEmail(
$this->modx->smarty->fetch('email/default.tpl'),
[
'from' => $this->modx->getOption('emailsender'),
'fromName' => $this->modx->getOption('site_name'),
'sender' => $this->modx->getOption('emailsender'),
'subject' => $this->modx->lexicon('user_password_email_subject'),
'html' => true,
]
);

if (!$sent) {
return $this->failure($this->modx->lexicon('error_sending_email_to') . $this->object->get('email'));
}
}
}

/**
Expand Down
80 changes: 71 additions & 9 deletions core/src/Revolution/Processors/Security/User/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
use MODX\Revolution\modUserGroupMember;
use MODX\Revolution\modUserProfile;
use MODX\Revolution\modX;
use MODX\Revolution\Registry\modRegister;
use MODX\Revolution\Registry\modRegistry;

/**
* Update a user.
Expand Down Expand Up @@ -285,6 +287,7 @@ public function setUserGroups() {
*/
public function afterSave() {
$this->setUserGroups();
$this->sendNotificationEmail();
if ($this->activeStatusChanged) {
$this->fireAfterActiveStatusChange();
}
Expand All @@ -305,6 +308,62 @@ public function fireAfterActiveStatusChange() {
);
}

/**
* Send the password notification email, if specified
*
* @return void
* @throws \Exception
*/
public function sendNotificationEmail() {
if ($this->getProperty('passwordgenmethod') === 'user_email_specify') {
$activationHash = bin2hex(random_bytes(32));

/** @var modRegistry $registry */
$registry = $this->modx->getService('registry', 'registry.modRegistry');
/** @var modRegister $register */
$register = $registry->getRegister('user', 'registry.modDbRegister');
$register->connect();
$register->subscribe('/pwd/change/');
$register->send('/pwd/change/', [$activationHash => $this->object->get('username')], ['ttl' => 86400]);

$this->modx->lexicon->load('core:login');

// Send activation email
$message = $this->modx->lexicon('user_password_email');
$placeholders = array_merge($this->modx->config, $this->object->toArray());
$placeholders['hash'] = $activationHash;

// Store previous placeholders
$ph = $this->modx->placeholders;
// now set those useful for modParser
$this->modx->setPlaceholders($placeholders);
$this->modx->getParser()->processElementTags('', $message, true, false, '[[', ']]', [], 10);
$this->modx->getParser()->processElementTags('', $message, true, true, '[[', ']]', [], 10);
// Then restore previous placeholders to prevent any breakage
$this->modx->placeholders = $ph;

$this->modx->getService('smarty', 'smarty.modSmarty', '', ['template_dir' => $this->modx->getOption('manager_path') . 'templates/default/']);

$this->modx->smarty->assign('_config', $this->modx->config);
$this->modx->smarty->assign('content', $message, true);

$sent = $this->object->sendEmail(
$this->modx->smarty->fetch('email/default.tpl'),
[
'from' => $this->modx->getOption('emailsender'),
'fromName' => $this->modx->getOption('site_name'),
'sender' => $this->modx->getOption('emailsender'),
'subject' => $this->modx->lexicon('user_password_email_subject'),
'html' => true,
]
);

if (!$sent) {
return $this->failure($this->modx->lexicon('error_sending_email_to') . $this->object->get('email'));
}
}
}

/**
* {@inheritDoc}
* @return array|string
Expand All @@ -314,20 +373,23 @@ public function cleanup()
$userArray = $this->object->toArray();
$profile = $this->object->getOne('Profile');
if ($profile) {
$userArray = array_merge($profile->toArray(),$userArray);
$userArray = array_merge($profile->toArray(), $userArray);
}
unset($userArray['password'], $userArray['cachepwd'], $userArray['sessionid'], $userArray['salt']);

$passwordNotifyMethod = $this->getProperty('passwordnotifymethod');
if (!empty($passwordNotifyMethod) && !empty($this->newPassword) && $passwordNotifyMethod == 's') {
return $this->success($this->modx->lexicon('user_updated_password_message',
[
'username' => $this->object->get('username'),
'password' => $this->newPassword,
]
), $this->object);
} else {
return $this->success('',$this->object);
return $this->success(
$this->modx->lexicon(
'user_updated_password_message',
[
'username' => $this->object->get('username'),
'password' => $this->newPassword,
]
),
$this->object
);
}
return $this->success('', $this->object);
}
}
6 changes: 4 additions & 2 deletions core/src/Revolution/Processors/Security/User/Validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,14 @@ public function alreadyExists($name) {
public function checkPassword() {
$newPassword = $this->processor->getProperty('newpassword',null);
$id = $this->processor->getProperty('id');
if ($newPassword !== null && $newPassword != 'false' || empty($id)) {

$passwordGenerationMethod = $this->processor->getProperty('passwordgenmethod','g');
if ($passwordGenerationMethod !== 'user_email_specify' && ($newPassword !== null && $newPassword != 'false' || empty($id))) {
$passwordNotifyMethod = $this->processor->getProperty('passwordnotifymethod',null);
if (empty($passwordNotifyMethod)) {
$this->processor->addFieldError('password_notify_method',$this->modx->lexicon('user_err_not_specified_notification_method'));
}
$passwordGenerationMethod = $this->processor->getProperty('passwordgenmethod','g');

if ($passwordGenerationMethod == 'g') {
$autoPassword = $this->user->generatePassword();
$this->user->set('password', $autoPassword);
Expand Down
7 changes: 7 additions & 0 deletions manager/assets/modext/widgets/security/modx.panel.user.js
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,13 @@ Ext.extend(MODx.panel.User,MODx.FormPanel,{
,xtype: 'radio'
,inputValue: 'spec'
,value: 'spec'
},{
id: 'modx-user-password-genmethod-user-email-specify'
,name: 'passwordgenmethod'
,boxLabel: _('password_gen_user_email_specify')
,xtype: 'radio'
,inputValue: 'user_email_specify'
,value: 'user_email_specify'
}]
},{
id: 'modx-user-panel-newpassword'
Expand Down

0 comments on commit aab292e

Please sign in to comment.