Issue/126 - Android Autofill #141

se1exin · 2019-02-09T11:48:11Z

This PR should be considered a Work In Progress for two reasons:

Compared to the iOS implementation and experience, so far Android is only about half way. The simplest form of AutoFill is working - with credentials being offered on-screen in the app being logged-into (e.g. Google Chrome). However, kicking the user temporarily into the Buttercup app to choose a credential (like in the iOS implementation) is not yet implemented, and appears to be a much larger undertaking than it was with iOS. Whilst it should indeed be possible to implement the kick-back to Buttercup to choose an (up-to-date) credential, I thought I would put the PR up in case you wanted to get at least what has been implemented into release soon rather than waiting for the whole thing to be complete.

Edit: Point 1 is now working as well

In case you are keen to release based on point 1, there is one other factor to consider first. On Android there is no obvious way (that I can find) to set Butercup to be the AutoFill provider in the Android Settings (which can be a labyrinth at the best of times). The recommended way in the Android dev docs is to request if the user wants to setup autofill, and then programmatically switch to the AutoFill selection settings in the Android Settings app, at which point the user can toggle Buttercup on. I am currently immediately switching directly to the Android Settings page as soon as the app loads, which is obviously a terrible idea, so I would like to formally request for feedback on how, where, and when the app should ask the user if they would like to enable AutoFill. I think this also applies for iOS as well, as currently it is up to the user to manually go and enable AutoFill in the iOS settings.

Edit: Point 2 has been implemented using the same approach as enabling Touch ID on an Archive

Finally, I'm also going to suggest that the conversation around point 2 be held in the original issue #126, as I feel it is more likely to be seen and participated by the community there.

Closes #71, Closes #126

…etect HTML based inputs

…ues, add Web Domain in results

…ying credentials across to Android AutoFill service

…utofill on Android

…th Android sharing redux state between main app and autofill app.

perry-mitchell · 2019-02-11T21:16:21Z

Great stuff @se1exin! I'm not surprised that it sounds more complicated. Thanks a lot for looking in to this.. We're happy to not rush things, if it's your opinion that it's still possible to improve the UX.

Let's continue in #126!

… with new ReactNativeHost instance

…hanges

se1exin · 2019-02-16T01:52:07Z

@perry-mitchell This PR is no longer a WIP - its now ready for testing and hopefully merging!

jokeyrhyme · 2019-02-16T06:06:03Z

