Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anonymous edits seem to be failing #63

Open
diegodlh opened this issue Mar 9, 2021 · 13 comments
Open

Anonymous edits seem to be failing #63

diegodlh opened this issue Mar 9, 2021 · 13 comments

Comments

@diegodlh
Copy link
Contributor

diegodlh commented Mar 9, 2021

  • 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?

@maxlath
Copy link
Owner

maxlath commented Mar 9, 2021

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?

@GiulioC
Copy link

GiulioC commented Mar 10, 2021

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.

@maxlath
Copy link
Owner

maxlath commented Mar 10, 2021

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

@diegodlh
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@maxlath
Copy link
Owner

maxlath commented Mar 10, 2021

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?

@diegodlh
Copy link
Contributor Author

diegodlh commented Mar 10, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor Author

diegodlh commented Mar 11, 2021

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 added a commit to diegodlh/zotero-cita that referenced this issue Mar 12, 2021
@maxlath
Copy link
Owner

maxlath commented Mar 13, 2021

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

@diegodlh
Copy link
Contributor Author

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!!

@maxlath
Copy link
Owner

maxlath commented Mar 20, 2021

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?

@diegodlh
Copy link
Contributor Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants