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

[HOLD for payment 2023-03-06] [HOLD for payment 2023-02-22] Implement the security code autofill for magic codes #14853

Closed
4 tasks
mountiny opened this issue Feb 6, 2023 · 19 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering NewFeature Something to build that is a new item. Weekly KSv2

Comments

@mountiny
Copy link
Contributor

mountiny commented Feb 6, 2023

Problem

We are going passwordless and we are using magic links and magic codes to sign in users. This feature is now under beta and its being slowly rolled out.

Internal tracking issue is here.

Why is it important

This is improving UX and sets NewDot on par with other apps using similar security passcodes.

Solution

Lets implement the security passcode autofill from SMS for all the platforms which support this, afaik:

  • iOS
  • mWeb
  • macOS Safari
  • Android

For example here is Apple documentation.

Lets also research if there are any other platforms which this can be implemented for. ie MacOS Chrome

@mountiny mountiny added Engineering Daily KSv2 NewFeature Something to build that is a new item. labels Feb 6, 2023
@mountiny mountiny self-assigned this Feb 6, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Feb 6, 2023
@mountiny mountiny changed the title Implement the security code autofill for iOS, MacOS Safari and Android Implement the security code autofill for magic codes Feb 6, 2023
@Expensify Expensify unlocked this conversation Feb 6, 2023
@mountiny
Copy link
Contributor Author

mountiny commented Feb 7, 2023

Waiting for Callstack

@narefyev91
Copy link
Contributor

Hello, I'm Nicolay from Callstack and I'm interested in taking and analysing this issue to work on a fix for it.

@mountiny mountiny added Weekly KSv2 and removed Daily KSv2 labels Feb 8, 2023
@mountiny
Copy link
Contributor Author

mountiny commented Feb 8, 2023

I am making this Weekly but please @narefyev91 continue adding daily updates as you work on this. thank you 🙇

@narefyev91
Copy link
Contributor

Yesterday spent time to investigate potential solutions around react native (ios & android) and web. Prepared internal proposal for review. Found additional workflow changes for android (Hash implementation)

@narefyev91
Copy link
Contributor

Today - investigated solution for android + trello, also was able to run locally web application on real mobile device and was able to make same behaviour as will be for mobile app - after sms came - code will be extracted for both mobile ios and mobile ios web inside keyboard (as we expected). Also investigate any required changes for sms body to make it working (for ios nothing is needed, body totally correct). Tomorrow plan to push proposal solution for IOS to review

@narefyev91
Copy link
Contributor

narefyev91 commented Feb 10, 2023

Proposal

Problem

We need to autofill code from SMS inside verification field to allow users not to manually typing received code.

Solution

For IOS:
We need to use standard props from React Native TextInput: autoComplete and textContentType - both combination of props will grant ability to auto fetch code from SMS.
Props review: textContentType="oneTimeCode", autoComplete="one-time-code"

For Android
We need to make changes for SMS body - the most important to include hash inside it. Hash is very important - it's connecting app with google play service - and allow to avoid ask users to grant READ_SMS permission from the app (Google restrict this permission check).
For make it working inside the app we need to use standard prop option from React Native TextInput: [autoComplete]
Prop review: autoComplete="sms-otp"

For Web
For web (both IOS and Android) we need to use standard prop option from React Native TextInput: [autoComplete]
Prop review: autoComplete="one-time-code"

Based on all this we will use Platform.select to apply correct props based on the platform.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 10, 2023
@mountiny
Copy link
Contributor Author

@narefyev91 Nice thank you I think we should start with iOS and Web before we figure out how to do this for Android too. We can get it ready in the frontend but the hash needs to be added and will be done so later I assume.

One more thing before proceeding, who about MacOS safari and Messages integration. Is this covered by the Web autocomplete?

@mountiny mountiny added Weekly KSv2 and removed Daily KSv2 labels Feb 10, 2023
@narefyev91
Copy link
Contributor

@narefyev91 Nice thank you I think we should start with iOS and Web before we figure out how to do this for Android too. We can get it ready in the frontend but the hash needs to be added and will be done so later I assume.

One more thing before proceeding, who about MacOS safari and Messages integration. Is this covered by the Web autocomplete?

Yes for MacOS safari it will rely on web autocomplete
Screenshot 2023-02-10 at 16 09 16

@mountiny
Copy link
Contributor Author

Amazing, would you like to raise the PRs to enable these? We could even do the Android as well but we cannot test it now.

Dont forget to follow these instructions to sign your commits https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#begin-coding-your-solution-in-a-pull-request

@narefyev91
Copy link
Contributor

Amazing, would you like to raise the PRs to enable these? We could even do the Android as well but we cannot test it now.

Dont forget to follow these instructions to sign your commits https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#begin-coding-your-solution-in-a-pull-request

Sure - i will also add in PR comments how to generate hash code (it will work for debug.keystore)

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Feb 11, 2023
@mountiny
Copy link
Contributor Author

PR merged

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Feb 15, 2023
@melvin-bot melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label Feb 15, 2023
@melvin-bot melvin-bot bot changed the title Implement the security code autofill for magic codes [HOLD for payment 2023-02-22] Implement the security code autofill for magic codes Feb 15, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 15, 2023
@MelvinBot
Copy link

Reviewing label has been removed, please complete the "BugZero Checklist".

@MelvinBot
Copy link

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.71-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-02-22. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@JmillsExpensify
Copy link

@mountiny should I assign myself or you've got it?

@mountiny
Copy link
Contributor Author

mountiny commented Feb 15, 2023

@JmillsExpensify I think for callstack we dont have to pay anything on Upwork right? so we might want to close this and create just a follow up to add the hash to the Android notifications, so the autofill works on Android native too

@JmillsExpensify
Copy link

Ah yep, good call! Wish we had any easier way to see that haha, though regardless I'll leave it to ya'll. 🙌🏼

@mountiny
Copy link
Contributor Author

This seems to be all good, closing. Thanks @narefyev91!

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Feb 27, 2023
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2023-02-22] Implement the security code autofill for magic codes [HOLD for payment 2023-03-06] [HOLD for payment 2023-02-22] Implement the security code autofill for magic codes Feb 27, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 27, 2023
@MelvinBot
Copy link

Reviewing label has been removed, please complete the "BugZero Checklist".

@MelvinBot
Copy link

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.76-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-03-06. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants