MyBB Community Forums

Full Version: Converted logins don't work if admin changes password
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
I migrated a forum from XenForo to MyBB 1.8.3 (now upgraded 1.8.4).

Users that login are correctly having their passwords updated in the database and the loginconvert* column values are blanked out.

However, if a user hasn't logged in to MyBB since the migration and an admin changes their password, the loginconvert* columns are not blanked/nullified. The MyBB password columns are however updated correctly.

It seems that the loginconvert password takes precedence over the MyBB password even if there is a MyBB password. Therefore, if the admin changes the password it is like the password was never changed since the user is still authenticated against the loginconvert password.

I think there are two separate issues here;

1. When a user password is changed by an admin, the loginconvert fields need to be blanked.
2. The MyBB password should take precedence over the loginconvert password regardless.

For now, I've added a trigger to the database to resolve the issue;

DELIMITER //
CREATE TRIGGER upd_<prefix>_users BEFORE UPDATE ON <prefix>_users
FOR EACH ROW
BEGIN
  IF NEW.password <> "" THEN
    SET NEW.passwordconvert = NULL;
    SET NEW.passwordconverttype = NULL;
    SET NEW.passwordconvertsalt = NULL;
  END IF;
END;//
DELIMITER ;

(2015-02-27, 08:39 AM)ukjbrown Wrote: [ -> ]I migrated a forum from XenForo to MyBB 1.8.3 (now upgraded 1.8.4).

Users that login are correctly having their passwords updated in the database and the loginconvert* column values are blanked out.

However, if a user hasn't logged in to MyBB since the migration and an admin changes their password, the loginconvert* columns are not blanked/nullified. The MyBB password columns are however updated correctly.

It seems that the loginconvert password takes precedence over the MyBB password even if there is a MyBB password. Therefore, if the admin changes the password it is like the password was never changed since the user is still authenticated against the loginconvert password.

I think there are two separate issues here;

1. When a user password is changed by an admin, the loginconvert fields need to be blanked.
2. The MyBB password should take precedence over the loginconvert password regardless.

For now, I've added a trigger to the database to resolve the issue;


DELIMITER //
CREATE TRIGGER upd_<prefix>_users BEFORE UPDATE ON <prefix>_users
FOR EACH ROW
BEGIN
  IF NEW.password <> "" THEN
    SET NEW.passwordconvert = NULL;
    SET NEW.passwordconverttype = NULL;
    SET NEW.passwordconvertsalt = NULL;
  END IF;
END;//
DELIMITER ;

Not tested yet, but I think the fix may be to add the following to line 125 on loginconvert.php
// User has already got a MyBB password (must have been updated through Admin CP)
if(isset($user['password']) && $user['password'] != '')
{
	$update = array(
		"passwordconverttype"	=> "",
		"passwordconvert"		=> "",
		"passwordconvertsalt"	=> "",
	);

	$db->update_query("users", $update, "uid='{$user['uid']}'");
	return;
}
In 1.8.5 the update_password is to be deprecated. See [Issue #1760].

So the best for the 1.8.5 Merge System would be to replace:
$plugins->add_hook("member_resetpassword_process", "loginconvert_pw_reset");

/*** ~~~~ ***/

function loginconvert_pw_reset()
{
	global $db, $user;

	// Someone reseted their password, clear the passwordconvert columns for this user
	$update = array(
		"passwordconvert" => "",
		"passwordconverttype" => "",
		"passwordconvertsalt" => ""
	);
	$db->update_query("users", $update, "uid={$user['uid']}");
}

To:
$plugins->add_hook("datahandler_user_update", "loginconvert_pw_reset");

/*** ~~~~ ***/

function loginconvert_pw_reset(&$user_handler)
{
	if(isset($user_handler->user_update_data['password']))
	{
		global $db, $user;

		// Someone reseted their password, clear the passwordconvert columns for this user
		$update = array(
			"passwordconvert" => "",
			"passwordconverttype" => "",
			"passwordconvertsalt" => ""
		);
		$db->update_query("users", $update, "uid={$user_handler->uid}");
	}
}

For 1.8.5 or whenever the before mentioned issue is accepted.
The reset part is never supposed to be released anyways, it was something I pushed before I noticed that there is an easier way.