Separate out temporary users from 'assert' param in ApiMain
Closed, DeclinedPublic
Actions

Assigned To

None

Authored By

	Tchanders
	Nov 20 2023, 11:35 AM

Description

Background

ApiMain::checkAsserts checks that the calling user is the right type of user. It accepts one of anon, user or bot.

Currently, temporary accounts are considered to be in the user category:

switch ( $params['assert'] ) {
	case 'anon':
		if ( $user->isRegistered() ) {
			$this->dieWithError( 'apierror-assertanonfailed' );
		}
		break;
	case 'user':
		if ( !$user->isRegistered() ) {
			$this->dieWithError( 'apierror-assertuserfailed' );
		}
		break;
	case 'bot':
		if ( !$this->getAuthority()->isAllowed( 'bot' ) ) {
			$this->dieWithError( 'apierror-assertbotfailed' );
		}
		break;
}

This will break the assumption by any callers that assert=user filters for users who have filled in a registration form. However, if we consider temporary users as separate, that will break assumption that assert=user filters for anyone with a row in the user table.

We discussed how to handle cases like this in T337103: Decide a standard approach for classifying temporary, IP and registered users and decided that we would give temporary users their own category.

What needs doing

Since only a single value can be given, it seems sensible to allow temporary users to be filtered alongside either user type, as well as on their own. In which case we should add the following to the assert param:

Value	Checks for
temp	User::isTemp()
anonOrTemp	!User::isNamed()
userOrTemp	User::isRegistered()

...and change the check for the user value to User::isNamed.

Acceptance criteria

The new values temp, anonOrTemp and userOrTemp are added
The check for user is updated
The changes are recorded in the release notes

Related Objects
Search...

Status	Assigned	Task
		Restricted Task
Resolved	kostajh	T294511 2021 Security Team wikireplicas audit
Declined	None	T284948 Raw IPs of logged-out users disclosed in wiki-replicas
In Progress	Niharika	T324492 Temporary accounts - MVP
Open	None	T326816 [Epic] Update features for temporary accounts
Open	None	T331637 Update features in MediaWiki core for IP Masking
Resolved	Dreamy_Jazz	T341228 Update action API to handle temporary users
Declined	None	T351632 Separate out temporary users from 'assert' param in ApiMain

Event Timeline

Tchanders created this task.Nov 20 2023, 11:35 AM

According to https://www.mediawiki.org/wiki/API:Assert, a use-case of this parameter is to check whether you are logged in, are logged into an account that has the bot right, etc.

At the very least we should do this, since without it a temporary account would pass the check, while the caller might be checking they're logged in:

The check for user is updated

I'm not quite sure what the use-case for assert=anon is, but its existence implies that assert=temp would be useful.

I'm comfortable with removing anonOrTemp and userOrTemp from the acceptance criteria, if they seem unlikely to be useful.

Tchanders mentioned this in T341228: Update action API to handle temporary users.Nov 20 2023, 12:16 PM

...and change the check for the user value to User::isNamed.

This would be a breaking change for the API consumers, e.g. it would require updating this in VisualEditor: https://gerrit.wikimedia.org/g/mediawiki/extensions/VisualEditor/+/3e96e895b38f26a5271d972aabe7fc1740cc9a13/modules/ve-mw/init/_targets/ve.init.mw.Article_target.js#1529. I would suggest keeping the current meaning of assert=user.

I'm also unsure if we need to add assert=temp and assert=named, because assertuser=... (with the current username) can already be used to achieve the same thing. The VisualEditor code above does it.

For context, assert=user is basically a fallback check to ensure that your login session hasn't expired, and that you didn't mess up the cookie handling in your code, so that your IP address won't be exposed when editing. assert=bot is similarly a fallback to ensure that you're logged in as the right account on the right wiki. Apparently this used to be documented better: https://www.mediawiki.org/w/index.php?title=Extension:AssertEdit&oldid=889947#Rationale but it got lost when that extension was merged into MediaWiki core many years ago.

For context, assert=user is basically a fallback check to ensure that your login session hasn't expired, and that you didn't mess up the cookie handling in your code, so that your IP address won't be exposed when editing.

Thanks, this is really helpful. I had assumed that it would be a problem if you had unknowingly created a temporary account already but were meaning to do something as a particular user. If that was a likely use-case, it seemed to me that keeping the assert param as it is would be a breaking change... But if it's only really for preventing IP leakage, and users would use assertuser= for the use-case I described here, then it seems there's nothing to do here except maybe update some documentation.

Declining, and here's a patch for updating the documentation: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/978055

I also documented the other parameters and the user cases at https://www.mediawiki.org/wiki/API:Assert:

Thanks @matmarex

For context, assert=user is basically a fallback check to ensure that your login session hasn't expired, and that you didn't mess up the cookie handling in your code,

Correct.

so that your IP address won't be exposed when editing.

Not necessarily. It could as well be "so that you don't wind up accidentally making edits while not logged into a registered account".

Apparently this used to be documented better: https://www.mediawiki.org/w/index.php?title=Extension:AssertEdit&oldid=889947#Rationale but it got lost when that extension was merged into MediaWiki core many years ago.

I read that as supporting assert=user rejecting temporary names.

I'd recommend you reopen this task and implement it as proposed. If someone really objects to the supposed change in the meaning of "user", deprecate "user" in favor of "registered" or the like.

Separate out temporary users from 'assert' param in ApiMainClosed, DeclinedPublicActions