A preliminary investigation (T326759) has found that the OATHAuth and WebAuthn extensions may be affected by IP Masking
Description
Details
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
In Progress | • Niharika | T324492 Temporary accounts - MVP | |||
Open | None | T326816 Update features for IP Masking | |||
Open | None | T355377 Update MediaWiki Platform team owned products for IP masking | |||
Resolved | pmiazga | T326925 Prepare OATHAuth and WebAuthn extensions for IP Masking |
Event Timeline
The WebAuthn extension requires one change to fully support IPMasking. The only place that required investigation is checkPermissions method - https://gerrit.wikimedia.org/g/mediawiki/extensions/WebAuthn/+/122deaa8a99b868c08f9fa1e5e5d4ab0a4387bf3/src/Api/WebAuthn.php#112
protected function checkPermissions( string $func ): void { $registered = $this->getRegisteredFunctions(); $functionConfig = $registered[$func]; $mustBeLoggedIn = $functionConfig['mustBeLoggedIn']; if ( $mustBeLoggedIn === true ) { $user = $this->getUser(); if ( !$user->isRegistered() ) { $this->dieWithError( [ 'apierror-mustbeloggedin', $this->msg( 'action-oathauth-enable' ) ] ); } } $funcPermissions = $functionConfig['permissions']; if ( $funcPermissions ) { $this->checkUserRightsAny( $funcPermissions ); } }
We need to change from if ( !$user->isRegistered() ) { to if ( !$user->isNamed() ) { to throw present this error to both anon and temp users.
Change 991081 had a related patch set uploaded (by Pmiazga; author: Pmiazga):
[mediawiki/extensions/WebAuthn@master] Temp users require to create account in order to use WebAuthn
Change 991084 had a related patch set uploaded (by Pmiazga; author: Pmiazga):
[mediawiki/extensions/OATHAuth@master] OATH validation is available only to named users
In the OATHAuth extension we are required to change two places:
a) https://gerrit.wikimedia.org/g/mediawiki/extensions/OATHAuth/+/320d0c5d17d1fb902059ced24ccb6dc16bb33864/src/Api/Module/ApiOATHValidate.php#63
b) https://gerrit.wikimedia.org/g/mediawiki/extensions/OATHAuth/+/320d0c5d17d1fb902059ced24ccb6dc16bb33864/src/Api/Module/ApiQueryOATH.php#90
In both places we check if ( !$user->isAnon() ) -> but OATH is available only for named users -> therefore those checks should be rewritten to
if ( $user->isNamed() ).
The OATHAuth extension uses $user->name() in multiple places but does not display it to other users. It is used only for server logging, creating cache keys or selecting the correct gender form when displaying messages.
Change 991084 merged by jenkins-bot:
[mediawiki/extensions/OATHAuth@master] OATH validation is available only to named users
Change 991081 merged by jenkins-bot:
[mediawiki/extensions/WebAuthn@master] Temp users require to create account in order to use WebAuthn
In OATHAuth, the current permission configuration (in extension.json) adds oathauth-enable to user, that should probably be named.
I can find nothing else with a quick grep. disableOATHAuthForUser.php could maybe complain if the user parameter is a temp user, but then it's pretty unlikely that anyone would invoke it with a temp user.
Thanks, that's a great find. I'll look into it as I'm wondering if changing this can break things - I mean mess up exiting permissions as we change from user to named.
Sorry, I double-checked and temporary users are not actually part of user (filed T355741: 'named' user group is pointless about that). So nothing left to do here.