Skip to content

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Jun 11, 2025

Description

See https://linear.app/a8c/issue/AINFRA-730

Problem: When moving networking from framework to Swift package, the architecture checks no longer work. This is a problem for the current application password name logic:

private var applicationPasswordName: String {
get async {
#if !os(watchOS)
let bundleIdentifier = Bundle.main.bundleIdentifier ?? "Unknown"
let model = await UIDevice.current.model
let identifierForVendor = await UIDevice.current.identifierForVendor?.uuidString ?? ""
return "\(bundleIdentifier).ios-app-client.\(model).\(identifierForVendor)"
#endif
#if os(watchOS)
return "" // This is not needed on watchOS as the watch does not create application passwords.
#endif
}
}

In practice, this means that when building the watch app we try to access UIDevice which is not available.

I tried replacing #if !os(watchOS) with #if canImport(UIKit) but got the same problem, surprisingly.

Solution: I pushed UIDevice off DefaultApplicationPasswordUseCase, leaving it up to the caller to pass the information. This way, the iOS app target can in a DefaultApplicationPasswordUseCase with the UIDevice data, and the watch code is none the wiser.

See also the documentation comment in the new DeviceModelVersionInfo data transfer object.

Steps to reproduce & Testing information

I'm not too sure how to test this on device, can you help me with that? Notice CI is green in the meantime.

Screenshots


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@mokagio mokagio added this to the 22.7 milestone Jun 11, 2025
@mokagio mokagio self-assigned this Jun 11, 2025
@mokagio mokagio added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Jun 11, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jun 11, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Number30351
VersionPR #15729
Bundle IDcom.automattic.alpha.woocommerce
Commit4adfc35
Installation URL61fjjkhflkhjg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@mokagio mokagio changed the title AINFRA-730 Decouple application password name logic from knowing the OS Jun 11, 2025
@mokagio mokagio force-pushed the ainfra-730-remove-watchos-knowledge-from-applicationusecase branch from 6a4ff81 to d61de8c Compare June 11, 2025 02:24
@mokagio mokagio requested a review from a team June 11, 2025 02:28
@mokagio mokagio marked this pull request as ready for review June 11, 2025 02:28
@itsmeichigo itsmeichigo assigned itsmeichigo and unassigned mokagio Jun 11, 2025
get async {
#if !os(watchOS)
let bundleIdentifier = Bundle.main.bundleIdentifier ?? "Unknown"
#if !os(watchOS)
Copy link
Contributor

Choose a reason for hiding this comment

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

When you mention pushing UIDevice off DefaultApplicationPasswordUseCase, I thought this check for watchOS would be removed and the usage of UIDevice below would be replaced with DeviceModelIdentifierInfo. @mokagio could you please clarify this again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry @itsmeichigo . I simply did not push the rest of the work 🤦‍♂️

@itsmeichigo
Copy link
Contributor

I'm not too sure how to test this on device, can you help me with that? Notice CI is green in the meantime.

DefaultApplicationPasswordUseCase can be tested by trying to log in to a Jurassic Ninja site using site credentials. When creating the JN site, please ensure to disable Jetpack installation on the site to ensure the site credentials login flow succeeds without any security measures blocking it.

@mokagio mokagio force-pushed the ainfra-730-remove-watchos-knowledge-from-applicationusecase branch from d61de8c to 4adfc35 Compare June 11, 2025 05:09
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

I tested site credential login with a JN site and confirmed that the login was successful after this fix:

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-06-11.at.13.15.09.mp4

I also checked with Proxyman and confirmed that the application name was correct.

@mokagio
Copy link
Contributor Author

mokagio commented Jun 11, 2025

@itsmeichigo thank you!

For what is worth, I also tested it and verified the new code gets called and the subsequent API request is successful resulting a login.

image

image

@mokagio mokagio merged commit 36efc72 into trunk Jun 11, 2025
13 checks passed
@mokagio mokagio deleted the ainfra-730-remove-watchos-knowledge-from-applicationusecase branch June 11, 2025 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants