Gergő Tisza has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311889

Change subject: Add HTMLFormField class for MWRestrictions and use it for bot 
passwords
......................................................................

Add HTMLFormField class for MWRestrictions and use it for bot passwords

Change-Id: Ib50238e3be5eec63eb5df97154b60dc4ca33d581
---
M autoload.php
M includes/htmlform/HTMLFormField.php
A includes/htmlform/fields/HTMLRestrictionsField.php
M includes/specials/SpecialBotPasswords.php
M includes/utils/MWRestrictions.php
M languages/i18n/en.json
M languages/i18n/qqq.json
A tests/phpunit/includes/htmlform/HTMLRestrictionsFieldTest.php
8 files changed, 200 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/89/311889/1

diff --git a/autoload.php b/autoload.php
index 198e477..1b21365 100644
--- a/autoload.php
+++ b/autoload.php
@@ -544,6 +544,7 @@
        'HTMLMultiSelectField' => __DIR__ . 
'/includes/htmlform/fields/HTMLMultiSelectField.php',
        'HTMLNestedFilterable' => __DIR__ . 
'/includes/htmlform/HTMLNestedFilterable.php',
        'HTMLRadioField' => __DIR__ . 
'/includes/htmlform/fields/HTMLRadioField.php',
+       'HTMLRestrictionsField' => __DIR__ . 
'/includes/htmlform/fields/HTMLRestrictionsField.php',
        'HTMLSelectAndOtherField' => __DIR__ . 
'/includes/htmlform/fields/HTMLSelectAndOtherField.php',
        'HTMLSelectField' => __DIR__ . 
'/includes/htmlform/fields/HTMLSelectField.php',
        'HTMLSelectLimitField' => __DIR__ . 
'/includes/htmlform/fields/HTMLSelectLimitField.php',
diff --git a/includes/htmlform/HTMLFormField.php 
b/includes/htmlform/HTMLFormField.php
index 8604ba2..4afdea7 100644
--- a/includes/htmlform/HTMLFormField.php
+++ b/includes/htmlform/HTMLFormField.php
@@ -821,7 +821,7 @@
        /**
         * Determine the help text to display
         * @since 1.20
-        * @return string HTML
+        * @return string|null HTML
         */
        public function getHelpText() {
                $helptext = null;
diff --git a/includes/htmlform/fields/HTMLRestrictionsField.php 
b/includes/htmlform/fields/HTMLRestrictionsField.php
new file mode 100644
index 0000000..f97962f
--- /dev/null
+++ b/includes/htmlform/fields/HTMLRestrictionsField.php
@@ -0,0 +1,121 @@
+<?php
+
+/**
+ * Class for updating an MWRestrictions value (which is, currently, basically 
just an IP address
+ * list).
+ *
+ * Will be represented as a textarea with one address per line, with 
intelligent defaults for
+ * label, help text and row count.
+ *
+ * The value returned will be an MWRestrictions or the input string if it was 
not a list of
+ * valid IP ranges.
+ */
+class HTMLRestrictionsField extends HTMLTextAreaField {
+       const DEFAULT_ROWS = 5;
+
+       public function __construct( array $params ) {
+               parent::__construct( $params );
+               if ( !$this->mLabel ) {
+                       $this->mLabel = $this->msg( 
'restrictionsfield-label')->parse();
+               }
+       }
+
+       public function getHelpText() {
+               $helpText = parent::getHelpText();
+               if ( $helpText === null ) {
+                       $helpText = $this->msg( 
'restrictionsfield-help')->parse();
+               }
+               return $helpText;
+       }
+
+       /**
+        * @param WebRequest $request
+        * @return string|MWRestrictions Restrictions object or original string 
if invalid
+        */
+       function loadDataFromRequest( $request ) {
+               if ( !$request->getCheck( $this->mName ) ) {
+                       return $this->getDefault();
+               }
+
+               $value = rtrim( $request->getText( $this->mName ), "\r\n" );
+               $ips = $value === '' ? [] : explode( PHP_EOL, $value );
+               try {
+                       return MWRestrictions::newFromArray( [ 'IPAddresses' => 
$ips ] );
+               } catch ( InvalidArgumentException $e ) {
+                       return $value;
+               }
+       }
+
+       /**
+        * @return MWRestrictions
+        */
+       public function getDefault() {
+               $default = parent::getDefault();
+               if ( $default === null ) {
+                       $default = MWRestrictions::newDefault();
+               }
+               return $default;
+       }
+
+       /**
+        * @param string|MWRestrictions $value The value the field was 
submitted with
+        * @param array $alldata The data collected from the form
+        *
+        * @return bool|string True on success, or String error to display, or
+        *   false to fail validation without displaying an error.
+        */
+       public function validate( $value, $alldata ) {
+               if ( $this->isHidden( $alldata ) ) {
+                       return true;
+               }
+
+               if (
+                       isset( $this->mParams['required'] ) && 
$this->mParams['required'] !== false
+                       && $value instanceof MWRestrictions && 
!$value->toArray()['IPAddresses']
+               ) {
+                       return $this->msg( 'htmlform-required' )->parse();
+               }
+
+               if ( is_string( $value ) ) {
+                       // MWRestrictions::newFromArray failed; one of the IP 
ranges must be invalid
+                       $status = Status::newGood();
+                       foreach ( explode( PHP_EOL,  $value ) as $range ) {
+                               if ( !\IP::isIPAddress( $range ) ) {
+                                       $status->fatal( 
'restrictionsfield-badip', $range );
+                               }
+                       }
+                       if ( $status->isOK() ) {
+                               $status->fatal( 'unknown-error' );
+                       }
+                       return $status->getWikiText();
+               }
+
+               if ( isset( $this->mValidationCallback ) ) {
+                       return call_user_func( $this->mValidationCallback, 
$value, $alldata, $this->mParent );
+               }
+
+               return true;
+       }
+
+       /**
+        * @param string|MWRestrictions $value
+        * @return string
+        */
+       public function getInputHTML( $value ) {
+               if ( $value instanceof MWRestrictions ) {
+                       $value = implode( PHP_EOL, 
$value->toArray()['IPAddresses'] );
+               }
+               return parent::getInputHTML( $value );
+       }
+
+       /**
+        * @param MWRestrictions $value
+        * @return string
+        */
+       public function getInputOOUI( $value ) {
+               if ( $value instanceof MWRestrictions ) {
+                       $value = implode( PHP_EOL, 
$value->toArray()['IPAddresses'] );
+               }
+               return parent::getInputOOUI( $value );
+       }
+}
diff --git a/includes/specials/SpecialBotPasswords.php 
b/includes/specials/SpecialBotPasswords.php
index 9975e41..ed3cd5e 100644
--- a/includes/specials/SpecialBotPasswords.php
+++ b/includes/specials/SpecialBotPasswords.php
@@ -146,19 +146,9 @@
                        ];
 
                        $fields['restrictions'] = [
-                               'type' => 'textarea',
-                               'label-message' => 
'botpasswords-label-restrictions',
+                               'class' => 'HTMLRestrictionsField',
                                'required' => true,
-                               'default' => 
$this->botPassword->getRestrictions()->toJson( true ),
-                               'rows' => 5,
-                               'validation-callback' => function ( $v ) {
-                                       try {
-                                               MWRestrictions::newFromJson( $v 
);
-                                               return true;
-                                       } catch ( InvalidArgumentException $ex 
) {
-                                               return $ex->getMessage();
-                                       }
-                               },
+                               'default' => 
$this->botPassword->getRestrictions(),
                        ];
 
                } else {
@@ -282,7 +272,7 @@
                $bp = BotPassword::newUnsaved( [
                        'centralId' => $this->userId,
                        'appId' => $this->par,
-                       'restrictions' => MWRestrictions::newFromJson( 
$data['restrictions'] ),
+                       'restrictions' => $data['restrictions'],
                        'grants' => array_merge(
                                MWGrants::getHiddenGrants(),
                                preg_replace( '/^grant-/', '', $data['grants'] )
diff --git a/includes/utils/MWRestrictions.php 
b/includes/utils/MWRestrictions.php
index 617e8f5..caf88a1 100644
--- a/includes/utils/MWRestrictions.php
+++ b/includes/utils/MWRestrictions.php
@@ -27,6 +27,7 @@
 
        /**
         * @param array $restrictions
+        * @throws InvalidArgumentException
         */
        protected function __construct( array $restrictions = null ) {
                if ( $restrictions !== null ) {
@@ -44,6 +45,7 @@
        /**
         * @param array $restrictions
         * @return MWRestrictions
+        * @throws InvalidArgumentException
         */
        public static function newFromArray( array $restrictions ) {
                return new self( $restrictions );
@@ -52,6 +54,7 @@
        /**
         * @param string $json JSON representation of the restrictions
         * @return MWRestrictions
+        * @throws InvalidArgumentException
         */
        public static function newFromJson( $json ) {
                $restrictions = FormatJson::decode( $json, true );
diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index 67e6491..bbf453d 100644
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -563,7 +563,6 @@
        "botpasswords-label-resetpassword": "Reset the password",
        "botpasswords-label-grants": "Applicable grants:",
        "botpasswords-help-grants": "Each grant gives access to listed user 
rights that a user account already has. See the [[Special:ListGrants|table of 
grants]] for more information.",
-       "botpasswords-label-restrictions": "Usage restrictions:",
        "botpasswords-label-grants-column": "Granted",
        "botpasswords-bad-appid": "The bot name \"$1\" is not valid.",
        "botpasswords-insert-failed": "Failed to add bot name \"$1\". Was it 
already added?",
@@ -4207,5 +4206,8 @@
        "unlinkaccounts-success": "The account was unlinked.",
        "authenticationdatachange-ignored": "The authentication data change was 
not handled. Maybe no provider was configured?",
        "userjsispublic": "Please note: JavaScript subpages should not contain 
confidential data as they are viewable by other users.",
-       "usercssispublic": "Please note: CSS subpages should not contain 
confidential data as they are viewable by other users."
+       "usercssispublic": "Please note: CSS subpages should not contain 
confidential data as they are viewable by other users.",
+       "restrictionsfield-badip": "Invalid IP address or range: $1",
+       "restrictionsfield-label": "Allowed IP ranges:",
+       "restrictionsfield-help": "One IP address or CIDR range per line. To 
enable everything, use<br><code>0.0.0.0/0</code><br><code>::/0</code>"
 }
diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index 163b613..a8921db 100644
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -747,7 +747,6 @@
        "botpasswords-label-resetpassword": "Label for the checkbox to reset 
the actual password for the current bot password.",
        "botpasswords-label-grants": "Label for the checkmatrix for selecting 
grants allowed when the bot password is used.\n\ngrant: Vidu 
http://komputeko.net/index_en.php?vorto=grant sed \"konced/i\" egale funkcius.",
        "botpasswords-help-grants": "Help text for the grant selection 
checkmatrix.",
-       "botpasswords-label-restrictions": "Label for the textarea field in 
which JSON defining access restrictions (e.g. which IP address ranges are 
allowed) is entered.",
        "botpasswords-label-grants-column": "Label for the checkbox column on 
the checkmatrix for selecting grants allowed when the bot password is used.",
        "botpasswords-bad-appid": "Used as an error message when an invalid 
\"bot name\" is supplied on [[Special:BotPasswords]]. Parameters:\n* $1 - The 
rejected bot name.",
        "botpasswords-insert-failed": "Error message when saving a new bot 
password failed. It's likely that the failure was because the user resubmitted 
the form after a previous successful save. Parameters:\n* $1 - Bot name",
@@ -4391,5 +4390,8 @@
        "unlinkaccounts-success": "Account unlinking form success message",
        "authenticationdatachange-ignored": "Shown when authentication data 
change was unsuccessful due to configuration problems.\n\nCf. e.g. 
{{msg-mw|Passwordreset-ignored}}.",
        "userjsispublic": "A reminder to users that Javascript subpages are not 
preferences but normal pages, and thus can be viewed by other users and the 
general public. This message is shown to a user whenever they are editing a 
subpage in their own user-space that ends in .js. See also 
{{msg-mw|usercssispublic}}.",
-       "usercssispublic": "A reminder to users that CSS subpages are not 
preferences but normal pages, and thus can be viewed by other users and the 
general public. This message is shown to a user whenever they are editing a 
subpage in their own user-space that ends in .css. See also 
{{msg-mw|userjsispublic}}"
+       "usercssispublic": "A reminder to users that CSS subpages are not 
preferences but normal pages, and thus can be viewed by other users and the 
general public. This message is shown to a user whenever they are editing a 
subpage in their own user-space that ends in .css. See also 
{{msg-mw|userjsispublic}}",
+       "restrictionsfield-badip": "An error message shown when one entered an 
invalid IP address or range in a restrictions field (such as 
Special:BotPassword). $1 is the IP address.",
+       "restrictionsfield-label": "Field label shown for restriction fields 
(e.g. on Special:BotPassword).",
+       "restrictionsfield-help": "Placeholder text displayed in restriction 
fields (e.g. on Special:BotPassword)."
 }
diff --git a/tests/phpunit/includes/htmlform/HTMLRestrictionsFieldTest.php 
b/tests/phpunit/includes/htmlform/HTMLRestrictionsFieldTest.php
new file mode 100644
index 0000000..8d898c6
--- /dev/null
+++ b/tests/phpunit/includes/htmlform/HTMLRestrictionsFieldTest.php
@@ -0,0 +1,63 @@
+<?php
+
+class HTMLRestrictionsFieldTest extends PHPUnit_Framework_TestCase {
+       public function testConstruct() {
+               $field = new HTMLRestrictionsField( [ 'fieldname' => 
'restrictions' ] );
+               $this->assertNotEmpty( $field->getLabel(), 'has a default 
label' );
+               $this->assertNotEmpty( $field->getHelpText(), 'has a default 
help text' );
+               $this->assertEquals( MWRestrictions::newDefault(), 
$field->getDefault(),
+                       'defaults to the default MWRestrictions object' );
+
+               $field = new HTMLRestrictionsField( [
+                       'fieldname' => 'restrictions',
+                       'label' => 'foo',
+                       'help' => 'bar',
+                       'default' => 'baz',
+               ] );
+               $this->assertEquals( 'foo', $field->getLabel(), 'label can be 
customized' );
+               $this->assertEquals( 'bar', $field->getHelpText(), 'help text 
can be customized' );
+               $this->assertEquals( 'baz', $field->getDefault(), 'default can 
be customized' );
+       }
+
+       /**
+        * @dataProvider provideValidate
+        */
+       public function testForm( $text, $value ) {
+               $form = HTMLForm::factory( 'ooui', [
+                       'restrictions' => [ 'class' => 
HTMLRestrictionsField::class ],
+               ] );
+               $request = new FauxRequest( [ 'wprestrictions' => $text ], true 
);
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $context->setRequest( $request );
+               $form->setContext( $context );
+               $status = $form->setTitle( Title::newFromText( 'Main Page' ) 
)->prepareForm()
+                       ->setSubmitCallback( function () { return true; } 
)->trySubmit();
+
+               if ( $status instanceof StatusValue ) {
+                       $this->assertEquals( $value !== false , 
$status->isGood() );
+               } elseif ( $value === false ) {
+                       $this->assertNotSame( true, $status );
+               } else {
+                       $this->assertSame( true, $status );
+               }
+
+               if ( $value !== false ) {
+                       $restrictions = $form->mFieldData['restrictions'];
+                       $this->assertInstanceOf( MWRestrictions::class, 
$restrictions );
+                       $this->assertEquals( $value, 
$restrictions->toArray()['IPAddresses'] );
+               }
+
+               // sanity
+               $form->getHTML( $status );
+       }
+
+       public function provideValidate() {
+               return [
+                       // submitted text, value of 'IPAddresses' key or false 
for validation error
+                       [ null, [ '0.0.0.0/0', '::/0' ] ],
+                       [ '', [] ],
+                       [ "1.2.3.4\n::/0", [ '1.2.3.4', '::/0' ] ],
+                       [ "1.2.3.4\n::/x", false ],
+               ];
+       }
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/311889
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib50238e3be5eec63eb5df97154b60dc4ca33d581
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to