Last modified: 2014-10-07 07:14:08 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42541, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40541 - $wgSecureLogin does not redirect to http if wpStickHTTPS is unchecked
$wgSecureLogin does not redirect to http if wpStickHTTPS is unchecked
Status: VERIFIED FIXED
Product: MediaWiki
Classification: Unclassified
User login and signup (Other open bugs)
1.20.x
All All
: Highest major (vote)
: 1.21.x release
Assigned To: Chad H.
:
Depends on:
Blocks: 71716 39380
  Show dependency treegraph
 
Reported: 2012-09-27 01:30 UTC by Chris Steipp
Modified: 2014-10-07 07:14 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Chris Steipp 2012-09-27 01:30:16 UTC
When using $wgSecureLogin, if a user leaves wpStickHTTPS unchecked, they are stil redirected to an https page after login.

I think it's because getFullURL returns a protocol relative url by default now, so preg_replace( '/^https:/', 'http:', $redirectUrl ) has no effect.

(NB: fixing this seems to prevent a user from logging in without wpStickHTTPS checked, because their session cookies are set with the secure attribute, but they are immediately redirected to an insecure page, so their session cookie no longer exists in the request.)
Comment 1 Liangent 2012-09-27 04:05:53 UTC
Can we use insecure cookies when wpStickHTTPS is not checked?
Comment 2 Tyler Romeo 2012-09-27 04:33:34 UTC
I'll work on this if nobody else is, since I'm the one who filed the bug this blocks.
Comment 3 Chris Steipp 2012-09-27 13:14:10 UTC
Thanks Tyler. The redirect should be an easy fix. The cookies might be easier after Gerrit change #24026, or there might be a better way to do it.
Comment 4 Tyler Romeo 2012-09-27 15:54:24 UTC
Well, unfortunately it's not that easy a fix. I'm working on it now, but the big problem is that say, for example, a hook injects HTML into the login complete page. The redirect does not occur automatically, and instead the user is given a sort of "login success" page and a link returning to where they were. The returnto link is added using OutputPage::addReturnTo, which in turn uses the Linker, and in that workflow there is no way to change the protocol. Right now I'm working on adding an option to the Linker so that you can force the link to have a certain protocol.
Comment 5 Chris Steipp 2012-09-27 18:27:27 UTC
I just added a patch for the cookie handling, that I think will allow insecure links/redirects to work. Gerrit change #25525.
Comment 6 Tyler Romeo 2012-09-27 18:40:29 UTC
https://gerrit.wikimedia.org/r/25530
Comment 7 Chris Steipp 2012-09-29 15:37:44 UTC
Roan pointed out that Gerrit change #25530 results in an infinite redirect if $wgServer includes an http:// protocol. This is because when $wgServer includes the protocol, it's assumed by the code that this means that there is no ssl available, so an http:// url is returned from wfExpandUrl(), even when PROTO_HTTPS is given.

I added an ugly hack to wfExpandUrl in Gerrit change #25721 to ignore this assumption if the site owner has enabled $wgSecureLogin. However, I think it would be best to back up and actually define a couple of things:

1) What does it mean if $wgServer starts with http://? Is it that there is no ssl available / intended, and MediaWiki should never generate an https link? Or should we assume that it's a default configuration, and we can override it if other settings like $wgSecureLogin are set?

2) If $wgServer starting with http:// means that the site should not use tls, but a site also includes $wgSecureLogin set to true, what *should* this mean to mediawiki? Should it be treated as an error? Or should one of the settings be overridden?

My preference for #2 is that we assume that the most secure preference is desired. I wouldn't even mind changing the protocol of $wgServer to be relative if $wgSecureLogin is true in Setup.php. But again, I'm not sure all of the places where assumptions are made about #1 in the code, and in our user base.
Comment 8 Tyler Romeo 2012-09-29 16:02:31 UTC
(In reply to comment #7)
> I added an ugly hack to wfExpandUrl in Gerrit change #25721 to ignore this assumption
> if the site owner has enabled $wgSecureLogin. However, I think it would be best
> to back up and actually define a couple of things:
> 
> 1) What does it mean if $wgServer starts with http://? Is it that there is no
> ssl available / intended, and MediaWiki should never generate an https link? Or
> should we assume that it's a default configuration, and we can override it if
> other settings like $wgSecureLogin are set?

Honestly, if MediaWiki is explicitly requesting an HTTPS link, I think it is erroneous to produce something different, especially with something as ambiguous as what $wgServer's scheme is. I think PROTO_HTTPS should give https regardless of $wgServer.