android/app/build.gradle

        release {
+            manifestPlaceholders = [usesCleartextTraffic:"true"]


I've not done much Android or React Native development, but is it normal that a password manager needs to enable non-HTTPS communication? Do you know what plaintext HTTP resources this app requests?

Good find @jokeyrhyme! Cleartext was needed to allow access to the metro bundler in dev - I must have forgotten to switch it back to off for release.

This is fixed in feccc8a

jokeyrhyme · 2019-02-16T19:43:05Z

Hmmm, the build is failing, which seems normal for this project: https://travis-ci.org/buttercup/buttercup-mobile/builds/494137819

However, previously, the "Node.js:8" job was passing: https://travis-ci.org/buttercup/buttercup-mobile/builds/481638329

The rest of this looks fine to me (for what it's worth) :)

se1exin · 2019-02-16T23:02:09Z

@jokeyrhyme I've fixed the nodejs test, but after a few attempts was not able to get iOS and Android tests to work. The tests we're already failing before I came along, and I'm not all that familiar with travis, so I'll defer fixing those to someone with more experience.

jokeyrhyme · 2019-02-16T23:07:48Z

@se1exin fair enough, yeah, looks good :)

perry-mitchell · 2019-02-20T18:10:39Z

Hmmm, the build is failing, which seems normal for this project: travis-ci.org/buttercup/buttercup-mobile/builds/494137819

Yeah the tests are a mess.. they pass locally but the CIs seem to have major issues. That's a separate issue 😅

This PR is no longer a WIP - its now ready for testing and hopefully merging!

Amazing! I'll make sure it's taken care of this week!

perry-mitchell

Just a couple of minor items!

source/shared/autofill.js

perry-mitchell · 2019-02-26T16:46:08Z

source/shared/autofill.js

+export function autoFillEnabledForSource(sourceID) {
+    return new Promise((resolve, reject) => {
+        if (autoFillAvailable) {
+            return AutoFillBridge.getAutoFillEnabledSources().then(autoFillSources => {


And this too.. Otherwise perhaps wrapping it in something like:

return Promise.resolve() .then(() => { // ... return false; });

Not sure about this one - the promise resolves on all paths, unless I'm missing something?

perry-mitchell · 2019-02-26T16:46:25Z

source/shared/autofill.js

+export function getAutoFillSystemStatus() {
+    return new Promise((resolve, reject) => {
+        if (autoFillAvailable) {
+            return AutoFillBridge.getAutoFillSystemStatus().then(isAutoFillEnabled => {


Again for this one - the promise resolves on all paths, unless I'm missing something?

I mean it's alright of course, it's just a bit weird seeing the promise being returned when using the new Promise((resolve, reject) => ... format. I'm fine with this as it works of course.

Alternatively (maybe):

export function getAutoFillSystemStatus() { return autoFillAvailable ? AutoFillBridge.getAutoFillSystemStatus() : Promise.resolve(false); }

index.shared.js

perry-mitchell · 2019-02-26T16:51:29Z

@se1exin Sorry for the delay.. Has been a busy week and I'm recovering from a bug. I'm keen to get this released.. It's a huge PR, and definitely over my head. My idea is that once again, after it's reviewed, I'll merge it to a holding branch where we'll test it before going to master.

…issue/126

se1exin · 2019-03-04T06:41:04Z

@perry-mitchell all good, hope your feeling better!

I've adding the missing resolves (not sure how I missed them originally..), however I'm not sure about the last 2 in the review (see review comments). Happy for you to just re-write the Promise logic for those if your can see an issue that I can't :)

sallar · 2019-03-04T19:25:45Z

Thanks @se1exin for this! I've just tested it, and the autofill works great. However I have a slight problem with a few random errors that keep occurring, which Im not sure are caused by this PR. ~~But I'll investigate a bit more.~~

Edit: I have found a few issues:

Right after adding a webdav vault, I see an error saying Unknown type: null. I can dismiss the error and everything looks fine.
In some pages username is not recognised. Examples:
- https://codepen.io/ianpirro/full/DzKIJ/
- https://www.tutorialrepublic.com/snippets/preview.php?topic=bootstrap&file=sign-in-from-with-facebook-and-twitter-button

sallar

Great work! Just a slight change needed from my part

sallar · 2019-03-04T19:32:45Z

source/shared/sheets.js

+    const itemSystemSettings = "Android Settings";
+    const itemCancel = "Cancel";
+    return Promise.all([getAutoFillSystemStatus(), autoFillEnabledForSource(currentSourceID)]).then(
+        results => {


maybe destructure this instead of assigning new constants later:

Suggested change

results => {

([autoFillEnabled, archiveEnabled]) => {

perry-mitchell

Looks good to me!

perry-mitchell · 2019-03-09T11:52:05Z

source/shared/autofill.js

+export function getAutoFillSystemStatus() {
+    return new Promise((resolve, reject) => {
+        if (autoFillAvailable) {
+            return AutoFillBridge.getAutoFillSystemStatus().then(isAutoFillEnabled => {


I mean it's alright of course, it's just a bit weird seeing the promise being returned when using the new Promise((resolve, reject) => ... format. I'm fine with this as it works of course.

se1exin · 2019-03-13T06:51:04Z

Hey! @perry-mitchell @sallar

Sorry for the delay in getting back to you both on this, work and life has been quite busy for the last few weeks. It also doesn't seem to be easing off so at this point I'm not sure when I will get a chance to address the feedback items sorry. Not sure what I can do to help at the moment but thought I would let you know so your weren't left hanging.

perry-mitchell · 2019-03-15T17:44:18Z

Thanks @se1exin - We know the feeling! We'll take care of the final adjustments shortly. Take care 👍

seddarj · 2019-04-21T09:19:37Z

Great work 👏 ! Can't wait to see this merged!

hayr-hotoca · 2022-01-01T07:54:14Z

Hi great work btw @se1exin !

But how do you retrieve the domain name from browser apps? My machine the getWebDomain() method always return null?

se1exin · 2022-01-01T11:53:37Z

Hi great work btw @se1exin !

But how do you retrieve the domain name from browser apps? My machine the getWebDomain() method always return null?

Hi @lincdh thanks, it's been a few years since I implemented this so my memory isn't very fresh on the inner details, but the first think I would check is your service config contains the apps you want to read from: android/app/src/main/res/xml/autofill_service_configuration.xml

Good luck 🙂

hayr-hotoca · 2022-01-02T11:56:08Z

@se1exin I tried to create another android project to test your code but the same result. Did it looks somewhat like below when you debug?:
Also if you have spare time could you have a look at my autofill implementation(mostly with your code) at https://github.com/lincdh/autofilljava? Thanks!

perry-mitchell · 2022-01-03T13:03:42Z

@lincdh Sorry, this isn't the right forum for debugging your project. Please use this codebase as a reference for your work, however. Best of luck :)

se1exin added 4 commits January 31, 2019 21:28

buttercup#126 - Implement basic Android AutoFill with hueristics to d…

35cce71

…etect HTML based inputs

Change grade back to previous version, fix Android 8 debugging RN iss…

1a2c2a1

…ues, add Web Domain in results

buttercup#126 - Start implementing AutoFillBridge on Android for prox…

479ce51

…ying credentials across to Android AutoFill service

buttercup#126 - Refactor AutoFillHelpers, finish one-way out of app a…

90f3bc2

…utofill on Android

This was referenced Feb 9, 2019

Android Autofill service #126

Closed

Form prefill #71

Closed

buttercup#126 - Implement AutoFill from App. Not complete - issues wi…

ef9ff18

…th Android sharing redux state between main app and autofill app.

se1exin added 6 commits February 12, 2019 08:31

buttercup#126 - Fix react state sharing by starting autofill activity…

231700c

… with new ReactNativeHost instance

buttercup#126 - Refactor Android Autofill UI and RN Entry point.

0a65076

buttercup#126 - Implement per-archive Autofill setting

617f571

Clean up log and fix promise

e767021

buttercup#126 - Update iOS Autofill implementation to match Android c…

ebc424c

…hanges

buttercup#126 - Remove index.io.autofill.js in favour of shared approach

720a637

se1exin changed the title ~~[WIP] Issue/126 - Android Autofill~~ Issue/126 - Android Autofill Feb 16, 2019

jokeyrhyme reviewed Feb 16, 2019

View reviewed changes

se1exin added 2 commits February 16, 2019 19:04

Remove cleartext from Android release

87b0034

Set cleartext to false in Android release

feccc8a

jokeyrhyme approved these changes Feb 16, 2019

View reviewed changes

Fix nodejs tests

3656b92

perry-mitchell self-requested a review February 20, 2019 18:10

perry-mitchell requested changes Feb 26, 2019

View reviewed changes

perry-mitchell and others added 2 commits February 26, 2019 18:52

Merge branch 'master' into issue/126

30c9379

Add missing Promise resolves

eb3655b

Merge branch 'issue/126' of github.com:se1exin/buttercup-mobile into …

b0ccb65

…issue/126

sallar requested changes Mar 4, 2019

View reviewed changes

perry-mitchell approved these changes Mar 9, 2019

View reviewed changes

perry-mitchell changed the base branch from master to android_autofill April 21, 2019 17:51

perry-mitchell merged commit 788c4b4 into buttercup:android_autofill Apr 21, 2019

buttercup locked as resolved and limited conversation to collaborators Jan 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/126 - Android Autofill #141

Issue/126 - Android Autofill #141

se1exin commented Feb 9, 2019 •

edited by perry-mitchell

Loading

perry-mitchell commented Feb 11, 2019

se1exin commented Feb 16, 2019

jokeyrhyme Feb 16, 2019

se1exin Feb 16, 2019

jokeyrhyme commented Feb 16, 2019

se1exin commented Feb 16, 2019

jokeyrhyme commented Feb 16, 2019

perry-mitchell commented Feb 20, 2019

perry-mitchell left a comment

perry-mitchell Feb 26, 2019

se1exin Mar 4, 2019

perry-mitchell Feb 26, 2019

se1exin Mar 4, 2019 •

edited

Loading

perry-mitchell Mar 9, 2019

jokeyrhyme Mar 9, 2019

perry-mitchell commented Feb 26, 2019

se1exin commented Mar 4, 2019

sallar commented Mar 4, 2019 •

edited

Loading

sallar left a comment

sallar Mar 4, 2019

perry-mitchell left a comment

perry-mitchell Mar 9, 2019

se1exin commented Mar 13, 2019

perry-mitchell commented Mar 15, 2019

seddarj commented Apr 21, 2019

hayr-hotoca commented Jan 1, 2022

se1exin commented Jan 1, 2022

hayr-hotoca commented Jan 2, 2022 •

edited

Loading

perry-mitchell commented Jan 3, 2022

		release {
		manifestPlaceholders = [usesCleartextTraffic:"true"]

Issue/126 - Android Autofill #141

Issue/126 - Android Autofill #141

Conversation

se1exin commented Feb 9, 2019 • edited by perry-mitchell Loading

perry-mitchell commented Feb 11, 2019

se1exin commented Feb 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jokeyrhyme commented Feb 16, 2019

se1exin commented Feb 16, 2019

jokeyrhyme commented Feb 16, 2019

perry-mitchell commented Feb 20, 2019

perry-mitchell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

se1exin Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perry-mitchell commented Feb 26, 2019

se1exin commented Mar 4, 2019

sallar commented Mar 4, 2019 • edited Loading

sallar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perry-mitchell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

se1exin commented Mar 13, 2019

perry-mitchell commented Mar 15, 2019

seddarj commented Apr 21, 2019

hayr-hotoca commented Jan 1, 2022

se1exin commented Jan 1, 2022

hayr-hotoca commented Jan 2, 2022 • edited Loading

perry-mitchell commented Jan 3, 2022

se1exin commented Feb 9, 2019 •

edited by perry-mitchell

Loading

se1exin Mar 4, 2019 •

edited

Loading

sallar commented Mar 4, 2019 •

edited

Loading

hayr-hotoca commented Jan 2, 2022 •

edited

Loading