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

Fixed #6787- SuiteCRM 7.11 User password not saving and could not change #6779

Merged
merged 8 commits into from Jan 29, 2019
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
55 changes: 35 additions & 20 deletions modules/Users/User.php
Expand Up @@ -5,7 +5,7 @@
* SugarCRM, Inc. Copyright (C) 2004-2013 SugarCRM Inc.
*
* SuiteCRM is an extension to SugarCRM Community Edition developed by SalesAgility Ltd.
* Copyright (C) 2011 - 2018 SalesAgility Ltd.
* Copyright (C) 2011 - 2019 SalesAgility Ltd.
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License version 3 as published by the
Expand Down Expand Up @@ -587,26 +587,24 @@ public static function getLicensedUsersWhere()
}

/**
* Normally a bean returns ID from save() method if it was
* Normally a bean returns ID from save() method if it was
* success and false (or maybe null) is something went wrong.
* BUT (for some reason) if User bean saved properly except
* the email addresses of it, this User::save() method also
* BUT (for some reason) if User bean saved properly except
* the email addresses of it, this User::save() method also
* return a false.
* It's a confusing ambiguous return value for caller method.
*
* To handle this issue when save method can not save email
* It's a confusing ambiguous return value for caller method.
*
* To handle this issue when save method can not save email
* addresses and return false it also set the variable called
* User::$lastSaveErrorIsEmailAddressSaveError to true.
*
* @global User $current_user
* @global array $sugar_config
* @global array $mod_strings
*
* @param bool $check_notify
* @return boolean
* @return bool|string
* @throws SuiteException
*/
public function save($check_notify = false)
{
global $current_user, $sugar_config, $mod_strings;
global $current_user, $mod_strings;

$msg = '';

Expand Down Expand Up @@ -643,8 +641,6 @@ public function save($check_notify = false)
}

if ($this->factor_auth && $isUpdate && is_admin($current_user)) {
$tmpUser = BeanFactory::getBean('Users', $this->id);

$factorAuthFactory = new FactorAuthFactory();
$factorAuth = $factorAuthFactory->getFactorAuth($this);

Expand All @@ -653,10 +649,6 @@ public function save($check_notify = false)
}
}


$query = "SELECT count(id) as total from users WHERE " . self::getLicensedUsersWhere();


// is_group & portal should be set to 0 by default
if (!isset($this->is_group)) {
$this->is_group = 0;
Expand All @@ -678,12 +670,18 @@ public function save($check_notify = false)
// set some default preferences when creating a new user
$setNewUserPreferences = empty($this->id) || !empty($this->new_with_id);

if (!$this->verify_data()) {
SugarApplication::appendErrorMessage($this->error_string);
header('Location: index.php?action=Error&module=Users');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk, is it possible here to use the proper function called SugarApplication::redirect() instead?

exit;
}


$retId = parent::save($check_notify);
if (!$retId) {
LoggerManager::getLogger()->fatal('save error: User is not saved, Person ID is not returned.');
}
if ($retId != $this->id) {
if ($retId !== $this->id) {
LoggerManager::getLogger()->fatal('save error: User is not saved properly, returned Person ID does not match to User ID.');
}
// set some default preferences when creating a new user
Expand All @@ -697,6 +695,23 @@ public function save($check_notify = false)

$this->savePreferencesToDB();

if ((isset($_POST['old_password']) || $this->portal_only) &&
(isset($_POST['new_password']) && !empty($_POST['new_password'])) &&
(isset($_POST['password_change']) && $_POST['password_change'] === 'true')) {
if (!$this->change_password($_POST['old_password'], $_POST['new_password'])) {
if (isset($_POST['page']) && $_POST['page'] === 'EditView') {
SugarApplication::appendErrorMessage($this->error_string);
header("Location: index.php?action=EditView&module=Users&record=" . $_POST['record']);
exit;
}
if (isset($_POST['page']) && $_POST['page'] === 'Change') {
SugarApplication::appendErrorMessage($this->error_string);
header("Location: index.php?action=ChangePassword&module=Users&record=" . $_POST['record']);
exit;
}
}
}

// User Profile specific save for Email addresses
$this->lastSaveErrorIsEmailAddressSaveError = false;
if (!$this->emailAddress->saveAtUserProfile($_REQUEST)) {
Expand Down
19 changes: 19 additions & 0 deletions tests/_support/Step/Acceptance/UsersTester.php
Expand Up @@ -12,4 +12,23 @@ public function gotoProfile()
$I = new NavigationBarTester($this->getScenario());
$I->clickUserMenuItem('Profile');
}

/**
* Navigate to users module
*/
public function gotoUsers()
{
$I = new NavigationBarTester($this->getScenario());
$I->clickUserMenuItem('Admin');
$I->see('ADMINISTRATION');
}

/**
* Logout a user
*/
public function logoutUser()
{
$I = new NavigationBarTester($this->getScenario());
$I->clickUserMenuItem('#logout_link');
}
}
2 changes: 1 addition & 1 deletion tests/acceptance/modules/Users/UsersCest.php
Expand Up @@ -33,7 +33,7 @@ public function _before(AcceptanceTester $I)
$this->fakeDataSeed = rand(0, 2048);
$this->fakeData->seed($this->fakeDataSeed);
}

public function testEmailSettingsMailAccountAdd(AcceptanceTester $I, UsersTester $Users, WebDriverHelper $webDriverHelper)
{
$instanceUrl = $webDriverHelper->getInstanceURL();
Expand Down