AuthManager: Allow for flagging fields as "sensitive"

This can allow AuthenticationRequests to flag certain fields as
sensitive, so e.g. the API can insist they be in the POST body rather
than in the query string.

Change-Id: I7b12aa4cd8f5a570f0df7213c0f9084b5a4d4de7

diff --git a/includes/api/ApiAuthManagerHelper.php b/includes/api/ApiAuthManagerHelper.php
index c970f93..d330862 100644 (file)
--- a/includes/api/ApiAuthManagerHelper.php
+++ b/includes/api/ApiAuthManagerHelper.php
@@ -326,6 +326,7 @@ class ApiAuthManagerHelper {
                        $this->formatMessage( $ret, 'label', $field['label'] );
                        $this->formatMessage( $ret, 'help', $field['help'] );
                        $ret['optional'] = !empty( $field['optional'] );
+                       $ret['sensitive'] = !empty( $field['sensitive'] );
 
                        $retFields[$name] = $ret;
                }

diff --git a/includes/auth/AuthenticationRequest.php b/includes/auth/AuthenticationRequest.php
index f6f949e..2474b8b 100644 (file)
--- a/includes/auth/AuthenticationRequest.php
+++ b/includes/auth/AuthenticationRequest.php
@@ -102,6 +102,8 @@ abstract class AuthenticationRequest {
         *  - label: (Message) Text suitable for a label in an HTML form
         *  - help: (Message) Text suitable as a description of what the field is
         *  - optional: (bool) If set and truthy, the field may be left empty
+        *  - sensitive: (bool) If set and truthy, the field is considered sensitive. Code using the
+        *      request should avoid exposing the value of the field.
         *
         * @return array As above
         */
@@ -315,6 +317,8 @@ abstract class AuthenticationRequest {
                                        $options['optional'] = !empty( $options['optional'] );
                                }
 
+                               $options['sensitive'] = !empty( $options['sensitive'] );
+
                                if ( !array_key_exists( $name, $merged ) ) {
                                        $merged[$name] = $options;
                                } elseif ( $merged[$name]['type'] !== $options['type'] ) {
@@ -333,6 +337,7 @@ abstract class AuthenticationRequest {
                                        }
 
                                        $merged[$name]['optional'] = $merged[$name]['optional'] && $options['optional'];
+                                       $merged[$name]['sensitive'] = $merged[$name]['sensitive'] || $options['sensitive'];
 
                                        // No way to merge 'value', 'image', 'help', or 'label', so just use
                                        // the value from the first request.

diff --git a/includes/auth/PasswordAuthenticationRequest.php b/includes/auth/PasswordAuthenticationRequest.php
index 187c29a..8550f3e 100644 (file)
--- a/includes/auth/PasswordAuthenticationRequest.php
+++ b/includes/auth/PasswordAuthenticationRequest.php
@@ -53,6 +53,7 @@ class PasswordAuthenticationRequest extends AuthenticationRequest {
                                'type' => 'password',
                                'label' => wfMessage( $passwordLabel ),
                                'help' => wfMessage( 'authmanager-password-help' ),
+                               'sensitive' => true,
                        ],
                ];
 
@@ -68,6 +69,7 @@ class PasswordAuthenticationRequest extends AuthenticationRequest {
                                'type' => 'password',
                                'label' => wfMessage( $retypeLabel ),
                                'help' => wfMessage( 'authmanager-retype-help' ),
+                               'sensitive' => true,
                        ];
                }
 

diff --git a/tests/phpunit/includes/auth/AuthenticationRequestTest.php b/tests/phpunit/includes/auth/AuthenticationRequestTest.php
index a7df221..7d2ba8d 100644 (file)
--- a/tests/phpunit/includes/auth/AuthenticationRequestTest.php
+++ b/tests/phpunit/includes/auth/AuthenticationRequestTest.php
@@ -172,6 +172,7 @@ class AuthenticationRequestTest extends \MediaWikiTestCase {
                                'type' => 'string',
                                'label' => $msg,
                                'help' => $msg,
+                               'sensitive' => true,
                        ],
                        'string3' => [
                                'type' => 'string',
@@ -206,6 +207,7 @@ class AuthenticationRequestTest extends \MediaWikiTestCase {
                $expect = $req1->getFieldInfo();
                foreach ( $expect as $name => &$options ) {
                        $options['optional'] = !empty( $options['optional'] );
+                       $options['sensitive'] = !empty( $options['sensitive'] );
                }
                unset( $options );
                $this->assertEquals( $expect, $fields );
@@ -225,8 +227,10 @@ class AuthenticationRequestTest extends \MediaWikiTestCase {
 
                $fields = AuthenticationRequest::mergeFieldInfo( [ $req1, $req2 ] );
                $expect += $req2->getFieldInfo();
+               $expect['string1']['sensitive'] = true;
                $expect['string2']['optional'] = false;
                $expect['string3']['optional'] = false;
+               $expect['string3']['sensitive'] = false;
                $expect['select']['options']['bar'] = $msg;
                $this->assertEquals( $expect, $fields );
 
@@ -237,6 +241,7 @@ class AuthenticationRequestTest extends \MediaWikiTestCase {
                $fields = AuthenticationRequest::mergeFieldInfo( [ $req1, $req2 ] );
                $expect += $req2->getFieldInfo();
                $expect['string1']['optional'] = false;
+               $expect['string1']['sensitive'] = true;
                $expect['string3']['optional'] = false;
                $expect['select']['optional'] = false;
                $expect['select']['options']['bar'] = $msg;
@@ -246,7 +251,11 @@ class AuthenticationRequestTest extends \MediaWikiTestCase {
 
                $fields = AuthenticationRequest::mergeFieldInfo( [ $req1, $req2 ] );
                $expect = $req1->getFieldInfo() + $req2->getFieldInfo();
+               foreach ( $expect as $name => &$options ) {
+                       $options['sensitive'] = !empty( $options['sensitive'] );
+               }
                $expect['string1']['optional'] = false;
+               $expect['string1']['sensitive'] = true;
                $expect['string2']['optional'] = true;
                $expect['string3']['optional'] = true;
                $expect['select']['optional'] = false;

diff --git a/tests/phpunit/includes/auth/AuthenticationRequestTestCase.php b/tests/phpunit/includes/auth/AuthenticationRequestTestCase.php
index aa0e3c7..b5c8a36 100644 (file)
--- a/tests/phpunit/includes/auth/AuthenticationRequestTestCase.php
+++ b/tests/phpunit/includes/auth/AuthenticationRequestTestCase.php
@@ -32,6 +32,13 @@ abstract class AuthenticationRequestTestCase extends \MediaWikiTestCase {
                        if ( isset( $data['image'] ) ) {
                                $this->assertType( 'string', $data['image'], "Field $field, image" );
                        }
+                       if ( isset( $data['sensitive'] ) ) {
+                               $this->assertType( 'bool', $data['sensitive'], "Field $field, sensitive" );
+                       }
+                       if ( $data['type'] === 'password' ) {
+                               $this->assertTrue( !empty( $data['sensitive'] ),
+                                       "Field $field, password field must be sensitive" );
+                       }
 
                        switch ( $data['type'] ) {
                                case 'string':