with WebAuthn 2FA turned on.

The bug is in that extension. Its MediaWiki\Extension\WebAuthn\Auth\WebAuthnAuthenticationRequest::getFieldInfo() method is not including the required 'label' property in the field definitions it returns.

The extension should also add PHPUnit tests based on core's MediaWiki\Auth\AuthenticationRequestTestCase to catch things like this.

I note the documentation of AuthenticationRequest::getFieldInfo() doesn't really say whether (any of) these keys are required, optional etc (while this isn't an excuse, it doesn't help)

	/**
	 * Fetch input field info
	 *
	 * The field info is an associative array mapping field names to info
	 * arrays. The info arrays have the following keys:
	 *  - type: (string) Type of input. Types and equivalent HTML widgets are:
	 *     - string: <input type="text">
	 *     - password: <input type="password">
	 *     - select: <select>
	 *     - checkbox: <input type="checkbox">
	 *     - multiselect: More a grid of checkboxes than <select multi>
	 *     - button: <input type="submit"> (uses 'label' as button text)
	 *     - hidden: Not visible to the user, but needs to be preserved for the next request
	 *     - null: No widget, just display the 'label' message.
	 *  - options: (array) Maps option values to Messages for the
	 *      'select' and 'multiselect' types.
	 *  - value: (string) Value (for 'null' and 'hidden') or default value (for other types).
	 *  - 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.
	 *  - skippable: (bool) If set and truthy, the client is free to hide this
	 *      field from the user to streamline the workflow. If all fields are
	 *      skippable (except possibly a single button), no user interaction is
	 *      required at all.
	 *
	 * All AuthenticationRequests are populated from the same data, so most of the time you'll
	 * want to prefix fields names with something unique to the extension/provider (although
	 * in some cases sharing the field with other requests is the right thing to do, e.g. for
	 * a 'password' field).
	 *
	 * @return array As above
	 * @phan-return array<string,array{type:string,options?:array,value?:string,label:Message,help:Message,optional?:bool,sensitive?:bool,skippable?:bool}>
	 */
	abstract public function getFieldInfo();

Shall I file a separate task to add more annotation?

Go ahead if you'd like. FYI, reading from AuthenticationRequestTestCase,

• 'type' is required.
• 'options' is required if 'type' is "select" or "multiselect".
• 'value' is optional.
• 'label' is required.
• 'help' is required unless 'type' is "null".
• 'optional' is optional.
• 'sensitive' must be set true if 'type' is "password", otherwise it's optional.
• 'skippable' is optional.

The phan-return field does specify what's optional, granted that's not exactly user-friendly.
FWIW value is required when type is null or hidden.

Annotation with [required] or [optional] or something like that would probably work.

In T247710#5972159, @Anomie wrote:

with WebAuthn 2FA turned on.

The bug is in that extension. Its MediaWiki\Extension\WebAuthn\Auth\WebAuthnAuthenticationRequest::getFieldInfo() method is not including the required 'label' property in the field definitions it returns.

The extension should also add PHPUnit tests based on core's MediaWiki\Auth\AuthenticationRequestTestCase to catch things like this.

'help' too

It seems odd/pointless to be defining messages when 'type' is set to 'hidden' - though, does this actually do anything?

If the messages aren't being used, do I define empty ones and get them marked as not for translation etc? Or is there something else I'm missing?

'hidden' fields are only hidden on the web interface. They do show up in the API.

Still seen on 1.36.0-wmf.35. Only a couple times a week. Suggests it's not something that happens when users do things correctly, but it does mean that when users use the API incorrectly, that it causes an increase in server-side errors (5xx) instead of client warnings (2xx/4xx), and the noise in production error monitoring and deployment checks as a result.

Tagging per mw:Maintainers entry for AuthManager and OAuth.

In T247710#6962833, @Krinkle wrote:

Only a couple times a week. Suggests it's not something that happens when users do things correctly, but it does mean that when users use the API incorrectly, that it causes an increase in server-side errors (5xx) instead of client warnings (2xx/4xx)

I don't think so, this will break an otherwise successful login attempt if it happens via WebAuthn and via the API (which is probably a pretty infrequent combination).

Seem 'label' (something else as well) should be returned by getFieldInfo() method, I will check if it will not break something. -> so the annotation (from the task https://phabricator.wikimedia.org/T247886) to abstract method getFieldInfo() is to be changed as well. Will that be ok if the changes from https://phabricator.wikimedia.org/T247886 will be made here, together with the needed changes ? Thanks!

Label and help are always required. The annotation is correct, and the phpdoc description is kind of correct (it says "if set" for everything that's optional) but could be more obvious.

Change 692698 had a related patch set uploaded (by Art.tsymbar; author: arttsymbar):

[mediawiki/extensions/WebAuthn@master] Fix for an argument 3 passed to ApiAuthManagerHelper::formatMessage(). Adding label and help properties to fields information.

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

Hi, @Tgr ! Could you please describe the easiest way to reproduce the issue locally (if possible) - I need it to properly describe fields props: (label, help). Thanks!

I don't think it's worth the effort, just for adding some missing i18n messages. But if you want to reproduce them anyway, you need to use some AuthManager APIs (those are action=clientlogin, action=createaccount, action=changeauthenticationdata, action=removeauthenticationdata, action=query&meta=authmanagerinfo - I think WebAuthn only supports login, and authmanagerinfo mostly needs to be used in combination with the others, so you'd need to use clientlogin):

1. set up a user account with WebAuthn 2FA
2. open Special:ApiSandbox while logged out
3. select clientlogin in the Action field
4. click action=clientlogin on the left pane
5. auto-fill the logintoken field
6. at the bottom add username and password parameters and fill them out
7. submit the request

The response should contain information about the next request that is expected; that should be a WebAuthnAuthenticationRequest.

Change 692698 merged by jenkins-bot:

[mediawiki/extensions/WebAuthn@master] Fix for an argument 3 passed to ApiAuthManagerHelper::formatMessage(). Adding label and help properties to fields information.

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