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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fill OTP fields with SMS codes #900

Merged
merged 15 commits into from Jul 2, 2020
Merged

Fill OTP fields with SMS codes #900

merged 15 commits into from Jul 2, 2020

Conversation

fmeum
Copy link
Member

@fmeum fmeum commented Jun 29, 2020

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

馃摐 Description

Allow users to fill in OTPs extracted from SMS using Google Play Services.

This PR is functional, but the UI is ugly and I can't get the API support checks to resolve (see FIXME). Please give this a try when you have the time and report whether it works for you or not (you can use PayPal for testing).

馃挕 Motivation and Context

馃挌 How did you test it?

Not much yet, but it works with PayPal.

馃摑 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

馃敭 Next steps

馃摳 Screenshots / GIFs

@fmeum fmeum added this to the 1.10.0 milestone Jun 29, 2020
@msfjarvis msfjarvis self-assigned this Jun 29, 2020
@fmeum
Copy link
Member Author

fmeum commented Jun 29, 2020

I pushed a commit that introduces both free and nonFree flavors. For F-Droid builds, we can simply specify gradle: free to build the free one.

fmeum and others added 3 commits June 29, 2020 14:05
* develop:
  Allow importing TOTP configuration through QR codes (#903)
  Workaround to prevent crash on first run (#898)
* develop:
  Revert "Support directly importing secrets" (#904)
@msfjarvis msfjarvis mentioned this pull request Jul 1, 2020
9 tasks
msfjarvis and others added 4 commits July 2, 2020 00:30
* develop:
  Scroll to files and enter folders when created (#909)
  Run a treewide reformat (#908)
  Improve how secrets and stored and used (#907)
  Improve and refactor Autofill heuristics (#905)
  Use PreferenceKeys file to manage SharedPreferences keys. (#891)
  Bump version
  Prepare release 1.9.2
  update changelog
  Workaround to prevent crash on first run (#898)
  build: bump version
  Prepare release 1.9.1
  Backport Actions fixes (#894)
  Remove API 30 from pull request test matrix (#879)
  CHANGELOG: reword to better clarify fixes
  Prevent cached passwords from being wiped (#884)
  Use remembered credential even if it is empty (#880)
  Reset SSH passphrase after SSH key import (#885)
  Add relnotes for #871 (#872)
  Add org.gnu.icecat as a trusted multi-origin browser (#871)
* develop:
  build: upgrade Gradle wrapper (#911)
@fmeum fmeum marked this pull request as ready for review July 2, 2020 09:29
@fmeum
Copy link
Member Author

fmeum commented Jul 2, 2020

Should be ready for review now.

I gave up on getting the API calls for ongoing requests and permission to work. They must not be run on the main thread, but just never return if run with runBlocking(Dispatchers.IO). @msfjarvis If you know how to deal with the Task API and know how to fix this, I will gladly carry that out. But since we require a user gesture to activate the feature anyway, I don't think that these APIs are worth calling.

Regarding the GitHub workflows: I only added nonFreeRelease as a build variant since that is what we will be deploying to the Play Store. I would vote for keeping the default builds here on GitHub to be of the free variant, which also makes them identical with the F-Droid builds. We could however additionally build nonFree apks, I just don't know how to change the workflows accordingly.

Copy link
Member

@msfjarvis msfjarvis left a comment

Choose a reason for hiding this comment

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

Should be ready for review now.

I gave up on getting the API calls for ongoing requests and permission to work. They must not be run on the main thread, but just never return if run with runBlocking(Dispatchers.IO). @msfjarvis If you know how to deal with the Task API and know how to fix this, I will gladly carry that out. But since we require a user gesture to activate the feature anyway, I don't think that these APIs are worth calling.

That's fine, I don't think it's worth the hassle if what's here works.

Regarding the GitHub workflows: I only added nonFreeRelease as a build variant since that is what we will be deploying to the Play Store. I would vote for keeping the default builds here on GitHub to be of the free variant, which also makes them identical with the F-Droid builds. We could however additionally build nonFree apks, I just don't know how to change the workflows accordingly.

I think we should be attaching APKs for both variants as well as a nonFree bundle which I then deploy to the Play Store. The workflow is currently broken because it doesn't account for path changes, I'll fix that in a second.

msfjarvis and others added 3 commits July 2, 2020 15:20
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis closed this Jul 2, 2020
@msfjarvis msfjarvis reopened this Jul 2, 2020
@msfjarvis
Copy link
Member

Actions is not having a fun time with the new variants...

* develop:
  Fix up URIish instances with @ in user name (#913)
@fmeum fmeum removed the in progress label Jul 2, 2020
@fmeum fmeum merged commit ca9c951 into develop Jul 2, 2020
@fmeum fmeum deleted the feature/autofill_sms_otp branch July 2, 2020 11:49
@fmeum
Copy link
Member Author

fmeum commented Jul 2, 2020

Should we provide separate snapshot builds for the nonFree variant? Ideally, we would get a lot of testing in for this feature as it could potentially lead to crashes on devices with non-standard Google Play Services.

@msfjarvis
Copy link
Member

Should we provide separate snapshot builds for the nonFree variant? Ideally, we would get a lot of testing in for this feature as it could potentially lead to crashes on devices with non-standard Google Play Services.

I'll look into it.

msfjarvis added a commit to fmeum/Android-Password-Store that referenced this pull request Jul 14, 2020
* develop: (77 commits)
  Add debug icon and update color palette (android-password-store#931)
  Revert "Work around Chrome Autofill issue (android-password-store#921)" (android-password-store#933)
  github: remove freeDebug variant from pull request matrix (android-password-store#932)
  Properly guard against invalid renaming (android-password-store#929)
  Fix navigation bar theming and reformat (android-password-store#930)
  Exclude third_party scope from reformats (android-password-store#927)
  Move password export to the IO dispatcher (android-password-store#918)
  Mention android-password-store#482 being fixed in the changelog (android-password-store#925)
  global: set an import order rule and reformat with it (android-password-store#924)
  styles: re-add alertDialogTheme override (android-password-store#923)
  Work around Chrome Autofill issue (android-password-store#921)
  Major UI overhaul and the introduction of a new icon (android-password-store#920)
  Update Public Suffix List data (android-password-store#917)
  Migrate to ActivityResultContracts (android-password-store#910)
  release: script improvements (android-password-store#915)
  Deploy both variants to snapshot directory (android-password-store#914)
  Fill OTP fields with SMS codes (android-password-store#900)
  Fix up URIish instances with @ in user name (android-password-store#913)
  build: upgrade Gradle wrapper (android-password-store#911)
  Scroll to files and enter folders when created (android-password-store#909)
  ...

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@fmeum
Copy link
Member Author

fmeum commented Aug 15, 2020

We should be back on F-Droid soon: https://gitlab.com/fdroid/fdroiddata/-/merge_requests/7141#note_396471590

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.

None yet

2 participants