Replace native WP.com sign in with web-based sign in #23675

crazytonyli · 2024-10-17T00:16:16Z

The web based sign-in flow is enabled on debug builds. But it will be hidden under a feature flag later.

sign-in.mp4

Regression Notes

Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

wpmobilebot · 2024-10-17T00:28:30Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr23675-7389522
	Version	25.4.2
	Bundle ID	org.wordpress.alpha
	Commit	`7389522`
	App Center Build	WPiOS - One-Offs #10921

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2024-10-17T00:30:13Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr23675-7389522
	Version	25.4.2
	Bundle ID	com.jetpack.alpha
	Commit	`7389522`
	App Center Build	jetpack-installable-builds #9961

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

kean

I tested the flow, including logging-in with a new account in the web view and using passkeys. The flow works well.

It is a bit of a downgrade compared to the current implementation where the app prompts you to use one of the saved credentials from the Apple Passwords on first launch. Perhaps web views should be reserved for self-hosted sites and for wp.com account creation.

kean · 2024-10-17T17:15:31Z

WordPress/Classes/Login/WordPressDotComAuthenticator.swift

+            } else {
+                wpAssertionFailure("WP.com web login failed", userInfo: ["error": "\(error)"])
+            }
+            return


(nit) Extract the first part into a separate method so that there is no return in the middle of this long method.

Addressed in 43409de

kean · 2024-10-17T17:17:31Z

WordPress/Classes/System/WordPressAuthenticatorProtocol.swift

+            // TODO: Replce with a remote feature flag.
+            // Enable web-based login for debug builds until the remote feature flag is available.
+            #if DEBUG
+            let webLoginEnabled = true


If there is no need to update the flag remotely, you can use extension FeatureFlag: RolloutConfigurableFlag { to implement the percentage-based rollout locally.

In that case, we'd need to release the app to update the rollout percentage, right?

Personally, I'd prefer a remote feature flag because we can completely turn it off if there are horrible issues.

kean · 2024-10-17T17:19:01Z

WordPress/Classes/Login/WordPressDotComAuthenticator.swift

+    func signIn(from viewController: UINavigationController) async {
+        let token: String
+        do {
+            token = try await authenticate(from: viewController)


The animation at the top is distracting.

Untitled.mp4

I'm not sure why it looks like this for me. It's different on your recording. I think there is a bug with the first welcome page.

Can we present this login screen as a pushed navigation item instead of a popover?

That'd be more consistent with the current setup

🤔 I don't think there is an API for that. https://developer.apple.com/documentation/authenticationservices/aswebauthenticationsession

Hmm I thought we had the ability to do it in the SwiftUI setup for self-hosted sites, but it's ok – we can dig into that further once we're not relying on a bunch of WPAuthenticator stuff anymore.

I have a few suggestions about this in #23702

Hmm I thought we had the ability to do it in the SwiftUI setup for self-hosted sites

I just checked. The application password flow for self-hosted site also presents Safari view as a modal.

dangermattic · 2024-10-31T01:00:11Z

	1 Warning
⚠️	This PR is assigned to the milestone 25.5. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

kean

LGTM

Still unsure it's a good idea to replace the existing native flow that work OK
Feature flag is missing

crazytonyli · 2024-11-03T20:52:31Z

Feature flag is missing

The remote feature flag has not landed yet. Once it's landed, I'll update the client code.

crazytonyli added 3 commits October 17, 2024 12:58

Consolidate tracking of web-based WP.com login

2212307

Add a clousre to overwrite handling of the WP.com login button

89816af

Replace native WP.com sign in with web-based sign in

3660f1c

crazytonyli added the Login label Oct 17, 2024

crazytonyli added this to the 25.5 milestone Oct 17, 2024

crazytonyli requested review from jkmassel and kean October 17, 2024 00:16

Add a release note

319f7e3

kean reviewed Oct 17, 2024

View reviewed changes

crazytonyli added 3 commits October 18, 2024 10:42

Extract error alert into a function

43409de

Handle a scenario where users deny access on web login

37d8246

Disable web log for UI tests

7e582a0

crazytonyli mentioned this pull request Oct 18, 2024

Move using stored credentials prompt to the WP.com sign in flow #23677

Merged

14 tasks

crazytonyli requested a review from kean October 31, 2024 00:58

Merge branch 'trunk' into task/wpcom-web-based-authentication-rollout

a218717

Merge branch 'trunk' into task/wpcom-web-based-authentication-rollout

7389522

crazytonyli mentioned this pull request Nov 1, 2024

Consolidate sign in process in WordPressDotComAuthenticator #23749

Merged

14 tasks

kean approved these changes Nov 1, 2024

View reviewed changes

crazytonyli added this pull request to the merge queue Nov 3, 2024

Merged via the queue into trunk with commit d6c77da Nov 3, 2024
24 checks passed

crazytonyli deleted the task/wpcom-web-based-authentication-rollout branch November 3, 2024 21:20

Replace native WP.com sign in with web-based sign in #23675

Replace native WP.com sign in with web-based sign in #23675

Uh oh!

Conversation

crazytonyli commented Oct 17, 2024

Regression Notes

Uh oh!

wpmobilebot commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kean left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kean Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dangermattic commented Oct 31, 2024

Uh oh!

kean left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crazytonyli commented Nov 3, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wpmobilebot commented Oct 17, 2024 •

edited

Loading

wpmobilebot commented Oct 17, 2024 •

edited

Loading

kean Oct 17, 2024 •

edited

Loading

kean left a comment •

edited

Loading