Be explicit about not needing a real token.

REST handlers that delegate to action API modules need to supply a
known-good token when the REST endpoint has been accessed in a way that
is safe against CSRF. This was done by returning null from getToken(),
which seems surprising and brittle. Having a method called needsToken()
makes the code easier to understand.

Follow-Up-To: If41749722b28c8c0e9898b3d3e7937167653fb10
Change-Id: I04148a7e000c3c73241bc20fe1582880b16b0056
This commit is contained in:
daniel 2023-11-21 19:08:04 +01:00 committed by Aaron Schulz
parent 95278bbfa1
commit 5182ccbf8e
3 changed files with 19 additions and 3 deletions

View file

@ -73,7 +73,9 @@ class CreationHandler extends EditHandler {
);
}
$token = $this->getToken() ?? $this->getUser()->getEditToken();
// Use a known good CSRF token if a token is not needed because we are
// using a method of authentication that protects against CSRF, like OAuth.
$token = $this->needsToken() ? $this->getToken() : $this->getUser()->getEditToken();
$params = [
'action' => 'edit',

View file

@ -105,7 +105,9 @@ class UpdateHandler extends EditHandler {
);
}
$token = $this->getToken() ?? $this->getUser()->getEditToken();
// Use a known good CSRF token if a token is not needed because we are
// using a method of authentication that protects against CSRF, like OAuth.
$token = $this->needsToken() ? $this->getToken() : $this->getUser()->getEditToken();
$params = [
'action' => 'edit',

View file

@ -53,7 +53,7 @@ trait TokenAwareHandlerTrait {
throw new LogicException( 'This trait must be used on handler classes.' );
}
if ( $this->getSession()->getProvider()->safeAgainstCsrf() ) {
if ( !$this->needsToken() ) {
return null;
}
@ -61,6 +61,18 @@ trait TokenAwareHandlerTrait {
return $body['token'] ?? '';
}
/**
* Determines whether a CSRF token is needed.
*
* Returns false if the request has been authenticated in a way that
* protects against CSRF, such as OAuth.
*
* @return bool
*/
protected function needsToken(): bool {
return !$this->getSession()->getProvider()->safeAgainstCsrf();
}
/**
* Returns a standard error message to use when the given CSRF token is invalid.
* In the future, this trait may also provide a method for checking the token.