Use a webview for WP.com login #21414
Conversation
Generated by 🚫 Danger |
|
 |
App Name |  WordPress |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr21414-20df6dc | |
| Commit | 20df6dc | |
| Direct Download | wordpress-prototype-build-pr21414-20df6dc.apk |
|
 |
App Name |  Jetpack |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr21414-20df6dc | |
| Commit | 20df6dc | |
| Direct Download | jetpack-prototype-build-pr21414-20df6dc.apk |
|
|
||
| runBlocking { | ||
| val tokenResult = loginClient.exchangeAuthCodeForToken(code, BuildConfig.OAUTH_REDIRECT_URI) | ||
| accountStore.updateAccessToken(tokenResult.getOrThrow()) |
There was a problem hiding this comment.
I think you'd want to check if the token belongs to the current user before saving it. Users can log in with any account from the login webpage.
There was a problem hiding this comment.
This should be for first-time authentication only – I'm thinking we'll want a reauthenticate method that does what you say – forces login to a specific account
|
@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:
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? wplogin.mp4 |
| @@ -113,7 +113,18 @@ | |||
| <activity | |||
| android:name=".ui.accounts.LoginActivity" | |||
| android:theme="@style/LoginTheme.TransparentSystemBars" | |||
| android:windowSoftInputMode="adjustResize" /> | |||
| android:windowSoftInputMode="adjustResize" | |||
| android:exported="true"> | |||
There was a problem hiding this comment.
Sonarcloud flags this as a security risk. Does it need to be exported?
There was a problem hiding this comment.
My understanding is that this was required in order to handle custom URL schemes, but I might be incorrect!
There was a problem hiding this comment.
Ah, I suspect you're right about that. This Sonarcloud page mentions ways to fix this warning.
That's really odd – I wonder if maybe the app needs to be fully uninstalled? You're being redirected to
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. |
I tried again, this time using a new emulator instance (so no previous install), and I'm still seeing this behavior. |
Project dependencies changesThe following changes in project dependencies were detected (configuration listtree+\--- androidx.browser:browser:1.5.0 -> 1.8.0 (*) |
jkmassel
force-pushed
the
use/webview-for-dotcom-login
branch
from
eb5c5a5
to
d93b1f8
Compare
|
@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! |
|
@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. redirect.mp4With a 2FA account, I still get stuck in some sort of passkey loop which I don't understand. passkey.mp4 |
Fixes a fairly common login issue reported in https://a8c.slack.com/archives/C02AVAR9B/p1728576885606729.
To Test:
Regression Notes
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).
What I did to test those areas of impact (or what existing automated tests I relied on)
n/a
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:
RELEASE-NOTES.txtif necessary.Testing Checklist (strike-out the not-applying and unnecessary ones):