billylo1 / covidpass

Web app for adding Ontario vaccination records to Apple Wallet (forked from covidpass in EU)
https://grassroots.vaccine-ontario.ca
MIT License
79 stars 12 forks source link

QR Code does not generate correct link (URL encoding error) #9

Closed jmees closed 2 years ago

jmees commented 2 years ago

Organization I received vaccination from has spaces in the name (e.g. organization=Some Pharmacy) which breaks the URL generated by the QR Code. If I manually add the "%20" (e.g. organization=Some%20Pharmacy) the link works and validates the pass correctly. Tested several QR Scanners, all with the same result.

straxus commented 2 years ago

@jmees can you please try scanning this with https://verifier.vaccine-ontario.ca ? It should work correctly with that

mutantlog commented 2 years ago

I wrote an email about the same issue, and I just tested scanning my code with that website. It did work, but it's not a convenient work-around both because it requires asking the business to navigate to that website first, and because it defaulted to my front facing camera in Chrome my Pixel 3a running Android 11, and I'm not aware of an easy way to change it to use the rear camera.

straxus commented 2 years ago

@mutantlog I ran into the same issue with the verifier using the front-facing camera, and I found that if I reloaded the page then it seemed to switch correctly to the back camera (I'm on a Pixel 5 with Chrome).

Given that we currently have >375k of the current passports generated as of this moment, encoding the URL's not going to be a viable option for a fix for the existing userbase - I think we'll need to detect a partial match which is indicative of an organization name with a space in it and direct the scanning device to try again with the official verifier app URL noted above. It's not ideal, but it may be the best option given where we're at right now

mutantlog commented 2 years ago

Please verify it, because I'm not an experienced developer, but I added a pull request to at least handle this for new passes generated in the future. It doesn't fix previously generated passes, but it does appear to work with third party QR code scanners as well as https://verifier.vaccine-ontario.ca

jmees commented 2 years ago

@straxus Thanks for the camera advice (reloading page to get the back camera). I can confirm the verifier.vaccine-ontario.ca does work for my QR Code (where other scanners do not). The current result with other scanners is the X page (Not Found) with the advice to remove the pass and to re-register.

FYI, not sure how the key strength is distributed across all the parameters, but most of my organization name was captured (just not the trailing +Pharmacy). The unfortunate part is that the "dose" parameter is also lost, since it follows organization.

My two cents, have a two step resolution (once the issue is patched). The one you suggested (use official scanning recommendation) along with an additional note that an "updated passport" is available for this customer (if they re-submit their document). This would all happen if a partial match succeeded. Giving control to the user and unburden the organization scanning over time might be the best way forward.

PeterS6g commented 2 years ago

Good Fix! I'm definitely going to re-generate my QR code when this goes live. Sure, my current QR code one works with https://verifier.vaccine-ontario.ca, but when you're trying to go into some restaurant, you never know what QR reader the wait staff may be using.

straxus commented 2 years ago

@billylo1 actually implemented a fix for this in parallel, and it got deployed out this morning - it should be out there now, and any QR codes generated from this point forward should work with third-party scanners and be URLEncoded.

@mutantlog thanks for the PR! Sorry we're not going to be able to adopt it directly since we fixed in parallel, but your solution would have worked as well and I appreciate the time you took to contribute this fix!

mutantlog commented 2 years ago

Thanks @straxus . As I said in the pull request, I'm not sure if I did it in the most elegant way, but I copied @billylo1's fix from pass.ts into photo.ts to ensure they're both generating the same QR code.

jmees commented 2 years ago

@straxus, @billylo1 Issue still open... just tried re-processing my MoH document to generate a new pass image. The new image produces the same URL error (unless using verifier.vaccine-ontario.ca). The space in the organization name is still the cause of the issue, as before.

FYI, verified it was the latest build based on the new dose name appearing on the pass. QR message still not encoded ( i.e. encodeURIComponent() or escape() ).

straxus commented 2 years ago

@jmees thanks for the verification - if I understand correctly, the Wallet pass is OK now but the photo is not? I believe that corroborates what @mutantlog posted as well, and we will implement a fix for that shortly (the urlencode fix was applied to the pass but not the photo gen code)

mutantlog commented 2 years ago

I think @billylo1 corrected this in b38540a and it can now be closed? I re-generated both passes and photos, and the latter now also validates with Barcode Scanner+ on Android.

straxus commented 2 years ago

Correct, this should now be fixed fixed :D Thanks for verifying!

jmees commented 2 years ago

@straxus @billylo1 et al. Also verified that the image pass works on all scanners I have tested. Great job everyone, thanks!