[Pushed] mysql

[satdav]

hi I am getting error message belowI can't see this on the phpmyadmin

---

MyBB has experienced an internal SQL error and cannot continue.

SQL Error:
1060 - Duplicate column name 'authsecret'
Query:
ALTER TABLE mybb_adminoptions CHANGE 2fasecret authsecret varchar(16) NOT NULL default ''

[Josh H.]

Have you already run the upgrade script before? Even partial completion?

Or did you fix the 2FA bug from 1.8.4 manually?

It's trying to change 2fasecret to a column name that already exists.

[Destroy666]

I think this should be considered a bug - the new name's existence should be checked and a column should be removed if it causes a duplicate SQL error. We do that for every new column, but not for any renamed one, which is weird. So moving to Bug Reports.

[Jones H]

IMHO that's a pretty rare case: We check whether "2fasecret" exists and if so we rename it: https://github.com/mybb/mybb/blob/featur...33.php#L31 If there's already a "authsecret" column and a "2fasecret" something went horribly wrong (or the database was changed manually). Dropping the column shouldn't be done here as there may be boards which renamed it manually (SQLite/PgSQL users) so we'd disable 2FA for those users and they may not notice it. The way it's been done should work for all usual cases, including rerunning the update.

[Destroy666]

It's an edge case, yes. But I think it should be fixed nonetheless.

Dropping the column shouldn't be done here as there may be boards which renamed it manually (SQLite/PgSQL users) so we'd disable 2FA for those users and they may not notice it.

They would need to predict our name to have the same column applied manually, which is an edge case too. And we can easily avoid any problems with that by selecting current values from the authsecret column before its deletion and transferring them to the invalid 2fasecret (using a query like UPDATE mybb_adminoptions SET 2fasecret=authsecret), which will then be renamed to authsecret without an error.


And it doesn't apply only to this column - I found several other instances of lacks of column deletion before renaming IIRC.

[Jones H]

But how do you want to know which of those column contains the correct values? If that column is added manually but the user forgot to edit the files it's "2fasecret", if he also changed the files it's "authsecret". IMHO if we fix this the easiest way would be to change

if($db->field_exists('2fasecret', 'adminoptions'))

to

if($db->field_exists('2fasecret', 'adminoptions') && !$db->field_exists('authsecret', 'adminoptions'))

[Destroy666]

Yeah, the fix above should be ok.

[Jones H]

Hi,

Thank you for your report. We have pushed this issue to our Github repository for further analysis where you can track our commits and progress with fixing this bug. Discussions regarding this bug may also take place there too.

Follow this link to visit the issue on Github: https://github.com/mybb/mybb/issues/2116

Thanks for contributing to MyBB!

Regards,
The MyBB Group

[satdav]

Hello I done a merge from phpbb to mybb