Page MenuHomePhabricator

Disabling PHP session handling breaks OAuth requests utilising CSRF tokens
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Set $wgPHPSessionHandling = 'disable'; in LocalSettings.php
  • Enable the OAuth extension
  • Create a new OAuth (2.0) consumer, not owner-only, not confidential, and approve it
  • Proceed through the OAuth authorization_code flow and obtain an access token
  • Make a request to api.php?action=query&meta=tokens&type=csrf with an Authorization: Bearer <access_token> header to obtain a CSRF token
  • Make a request to api.php?action=edit with the same Authorization header, and the CSRF token passed in the body as token

What happens?:

An error is returned which states "Invalid CSRF token" and the edit doesn't occur.

What should have happened instead?:

The edit should be successfully made.

Software version (skip for WMF-hosted wikis like Wikipedia): 1.39.5

Other information (browser name/version, screenshots, etc.):

The problem also occurs on the api.php?action=checktoken&type=csrf route (which simply returns "invalid"), and presumably any other authenticated route which requires a CSRF token.

Event Timeline

I note that in T302623#8566372, it is clear that there is an intention to phase out PHP session handling. This seems like a hard blocker right now.

JaydenKieran claimed this task.

This issue was fixed in T403519 when PHP session handling was disabled in WMF production before this issue was fixed, causing it to break OAuth apps.

Thanks for noticing! Too bad we missed this task at the time.

As you say this is now fixed in master. We should probably backport to supported branches and add a note to the docs that this setting doesn't really work in older versions.

Oops, sorry we missed this bug report @JaydenKieran. It certainly would have saved me a headache if I had known about it before T403519. As you can see we now have a better triage mechanism with the MediaWiki-Platform-Team tag, so this shouldn't happen again.

Backporting the fix is a bit tricky. The patch https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1186595 may be unreliable without https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1181220, which was also a bumpy ride, requiring at least one followup in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1181796 that I remember. I'm not sure if I want to spend time testing them on the older versions, or risk breaking the releases.

I would say we should just note in the documentation that $wgPHPSessionHandling = 'disable' was buggy before MW 1.45 and that it was incompatible with at least the OAuth extension.

Tgr reassigned this task from JaydenKieran to matmarex.

Fair point. Updated the docs.