Page MenuHomePhabricator

Require token to logout (PS2)

Authored By
Bawolff
Feb 10 2016, 12:15 AM
Size
6 KB
Subscribers
None

Require token to logout (PS2)

From 4be85fc229bae7ba5e722840a6eefab12d731793 Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Sun, 7 Feb 2016 13:28:13 -0500
Subject: [PATCH] [SECURITY] [API BREAKING CHANGE] Require logout actions to
have a token.
Special:Userlogout now requires a token
Api action=logout requires a csrf token and the request to be POSTed
Bug: T25227
Change-Id: If758b2a9e7a7efb50caca8448ff74be30c195c6b
---
RELEASE-NOTES-1.27 | 1 +
includes/api/ApiLogout.php | 14 +++++++++++++-
includes/skins/SkinTemplate.php | 14 +++++++-------
includes/specials/SpecialUserlogout.php | 22 ++++++++++++++++++++++
languages/i18n/en.json | 4 +++-
languages/i18n/qqq.json | 4 +++-
6 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27
index 5b9b2b8..b9b1334 100644
--- a/RELEASE-NOTES-1.27
+++ b/RELEASE-NOTES-1.27
@@ -198,6 +198,7 @@ production.
merely need to change the username and password used after setting up a bot
password.
* action=upload no longer understands statuskey, asyncdownload or leavemessage.
+* (T25227) action=logout now requires to be posted and have a csrf token.
=== Action API internal changes in 1.27 ===
* ApiQueryORM removed.
diff --git a/includes/api/ApiLogout.php b/includes/api/ApiLogout.php
index b40f5a3..8bb7fc3 100644
--- a/includes/api/ApiLogout.php
+++ b/includes/api/ApiLogout.php
@@ -56,13 +56,25 @@ class ApiLogout extends ApiBase {
return false;
}
+ public function mustBePosted() {
+ return true;
+ }
+
+ public function needsToken() {
+ return 'csrf';
+ }
+
protected function getExamplesMessages() {
return array(
- 'action=logout'
+ 'action=logout&token=123ABC'
=> 'apihelp-logout-example-logout',
);
}
+ protected function getWebUITokenSalt( Array $params ) {
+ return 'logoutToken';
+ }
+
public function getHelpUrls() {
return 'https://www.mediawiki.org/wiki/API:Logout';
}
diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php
index 1328870..e012a01 100644
--- a/includes/skins/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
@@ -568,16 +568,15 @@ class SkinTemplate extends Skin {
$page = Title::newFromText( $request->getVal( 'title', '' ) );
}
$page = $request->getVal( 'returnto', $page );
- $a = array();
+ $returnto = array();
if ( strval( $page ) !== '' ) {
- $a['returnto'] = $page;
+ $returnto['returnto'] = $page;
$query = $request->getVal( 'returntoquery', $this->thisquery );
if ( $query != '' ) {
- $a['returntoquery'] = $query;
+ $returnto['returntoquery'] = $query;
}
}
- $returnto = wfArrayToCgi( $a );
if ( $this->loggedin ) {
$personal_urls['userpage'] = array(
'text' => $this->username,
@@ -635,9 +634,10 @@ class SkinTemplate extends Skin {
$personal_urls['logout'] = array(
'text' => $this->msg( 'pt-userlogout' )->text(),
'href' => self::makeSpecialUrl( 'Userlogout',
- // userlogout link must always contain an & character, otherwise we might not be able
+ // Note: userlogout link must always contain an & character, otherwise we might not be able
// to detect a buggy precaching proxy (bug 17790)
- $title->isSpecial( 'Preferences' ) ? 'noreturnto' : $returnto
+ ( $title->isSpecial( 'Preferences' ) ? array() : $returnto )
+ + array( 'logoutToken' => $this->getUser()->getEditToken( 'logoutToken', $this->getRequest() ) )
),
'active' => false
);
@@ -656,7 +656,7 @@ class SkinTemplate extends Skin {
);
$createaccount_url = array(
'text' => $this->msg( 'pt-createaccount' )->text(),
- 'href' => self::makeSpecialUrl( 'Userlogin', "$returnto&type=signup" ),
+ 'href' => self::makeSpecialUrl( 'Userlogin', $returnto + array( "type" => "signup" ) ),
'active' => $title->isSpecial( 'Userlogin' ) && $is_signup,
);
diff --git a/includes/specials/SpecialUserlogout.php b/includes/specials/SpecialUserlogout.php
index 6e34690..e6bdc89 100644
--- a/includes/specials/SpecialUserlogout.php
+++ b/includes/specials/SpecialUserlogout.php
@@ -48,6 +48,28 @@ class SpecialUserlogout extends UnlistedSpecialPage {
$this->setHeaders();
$this->outputHeader();
+ $out = $this->getOutput();
+ $user = $this->getUser();
+ $request = $this->getRequest();
+
+ $logoutToken = $request->getVal( 'logoutToken' );
+ $urlParams = array(
+ 'logoutToken' => $user->getEditToken( 'logoutToken', $request )
+ ) + $request->getValues();
+ unset( $urlParams['title'] );
+ $continueLink = $this->getFullTitle()->getFullUrl( $urlParams );
+
+ if ( $logoutToken === null ) {
+ $this->getOutput()->addWikiMsg( 'userlogout-continue', $continueLink );
+ return;
+ }
+ if ( !$this->getUser()->matchEditToken(
+ $logoutToken, 'logoutToken', $this->getRequest(), 24*60*60
+ ) ) {
+ $this->getOutput()->addWikiMsg( 'userlogout-sessionerror', $continueLink );
+ return;
+ }
+
// Make sure it's possible to log out
$session = MediaWiki\Session\SessionManager::getGlobalSession();
if ( !$session->canSetUser() ) {
diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index 2e1df41..e6fffd8 100644
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -4033,5 +4033,7 @@
"sessionprovider-generic": "$1 sessions",
"sessionprovider-mediawiki-session-cookiesessionprovider": "cookie-based sessions",
"sessionprovider-nocookies": "Cookies may be disabled. Ensure you have cookies enabled and start again.",
- "randomrootpage": "Random root page"
+ "randomrootpage": "Random root page",
+ "userlogout-continue": "If you wish to log out please [$1 continue to the log out page].",
+ "userlogout-sessionerror": "Log out failed due to session error. Please [$1 try again]."
}
diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index fec3079..1deb3ad 100644
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -4208,5 +4208,7 @@
"sessionprovider-generic": "Used to create a generic session type description when one isn't provided via the proper message. Should be phrased to make sense when added to a message such as {{msg-mw|cannotloginnow-text}}.\n\nParameters:\n* $1 - PHP classname.",
"sessionprovider-mediawiki-session-cookiesessionprovider": "Description of the sessions provided by the CookieSessionProvider class, which use HTTP cookies. Should be phrased to make sense when added to a message such as {{msg-mw|cannotloginnow-text}}.",
"sessionprovider-nocookies": "Used to inform the user that sessions may be missing due to lack of cookies.",
- "randomrootpage": "{{doc-special|RandomRootPage}}"
+ "randomrootpage": "{{doc-special|RandomRootPage}}",
+ "userlogout-continue": "Shown if user attempted to log out without a token specified. Probably the user clicked on an old link that hasn't been updated to use the new system. $1 - url that user should click on in order to log out.",
+ "userlogout-sessionerror": "Shown when a user tries to log out with an invalid token. $1 is url with correct token that user should click."
}
--
2.0.1

Event Timeline