wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.99k stars 1.33k forks source link

Use a webview for WP.com login #21414

Open jkmassel opened 2 weeks ago

jkmassel commented 2 weeks ago

Fixes a fairly common login issue reported in https://a8c.slack.com/archives/C02AVAR9B/p1728576885606729.


To Test:


Regression Notes

  1. Potential unintended areas of impact Could be other issues around login – I tried very narrowly address the issue in this PR – nothing has been removed, so everything should work like it did before (for instance, use re-authentication).

  2. What I did to test those areas of impact (or what existing automated tests I relied on) n/a

  3. What automated tests I added (or what prevented me from doing so) There wouldn't be a lot of benefit to automated tests at this point.


PR Submission Checklist:


Testing Checklist (strike-out the not-applying and unnecessary ones):

dangermattic commented 2 weeks ago
3 Errors
:no_entry_sign: Please add tests for class WPcomLoginHelper (or add unit-tests-exemption label to ignore this).
:no_entry_sign: Please add tests for class WPcomLoginClient (or add unit-tests-exemption label to ignore this).
:no_entry_sign: Please add tests for class WPcomLoginError (or add unit-tests-exemption label to ignore this).
1 Message
:book: This PR is still a Draft: some checks will be skipped.

Generated by :no_entry_sign: Danger

wpmobilebot commented 2 weeks ago
WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21414-ade3342
Commitade3342db8acdc42b8b441234d2012e97f0c76d8
Direct Downloadwordpress-prototype-build-pr21414-ade3342.apk
Note: Google Login is not supported on these builds.
wpmobilebot commented 2 weeks ago
Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21414-ade3342
Commitade3342db8acdc42b8b441234d2012e97f0c76d8
Direct Downloadjetpack-prototype-build-pr21414-ade3342.apk
Note: Google Login is not supported on these builds.
nbradbury commented 2 weeks ago

@jkmassel I'm running into some issues with this. When I try to login with a 2FA account, I see this screen and can't go any further:

passkey

If I try with a non-2FA account, after I login I'm not redirected to the app - instead I'm still in the browser. Is this intended? I also did not expect to be taken to the browser to login. Is it not possible to do this in a WebView within the app?

https://github.com/user-attachments/assets/b7bf75e6-6ad3-42d6-9048-44dba946c70a

jkmassel commented 2 weeks ago

If I try with a non-2FA account, after I login I'm not redirected to the app - instead I'm still in the browser. Is this intended?

That's really odd – I wonder if maybe the app needs to be fully uninstalled? You're being redirected to http://android.a8c.com, which is the old behaviour 🤔

Is it not possible to do this in a WebView within the app

Good question! I had assumed that we'd want to use the default browser for password managers, etc – if we can do it in an in-app browser that would be nicer for sure.

nbradbury commented 2 weeks ago

That's really odd – I wonder if maybe the app needs to be fully uninstalled? You're being redirected to http://android.a8c.com, which is the old behaviour

I tried again, this time using a new emulator instance (so no previous install), and I'm still seeing this behavior.

wpmobilebot commented 6 days ago

Project dependencies changes

The following changes in project dependencies were detected (configuration wordpressVanillaReleaseRuntimeClasspath):

list ``` ```
tree ```diff +\--- androidx.browser:browser:1.5.0 -> 1.8.0 (*) ```
jkmassel commented 6 days ago

@nbradbury – could you give this a try again? It now uses Chrome Tabs in-app, which should share all of password management goodness we need.

I tried it with a Google-synced passkey and it worked, and I tried with a Yubikey and that worked too (on my ancient Galaxy S9) so I'm hopeful it works for you too!

nbradbury commented 5 days ago

could you give this a try again? It now uses Chrome Tabs in-app, which should share all of password management goodness we need.

@jkmassel Chrome Custom tabs is a much better solution, but I'm still seeing issues. With a non-2FA account, I end up being redirected to the Automattic home page.

https://github.com/user-attachments/assets/b9cd6021-dd60-4875-9c54-63019b3b223d

With a 2FA account, I still get stuck in some sort of passkey loop which I don't understand.

https://github.com/user-attachments/assets/54d6515e-8273-4f1e-9e62-cb6398ef79b9

sonarcloud[bot] commented 2 days ago

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE