Last modified: 2012-11-25 00:59:30 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T39643, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 37643 - Session may not be started for non-logged-in API edits, causing captcha to fail
Session may not be started for non-logged-in API edits, causing captcha to fail
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
ConfirmEdit (CAPTCHA extension) (Other open bugs)
master
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-need-review
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-16 02:07 UTC by Brad Jorsch
Modified: 2012-11-25 00:59 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Brad Jorsch 2012-06-16 02:07:19 UTC
This extension may use the session to store the captcha data, but does not ensure that the session has actually been started. Setup.php will normally take care of this, because in most cases the user will be logged in or at least have done other things that would cause the session cookie to be set.

But if the edit is being performed by a non-logged-in user who hasn't gotten a session cookie yet, Setup.php will not start the session. The captcha data will thus never be saved, so the user cannot ever pass the captcha.

Gerrit changeset coming shortly.
Comment 1 Brad Jorsch 2012-06-16 02:28:40 UTC
Gerrit change #11722
Comment 2 Platonides 2012-08-21 14:13:37 UTC
How is it possible? Anyone trying to edit will get a session.
Comment 3 Brad Jorsch 2012-08-21 15:47:04 UTC
(In reply to comment #2)
> How is it possible? Anyone trying to edit will get a session.

As described: If you're not logged in, you don't get a session cookie until you try to perform certain actions. And the steps required to make an edit via the API do not include any session-creating actions.

I discovered this bug by doing exactly that while testing API captcha support.

At any rate, the gerrit change doesn't hurt anything if there is already a session active. It just starts a session if there isn't one, which really should be done by anything that is trying to use the session.
Comment 4 Chris Steipp 2012-09-27 20:26:32 UTC
Brad, can you add some details about the use case that you're seeing fail?

It's probably fine, but I would hate to start any sessions that aren't needed, since that affects caching. But if there are a few cases where we are missing sessions, then this is probably a good way to set it.
Comment 5 Brad Jorsch 2012-09-28 00:37:40 UTC
Ok, step by step.

1. Make sure ConfirmEdit is enabled and that it will be using CaptchaSessionStore.
2. Clear your cookies, if necessary.
3. Make an API request to get an edit token, e.g. http://localhost/w/api.php?action=query&titles=Sandbox&prop=info&intoken=edit or http://localhost/w/api.php?action=tokens. Note you will not receive any session cookie from either of these requests.
4. Make an API request to edit a page, which includes at least one new link. For example, http://localhost/w/api.php?action=edit&token=%2B%5C&summary=Test&text=http://www.example.com/&title=Sandbox. You should get back a response with captcha information. You will receive no cookie, even though ConfirmEdit tried to save data in the session.
5. Make another API request, supplying the correct captcha information. For example, if MathCaptcha gave back "3 + 4 = ", you might send http://localhost/w/api.php?action=edit&token=%2B%5C&summary=Test&text=http://www.example.com/&title=Sandbox&captchaid=1234567890&captchaword=7. You will get back another response with a different captcha request, and still no cookie.

When using my patch, in step 4 you ''will'' receive a session cookie, so the edit will be able to succeed in step 5. In the much more common case that you already have a session cookie heading into step 4 (e.g. you're logged in), the "session_id() === ''" test in my patchet will be false so nothing much will change.
Comment 6 db [inactive,noenotif] 2012-11-24 21:30:48 UTC
(In reply to comment #1)
> Gerrit change #11722

Status Merged, bug resolved?
Comment 7 Brad Jorsch 2012-11-25 00:59:30 UTC
(In reply to comment #6)
> (In reply to comment #1)
> > Gerrit change #11722
> 
> Status Merged, bug resolved?

Yes.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links