Page MenuHomePhabricator

Fix validation for scopes parameter in create client endpoint
Closed, ResolvedPublic

Description

Event Timeline

Commenting out validation of specific values in the "scopes" parameter validation on my local allowed creation of oauth clients with arbitrary scope values (oarc_grants column in table oauth_registered_consumer):

			'scopes' => [
				self::PARAM_SOURCE => 'post',
/*
				ParamValidator::PARAM_TYPE => [
					'basic',
					'mwoauth-authonly',
					'mwoauth-authonlyprivate'
				],
*/
				ParamValidator::PARAM_REQUIRED => true,
				ParamValidator::PARAM_ISMULTI => true
			]

Here's the command I used to create a test client locally (as this was just against my local, there's no security risk in posting the cookie value):

curl -H "Accept: application/json" --cookie "default_session=fhbm9rbaou605bt72kam4rpgjfqdd3an;defaultToken=8cad97b3037b4aa511f3eb3fce275ca0;defaultUserID=1;defaultUserName=Admin" -X POST -F name=TestFromCurl322 -F description=foo -F [email protected] -F is_confidential=1 -F grant_types=authorization_code -F scopes=editprotected\|createeditmovepage -F callback_url=http://default.web.mw.localhost:8080/mediawiki/ http://default.web.mw.localhost:8080/mediawiki/rest.php/oauth2/client

Manual testing shows that including an invalid scope with the parameter validation removed strips the invalid scope but proceeds with client creation. So we don't allow creation of garbage clients, but this isn't terribly helpful to people who try to create a client with certain scopes then silently receive a client with different ones.

Maybe we should modify RequestClient::adjustScopes() to dynamically generate the list of allowed scopes based on ScopeRepository::getAllowedScopes()? That'd let us reject invalid scopes at the REST parameter validation level.

Note that the "basic" scope, which we hard-code forced inclusion of in RequestClient::adjustScopes(), IS included in the allowed scopes from ScopeRepository::getAllowedScopes(), at least for my local. Presumably, that's the same for production wikis and just about every wiki that doesn't have a need to do something really unusual.

Change 631534 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/extensions/OAuth@master] Dynamic validation for the OAuth client creation REST endpoint scopes param

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

Change 631534 merged by jenkins-bot:
[mediawiki/extensions/OAuth@master] Dynamic validation for the OAuth client creation REST endpoint scopes param

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

Verified on beta. Thanks, Bill!

Verified that creating a read-only app works, but I'm still getting the error for read/write apps