jenkins-bot has submitted this change and it was merged.
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.
Also make CreationReasonAuthenticationRequest non-required so that
the list of required form fields is more in sync with that of
pre-AuthManager code.
Bug: T85853
Change-Id: I9d33bd22295758cc532a260b1848616b41d94f12
---
M includes/auth/AuthenticationRequest.php
M includes/auth/CreationReasonAuthenticationRequest.php
M tests/phpunit/includes/auth/AuthenticationRequestTest.php
3 files changed, 35 insertions(+), 9 deletions(-)
Approvals:
Anomie: Looks good to me, approved
Bartosz Dziewoński: Looks good to me, but someone else must approve
jenkins-bot: Verified
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/includes/auth/CreationReasonAuthenticationRequest.php
b/includes/auth/CreationReasonAuthenticationRequest.php
index 1711aec..146470e 100644
--- a/includes/auth/CreationReasonAuthenticationRequest.php
+++ b/includes/auth/CreationReasonAuthenticationRequest.php
@@ -10,6 +10,8 @@
/** @var string Account creation reason (only used when creating for
someone else) */
public $reason;
+ public $required = self::OPTIONAL;
+
public function getFieldInfo() {
return [
'reason' => [
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: merged
Gerrit-Change-Id: I9d33bd22295758cc532a260b1848616b41d94f12
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