In reality, I would love to just finish https://gerrit.wikimedia.org/r/22167 and stop using wfExpandUrl in new code altogether.
Comment 9 Chris Steipp 2012-10-01 19:55:53 UTC
(In reply to comment #8)
> Honestly, if MediaWiki is explicitly requesting an HTTPS link, I think it is
> erroneous to produce something different, especially with something as
> ambiguous as what $wgServer's scheme is. I think PROTO_HTTPS should give https
> regardless of $wgServer.

I totally agree, in theory. However, even when I do that, if I load a page over ssl on a site where $wgServer starts with http://, all of the links to load.php and api.php use http instead of https. So the assumption that $wgServer can be used to generate urls with the correct protocol seems to be used all over the place.

I tested out just setting $wgServer to be protocol-relative if $wgSecureLogin is true, and it's working well. I don't like the idea of secretly flagging or changing a user's config, but in this case it seems like it may make sense to fix up conflicting config options?
Comment 10 Tyler Romeo 2012-10-01 21:05:24 UTC
Yeah, I mean if a user is turning on wgSecureLogin (or a similar option), they should already be aware (hopefully) that this implies HTTPS. At the very least, the bug of a infinite redirect will be replaced with that of getting an error when you try to browse HTTPS when none exists.
Comment 11 Tyler Romeo 2012-10-01 23:31:18 UTC
Closing this as resolved since the actual bug has indeed been fixed. Opened a separate report (bug 40679) for the redirect issue.
Comment 12 Nemo 2013-04-11 08:44:37 UTC
I don't understand on what release this is supposed to be fixed and at what conditions ([[mw:Manual:$wgSecureLogin]] says nothing).
wiki.openstreetmap.org is at 1.20.3 (beb501d) and I'm always stuck on HTTPS with $wgSecureLogin = true; $wgServer = '//wiki.openstreetmap.org';
Comment 13 Nemo 2013-08-21 08:14:23 UTC
Indee this seems to be hitting us again, see Chris and Tim on https://gerrit.wikimedia.org/r/#/c/79960/
4eopening this because the fix never worked for me on released versions of MW and its status has to be checked.
Comment 14 Greg Grossmeier 2013-08-27 15:46:42 UTC
That change was merged on the 21st, but the "included in" info on the change only says wmf12. We're on 13/14 now.

Chris/Chad: is this live on the testwikis+mediawiki.org. We have another report of the issue from S in bug 53379.
Comment 15 Tyler Romeo 2013-08-27 15:52:23 UTC
(In reply to comment #14)
> That change was merged on the 21st, but the "included in" info on the change
> only says wmf12. We're on 13/14 now.
> 
> Chris/Chad: is this live on the testwikis+mediawiki.org. We have another
> report
> of the issue from S in bug 53379.

I can't comment as to what is deployed, but I tested testwiki+mediawiki earlier this week and the specific issue mentioned at the top of this bug report doesn't occur.
Comment 16 Chris Steipp 2013-08-27 17:08:27 UTC
This is different from S's bug. The functionality is now present in both core and centralauth with the patches that were merged into master, and deployed to wmf14 by Chad on Saturday.

After bug 52283, we no longer have the wpStickHTTPS checkbox on the login form, instead we use the user's originating page + preference + geoip to make the determination.
Comment 17 Nemo 2013-08-27 18:13:14 UTC
So what's the alternative to $wgSecureLoginDefaultHTTPS now? What happens (now and in 1.21) if $wgSecureLogin is true but $wgServer is http:// ?
Comment 18 Tyler Romeo 2013-08-27 18:18:15 UTC
(In reply to comment #17)
> So what's the alternative to $wgSecureLoginDefaultHTTPS now?

Nothing, because it doesn't make sense anymore. $wgSecureLoginDefaultHTTPS was made to decide what the default value of the mStickHTTPS checkbox was. However, that checkbox doesn't exist anymore, so there's no purpose in having a default value for it.

Whether you're sent to HTTPS, rather than being decided by a checkbox, is decided by a combination of what protocol you were previously browsing and the user preference prefershttps.

> What happens (now
> and in 1.21) if $wgSecureLogin is true but $wgServer is http:// ?

I forget what release it was in, but having $wgSecureLogin as true and $wgServer with an HTTP protocol is considered a configuration error, so a patch was added that automatically shuts off $wgSecureLogin in Setup.php if that condition matches.
Comment 19 Nemo 2013-08-27 18:21:33 UTC
(In reply to comment #18)
> (In reply to comment #17)
> > So what's the alternative to $wgSecureLoginDefaultHTTPS now?
> 
> Nothing, because it doesn't make sense anymore. $wgSecureLoginDefaultHTTPS
> was
> made to decide what the default value of the mStickHTTPS checkbox was.
> However,
> that checkbox doesn't exist anymore, so there's no purpose in having a
> default
> value for it.
> 
> Whether you're sent to HTTPS, rather than being decided by a checkbox, is
> decided by a combination of what protocol you were previously browsing and
> the
> user preference prefershttps.

What are the combinations?

> 
> > What happens (now
> > and in 1.21) if $wgSecureLogin is true but $wgServer is http:// ?
> 
> I forget what release it was in, but having $wgSecureLogin as true and
> $wgServer with an HTTP protocol is considered a configuration error, so a
> patch
> was added that automatically shuts off $wgSecureLogin in Setup.php if that
> condition matches.

Yes, that's what the configuration suggests. I still don't understand how one is supposed to send logged in users to http after the secure login.
Comment 20 Tyler Romeo 2013-08-27 18:37:12 UTC
(In reply to comment #19)
> What are the combinations?

1. If you have prefershttps turned on, you always go to HTTPS.
2. If you don't have prefershttps turned on, you are sent to whatever protocol you were originally on. In other words, if you came from HTTP, you go back to HTTP. If you came from HTTPS, you go back to HTTPS.
Comment 21 Chris Steipp 2013-08-27 19:01:44 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > What are the combinations?
> 
> 1. If you have prefershttps turned on, you always go to HTTPS.
> 2. If you don't have prefershttps turned on, you are sent to whatever
> protocol
> you were originally on. In other words, if you came from HTTP, you go back to
> HTTP. If you came from HTTPS, you go back to HTTPS.

The full set of options I was using while implementing this ended up as:

preferhttps	geoip-https	browsing	final protocol
true		no		http		http
true		no		https		https
true		yes		http		https
true		yes		https		https
false		no		http		http
false		no		https		https (w/ insec cookies)
false		yes		http		http
false		yes		https		https (w/ insec cookies?)
Comment 22 Nemo 2013-09-10 07:20:46 UTC
OSM upgraded to 1.21.2 (404e36a) and fixed https://trac.openstreetmap.org/ticket/3919 ; leaving the checkbox to stay on HTTPS successfully keeps me on HTTP.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links