Skip to content

Conversation

@adalpari
Copy link
Contributor

Description

This PR is offering the user the option of auto-generate an Application Password without requiring the user to log in again in the web view. To achieve that, we are using the current credentials to create and use an authenticated cookie.

Testing instructions

  1. Log into a self-hosted site
  2. Enable Application Password experimental feature
  3. Go to MySite screen and tap on "Authenticate using Application Password" card
  • Follow the flow, and check the AP is created by opening the "Media" screen for instance
Screen_recording_20251117_133029.mp4

@adalpari
Copy link
Contributor Author

@claude

@dangermattic
Copy link
Collaborator

dangermattic commented Nov 17, 2025

3 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

}
when (response) {
is WpRequestResult.Success -> {
val name = site.username // This should be the response name, but it's retuning a wrong value
Copy link

Choose a reason for hiding this comment

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

Critical Bug: This comment indicates a known issue but doesn't fix it. The username should come from the API response:

val name = response.response.data.username ?: site.username

Using the wrong username could cause authentication failures. Please investigate why the API response username is incorrect, or use a fallback as shown above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a note on how the endpoint works. I'm not using the wrong username

Comment on lines +83 to +85
} catch (e: Exception) {
appLogWrapper.e(AppLog.T.API, "Exception creating application password: ${e.message}")
_navigationEvent.emit(NavigationEvent.Error)
Copy link

Choose a reason for hiding this comment

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

Error Handling: Catching all exceptions with a generic error message makes debugging difficult. Consider:

  1. Catching specific exception types (NetworkException, AuthException, etc.)
  2. Providing user-friendly error messages based on the exception type
  3. Logging the full stack trace: appLogWrapper.e(AppLog.T.API, "Exception creating application password", e)

This will help both users understand what went wrong and developers debug production issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Users cannot take any action if the creation fails. We will redirect them to the webview flow

Comment on lines +61 to +66
val cookiesNonceProvider = CookiesNonceAuthenticationProvider.withSiteUrl(
url = site.url,
username = site.username,
password = site.password,
requestExecutor = requestExecutor
)
Copy link

Choose a reason for hiding this comment

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

Security Concern: This code uses the user's primary WordPress credentials (site.username and site.password) to create authentication cookies.

Questions to consider:

  1. Are these passwords encrypted at rest in the SiteModel?
  2. Should we add validation to ensure these fields are not empty?
  3. Should we add explicit user consent/warning that their main password will be used?

Recommendation:

fun getWpApiClientCookiesNonceAuthentication(site: SiteModel): WpApiClient {
    require(site.username.isNotBlank()) { "Site username is required for cookie authentication" }
    require(site.password.isNotBlank()) { "Site password is required for cookie authentication" }
    // ... rest of implementation
}

Also, consider adding KDoc explaining when this method should be used vs. the regular Application Password method.

Copy link
Contributor Author

@adalpari adalpari Nov 17, 2025

Choose a reason for hiding this comment

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

Security Concern

This is how the current flow work. So, we have no ther option than using the plan credentilals.

Should we add validation to ensure these fields are not empty?

Done!

requestBuilder.applicationPasswords().createForCurrentUser(
params = ApplicationPasswordCreateParams(
appId = appId.uuidString(),
name = "$appName-${System.currentTimeMillis()}"
Copy link

Choose a reason for hiding this comment

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

Naming: Using a timestamp makes the Application Password names hard to read in the WordPress admin. Consider a more user-friendly format:

name = "$appName-${Build.MODEL}-${SimpleDateFormat("yyyy-MM-dd", Locale.US).format(Date())}"
// Example: "android-wordpress-client-Pixel-7-2025-11-17"

This makes it easier for users to identify and manage their Application Passwords.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good catch! Done!

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 17, 2025

Project manifest changes for WordPress

The following changes in the WordPress's merged AndroidManifest.xml file were detected (build variant: wordpressVanillaRelease):

--- ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/base_manifest.txt	2025-11-17 13:57:15.769354158 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/head_manifest.txt	2025-11-17 13:57:24.699381245 +0000
@@ -224,6 +224,10 @@
             android:exported="false"
             android:theme="@style/WordPress.TransparentDialog" />
         <activity
+            android:name="org.wordpress.android.ui.accounts.login.applicationpassword.ApplicationPasswordAutoAuthDialogActivity"
+            android:exported="false"
+            android:theme="@style/WordPress.TransparentDialog" />
+        <activity
             android:name="org.wordpress.android.ui.accounts.LoginMagicLinkInterceptActivity"
             android:exported="true"
             android:theme="@style/NoDisplay" >

Go to https://buildkite.com/automattic/wordpress-android/builds/23914/canvas?sid=019a9217-3320-403a-b7ff-b54d139192c3, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 17, 2025

Project manifest changes for WordPress

The following changes in the WordPress's merged AndroidManifest.xml file were detected (build variant: jetpackVanillaRelease):

--- ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/base_manifest.txt	2025-11-17 13:57:34.769844750 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/head_manifest.txt	2025-11-17 13:57:44.409842404 +0000
@@ -358,6 +358,10 @@
             android:exported="false"
             android:theme="@style/WordPress.TransparentDialog" />
         <activity
+            android:name="org.wordpress.android.ui.accounts.login.applicationpassword.ApplicationPasswordAutoAuthDialogActivity"
+            android:exported="false"
+            android:theme="@style/WordPress.TransparentDialog" />
+        <activity
             android:name="org.wordpress.android.ui.accounts.LoginMagicLinkInterceptActivity"
             android:exported="true"
             android:theme="@style/NoDisplay" >

Go to https://buildkite.com/automattic/wordpress-android/builds/23914/canvas?sid=019a9217-3321-47c8-a230-31e820d31d44, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Contributor

Project dependencies changes

list
! Upgraded Dependencies
rs.wordpress.api:android:trunk-3d2dafdc1f8b058b4ed9101673fdf690671da73c, (changed from trunk-fb107b497caaf2b1f4ffcf9f487784792561a645)
rs.wordpress.api:kotlin:trunk-3d2dafdc1f8b058b4ed9101673fdf690671da73c, (changed from trunk-fb107b497caaf2b1f4ffcf9f487784792561a645)
tree
 +--- project :libs:fluxc
-|    \--- rs.wordpress.api:android:trunk-fb107b497caaf2b1f4ffcf9f487784792561a645
-|         +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.0 (*)
-|         +--- com.squareup.okhttp3:okhttp-tls:4.12.0
-|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.0 (*)
-|         |    +--- com.squareup.okio:okio:3.6.0 -> 3.16.2 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.24 (*)
-|         +--- net.java.dev.jna:jna:5.18.1
-|         +--- rs.wordpress.api:kotlin:trunk-fb107b497caaf2b1f4ffcf9f487784792561a645
-|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.0 (*)
-|         |    +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
-|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
-|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
+|    \--- rs.wordpress.api:android:trunk-3d2dafdc1f8b058b4ed9101673fdf690671da73c
+|         +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.0 (*)
+|         +--- com.squareup.okhttp3:okhttp-tls:4.12.0
+|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.0 (*)
+|         |    +--- com.squareup.okio:okio:3.6.0 -> 3.16.2 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.24 (*)
+|         +--- net.java.dev.jna:jna:5.18.1
+|         +--- rs.wordpress.api:kotlin:trunk-3d2dafdc1f8b058b4ed9101673fdf690671da73c
+|         |    +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.0 (*)
+|         |    +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
+|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
+|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
-\--- rs.wordpress.api:android:trunk-fb107b497caaf2b1f4ffcf9f487784792561a645 (*)
+\--- rs.wordpress.api:android:trunk-3d2dafdc1f8b058b4ed9101673fdf690671da73c (*)

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 17, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
FlavorJalapeno
Build TypeDebug
Versionpr22352-69dcf6e
Commit69dcf6e
Direct Downloadjetpack-prototype-build-pr22352-69dcf6e.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 17, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
FlavorJalapeno
Build TypeDebug
Versionpr22352-69dcf6e
Commit69dcf6e
Direct Downloadwordpress-prototype-build-pr22352-69dcf6e.apk
Note: Google Login is not supported on these builds.

@wordpress-mobile wordpress-mobile deleted a comment from claude bot Nov 17, 2025
…okiesNonceAuthentication' of https://github.com/wordpress-mobile/WordPress-Android into feat/CMM-942-auto-generate-Application-Password-with-CookiesNonceAuthentication
suspend fun loadConversations(): List<BotConversation> = withContext(ioDispatcher) {
val response = wpComApiClient.request { requestBuilder ->
requestBuilder.supportBots().getBotConverationList(BOT_ID)
requestBuilder.supportBots().getBotConversationList(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes have been done because after updating the RS library, the method signature changed. So, I needed to change the call to keeo the project working. No, need to open a new PR for this small change.

@adalpari adalpari marked this pull request as ready for review November 17, 2025 13:53
@adalpari adalpari requested a review from a team as a code owner November 17, 2025 13:53
@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

❌ Patch coverage is 17.48252% with 118 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.09%. Comparing base (7d993c6) to head (69dcf6e).

Files with missing lines Patch % Lines
...sword/ApplicationPasswordAutoAuthDialogActivity.kt 0.00% 52 Missing ⚠️
...word/ApplicationPasswordAutoAuthDialogViewModel.kt 45.45% 24 Missing ⚠️
...fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt 0.00% 23 Missing ⚠️
...org/wordpress/android/ui/mysite/MySiteViewModel.kt 0.00% 13 Missing ⚠️
...support/aibot/repository/AIBotSupportRepository.kt 33.33% 4 Missing ⚠️
...ordpress/android/ui/mysite/SiteNavigationAction.kt 0.00% 1 Missing ⚠️
...ationpassword/ApplicationPasswordViewModelSlice.kt 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22352      +/-   ##
==========================================
- Coverage   39.12%   39.09%   -0.04%     
==========================================
  Files        2205     2207       +2     
  Lines      106157   106292     +135     
  Branches    15049    15058       +9     
==========================================
+ Hits        41536    41556      +20     
- Misses      61129    61244     +115     
  Partials     3492     3492              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants