r98104 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r98103‎ | r98104 | r98105 >
Date:02:07, 26 September 2011
Author:aaron
Status:deferred
Tags:
Comment:
* Added new UserAccountRequest class
* Movied verify() et all to utility class
* Fixed missing static declarations
Modified paths:
  • /trunk/extensions/ConfirmAccount/ConfirmAccount.php (modified) (history)
  • /trunk/extensions/ConfirmAccount/dataclasses/ConfirmAccount.class.php (modified) (history)
  • /trunk/extensions/ConfirmAccount/dataclasses/UserAccountRequest.php (added) (history)
  • /trunk/extensions/ConfirmAccount/presentation/specialpages/actions/ConfirmAccount_body.php (modified) (history)
  • /trunk/extensions/ConfirmAccount/presentation/specialpages/actions/RequestAccount_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ConfirmAccount/ConfirmAccount.php
@@ -166,9 +166,11 @@
167167 $wgAutoloadClasses['UserCredentialsPage'] = "$dir/actions/UserCredentials_body.php";
168168 $wgSpecialPageGroups['UserCredentials'] = 'users';
169169
170 -# Data functions
171170 $dir = dirname( __FILE__ ) . '/dataclasses';
 171+# Utility functions
172172 $wgAutoloadClasses['ConfirmAccount'] = "$dir/ConfirmAccount.class.php";
 173+# Data access objects
 174+$wgAutoloadClasses['UserAccountRequest'] = "$dir/UserAccountRequest.php";
