Forums

Unfortunately no one can be told what FluxBB is - you have to see it for yourself.

You are not logged in.

#1 2010-08-01 13:12:11

Reines
Administrator
From: Scotland
Registered: 2008-05-11
Posts: 3,197
Website

Only admins should be able to run the upgrade script

How should we handle #30?

I'm not really a fan of checking the user is an admin the normal way. Firstly they may be logged out, and secondly if they are performing an upgrade the code will have changed, potentially meaning the check wouldn't work.

Does requiring the database password (used in config.php) sound sensible? Then we can be sure only authorised users can access the script. The only problem I see there is, what about SQLite which doesn't have a database password?

Offline

#2 2010-08-01 13:18:55

daris
Developer
From: Poland
Registered: 2008-05-09
Posts: 557

Re: Only admins should be able to run the upgrade script

it could be dangerous for multiple people to run the script at the same time.

Store temp file in cache directory when starting an update. And then for next user check if it exists.

Offline

#3 2010-08-02 02:27:46

Gary
Moderator
From: Sydney, Australia
Registered: 2009-09-07
Posts: 232

Re: Only admins should be able to run the upgrade script

It is definitely something important, as there are security risks and for the already suggested problems. Asking for the database password would be alright I guess, but aren't there other ways that this problem could be avoided overall (including SQLite)?

Offline

#4 2010-08-02 06:25:10

Smartys
Former Developer
Registered: 2008-04-27
Posts: 3,139
Website

Re: Only admins should be able to run the upgrade script

PorkoWog: That's exactly the question Reines is posing wink

Offline

#5 2010-08-02 08:39:33

Gary
Moderator
From: Sydney, Australia
Registered: 2009-09-07
Posts: 232

Re: Only admins should be able to run the upgrade script

I misread.

Possibly a way to go about it is to use the root administrator or number one administrator username and password to have access to upgrade the software? Or possibly by showing visitors and members a page saying that the forum is currently being upgraded and will be back shortly, whilst providing a way for administrators to access the upgrade page by their username and password?

Either way would work I guess.

Offline

#6 2010-08-02 10:27:46

Franz
Lead developer
From: Germany
Registered: 2008-05-13
Posts: 6,744
Website

Re: Only admins should be able to run the upgrade script

I'd go for first administrator username and password. Then again, that wouldn't work for us, so maybe all administrators?


fluxbb.de | develoPHP

"As code is more often read than written it's really important to write clean code."

Offline

#7 2010-08-02 10:29:20

Gary
Moderator
From: Sydney, Australia
Registered: 2009-09-07
Posts: 232

Re: Only admins should be able to run the upgrade script

Exactly, and if people complain about it then they should really think twice about who to give administrator rights/powers to.

Offline

#8 2010-08-02 13:42:40

SolykZ
Member
From: Mons, Belgium
Registered: 2010-05-15
Posts: 75
Website

Re: Only admins should be able to run the upgrade script

Why not *just* simply add a new column in the users table that define the permission to update? So you'll be able to define exactly who can update or not. (-:


SolykZ, apprentice modder.
...my English is probably the worst thing you'll ever see. PLEASE correct me if I make mistakes. (-:

Offline

#9 2010-08-02 14:56:23

Franz
Lead developer
From: Germany
Registered: 2008-05-13
Posts: 6,744
Website

Re: Only admins should be able to run the upgrade script

SolykZ wrote:

Why not *just* simply add a new column in the users table that define the permission to update? So you'll be able to define exactly who can update or not. (-:

But what if you don't define that?


fluxbb.de | develoPHP

"As code is more often read than written it's really important to write clean code."

Offline

#10 2010-08-02 15:03:09

SolykZ
Member
From: Mons, Belgium
Registered: 2010-05-15
Posts: 75
Website

Re: Only admins should be able to run the upgrade script

If the administrator does not defines that, that's his problem, no? (-:

Either we specifically define who can update, or everyone can do. For my part I prefer the first idea. q-:


/edit/
IMO, a good idea should be to add a new group: the superadmins. Their role to be able to update the board, manage some critical things like SMTP passwords, etc. It might also avoid takeovers, since a newly promoted administrator could not deprive a super administrator of his powers.

