Page MenuHomePhabricator

We need to use 'assertuser' in saving API requests
Closed, ResolvedPublic

Description

Current (v1) termbox sends assertuser for logged in users to "Verify the current user is the named user. ".

We need to do this otherwise if people log out in another tab (or delete their cookies etc..) we leak their IP address without warning.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 25 2019, 6:17 PM

I think we do.

The reason is that if the user logs out in another tab then we will "without warning" leak their IP address in the edit.

also IMHO this is not an extra story or anything. This is just part of implementing the save request

also IMHO this is not an extra story or anything. This is just part of implementing the save request

That's why it is a subtask of a story in the sprint. What would have been better procedure of creating this ticket to avoid any doubt in that regard?

Tarrow renamed this task from Verify if we need 'assertuser' in saving API requests to We need to use 'assertuser' in saving API requests.Apr 1 2019, 9:55 AM
Tarrow updated the task description. (Show Details)
Tarrow claimed this task.Apr 1 2019, 2:03 PM
Tarrow moved this task from To Do to Doing on the Wikidata-Termbox-Iteration-12 ✓ board.
Tarrow added a comment.Apr 1 2019, 3:00 PM

My suggested thoughts on implementing this:

  • Add username to the USER store module
  • Add actions and mutations for setting username in store
  • set username

OPTION 1

  • Add additional optional parameter to WritingEntityRepository constructor and Axios Implementation of it for username
  • Use parameter in request in AxiosWritingEntityRepository
  • Include username parameter in the TermboxFactory.getWritingEntityRepository
  • change factory to generate a new WritingEntityRepository
  • get a corrected usernamed repo in save action

OPTION 2

  • add additional optional parameter to saveEntity in WritingEntityRepository
  • use additional parameter in request if present

FTR mw.user.isAnon() & mw.user.getName() are where interesting user information can be found.

Change 501159 had a related patch set uploaded (by Tarrow; owner: Tarrow):
[wikibase/termbox@master] Introduce axios factory

https://gerrit.wikimedia.org/r/501159

Change 501513 had a related patch set uploaded (by Tarrow; owner: Tarrow):
[wikibase/termbox@master] DNM Add assertUser interceptor

https://gerrit.wikimedia.org/r/501513

Change 501159 merged by jenkins-bot:
[wikibase/termbox@master] Introduce axios factory

https://gerrit.wikimedia.org/r/501159

Change 501513 merged by jenkins-bot:
[wikibase/termbox@master] Add assertUser interceptor

https://gerrit.wikimedia.org/r/501513

Tarrow added a comment.Apr 8 2019, 9:11 AM

In order to give this a product test you could try something like the following:

  • log into beta wikidata
  • open a page with the new term box
  • duplicate the tab
  • in one tab log out
  • in the other tab enter edit mode
  • make an edit and click save
  • check that there is now no edit in that entity's history
Pablo-WMDE closed this task as Resolved.Apr 8 2019, 10:38 AM

Change 503034 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[wikibase/termbox@master] mock-entry: configure wgUserName

https://gerrit.wikimedia.org/r/503034

Change 503034 merged by jenkins-bot:
[wikibase/termbox@master] mockup-entry: configure wgUserName

https://gerrit.wikimedia.org/r/503034