Fork me on GitHub
Subscribe 3

Ticket #30 (fixed enhancement)

Only admins should be able to run the upgrade script

  • Created: 2010-05-28 13:30:37
  • Reported by: Reines
  • Assigned to: Reines
  • Milestone: 1.4.3
  • Component: upgrading
  • Priority: high

Currently when the forum database requires an upgrade, anyone visiting the forum is informed of this and able to run the upgrade.

In some ways this is sensible:

  • The admin might not happen to be logged in.

  • Some login related code may have changed and users aren't recognized until the database has been upgraded.

However, it is also dangerous in a production environment. Normal users should not be able to run the upgrade, and similarly it could be dangerous for multiple people to run the script at the same time.

I suggest only logged in administrators are given permission to run the upgrade script, and everyone else is shown a message simply saying the forums are currently being upgraded. To solve the issue of admins not being logged in, a new constant could be used to bypass the permission check.

History

Reines 2010-07-21 23:06:49

  • Milestone changed from 1.4.1 to 1.4.2.

fg123 2010-08-08 06:08:14

I guess I agree. smile But then, you only run the script when it's a big update, like from 1.4 to 1.5. For subversions, like 1.4.1 to 1.4.2, there is no update script...

Franz 2010-08-08 09:43:36

fg123 wrote:

For subversions, like 1.4.1 to 1.4.2, there is no update script...

Oh yes, there is. I suggest you read the following forum topic where this has been discussed at length: http://fluxbb.org/forums/viewtopic.php?id=4476

Reines 2010-08-09 12:30:10

  • Milestone changed from 1.4.2 to 1.4.3.

Reines 2010-10-20 19:06:01

Commit 80c599c to fluxbb fluxbb-1.4

Adding a more user friendly update required message, and forcing the admin to enter the database password to perform a database update. #30 and #153.

Reines 2010-10-20 19:06:43

  • Status changed from open to fixed.

I have implemented this by making the user enter the database password, which is stored in config.php. If they are using SQLite then the database name is used instead.

This seems to work, but should be tested with other database types (mainly SQLite...) - I've only tested with MySQL so far.

http://github.com/fluxbb/fluxbb/commit/ … 76cf6ffc48

Franz 2010-10-20 21:52:39

No lock file?

Reines 2010-10-20 22:37:36

Does this look vaguely sensible?

Franz 2010-10-21 06:58:06

Looks good to me.

Franz 2010-10-22 22:47:30

  • Status changed from fixed to open.

Sorry to be mean, but I believe this doesn't yet work when JavaScript is disabled.

I didn't quite understand why not, but then you are more into this script than I am. Please confirm.

Reines 2010-10-22 23:05:02

Cheers, I missed that link. Should be sorted now.

Reines 2010-10-22 23:05:17

  • Status changed from open to fixed.

quy 2010-10-26 15:35:10

Should the "Database password" field be password type?

Franz 2010-10-26 18:36:26

Most definitely, quy. We just changed it to be of that type in places like the install script. Why not?

Reines 2010-10-27 10:09:53

Yes it should, my bad. Fixed now, cheers.

quy 2010-10-29 05:22:36

The database password check can be bypassed by appending uid=##### to db_update.php

For example: http://localhost/fluxbb143/db_update.php?uid=9999

Reines 2010-11-08 16:00:40

  • Status changed from fixed to open.

Thanks quy, I have a patch but it's at home, I'll apply it later.

I've also re-opened this because the styling doesn't quite work on the old skins (Oxygen etc.).

Franz 2010-12-15 14:51:44

I don't know whether you'll be doing that with your patch, but I think getting a message that no valid password was entered when directly accessing the update script is a little bad on usability.

The script should simply show the upgrade message when being called for the first time, too.

Reines 2011-01-21 17:22:19

I've created a pull request that should finished this. If someone could give it a test and then apply it (or just confirm it seems okay and I'll apply it) that would be great.

PS. The easiest way to apply it actually is probably using the "fork queue" section on GitHub - you can just select the commit from my fork and hit apply.

quy 2011-01-21 22:02:31

Confirmed to be fine.

Reines 2011-01-21 22:13:21

  • Status changed from open to fixed.

Cheers, I've merged it. This ticket should be sorted now hopefully smile

quy 2011-01-21 23:57:18

  • Status changed from fixed to open.

Sorry I should have tested this for non-English installations. The language reverts to English upon errors and completion of installation because "default_lang" field is not in the second form for install language selection.

Reines 2011-01-22 00:12:20

  • Status changed from open to fixed.

Thanks, I think this should be fixed now. If you could check it again that'd be great.

PS. I've separated out the install_lang and default_lang instead of merging the two variables, since potentially someone may wish to install in one language but have another as the default.

quy 2011-01-22 00:51:38

Perfect! Thanks.