-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CMM-942 auto generate application password with cookies nonce authentication #22352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
CMM-942 auto generate application password with cookies nonce authentication #22352
Conversation
…word-with-CookiesNonceAuthentication
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 |
There was a problem hiding this comment.
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.usernameUsing the wrong username could cause authentication failures. Please investigate why the API response username is incorrect, or use a fallback as shown above.
There was a problem hiding this comment.
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
| } catch (e: Exception) { | ||
| appLogWrapper.e(AppLog.T.API, "Exception creating application password: ${e.message}") | ||
| _navigationEvent.emit(NavigationEvent.Error) |
There was a problem hiding this comment.
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:
- Catching specific exception types (NetworkException, AuthException, etc.)
- Providing user-friendly error messages based on the exception type
- 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.
There was a problem hiding this comment.
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
| val cookiesNonceProvider = CookiesNonceAuthenticationProvider.withSiteUrl( | ||
| url = site.url, | ||
| username = site.username, | ||
| password = site.password, | ||
| requestExecutor = requestExecutor | ||
| ) |
There was a problem hiding this comment.
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:
- Are these passwords encrypted at rest in the
SiteModel? - Should we add validation to ensure these fields are not empty?
- 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.
There was a problem hiding this comment.
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!
...fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt
Show resolved
Hide resolved
...s/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogActivity.kt
Show resolved
Hide resolved
| requestBuilder.applicationPasswords().createForCurrentUser( | ||
| params = ApplicationPasswordCreateParams( | ||
| appId = appId.uuidString(), | ||
| name = "$appName-${System.currentTimeMillis()}" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
Project manifest changes for WordPressThe following changes in the --- ./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 |
Project manifest changes for WordPressThe following changes in the --- ./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 |
Project dependencies changeslist! 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 (*) |
|
| App Name | Jetpack | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22352-69dcf6e | |
| Commit | 69dcf6e | |
| Direct Download | jetpack-prototype-build-pr22352-69dcf6e.apk |
|
| App Name | WordPress | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22352-69dcf6e | |
| Commit | 69dcf6e | |
| Direct Download | wordpress-prototype-build-pr22352-69dcf6e.apk |
…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( |
There was a problem hiding this comment.
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.
|
Codecov Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|





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
Screen_recording_20251117_133029.mp4