From 83c98e7cd8cc530cb87ec86a0f55e099eea64a46 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 18 Aug 2016 13:03:10 -0400 Subject: [PATCH] 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 --- includes/api/ApiAuthManagerHelper.php | 1 + includes/auth/AuthenticationRequest.php | 5 +++++ includes/auth/PasswordAuthenticationRequest.php | 2 ++ .../phpunit/includes/auth/AuthenticationRequestTest.php | 9 +++++++++ .../includes/auth/AuthenticationRequestTestCase.php | 7 +++++++ 5 files changed, 24 insertions(+) diff --git a/includes/api/ApiAuthManagerHelper.php b/includes/api/ApiAuthManagerHelper.php index c970f932d7..d330862e79 100644 --- 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 f6f949ec7c..2474b8b830 100644 --- 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 187c29ae9f..8550f3e211 100644 --- 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 a7df221700..7d2ba8d749 100644 --- 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 aa0e3c70f5..b5c8a36cd1 100644 --- 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': -- 2.20.1