Last edited by SolykZ (2010-08-02 15:12:58)


SolykZ, apprentice modder.
...my English is probably the worst thing you'll ever see. PLEASE correct me if I make mistakes. (-:

Offline

#11 2010-08-02 15:57:57

FSX
Former Developer
From: NL
Registered: 2008-05-09
Posts: 818
Website

Re: Only admins should be able to run the upgrade script

I think a admin username and password should be provided. It isn't good to add a new column to the user table, because it'll only be used for updating and other than that it useless.

And adding a super admin group shouldn't be done. You only add people to the administrator group when you really trust them. Not when you think "oh, that's a nice lad."

Offline

#12 2010-08-02 16:24:31

Franz
Lead developer
From: Germany
Registered: 2008-05-13
Posts: 6,744
Website

Re: Only admins should be able to run the upgrade script

FSX wrote:

It isn't good to add a new column to the user table, because it'll only be used for updating and other than that it useless.

And we'd have to add it while upgrading tongue

FSX wrote:

And adding a super admin group shouldn't be done. You only add people to the administrator group when you really trust them. Not when you think "oh, that's a nice lad."

Well, I think a more flexible, possibly role-based, permission system would solve these problems and everybody could just shape the permission system the way they deem necessary.


fluxbb.de | develoPHP

"As code is more often read than written it's really important to write clean code."

Offline

#13 2010-08-02 17:25:20

adaur
Developer
From: France
Registered: 2010-01-07
Posts: 843
Website

Re: Only admins should be able to run the upgrade script

I think a new login to check if the user is admin is the best way to limit the script's access smile.


FeatherBB - A simple and lightweight new generation forum system
Based on FluxBB, written in PHP, using Slim Framework for a proper OOP-MVC architecture.

Offline

#14 2010-08-02 17:28:31

hussam
Member
Registered: 2010-06-13
Posts: 41

Re: Only admins should be able to run the upgrade script

I like Daris's idea of storing a temp file in cache directory when update starts. Then any extra instances of the updater will check if the temp file exits. If it does, it exits.

Offline

#15 2010-08-02 21:11:44

Gizzmo
Member
From: Earth, Milkyway Galaxy
Registered: 2008-04-30
Posts: 301
Website

Re: Only admins should be able to run the upgrade script

What so wrong with making the admin login first? Just check for a logged in admin. Then use daris' idea to make sure no other admins use the update script.


~Gizzmo - My Mods: Usermap (Github) - Default Avatar (Github)

Offline

#16 2010-08-02 23:07:30

Mpok
Member
From: France
Registered: 2008-05-12
Posts: 389

Re: Only admins should be able to run the upgrade script

1) The "#30" ticket is a REAL ISSUE (mean, actual way is AWFUL..).

2) For info : it's a rather "1.4 issue only", as we never had the pbm in 1.2. Though, there was the same pbm in 1.2. The difference is that db_update seems to be MUCH MORE used than previously.
In 1.2.x we pretty never used db_update, so the file was not even uploaded (or just once, then deleted), and there was no pbm.
In 1.4, we have to upload it, cos EVERY update NEED a 'db_update'... Even if the database is not modified at all..

3) Asking the database password is (imo) the worst thing to do to deal with the issue.

4) Like SolykZ solution : a new line in 'config' table.
(and new code in 'Admin->Options')

Offline

#17 2010-08-02 23:11:42

Reines
Administrator
From: Scotland
Registered: 2008-05-11
Posts: 3,197
Website

Re: Only admins should be able to run the upgrade script

Mpok wrote:

3) Asking the database password is (imo) the worst thing to do to deal with the issue.

Why? Presumably anyone with FTP access to upload the new files has database access too?

Mpok wrote:

4) Like SolykZ solution : a new line in 'config' table.

That requires board admins to configure things, which lots wont. What then happens when an update occurs and it hasn't been configured, or the admin forgot to update it and they can't perform the update?

Offline

#18 2010-08-02 23:39:56

Mpok
Member
From: France
Registered: 2008-05-12
Posts: 389

