r105215 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105214‎ | r105215 | r105216 >
Date:20:23, 5 December 2011
Author:preilly
Status:ok (Comments)
Tags:
Comment:
set mfsecure cookie if user is logged in
Modified paths:
  • /trunk/extensions/MobileFrontend/MobileFrontend.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/MobileFrontend.php
@@ -88,7 +88,7 @@
8989 }
9090
9191 class ExtMobileFrontend {
92 - const VERSION = '0.5.90';
 92+ const VERSION = '0.5.91';
9393
9494 /**
9595 * @var DOMDocument
@@ -585,6 +585,7 @@
586586 $this->sendXDeviceVaryHeader();
587587 $this->sendApplicationVersionVaryHeader();
588588 $this->checkUserStatus();
 589+ $this->checkUserLoggedIn();
589590
590591 if ( self::$title->isSpecial( 'Userlogin' ) && self::$isBetaGroupMember ) {
591592 self::$wsLoginToken = $wgRequest->getSessionData( 'wsLoginToken' );
@@ -601,6 +602,32 @@
602603 wfProfileOut( __METHOD__ );
603604 return true;
604605 }
 606+
 607+ /**
 608+ * @return bool
 609+ */
 610+ private function checkUserLoggedIn() {
 611+ global $wgUser, $wgCookieDomain, $wgRequest, $wgCookiePrefix;
 612+ wfProfileIn( __METHOD__ );
 613+ $tempWgCookieDomain = $wgCookieDomain;
 614+ $wgCookieDomain = $this->getBaseDomain();
 615+ $tempWgCookiePrefix = $wgCookiePrefix;
 616+ $wgCookiePrefix = '';
 617+
 618+ if ( $wgUser->isLoggedIn() ) {
 619+ $wgRequest->response()->setcookie( 'mfsecure', '1', 0, '' );
 620+ } else {
 621+ $mfSecure = $wgRequest->getCookie( 'mfsecure', '' );
 622+ if ( !empty( $mfSecure ) && $mfSecure == '1' ) {
 623+ $wgRequest->response()->setcookie( 'mfsecure', '', 0, '' );
 624+ }
 625+ }
 626+
 627+ $wgCookieDomain = $tempWgCookieDomain;
 628+ $wgCookiePrefix = $tempWgCookiePrefix;
 629+ wfProfileOut( __METHOD__ );
 630+ return true;
 631+ }
605632
606633 private function checkUserStatus() {
607634 wfProfileIn( __METHOD__ );

Follow-up revisions

RevisionCommit summaryAuthorDate
r1052161.18wmf1: MFT r105215preilly20:24, 5 December 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   21:33, 6 December 2011

What does 'secure' mean in this context? Normally we would only use 'secure' in some kind of reference to SSL, which may or may not be the case here.

It looks like the cookie expires at the end of the browser session; does this mean that it might keep this cookie around beyond the end of a *login* session, which may expire after some period of disuse?

#Comment by Preilly (talk | contribs)   22:28, 6 December 2011

We need to set a cookie to show that the user is logged in and that they need to stay on the SSL site only. This cookie is for the squid layer. I think it is fine that it expires at the end of the browser session as it would just be recreated on a subsequent visit.

#Comment by Krinkle (talk | contribs)   21:36, 6 December 2011

Please don't merge to branches (especially deployment branches) before code-review, except for emergency fixes perhaps.

#Comment by Preilly (talk | contribs)   22:29, 6 December 2011

I needed to get this into production for testing with the production caching layers... (squid and varnish.) Your comment is duly noted.

Status & tagging log