173175
174176 $dir = dirname( __FILE__ ) . '/schema';
175177 # Schema changes
Index: trunk/extensions/ConfirmAccount/dataclasses/ConfirmAccount.class.php
@@ -50,7 +50,7 @@
5151 *
5252 * @param sring $name
5353 */
54 - public function confirmEmail( $name ) {
 54+ public static function confirmEmail( $name ) {
5555 global $wgMemc;
5656 $dbw = wfGetDB( DB_MASTER );
5757 $dbw->update( 'account_requests',
@@ -68,7 +68,7 @@
6969 * @param string $token
7070 * @return string
7171 */
72 - public function confirmationTokenUrl( $token ) {
 72+ public static function confirmationTokenUrl( $token ) {
7373 $title = SpecialPage::getTitleFor( 'RequestAccount' );
7474 return $title->getFullUrl( array(
7575 'action' => 'confirmemail',
@@ -83,11 +83,52 @@
8484 * @param string $expiration
8585 * @return string
8686 */
87 - public function getConfirmationToken( $user, &$expiration ) {
 87+ public static function getConfirmationToken( $user, &$expiration ) {
8888 global $wgConfirmAccountRejectAge;
8989 $expires = time() + $wgConfirmAccountRejectAge;
9090 $expiration = wfTimestamp( TS_MW, $expires );
9191 $token = $user->generateToken( $user->getName() . $user->getEmail() . $expires );
9292 return $token;
9393 }
 94+
 95+ /**
 96+ * Verifies that it's ok to include the uploaded file
 97+ *
 98+ * @param string $tmpfile the full path of the temporary file to verify
 99+ * @param string $extension The filename extension that the file is to be served with
 100+ * @return Status object
 101+ */
 102+ public static function verifyAttachment( $tmpfile, $extension ) {
 103+ global $wgVerifyMimeType, $wgMimeTypeBlacklist;
 104+ # magically determine mime type
 105+ $magic =& MimeMagic::singleton();
 106+ $mime = $magic->guessMimeType( $tmpfile, false );
 107+ # check mime type, if desired
 108+ if ( $wgVerifyMimeType ) {
 109+ wfDebug ( "\n\nmime: <$mime> extension: <$extension>\n\n" );
 110+ # Check mime type against file extension
 111+ if ( !UploadBase::verifyExtension( $mime, $extension ) ) {
 112+ return Status::newFatal( 'uploadcorrupt' );
 113+ }
 114+ # Check mime type blacklist
 115+ if ( isset( $wgMimeTypeBlacklist ) && !is_null( $wgMimeTypeBlacklist )
 116+ && self::checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 117+ return Status::newFatal( 'filetype-badmime', $mime );
 118+ }
 119+ }
 120+ wfDebug( __METHOD__ . ": all clear; passing.\n" );
 121+ return Status::newGood();
 122+ }
 123+
 124+ /**
 125+ * Perform case-insensitive match against a list of file extensions.
 126+ * Returns true if the extension is in the list.
 127+ *
 128+ * @param string $ext
 129+ * @param array $list
 130+ * @return bool
 131+ */
 132+ protected static function checkFileExtension( $ext, $list ) {
 133+ return in_array( strtolower( $ext ), $list );
 134+ }
94135 }
Index: trunk/extensions/ConfirmAccount/dataclasses/UserAccountRequest.php
@@ -0,0 +1,257 @@
 2+<?php
 3+class UserAccountRequest {
 4+ /* Initially supplied fields */
 5+ protected $id;
 6+ protected $name;
 7+ protected $realName;
 8+ protected $email;
 9+ protected $registration;
 10+ protected $bio;
 11+ protected $notes;
 12+ protected $urls;
 13+ protected $type;
 14+ protected $areas;
 15+ protected $fileName;
 16+ protected $fileStorageKey;
 17+ protected $ip;
 18+ protected $emailToken;
 19+ protected $emailTokenExpires;
 20+ /* Fields set if user later confirms email */
 21+ protected $emailAuthTimestamp;
 22+ /* Fields used by the admins */
 23+ protected $deleted;
 24+ protected $rejectedTimestamp;
 25+ protected $heldTimestamp;
 26+ protected $user;
 27+ protected $comment;
 28+
 29+ private function __construct() {}
 30+
 31+ public static function newFromRow( Object $row ) {
 32+ $req = new self();
 33+
 34+ $req->id = (int)$row->acr_id;
 35+ $req->name = $row->acr_name;
 36+ $req->realName = $row->acr_real_name;
 37+ $req->email = $row->acr_email;
 38+ $req->registration = wfTimestampOrNull( TS_MW, $row->acr_registration );
 39+ $req->bio = $row->acr_bio;
 40+ $req->notes = $row->acr_notes;
 41+ $req->urls = $row->acr_urls;
 42+ $req->type = (int)$row->acr_type;
 43+ $req->areas = self::expandAreas( $row->acr_areas );
 44+ $req->fileName = $row->acr_filename;
 45+ $req->fileStorageKey = $row->acr_storage_key;
 46+ $req->ip = $row->acr_ip;
 47+ $req->emailToken = $row->acr_email_token; // MD5 of token
 48+ $req->emailTokenExpires = wfTimestampOrNull( TS_MW, $row->acr_email_token_expires );
 49+ $req->emailAuthTimestamp = wfTimestampOrNull( TS_MW, $row->acr_email_authenticated );
 50+ $req->deleted = (bool)$row->acr_deleted;
 51+ $req->rejectedTimestamp = wfTimestampOrNull( TS_MW, $row->acr_rejected );
 52+ $req->heldTimestamp = wfTimestampOrNull( TS_MW, $row->acr_held );
 53+ $req->user = (int)$row->acr_user;
 54+ $req->comment = $row->acr_comment;
 55+
 56+ return $req;
 57+ }
 58+
 59+ public static function newFromArray( array $fields ) {
 60+ $req = new self();
 61+
 62+ $req->id = isset( $fields['id'] )
 63+ ? (int)$fields['id']
 64+ : null; // determined on insertOn()
 65+ $req->name = $fields['name'];
 66+ $req->realName = $fields['real_name'];
 67+ $req->email = $fields['email'];
 68+ $req->registration = wfTimestampOrNull( TS_MW, $fields['registration'] );
 69+ $req->bio = $fields['bio'];
 70+ $req->notes = $fields['notes'];
 71+ $req->urls = $fields['urls'];
 72+ $req->type = (int)$fields['type'];
 73+ $req->areas = is_string( $fields['areas'] )
 74+ ? self::expandAreas( $fields['areas'] ) // DB format
 75+ : $fields['areas']; // already expanded
 76+ $req->fileName = $fields['filename'];
 77+ $req->fileStorageKey = $fields['storage_key'];
 78+ $req->ip = $fields['ip'];
 79+ $req->emailToken = $fields['email_token']; // MD5 of token
 80+ $req->emailTokenExpires = wfTimestampOrNull( TS_MW, $fields['email_token_expires'] );
 81+ // These fields are typically left to default on insertion...
 82+ $req->emailAuthTimestamp = isset( $fields['email_authenticated'] )
 83+ ? wfTimestampOrNull( TS_MW, $fields['email_authenticated'] )
 84+ : null;
 85+ $req->deleted = isset( $fields['deleted'] )
 86+ ? $fields['deleted']
 87+ : false;
 88+ $req->rejectedTimestamp = isset( $fields['rejected'] )
 89+ ? wfTimestampOrNull( TS_MW, $fields['rejected'] )
 90+ : null;
 91+ $req->heldTimestamp = isset( $fields['held'] )
 92+ ? wfTimestampOrNull( TS_MW, $fields['held'] )
 93+ : null;
 94+ $req->user = isset( $fields['user'] )
 95+ ? (int)$fields['user']
 96+ : 0;
 97+ $req->comment = isset( $fields['comment'] )
 98+ ? $fields['comment']
 99+ : '';
 100+
 101+ return $req;
 102+ }
 103+
 104+ public function getId() {
 105+ return $this->id;
 106+ }
 107+
 108+ public function getName() {
 109+ return $this->name;
 110+ }
 111+
 112+ public function getRealName() {
 113+ return $this->realName;
 114+ }
 115+
 116+ public function getEmail() {
 117+ return $this->email;
 118+ }
 119+
 120+ public function getRegistration() {
 121+ return $this->registration;
 122+ }
 123+
 124+ public function getBio() {
 125+ return $this->bio;
 126+ }
 127+
 128+ public function getNotes() {
 129+ return $this->notes;
 130+ }
 131+
 132+ public function getUrls() {
 133+ return $this->urls;
 134+ }
 135+
 136+ public function getAreas() {
 137+ return $this->areas;
 138+ }
 139+
 140+ public function getFileName() {
 141+ return $this->fileName;
 142+ }
 143+
 144+ public function getFileStorageKey() {
 145+ return $this->fileStorageKey;
 146+ }
 147+
 148+ public function getIP() {
 149+ return $this->ip;
 150+ }
 151+
 152+ public function getEmailToken() {
 153+ return $this->emailToken;
 154+ }
 155+
 156+ public function getEmailTokenExpires() {
 157+ return $this->emailTokenExpires;
 158+ }
 159+
 160+ public function getEmailAuthTimestamp() {
 161+ return $this->emailAuthTimestamp;
 162+ }
 163+
 164+ public function isDeleted() {
 165+ return $this->deleted;
 166+ }
 167+
 168+ public function getRejectTimestamp() {
 169+ return $this->rejectedTimestamp;
 170+ }
 171+
 172+ public function getHeldTimestamp() {
 173+ return $this->heldTimestamp;
 174+ }
 175+
 176+ public function getHandlingUser() {
 177+ return $this->user;
 178+ }
 179+
 180+ public function getHandlingComment() {
 181+ return $this->comment;
 182+ }
 183+
 184+ public function insertOn() {
 185+ $dbw = wfGetDB( DB_MASTER );
 186+ # Allow for some fields to be handled automatically...
 187+ $acr_id = is_null( $this->id )
 188+ ? $this->id
 189+ : $dbw->nextSequenceValue( 'account_requests_acr_id_seq' );
 190+ # Insert into pending requests...
 191+ $dbw->insert( 'account_requests',
 192+ array(
 193+ 'acr_id' => $acr_id,
 194+ 'acr_name' => strval( $this->name ),
 195+ 'acr_email' => strval( $this->email ),
 196+ 'acr_real_name' => strval( $this->realName ),
 197+ 'acr_registration' => $dbw->timestamp( $this->registration ),
 198+ 'acr_bio' => strval( $this->bio ),
 199+ 'acr_notes' => strval( $this->notes ),
 200+ 'acr_urls' => strval( $this->urls ),
 201+ 'acr_type' => strval( $this->type ),
 202+ 'acr_areas' => self::flattenAreas( $this->areas ),
 203+ 'acr_filename' => isset( $this->fileName )
 204+ ? $this->fileName
 205+ : null,
 206+ 'acr_storage_key' => isset( $this->fileStorageKey )
 207+ ? $this->fileStorageKey
 208+ : null,
 209+ 'acr_comment' => strval( $this->comment ),
 210+ 'acr_ip' => strval( $this->ip ), // possible use for spam blocking
 211+ 'acr_deleted' => (int)$this->deleted,
 212+ 'acr_email_token' => strval( $this->emailToken ), // MD5 of token
 213+ 'acr_email_token_expires' => $dbw->timestamp( $this->emailTokenExpires ),
 214+ ),
 215+ __METHOD__
 216+ );
 217+ $this->id = $acr_id; // set for accessors
 218+
 219+ return $this->id;
 220+ }
 221+
 222+ public function remove() {
 223+ if ( !$this->id ) {
 224+ throw new MWException( "Account request ID is not set." );
 225+ }
 226+ $dbw = wfGetDB( DB_MASTER );
 227+ $dbw->delete( 'account_requests', array( 'acr_id' => $this->id ), __METHOD__ );
 228+
 229+ return ( $dbw->affectedRows() > 0 );
 230+ }
 231+
 232+ /**
 233+ * Flatten areas of interest array
 234+ * Used by ConfirmAccountsPage
 235+ * @todo just serialize()
 236+ */
 237+ protected static function flattenAreas( array $areas ) {
 238+ $flatAreas = '';
 239+ foreach ( $areas as $area ) {
 240+ $flatAreas .= $area . "\n";
 241+ }
 242+ return $flatAreas;
 243+ }
 244+
 245+ /**
 246+ * Expand areas of interest to array
 247+ * Used by ConfirmAccountsPage
 248+ * @todo just unserialize()
 249+ */
 250+ public static function expandAreas( $areas ) {
 251+ $list = explode( "\n", $areas );
 252+ foreach ( $list as $n => $item ) {
 253+ $list[$n] = trim( "wpArea-" . str_replace( ' ', '_', $item ) );
 254+ }
 255+ unset( $list[count( $list ) - 1] );
 256+ return $list;
 257+ }
 258+}
Property changes on: trunk/extensions/ConfirmAccount/dataclasses/UserAccountRequest.php
___________________________________________________________________
Added: svn:eol-style
1259 + native
Index: trunk/extensions/ConfirmAccount/presentation/specialpages/actions/RequestAccount_body.php
@@ -335,7 +335,7 @@
336336 $this->showForm( wfMsgHtml( 'requestaccount-exts' ) );
337337 return false;
338338 }
339 - $veri = $this->verify( $this->mTempPath, $finalExt );
 339+ $veri = ConfirmAccount::verifyAttachment( $this->mTempPath, $finalExt );
340340 if ( !$veri->isGood() ) {
341341 $this->mPrevAttachment = '';
342342 $this->showForm( wfMsgHtml( 'uploadcorrupt' ) );
@@ -352,30 +352,25 @@
353353 $expires = null; // passed by reference
354354 $token = ConfirmAccount::getConfirmationToken( $u, $expires );
355355 # Insert into pending requests...
356 - $acr_id = $dbw->nextSequenceValue( 'account_requests_acr_id_seq' );
 356+ $req = UserAccountRequest::newFromArray( array(
 357+ 'name' => $u->getName(),
 358+ 'email' => $u->getEmail(),
 359+ 'real_name' => $u->getRealName(),
 360+ 'registration' => wfTimestampNow(),
 361+ 'bio' => $this->mBio,
 362+ 'notes' => $this->mNotes,
 363+ 'urls' => $this->mUrls,
 364+ 'filename' => isset( $this->mSrcName ) ? $this->mSrcName : null,
 365+ 'type' => $this->mType,
 366+ 'areas' => $this->mAreaSet,
 367+ 'storage_key' => isset( $key ) ? $key : null,
 368+ 'comment' => '',
 369+ 'email_token' => md5( $token ),
 370+ 'email_token_expires' => $expires,
 371+ 'ip' => wfGetIP(),
 372+ ) );
357373 $dbw->begin();
358 - $dbw->insert( 'account_requests',
359 - array(
360 - 'acr_id' => $acr_id,
361 - 'acr_name' => $u->getName(),
362 - 'acr_email' => $u->getEmail(),
363 - 'acr_real_name' => $u->getRealName(),
364 - 'acr_registration' => $dbw->timestamp(),
365 - 'acr_bio' => $this->mBio,
366 - 'acr_notes' => $this->mNotes,
367 - 'acr_urls' => $this->mUrls,
368 - 'acr_filename' => isset( $this->mSrcName ) ? $this->mSrcName : null,
369 - 'acr_type' => $this->mType,
370 - 'acr_areas' => self::flattenAreas( $this->mAreaSet ),
371 - 'acr_storage_key' => isset( $key ) ? $key : null,
372 - 'acr_comment' => '',
373 - 'acr_email_token' => md5( $token ),
374 - 'acr_email_token_expires' => $dbw->timestamp( $expires ),
375 - 'acr_ip' => wfGetIP(), // Possible use for spam blocking
376 - 'acr_deleted' => 0,
377 - ),
378 - __METHOD__
379 - );
 374+ $req->insertOn();
380375 # Send confirmation, required!
381376 $result = $this->sendConfirmationMail( $u, $token, $expires );
382377 if ( !$result->isOK() ) {
@@ -409,30 +404,6 @@
410405 }
411406
412407 /**
413 - * Flatten areas of interest array
414 - */
415 - protected static function flattenAreas( $areas ) {
416 - $flatAreas = '';
417 - foreach ( $areas as $area ) {
418 - $flatAreas .= $area . "\n";
419 - }
420 - return $flatAreas;
421 - }
422 -
423 - /**
424 - * Expand areas of interest to array
425 - * Used by ConfirmAccountsPage
426 - */
427 - public static function expandAreas( $areas ) {
428 - $list = explode( "\n", $areas );
429 - foreach ( $list as $n => $item ) {
430 - $list[$n] = trim( "wpArea-" . str_replace( ' ', '_', $item ) );
431 - }
432 - unset( $list[count( $list ) - 1] );
433 - return $list;
434 - }
435 -
436 - /**
437408 * Initialize the uploaded file from PHP data
438409 */
439410 protected function initializeUpload( $request ) {
@@ -443,49 +414,6 @@
444415 }
445416
446417 /**
447 - * Verifies that it's ok to include the uploaded file
448 - *
449 - * @param string $tmpfile the full path of the temporary file to verify
450 - * @param string $extension The filename extension that the file is to be served with
451 - * @return Status object
452 - */
453 - protected function verify( $tmpfile, $extension ) {
454 - # magically determine mime type
455 - $magic =& MimeMagic::singleton();
456 - $mime = $magic->guessMimeType( $tmpfile, false );
457 - # check mime type, if desired
458 - global $wgVerifyMimeType;
459 - if ( $wgVerifyMimeType ) {
460 - wfDebug ( "\n\nmime: <$mime> extension: <$extension>\n\n" );
461 - # check mime type against file extension
462 - if ( !UploadBase::verifyExtension( $mime, $extension ) ) {
463 - return Status::newFatal( 'uploadcorrupt' );
464 - }
465 -
466 - # check mime type blacklist
467 - global $wgMimeTypeBlacklist;
468 - if ( isset( $wgMimeTypeBlacklist ) && !is_null( $wgMimeTypeBlacklist )
469 - && $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
470 - return Status::newFatal( 'filetype-badmime', $mime );
471 - }
472 - }
473 - wfDebug( __METHOD__ . ": all clear; passing.\n" );
474 - return Status::newGood();
475 - }
476 -
477 - /**
478 - * Perform case-insensitive match against a list of file extensions.
479 - * Returns true if the extension is in the list.
480 - *
481 - * @param string $ext
482 - * @param array $list
483 - * @return bool
484 - */
485 - protected function checkFileExtension( $ext, $list ) {
486 - return in_array( strtolower( $ext ), $list );
487 - }
488 -
489 - /**
490418 * @private
491419 * @param int $limit number of accounts allowed to be requested from the same IP
492420 */
Index: trunk/extensions/ConfirmAccount/presentation/specialpages/actions/ConfirmAccount_body.php
@@ -754,7 +754,7 @@
755755 $this->mUsername = $this->mUsername ? $this->mUsername : $row->acr_name;
756756 $this->mBio = $this->mBio ? $this->mBio : $row->acr_bio;
757757 $this->mType = !is_null($this->mType) ? $this->mType : $row->acr_type;
758 - $rowareas = RequestAccountPage::expandAreas( $row->acr_areas );
 758+ $rowareas = UserAccountRequest::expandAreas( $row->acr_areas );
759759
760760 foreach( $this->mAreas as $area => $within ) {
761761 # If admin didn't set any of these checks, go back to how the user set them

Status & tagging log