Page MenuHomePhabricator
Files F28682008

T25227.patch
sbassett (Scott Bassett)
Actions

Authored By
sbassett
Apr 16 2019, 10:15 PM
Size
6 KB
Referenced Files
None
Subscribers
Anomie
Bawolff
bd808
• chasemp
Jorm
Ladsgroup
Legoktm
Reedy

T25227.patch
View Options

From 43c00bff590322a65f5ef6cae653ec70648f766e Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Tue, 16 Apr 2019 17:09:43 -0500
Subject: [PATCH] [SECURITY] [API BREAKING CHANGE] Require logout token.
Special:Userlogout now requires a token
Api action=logout requires a csrf token and the request to be POSTed
Bug: T25227
---
RELEASE-NOTES-1.33 | 1 +
includes/api/ApiLogout.php | 10 +++++++++-
includes/skins/SkinTemplate.php | 12 ++++++------
includes/specials/SpecialUserLogout.php | 22 ++++++++++++++++++++++
languages/i18n/en.json | 4 +++-
languages/i18n/qqq.json | 4 +++-
6 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index db5fea0..4f99e86 100644
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -169,6 +169,7 @@ For notes on 1.32.x and older releases, see HISTORY.
* (T216245) Autoblocks will now be spread by action=edit and action=move.
* action=query&meta=userinfo has a new uiprop, 'latestcontrib', that returns
the date of user's latest contribution.
+* (T25227) action=logout now requires to be posted and have a csrf token.
=== Action API internal changes in 1.33 ===
* A number of deprecated methods for API documentation, intended for overriding
diff --git a/includes/api/ApiLogout.php b/includes/api/ApiLogout.php
index c663d1e..39a96ac 100644
--- a/includes/api/ApiLogout.php
+++ b/includes/api/ApiLogout.php
@@ -59,13 +59,21 @@ class ApiLogout extends ApiBase {
Hooks::run( 'UserLogoutComplete', [ &$user, &$injected_html, $oldName ] );
}
+ public function mustBePosted() {
+ return true;
+ }
+
+ public function needsToken() {
+ return 'csrf';
+ }
+
public function isReadMode() {
return false;
}
protected function getExamplesMessages() {
return [
- 'action=logout'
+ 'action=logout&token=123ABC'
=> 'apihelp-logout-example-logout',
];
}
diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php
index 8f45f28..068e43a 100644
--- a/includes/skins/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
@@ -617,16 +617,15 @@ class SkinTemplate extends Skin {
$page = Title::newFromText( $request->getVal( 'title', '' ) );
}
$page = $request->getVal( 'returnto', $page );
- $a = [];
+ $returnto = [];
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'] = [
'text' => $this->username,
@@ -691,9 +690,10 @@ class SkinTemplate extends Skin {
$personal_urls['logout'] = [
'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 (T19790)
- $title->isSpecial( 'Preferences' ) ? 'noreturnto' : $returnto ),
+ ( $title->isSpecial( 'Preferences' ) ? [] : $returnto )
+ + [ 'logoutToken' => $this->getUser()->getEditToken( 'logoutToken', $this->getRequest() ) ] ),
'active' => false
];
}
diff --git a/includes/specials/SpecialUserLogout.php b/includes/specials/SpecialUserLogout.php
index a9b732e..aa5ad75 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 56ff467..b92fee7 100644
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -4216,5 +4216,7 @@
"passwordpolicies-policyflag-forcechange": "must change on login",
"passwordpolicies-policyflag-suggestchangeonlogin": "suggest change on login",
"easydeflate-invaliddeflate": "Content provided is not properly deflated",
- "unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage"
+ "unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage",
+ "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 c7f2733..6731a76 100644
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -4423,5 +4423,7 @@
"passwordpolicies-policyflag-forcechange": "Password policy flag that enforces changing invalid passwords on login.",
"passwordpolicies-policyflag-suggestchangeonlogin": "Password policy flag that suggests changing invalid passwords on login.",
"easydeflate-invaliddeflate": "Error message if the content passed to easydeflate was not deflated (compressed) properly",
- "unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected"
+ "unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected",
+ "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.20.1

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
7307018
Default Alt Text
T25227.patch (6 KB)
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL