wikibase-edit icon indicating copy to clipboard operation
wikibase-edit copied to clipboard

Anonymous edits seem to be failing

Open diegodlh opened this issue 3 years ago • 13 comments

  • wikibase-edit version: 4.11.8
  • Environment: browser version

Editing Wikidata entities anonymously seems to be failing with "badtoken: Invalid CSRF token". For example, I tried this:

const wdEdit = require('wikibase-edit')({ instance: 'https://www.wikidata.org' })
wdEdit.claim.create(
  {
    id: "Q15397819",
    property: "P2860",
    value: "Q13406268"
  }, {
    anonymous: true
  }
)

POST request is sent with token parameter set to +\ as per https://phabricator.wikimedia.org/T40417), but server responds with error badtoken: Invalid CSRF token.

Is this a known bug? Maybe they changed something in the API?

diegodlh avatar Mar 09 '21 16:03 diegodlh

It's not a known bug to me, but I'm not using the anonymous mode. Maybe @GiulioC who requested it in #51 knows something about it?

maxlath avatar Mar 09 '21 19:03 maxlath

It's been some time since I worked on it, however looking at my old code I can confirm the requests were made the same way as the one showed by @diegodlh:

const wbEdit = require('wikibase-edit')({ instance: 'https://www.wikidata.org/w/api.php' });

wbEdit.entity.create({
    descriptions: { it: object.description},
    labels: { it: object.label},
    claims: object.claims
}, {
    anonymous: true
});

The only difference being the wikidata instance url, maybe you need to check on that one. Hope this helps.

GiulioC avatar Mar 10 '21 09:03 GiulioC

I can't reproduce the error: I was able to create a claim on wikidata.org with anonymous=true with the same code as @diegodlh

maxlath avatar Mar 10 '21 12:03 maxlath

Thank you both for your help!

I'm developing a plugin for Zotero, which runs on Firefox runtime environment. The problem seems to occur because cookies are being dragged from previous requests. This happens even if Zotero/xulrunner is restarted, probably because cookies are being retrieved from the profile storage. Removing the file where cookies are stored solves the problem.

These logins in the order presented result in the following outcomes:

  1. anonymous login on a clean profile (i.e., no previous cookies): login successful
  2. regular user/password (account 1) login: login succesful
  3. regular user/password (account 2) login: login succesful (i.e., switching accounts does work)
  4. anonymous login again: login fails with "badtoken: Invalid CSRF token"
  5. regular user/password login: login succesful
  6. bot user/password login: login succesful
  7. regular or bot user/password login: login fails with "Cannot log in when using MediaWiki\Session\BotPasswordSessionProvider sessions"

Cookies being sent in the request to https://www.wikidata.org/w/api.php?action=login&format=json are the following:

centralauth_Session
centralauth_ss0-Token
centralauth_ss0-User
centralauth_Token
centralauth_User
GeoIP
loginnotify_prevlogins
ss0-centralauth_Session
ss0-wikidatawikiSession
wikidatawiki_BPsession
wikidatawikiSession
wikidatawikiss0-UserID
wikidatawikiss0-UserName
wikidatawikiUserID
wikidatawikiUserName
WMF-Last-Access
WMF-Last-Access-Global

I don't think the workaround I had to do, described in #62, is related; but I mention it anyway.

diegodlh avatar Mar 10 '21 13:03 diegodlh

I understand cookies are not refreshed in step 4 above (but yes in step 3) because wbeditentity request is sent directly (without sending a login request first) if anonymous=true. (BTW, I wonder whether #43 may be somehow related).

Do you think it be possible to send an anonymous login request that would return the +\ token (instead of hard-coding it for anonymous edits) and refresh the cookies?

Alternatively, I tried passing credentials: 'omit' to fetch in request.js (here), in an attempt to prevent cookies from being sent (a parameter that, I thought, may be passed to request in post.js if anonymous=true here), but cookies are sent anyway. I wonder if it may be related to what it says here that:

due to limitations of XMLHttpRequest, using credentials: 'omit' is not respected for same domains in browsers where this polyfill is active. Cookies will always be sent to same domains in older browsers.

Zotero is based on Firefox 60 ESR (an "old browser"?), but I'm afraid I don't understand enough about same-origin policies to judge this.

All in all, I guess this login-related bug will be affected by resolution of #50.

diegodlh avatar Mar 10 '21 14:03 diegodlh

I got stuck trying to reproduce a browser environment (CORS errors): any chance you have an easy way to setup an environment in which I could reproduce the error?

maxlath avatar Mar 10 '21 16:03 maxlath

Hi, Max. Yeah, same thing happened to me today when trying to see if I could reproduce the issue on latest version of Firefox (since Zotero is based on an old one). I think the way to go would be making a simple browser extension. I'll try to make some time to make one these days. I'll post it here if I do. Thanks!

diegodlh avatar Mar 10 '21 17:03 diegodlh

I managed to make a simple browser extension, using wikibase-edit in a background script so it doesn't complain about CORS. I included usage instructions in the Readme file.

However, I had to make some changes in the wikibase-edit login.js' getSessionCookies function because Cookie is a Forbidden header name and thus is not available to github/fetch xhr.getAllResponseHeaders() here (cross-fetch depends on github/fetch). Therefore, your code was failing here and here.

Anyway, github/fetch fails to set "cookie" request header later on. As per https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name "A forbidden header name (...) cannot be modified programmatically (...) because the user agent [browser] retains full control over them."

These changes I made (specified in the patches directory) are applied automatically after running npm install.

It is worth saying that these changes were not necessary in Zotero/Xulrunner. For some reason, 'set-cookie' headers are available to xhr.getAllRequestHeaders() there. In fact, that's the reason why I had to make the other patch described in #62.

I tried logins 1 to 7 in the order mentioned in my previous message, using the browser extension in Chrome, and managed to reproduce the same outcomes (successes and failures).

diegodlh avatar Mar 11 '21 21:03 diegodlh

I think I may have found a solution. Couldn't test it thoroughly and probably should be implemented in a better way, but it seems to solve the issue; that is, go through login attempts 1 to 7 above without login failures.

Basically it consists of calling the API's logout action at the end of the actionPost request to clear the session cookies.

To do so, I added a second then() here:

  return request('post', params)
  .then(throwErrorRes(`action=${action} error`, params))
  .then((body) => {
    request('post', {
      url: buildUrl(
        `${instance}/w/api.php`, {
          action: 'logout',
          format: 'json'
        }
      ),
      headers: {},
      body: {
        token: data.token
      }
    });
    return body;
  });

Sorry for the hasty implementation and for not testing it thoroughly enough. But I had to go and wanted to report my findings.

If you think this makes sense, we can think of a better implementation.

Thanks!

Edited: Fixed code above because function in last then wasn't returning body.

diegodlh avatar Mar 11 '21 22:03 diegodlh

I made an attempt to turn your patches into a configurable setup, see https://github.com/maxlath/wikibase-edit/blob/browser/docs/how_to.md#browser

Could you try this and confirm it works for you:

  • install wikibase-edit from that branch: npm install wikibase-edit@https://github.com/maxlath/wikibase-edit#browser
  • set letAgentHandleLoginCookies=true in the request config (same object as credentials and anonymous)

I also experimented with tests in a browser environment, but I could only make the anonymous edit work

maxlath avatar Mar 13 '21 20:03 maxlath

Sorry for the delay. It works like a charm! Just one minor thing (see below).

I can confirm that setting letAgentHandleLoginCookies=true when using the test browser extension I created does remove the need of applying the patch I mentioned in my comment above. Updated the test browser extension accordingly.

I see you included sending logout requests at the end, so the problem why I posted this issue seems to have been solved. This is also the case for the Zotero plugin I'm developing, for which (as mentioned above) "'set-cookie' headers are available to xhr.getAllRequestHeaders()", although the cookies are handled by the agent anyway in the end.

I can also confirm that I didn't need origin=* for anonymous edits, neither in the extension nor in the plugin.

Only error I found is that the .then(afterRequest) in post.js is not being called if there is an error in .then(throwErrorRes(...)) right above. For example, this happened to me if the user had no permissions to make the edit. This is fixed by using .finally(afterRequest) instead, which runs both on promise success or rejection. Could you make this change?

As for the tests in a browser environment you mentioned, I'm afraid I have no experience with unit tests (though I have to learn soon!), so I can't help there right now.

Thank you!!

diegodlh avatar Mar 19 '21 16:03 diegodlh

while it's great that it works, I have a concern there: logging in and out after each requests is quite a waste of time and resources, especially has I would assume that a single browser extension installation would never be used by several users at the same time. Maybe we should rather store the auth status used for the previous request (username or anonymous), and logout only before doing a new request with a different auth status?

maxlath avatar Mar 20 '21 10:03 maxlath

Hi, Max. Thank you again for taking the time to look at this.

I do agree that logging out just to make sure that saved cookies won't interfere (in a future login with different credentials) is a waste of time and resources. I agree that saving the auth status from the last request might help. In fact, just logging in (assuming the session has not expired) is a waste of time and resources as well.

Anyway, before reading this message of yours, I was having trouble with the log out being sent at the end of the request. I couldn't debug it in detail, but it seemed as if some errors (from the actual edit requests) would go unnoticed because of that last .then( / .finally(afterRequest) call. So I thought maybe calling the log out at the beginning would be better (and also safer, to make sure a failed log out wouldn't affect future logins).

However, in the end, I found a completely different way to deal with this whole thing. After a very long time searching, I finally found a way of managing cookies (that is, clear them) programmatically. Since Zotero is based on an old version of Firefox, I managed to do so with the Cookies Service. For current browsers using the WebExtensions API, there is a Cookies API, which I haven't had the chance to test, though.

I've been stuck on this for quite some time last week, so I have to move forward with my plugin now, using this workaround. Regarding your suggestion, although I think it would be useful to avoid having to log in repeatedly, I'm afraid I won't have time to work on that right now.

From my side, the issue is closed, using the workaround to clear cookies at the beginning. But maybe you want to keep it open to eventually work on saving auth status.

Thank you VERY MUCH again for your help!

diegodlh avatar Mar 22 '21 17:03 diegodlh