Re: Only admins should be able to run the upgrade script

Well...
- "Presumably anyone with FTP access to upload the new files has database access too" : ur right (but it can be 'implicit', we're not 'mandated' to tell the database password with the ftp one).
- "What then happens when an update occurs and it hasn't been configured, or the admin forgot to update it and they can't perform the update?" : ok also.

Offline

#19 2010-08-02 23:45:21

SolykZ
Member
From: Mons, Belgium
Registered: 2010-05-15
Posts: 75
Website

Re: Only admins should be able to run the upgrade script

We also can add default value '1' for the first admin. Then, at least one user can update the board. smile


SolykZ, apprentice modder.
...my English is probably the worst thing you'll ever see. PLEASE correct me if I make mistakes. (-:

Offline

#20 2010-08-02 23:49:26

Mpok
Member
From: France
Registered: 2008-05-12
Posts: 389

Re: Only admins should be able to run the upgrade script

@SolykZ : there is already a 'first admin'. It's the user's ID 2.

Offline

#21 2010-08-03 00:33:30

SolykZ
Member
From: Mons, Belgium
Registered: 2010-05-15
Posts: 75
Website

Re: Only admins should be able to run the upgrade script

No, I mean for the eventual 'can_update' new column in the users table. tongue
The id#2 is not a efficient value since the first admin could have been deleted and the "main" administrator could now be somebody else. smile

Last edited by SolykZ (2010-08-03 00:33:53)


SolykZ, apprentice modder.
...my English is probably the worst thing you'll ever see. PLEASE correct me if I make mistakes. (-:

Offline

#22 2010-08-03 01:15:00

Mpok
Member
From: France
Registered: 2008-05-12
Posts: 389

Re: Only admins should be able to run the upgrade script

Ok..
But adding a column in this table (users) JUST for that is not very efficient, no ?
So i've 'transcoded' it to an add in 'config' table (see my previous post).
Then Reines got a point.. smile

Offline

#23 2010-08-03 01:51:18

SolykZ
Member
From: Mons, Belgium
Registered: 2010-05-15
Posts: 75
Website

Re: Only admins should be able to run the upgrade script

Oh, yes. I did not noticed that you were talking of the config table, sorry. tongue
So, maybe a serialized array of ids corresponding to some users should do the work as espected, uh?

On adding it, we could so fill this config item with the first admin. It is just an idea, of course. (-:


SolykZ, apprentice modder.
...my English is probably the worst thing you'll ever see. PLEASE correct me if I make mistakes. (-:

Offline

#24 2010-08-03 08:01:39

Franz
Lead developer
From: Germany
Registered: 2008-05-13
Posts: 6,744
Website

Re: Only admins should be able to run the upgrade script

Mpok wrote:

In 1.4, we have to upload it, cos EVERY update NEED a 'db_update'... Even if the database is not modified at all..

Simply not true. It is only executed if one of the following three conditions is true:

  • the database has been modified

  • the parser was changed -> preparsing has to be re-done

  • the search was changed -> search indexing has to be re-done

However, adding a column to the user table would not work, as we would still have problems regarding cookies etc.: What if the admin isn't logged in? What if his cookie lifetime isn't long enough? You'd have to disable the check manually and we would be where we are now again.

I suppose having the administrator log in combined with a special file in the cache/ directory like daris suggested is the safest and best solution. Or maybe instead of the file another row in the config table (when it exists: update is running; if it doesn't, nobody has started yet - this would mean, though, that we'd have to regenerate the config cache or at least fetch the values from the config table on every request of db_update.php (which could be renamed now, by the way).


fluxbb.de | develoPHP

"As code is more often read than written it's really important to write clean code."

Offline

#25 2010-08-03 09:13:54

Gary
Moderator
From: Sydney, Australia
Registered: 2009-09-07
Posts: 232

Re: Only admins should be able to run the upgrade script

I think you've got it now Franz. Allowing the update to be done by an administrator logging in is the safest and best way to go about things. Along with daris' idea as well of course to avoid multiple updates at the same time.

Offline

Board footer

Powered by FluxBB