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

💪 Increase checks for scanned QR code #1081

Merged

Conversation

Abby-Wheelis
Copy link
Member

In order to ensure that only valid users are created, we need to make sure that the qr codes that are scanned: are QR codes, have not been canceled, start with the appropriate prefix (either emission or nrelopenpath), and include a token parameter.

This way, after we scan into the app, we are sure that the user has scanned the right thing and will have a valid opcode. Later in the login process, we check the opcode for validity.

I still need to test this with a physical phone before it can be merged, which I plan to do tonight.

Abby Wheelis added 2 commits October 24, 2023 14:10
we need to make sure that nobody scans a random qr code and gets into the app with an invalid token, we can help prevent that by checking that the qr code contains the right elements
added a log statement, and verifying that the first part of the opcode is "nrelopenpath" or "emission" -- the staging opcodes start with "emission", but sometimes we use production opcodes to test things in develpoment
@Abby-Wheelis
Copy link
Member Author

While testing this, I discovered that the QR code on the "save qr" page is not composed with a full URL, only the opcode. I'll work up a fix now.

make sure qr code is made with the whole url link
@Abby-Wheelis
Copy link
Member Author

Fix up for the QR code being created with opcode and not whole link in onboarding. In the QrCode component, I added a check to see if the value was a url, and if not, prefix the value with "nrelopenpath://login_token?token=".

Checks have been added when the code is scanned. I chose to check for "nrelopenpath" or "emission" to accomodate the dev environment, but open to changing this. The reason I did not condition what I check for on the global __DEV__ flag is that sometimes we login to the devapp with production opcodes for testing, but if we want I can add that condition, and we can just know to modify the code if we want to use the devapp to scan an qr code that starts with "nrelopenpath"

@Abby-Wheelis Abby-Wheelis marked this pull request as ready for review October 24, 2023 23:58
Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this as emission in the public repo, I will change it to nrelopenpath in the internal one.
Long term, this should be part of the app config
e-mission/e-mission-docs#985 (comment)

www/js/components/QrCode.tsx Outdated Show resolved Hide resolved
www/js/onboarding/WelcomePage.tsx Outdated Show resolved Hide resolved
Long term, this should be part of the app config
e-mission/e-mission-docs#985 (comment)
@shankari shankari merged commit 8dfcd29 into e-mission:onboarding_routing_sept_2023 Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants