Gergő Tisza has uploaded a new change for review. https://gerrit.wikimedia.org/r/291678
Change subject: Fix required field calculation in AuthenticationRequest ...................................................................... Fix required field calculation in AuthenticationRequest Instead of only flagging fields which are required by a request needed by all primairy providers, it should be enough if all requests needed by some primary provider require that field. Bug: T85853 Change-Id: I9d33bd22295758cc532a260b1848616b41d94f12 --- M includes/auth/AuthenticationRequest.php M tests/phpunit/includes/auth/AuthenticationRequestTest.php 2 files changed, 33 insertions(+), 9 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/78/291678/1 diff --git a/includes/auth/AuthenticationRequest.php b/includes/auth/AuthenticationRequest.php index 3c19b87..f314849 100644 --- a/includes/auth/AuthenticationRequest.php +++ b/includes/auth/AuthenticationRequest.php @@ -281,6 +281,21 @@ public static function mergeFieldInfo( array $reqs ) { $merged = []; + // fields that are required by some primary providers but not others are not actually required + $primaryRequests = array_filter( $reqs, function ( $req ) { + return $req->required === AuthenticationRequest::PRIMARY_REQUIRED; + } ); + $sharedRequiredPrimaryFields = array_reduce( $primaryRequests, function ( $shared, $req ) { + $required = array_keys( array_filter( $req->getFieldInfo(), function ( $options ) { + return empty( $options['optional'] ); + } ) ); + if ( $shared === null ) { + return $required; + } else { + return array_intersect( $shared, $required ); + } + }, null ); + foreach ( $reqs as $req ) { $info = $req->getFieldInfo(); if ( !$info ) { @@ -288,8 +303,14 @@ } foreach ( $info as $name => $options ) { - if ( $req->required !== self::REQUIRED ) { + if ( // If the request isn't required, its fields aren't required either. + $req->required === self::OPTIONAL + // If there is a primary not requiring this field, no matter how many others do, + // authentication can proceed without it. + || $req->required === self::PRIMARY_REQUIRED + && !in_array( $name, $sharedRequiredPrimaryFields, true ) + ) { $options['optional'] = true; } else { $options['optional'] = !empty( $options['optional'] ); diff --git a/tests/phpunit/includes/auth/AuthenticationRequestTest.php b/tests/phpunit/includes/auth/AuthenticationRequestTest.php index 84a0ea6..cac031c 100644 --- a/tests/phpunit/includes/auth/AuthenticationRequestTest.php +++ b/tests/phpunit/includes/auth/AuthenticationRequestTest.php @@ -243,14 +243,6 @@ $req1->required = AuthenticationRequest::PRIMARY_REQUIRED; - $fields = AuthenticationRequest::mergeFieldInfo( [ $req1 ] ); - $expect = $req1->getFieldInfo(); - foreach ( $expect as $name => &$options ) { - $options['optional'] = true; - } - unset( $options ); - $this->assertEquals( $expect, $fields ); - $fields = AuthenticationRequest::mergeFieldInfo( [ $req1, $req2 ] ); $expect += $req2->getFieldInfo(); $expect['string1']['optional'] = false; @@ -258,6 +250,17 @@ $expect['select']['optional'] = false; $expect['select']['options']['bar'] = $msg; $this->assertEquals( $expect, $fields ); + + $req2->required = AuthenticationRequest::PRIMARY_REQUIRED; + + $fields = AuthenticationRequest::mergeFieldInfo( [ $req1, $req2 ] ); + $expect = $req1->getFieldInfo() + $req2->getFieldInfo(); + $expect['string1']['optional'] = false; + $expect['string2']['optional'] = true; + $expect['string3']['optional'] = true; + $expect['select']['optional'] = false; + $expect['select']['options']['bar'] = $msg; + $this->assertEquals( $expect, $fields ); } /** -- To view, visit https://gerrit.wikimedia.org/r/291678 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9d33bd22295758cc532a260b1848616b41d94f12 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