457719 - The "You need to _log in_ before you can comment..." link in show_bug.cgi doesn't display the login form if ssl = "authenticated sessions"

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Frédéric Buclin]

Looks like a regression:

If ssl = "authenticated sessions" and you visit a bug at http://bugzilla/show_bug.cgi?id=1 while being logged out, you see the read-only form with a link in place of the comment "You need to _log in_ before you can comment...". But this link points to http://bugzilla/show_bug.cgi?id=1&GoAheadAndLogIn=1. Note the protocol: http://. As ssl != 'never', we should point to https:// (the "Log In" links in the header and footer correctly point to https://).

Related problem: for a reason I don't understand yet (I didn't try to), GoAheadAndLogIn=1 is ignored in the URL above and so the user clicking this link see the read-only form again. Now the link has been updated to point to https:// and clicking it again now correctly displays the login form. This behavior is confusing and the first URL above, even with http:// instead of https://, should display the login form.

In this bug, we should fix both issues at once: correctly set the link and correctly catch GoAheadAndLogIn=1 to display the login form immediately.

Comment 1

[David Lawrence [:dkl]]

Attachment: Patch to use SSL login link when logged out in show_bug.cgi (v1)

Patch to use sslbase when ssl != 'never' in show_bug.cgi.

Please review
Dave

Comment 2

[David Lawrence [:dkl]]

(In reply to comment #0)
> Related problem: for a reason I don't understand yet (I didn't try to),
> GoAheadAndLogIn=1 is ignored in the URL above and so the user clicking this
> link see the read-only form again. Now the link has been updated to point to
> https:// and clicking it again now correctly displays the login form. This
> behavior is confusing and the first URL above, even with http:// instead of
> https://, should display the login form.
> 

The GoAheadAndLogIn form var is removed due to the line in the following part of the login process:

Bugzilla/Auth/Login/CGI.pm

48:    $cgi->delete('Bugzilla_login', 'Bugzilla_password', 'GoAheadAndLogIn');

When the user is redirect to SSL, Bugzilla::CGI::require_https() rewrites the previous query string from the CGI object, and by that time the GoAheadAndLogIn part has already been removed. So we will need to figure out how to pass it along instead.

Dave

Comment 3

[Frédéric Buclin]

(In reply to comment #2)
> The GoAheadAndLogIn form var is removed due to the line in the following part
> of the login process:
> 
> Bugzilla/Auth/Login/CGI.pm
> 
> 48:    $cgi->delete('Bugzilla_login', 'Bugzilla_password', 'GoAheadAndLogIn');

I checked, and the only reason why we also delete GoAheadAndLogIn is to fix bug 183665. I just realized that this fix is imperfect anyway as I still manage to get an error thrown instead of being redirected to enter_bug.cgi. So I'm going to back out bug 183665. This will fix the 2nd part of comment 0 while your patch fixes the first part.

Comment 4

[Frédéric Buclin]

Comment on attachment 341133 [details] [diff] [review]
Patch to use SSL login link when logged out in show_bug.cgi (v1)

Yes, that's the right fix. If sslbase is empty, this won't hurt anything and https:// will still be used as the URL will remain relative. r=LpSolit

Comment 5

[David Lawrence [:dkl]]

tip:
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.134; previous revision: 1.133
done

3.2:
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.125.2.9; previous revision: 1.125.2.8
done