r71606 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r71605‎ | r71606 | r71607 >
Date:00:40, 25 August 2010
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
begin implementing better handling of bad authtoken
Modified paths:
  • /trunk/extensions/CentralNotice/CentralNotice.i18n.php (modified) (history)
  • /trunk/extensions/CentralNotice/SpecialCentralNotice.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/SpecialCentralNotice.php
——@@ -48,147 +48,153 @@
4949
5050 $method = $wgRequest->getVal( 'method' );
5151 // Handle form sumissions
52 - if ( $this->editable && $wgRequest->wasPosted() && $wgUser->matchEditToken( $wgRequest->getVal( 'authtoken' ) ) ) {
53 -
54 - // Handle removing campaigns
55 - $toRemove = $wgRequest->getArray( 'removeNotices' );
56 - if ( isset( $toRemove ) ) {
57 - // Remove campaigns in list
58 - foreach ( $toRemove as $notice ) {
59 - $this->removeNotice( $notice );
 52+ if ( $this->editable && $wgRequest->wasPosted() ) {
 53+
 54+ // Check authentication token
 55+ if ( $wgUser->matchEditToken( $wgRequest->getVal( 'authtoken' ) ) ) {
 56+
 57+ // Handle removing campaigns
 58+ $toRemove = $wgRequest->getArray( 'removeNotices' );
 59+ if ( isset( $toRemove ) ) {
 60+ // Remove campaigns in list
 61+ foreach ( $toRemove as $notice ) {
 62+ $this->removeNotice( $notice );
 63+ }
 64+
 65+ // Show list of campaigns
 66+ $this->listNotices();
 67+ $wgOut->addHTML( Xml::closeElement( 'div' ) );
 68+ return;
6069 }
6170
62 - // Show list of campaigns
63 - $this->listNotices();
64 - $wgOut->addHTML( Xml::closeElement( 'div' ) );
65 - return;
66 - }
67 -
68 - // Handle locking/unlocking campaigns
69 - $lockedNotices = $wgRequest->getArray( 'locked' );
70 - if ( isset( $lockedNotices ) ) {
71 - if ( $method == 'listNoticeDetail' ) {
72 - $notice = $wgRequest->getVal ( 'notice' );
73 - $this->updateLock( $notice, '1' );
74 - } else {
75 - // Build list of campaigns to lock
76 - $unlockedNotices = array_diff( $this->getNoticesName(), $lockedNotices );
77 -
78 - // Set locked/unlocked flag accordingly
79 - foreach ( $lockedNotices as $notice ) {
 71+ // Handle locking/unlocking campaigns
 72+ $lockedNotices = $wgRequest->getArray( 'locked' );
 73+ if ( isset( $lockedNotices ) ) {
 74+ if ( $method == 'listNoticeDetail' ) {
 75+ $notice = $wgRequest->getVal ( 'notice' );
8076 $this->updateLock( $notice, '1' );
 77+ } else {
 78+ // Build list of campaigns to lock
 79+ $unlockedNotices = array_diff( $this->getNoticesName(), $lockedNotices );
 80+
 81+ // Set locked/unlocked flag accordingly
 82+ foreach ( $lockedNotices as $notice ) {
 83+ $this->updateLock( $notice, '1' );
 84+ }
 85+ foreach ( $unlockedNotices as $notice ) {
 86+ $this->updateLock( $notice, '0' );
 87+ }
8188 }
82 - foreach ( $unlockedNotices as $notice ) {
83 - $this->updateLock( $notice, '0' );
84 - }
8589 }
86 - }
8790
88 - // Handle enabling/disabling campaigns
89 - $enabledNotices = $wgRequest->getArray( 'enabled' );
90 - if ( isset( $enabledNotices ) ) {
91 - if ( $method == 'listNoticeDetail' ) {
92 - $notice = $wgRequest->getVal ( 'notice' );
93 - $this->updateEnabled( $notice, '1' );
94 - } else {
95 - // Build list of campaigns to disable
96 - $disabledNotices = array_diff( $this->getNoticesName(), $enabledNotices );
97 -
98 - // Set enabled/disabled flag accordingly
99 - foreach ( $enabledNotices as $notice ) {
 91+ // Handle enabling/disabling campaigns
 92+ $enabledNotices = $wgRequest->getArray( 'enabled' );
 93+ if ( isset( $enabledNotices ) ) {
 94+ if ( $method == 'listNoticeDetail' ) {
 95+ $notice = $wgRequest->getVal ( 'notice' );
10096 $this->updateEnabled( $notice, '1' );
 97+ } else {
 98+ // Build list of campaigns to disable
 99+ $disabledNotices = array_diff( $this->getNoticesName(), $enabledNotices );
 100+
 101+ // Set enabled/disabled flag accordingly
 102+ foreach ( $enabledNotices as $notice ) {
 103+ $this->updateEnabled( $notice, '1' );
 104+ }
 105+ foreach ( $disabledNotices as $notice ) {
 106+ $this->updateEnabled( $notice, '0' );
 107+ }
101108 }
102 - foreach ( $disabledNotices as $notice ) {
103 - $this->updateEnabled( $notice, '0' );
104 - }
105109 }
106 - }
107110
108 - // Handle setting preferred campaigns
109 - $preferredNotices = $wgRequest->getArray( 'preferred' );
110 - if ( isset( $preferredNotices ) ) {
111 - // Set since this is a single display
112 - if ( $method == 'listNoticeDetail' ) {
113 - $notice = $wgRequest->getVal ( 'notice' );
114 - $this->centralNoticeDB->updatePreferred( $notice, '1' );
115 - }
116 - else {
117 - // Build list of campaigns to unset
118 - $unsetNotices = array_diff( $this->getNoticesName(), $preferredNotices );
119 -
120 - // Set flag accordingly
121 - foreach ( $preferredNotices as $notice ) {
 111+ // Handle setting preferred campaigns
 112+ $preferredNotices = $wgRequest->getArray( 'preferred' );
 113+ if ( isset( $preferredNotices ) ) {
 114+ // Set since this is a single display
 115+ if ( $method == 'listNoticeDetail' ) {
 116+ $notice = $wgRequest->getVal ( 'notice' );
122117 $this->centralNoticeDB->updatePreferred( $notice, '1' );
123118 }
124 - foreach ( $unsetNotices as $notice ) {
125 - $this->centralNoticeDB->updatePreferred( $notice, '0' );
 119+ else {
 120+ // Build list of campaigns to unset
 121+ $unsetNotices = array_diff( $this->getNoticesName(), $preferredNotices );
 122+
 123+ // Set flag accordingly
 124+ foreach ( $preferredNotices as $notice ) {
 125+ $this->centralNoticeDB->updatePreferred( $notice, '1' );
 126+ }
 127+ foreach ( $unsetNotices as $notice ) {
 128+ $this->centralNoticeDB->updatePreferred( $notice, '0' );
 129+ }
126130 }
127131 }
128 - }
129132
130 - $noticeName = $wgRequest->getVal( 'notice' );
 133+ $noticeName = $wgRequest->getVal( 'notice' );
 134+
 135+ // Handle range setting
 136+ $start = $wgRequest->getArray( 'start' );
 137+ $end = $wgRequest->getArray( 'end' );
 138+ if ( isset( $start ) && isset( $end ) ) {
 139+ $updatedStart = sprintf( "%04d%02d%02d%02d%02d00",
 140+ $start['year'],
 141+ $start['month'],
 142+ $start['day'],
 143+ $start['hour'],
 144+ $start['min'] );
 145+ $updatedEnd = sprintf( "%04d%02d%02d000000",
 146+ $end['year'],
 147+ $end['month'],
 148+ $end['day'] );
 149+ $this->updateNoticeDate( $noticeName, $updatedStart, $updatedEnd );
 150+ }
131151
132 - // Handle range setting
133 - $start = $wgRequest->getArray( 'start' );
134 - $end = $wgRequest->getArray( 'end' );
135 - if ( isset( $start ) && isset( $end ) ) {
136 - $updatedStart = sprintf( "%04d%02d%02d%02d%02d00",
137 - $start['year'],
138 - $start['month'],
139 - $start['day'],
140 - $start['hour'],
141 - $start['min'] );
142 - $updatedEnd = sprintf( "%04d%02d%02d000000",
143 - $end['year'],
144 - $end['month'],
145 - $end['day'] );
146 - $this->updateNoticeDate( $noticeName, $updatedStart, $updatedEnd );
147 - }
 152+ // Handle updates if no post content came through
 153+ if ( !isset( $lockedNotices ) && $method !== 'addNotice' ) {
 154+ if ( $method == 'listNoticeDetail' ) {
 155+ $notice = $wgRequest->getVal ( 'notice' );
 156+ $this->updateLock( $notice, 0 );
 157+ } else {
 158+ $allNotices = $this->getNoticesName();
 159+ foreach ( $allNotices as $notice ) {
 160+ $this->updateLock( $notice, '0' );
 161+ }
 162+ }
 163+ }
148164
149 - // Handle updates if no post content came through
150 - if ( !isset( $lockedNotices ) && $method !== 'addNotice' ) {
151 - if ( $method == 'listNoticeDetail' ) {
152 - $notice = $wgRequest->getVal ( 'notice' );
153 - $this->updateLock( $notice, 0 );
154 - } else {
155 - $allNotices = $this->getNoticesName();
156 - foreach ( $allNotices as $notice ) {
157 - $this->updateLock( $notice, '0' );
 165+ if ( !isset( $enabledNotices ) && $method !== 'addNotice' ) {
 166+ if ( $method == 'listNoticeDetail' ) {
 167+ $notice = $wgRequest->getVal ( 'notice' );
 168+ $this->updateEnabled( $notice, 0 );
 169+ } else {
 170+ $allNotices = $this->getNoticesName();
 171+ foreach ( $allNotices as $notice ) {
 172+ $this->updateEnabled( $notice, '0' );
 173+ }
158174 }
159175 }
160 - }
161176
162 - if ( !isset( $enabledNotices ) && $method !== 'addNotice' ) {
163 - if ( $method == 'listNoticeDetail' ) {
164 - $notice = $wgRequest->getVal ( 'notice' );
165 - $this->updateEnabled( $notice, 0 );
166 - } else {
167 - $allNotices = $this->getNoticesName();
168 - foreach ( $allNotices as $notice ) {
169 - $this->updateEnabled( $notice, '0' );
 177+ if ( !isset( $preferredNotices ) && $method !== 'addNotice' ) {
 178+ if ( $method == 'listNoticeDetail' ) {
 179+ $notice = $wgRequest->getVal ( 'notice' );
 180+ $this->centralNoticeDB->updatePreferred( $notice, 0 );
 181+ } else {
 182+ $allNotices = $this->getNoticesName();
 183+ foreach ( $allNotices as $notice ) {
 184+ $this->centralNoticeDB->updatePreferred( $notice, '0' );
 185+ }
170186 }
171187 }
172 - }
173188
174 - if ( !isset( $preferredNotices ) && $method !== 'addNotice' ) {
175 - if ( $method == 'listNoticeDetail' ) {
176 - $notice = $wgRequest->getVal ( 'notice' );
177 - $this->centralNoticeDB->updatePreferred( $notice, 0 );
178 - } else {
179 - $allNotices = $this->getNoticesName();
180 - foreach ( $allNotices as $notice ) {
181 - $this->centralNoticeDB->updatePreferred( $notice, '0' );
 189+ // Handle weight change
 190+ $updatedWeights = $wgRequest->getArray( 'weight' );
 191+ if ( isset( $updatedWeights ) ) {
 192+ foreach ( $updatedWeights as $templateId => $weight ) {
 193+ $this->updateWeight( $noticeName, $templateId, $weight );
182194 }
183195 }
 196+ } else {
 197+ $wgOut->addHTML( Xml::element( 'div', array( 'class' => 'cn-error' ), wfMsg( 'centralnotice-bad-authtoken' ) ) );
184198 }
185 -
186 - // Handle weight change
187 - $updatedWeights = $wgRequest->getArray( 'weight' );
188 - if ( isset( $updatedWeights ) ) {
189 - foreach ( $updatedWeights as $templateId => $weight ) {
190 - $this->updateWeight( $noticeName, $templateId, $weight );
191 - }
192 - }
193199 }
194200
195201 // Handle adding of campaign
Index: trunk/extensions/CentralNotice/CentralNotice.i18n.php
——@@ -119,6 +119,7 @@
120120 'centralnotice-banner-type' => 'Banner type:',
121121 'centralnotice-banner-hidable' => 'Static/Hidable',
122122 'centralnotice-banner-collapsible' => 'Collapsible',
 123+ 'centralnotice-bad-authtoken' => 'Invalid authentication token. Please try again.',
123124
124125 'right-centralnotice-admin' => 'Manage central notices',
125126 'right-centralnotice-translate' => 'Translate central notices',

Follow-up revisions

RevisionCommit summaryAuthorDate
r71632changing session failure message per comment at r71606kaldari17:28, 25 August 2010

Comments

#Comment by Catrope (talk | contribs)   16:08, 25 August 2010 
+	'centralnotice-bad-authtoken' => 'Invalid authentication token. Please try again.',

There's a better, long-existing message for this in core called sessionfaliure. Contents: 

There seems to be a problem with your login session;
this action has been canceled as a precaution against session hijacking.
Please hit "back" and reload the page you came from, then try again.
#Comment by Kaldari (talk | contribs)   17:29, 25 August 2010

fixed in r71632.

Status & tagging log