Page MenuHomePhabricator

Add ability to cite books by scanning their ISBN barcode in mobile web
Open, Needs TriagePublic

Description

Separating from T208134, which is about the same feature in mobile app.

Event Timeline

I myself am successfully using a JS compilation of zbar: https://github.com/Schnark/barcode-reader/blob/master/js/lib/zbar-worker.js, and IIRC the last time I tested it, it was much faster than quaggaJS, even though it also detects QR codes.

They main problem all these JS's have is that a live input via getUserMedia might not be able to properly focus (according to https://bugs.chromium.org/p/chromium/issues/detail?id=343894 it works in Chrome on Android, but probably still not on iOS), which for normal sized barcodes means that you just can't read them. Instead you have to take a photo with the camara app, select that photo instead of the live stream, repeat these steps if something went wrong, and delete all those photos manually once you are done. That's not very user friendly.

A very experimental (i.e. currently an experiment in Chrome only, might perhaps change sometime in the future) is https://wicg.github.io/shape-detection-api/#barcode-detection-api.

If you only want this feature in the Android app, then you should do the barcode scanning in Java, not in JavaScript.

Change 493494 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Citoid@master] WIP DEMO barcode scanning

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

Video throughput on my iPhoneX seems jittery, but this is an amazing feature.. SHIP IT ! ;)

Awesome! There's a slight delay with no loading animation in between when the barcode app closes and when the isbn gets inserted that is just a little too long for no feedback on my phone... is there any way to make that pending? And also, when you press back after denying camera access, you get an empty dialog. But yes ship it :D.

Change 493494 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Citoid@master] WIP DEMO barcode scanning

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

Awesome! There's a slight delay with no loading animation in between when the barcode app closes and when the isbn gets inserted that is just a little too long for no feedback on my phone... is there any way to make that pending? And also, when you press back after denying camera access, you get an empty dialog. But yes ship it :D.

Can you make a screen recording? The dialog should be closing fairly instantly - if it isn't that could be the shutdown code for the library causes the main thread to hang.

Awesome! There's a slight delay with no loading animation in between when the barcode app closes and when the isbn gets inserted that is just a little too long for no feedback on my phone... is there any way to make that pending? And also, when you press back after denying camera access, you get an empty dialog. But yes ship it :D.

Can you make a screen recording? The dialog should be closing fairly instantly - if it isn't that could be the shutdown code for the library causes the main thread to hang

Done: https://www.dropbox.com/s/wa9x5umn4hhx59v/mobizen_20190404_134118.mp4

I note the small camera icon in the top left of the system tray disappears at the same time as the barcode appears. I would suspect that the camera closing is causes the whole phone to freeze up for a second. Potentially we could emit the barcode detected event before closing the camera so at least the code is there when the phone freezes.

Changing .closed to .closing in the scanner widget appears to fix this, sending the detected code to the widget at the start of the teardown process, instead of the end.

@Mvolz I've pushed this change to the test server.

@Mvolz I've pushed this change to the test server.

Great, that seems to have fixed it.

I'm uneasy about this feature for security reasons. As far as I know, browsers will only ask for camera permissions once, and then the website is just allowed to use the camera whenever it wants.

I think this offers new nasty possibilities to someone who manages to take over an interface administrator account, or finds an XSS vulnerability anywhere in any of the code the sites run. Previously just about the worst thing they could do was fetching the IP address, but if our sites have camera permissions, they will also be able to take photos and record videos.

Maybe I am paranoid, but I wanted to point this out.

I'm uneasy about this feature for security reasons. As far as I know, browsers will only ask for camera permissions once, and then the website is just allowed to use the camera whenever it wants.

There is precedent in us asking for sensor data from users with the nearby pages feature asking for location (https://en.m.wikipedia.org/wiki/Special:Nearby). AFAIK there have been no concerns raised about this.

I think this offers new nasty possibilities to someone who manages to take over an interface administrator account, or finds an XSS vulnerability anywhere in any of the code the sites run. Previously just about the worst thing they could do was fetching the IP address, but if our sites have camera permissions, they will also be able to take photos and record videos.

There are undoubtedly terrible things that can happen if an attacker gets hold of an interface admin account (at least for as long as it goes unnoticed). In addition to gathering IPs and granted sensor data (GPS/camera), the user could exploit trust in the domain to prompt for a password, or payment information from donors. Additional private information could be harvested from various API calls, or by scraping Special:Preferences. A attacker could ask a much larger number of users for sensor data, again exploiting the trust in our domain. I don't think it's correct to characterise this as a large change in the data at risk. There are other pieces of work being undertaken to address this (such as enforcing our CORS policy, which would it make it harder to export stolen data).

@matmarex and I will create a security review task for this

JTannerWMF added subscribers: Jcross, sbassett.

Hey @sbassett and @Jcross can we have a review of this ticket/attached patches, or do we need to formally complete this form.

@JTannerWMF - we've taken a look and would appreciate it if you would fill out the Security Readiness Review form. Thanks!

@JTannerWMF - we've taken a look and would appreciate it if you would fill out the Security Readiness Review form. Thanks!

Will do – thank you, @Jcross!

Note: I'm assigning this task over to @JTannerWMF to file a security review task.

Per T269007#7067880 this has been security reviewed and just needs sign off from @marcella.

Test wiki created on Patch Demo by ESanders (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/1fe72fc2e1/w/