When something is entered in the 'shutdownhtml' parameter, every page except editparams.cgi is disabled... including the logout page. If one goes to log out, one sees the information entered in the 'shutdownhtml' param, and the footer indicates that the user has to 'log in'... but this is misleading, as the user is actually still logged in. As soon as the 'shutdownhtml' text is removed, anyone using that session will have access to that user's account because they literally cannot log out. If 'shutdownhtml' is active, and the user moves to another page, user should be told to log out (and allowed to do so) or logged out automatically. Implying that they have already been logged out (by displaying the 'Log In' link in the footer) is deceptive, and a potential breach of security.

We should just get rid of the footer when shutdownhtml is active. That would solve the whole problem.

How does that solve the problem? I don't get it... I envision (local example here): - Manager goes down to the floor to discuss some bug reports with manufacturing. - Manger logs in to one of the general terminals to bring up the LIs (because we have no public bugs, so you HAVE to log in to see what you want to see). - Manager pulls up bug; bug looks normal (including footer) - shutdownhtml gets enabled by bugzilla admin - Manager enters 2nd bug number in 'find' part of footer - Manager is taken to a page -- with no footer -- that says "Bugzilla is down for 20 minutes, sorry!" - Manager walks away from terminal - 20 minutes later, shutdownhtml is deactivated. Manager is still logged in, and anyone coming by that public terminal now has bugzilla access to anything he can see. As far as I can tell, your suggestion doesn't change the above scenario, save to add the parenthetical 'with no footer' comment. Am I missing something?

I'd say comment 2 is a pretty good argument for a more complete solution. I agree that the footer as a whole should be removed when Bugzilla is down (it gives a false impression if you show it as if you're loged out and it access the database if shown as if your logged in). It should be possible to just print a "log out" link (that is made to work!) at the end of the shutdownhtml that shows up if there is a login cookie (don't even need to check if it's valid, so there's no db access). The problem, as I see it, is that in order to log out, the database must be changed. Any change when Bugzilla is down may not actually take effect (if the admin does a DB rollback, has the DB in a read-only state, whatever). Maybe a better solution is to put a link in the description for a funtion that logs everybody out. Though the problem with that is the mass-logout is enforced on everybody, some who very much like being able to log in once and stay that way for all of eternity. Even with this downside, I think it's probably the best solution.

OK. Problem: Users at terminals cannot log out when Bugzilla is shut down, and when it comes back up, anybody can use their account if they walk away. He should close the browser, just like every other sane computer user, when he walks away from the terminal. I certainly hope he didn't tell the terminal to save his login across browser sessions...

Reassigning bugs that I'm not actively working on to the default component owner in order to try to make some sanity out of my personal buglist. This doesn't mean the bug isn't being dealt with, just that I'm not the one doing it. If you are dealing with this bug, please assign it to yourself.

kiko, erik, could we imagine that when the admin validates his changes in editparams.cgi, a function is automatically called which logs out all users at once? This way, users don't have to worry anymore.. there are all already logged out.

"If it's not a regression from 2.18 and it's not a critical problem with something that's already landed, let's push it off." - Dave

Make sure the user is logged out when the shutdown message is displayed.

unbitrot + mention that the user has been logged out only if he was logged in previously.

+ <p>You have been logged out. The cookie that was + remembering your login is now gone. i think the sentance about the cookie is not required. just say "you have been logged out".

(In reply to comment #10) > i think the sentance about the cookie is not required. > just say "you have been logged out". I want to let it here for consistency with the message you get when logging out under normal conditions. Moreover, this makes things really clear that your cookie is gone (personally, I would sleep better by knowing that ;) ).

Comment on attachment 189305 [details] [diff] [review] patch, v2 mkanat, wurblzap is too busy these days. Can you have a look at this patch, please?

Comment on attachment 189305 [details] [diff] [review] patch, v2 The patch looks good, and works properly. A few notes: + Users should be aware that for scripts in SHUTDOWNHTML_EXEMPT, they will still be logged in. + Checkin fix: I'd like a comment above the ->login ->logout part in CGI.pm that explains why we do that, or just refers to this bug. + Checkin fix: Inform the user, "For security reasons, you have been logged out automatically." instead of just informing them that they have been logged out (which will confuse them, since they didn't ask to be logged out).

This bug is apparently targeted for Bugzilla 2.18, so will need a backport to 2.18. (I'd imagine the 2.20 and 2.21 code should be identical.)

nits fixed. Carrying forward mkanat's review.

In 2.18, the patch is slightly different because Bugzilla->login() returns 'undef' if the user is not logged in and $user->id doesn't work.

Comment on attachment 189973 [details] [diff] [review] 2.20 backport, v1 >Index: CGI.pl > if (Param("shutdownhtml") && $0 !~ m:(^|[\\/])(do)?editparams\.cgi$:) { >+ my $user = Bugzilla->login(); If requirelogin is enabled, this prompts the user to login before showing him/her the shutdown message. I would recommend to change this to LOGIN_OPTIONAL like in 2.18 backport. Otherwise this would be good to go.

Comment on attachment 189962 [details] [diff] [review] patch for the tip, v2.1 This has the same small problem as 2.20 backport.

added LOGIN_OPTIONAL

added LOGIN_OPTIONAL

Hmm.

tip: Checking in Bugzilla.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v <-- Bugzilla.pm new revision: 1.19; previous revision: 1.18 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.32; previous revision: 1.31 done 2.20rc1: Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 1.242.2.1; previous revision: 1.242 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.30.2.2; previous revision: 1.30.2.1 done 2.18.3: Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 1.211.2.10; previous revision: 1.211.2.9 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.22.2.3; previous revision: 1.22.2.2 done