CMM-942 auto generate application password with cookies nonce authentication #22352
Conversation
…word-with-CookiesNonceAuthentication
Generated by 🚫 Danger |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
.../android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt
Outdated
Show resolved
Hide resolved
| } 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.
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.
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.
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.
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.
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.
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-20 09:42:33.583709188 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/head_manifest.txt 2025-11-20 09:42:41.573731527 +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/23944/canvas?sid=019aa0a0-ecbc-44d8-9273-44f57a25b160, click on the |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/base_manifest.txt 2025-11-20 09:42:28.001517525 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/head_manifest.txt 2025-11-20 09:42:36.061526549 +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/23944/canvas?sid=019aa0a0-ecbd-47c1-bc8d-c20456dc674a, 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.1 (*)
-| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
-| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.1 (*)
-| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.3 (*)
-| | \--- 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.1 (*)
-| | +--- 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.1 (*)
+| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.1 (*)
+| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.3 (*)
+| | \--- 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.1 (*)
+| | +--- 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-658258c | |
| Commit | 658258c | |
| Direct Download | jetpack-prototype-build-pr22352-658258c.apk |
|
| App Name | WordPress | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22352-658258c | |
| Commit | 658258c | |
| Direct Download | wordpress-prototype-build-pr22352-658258c.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.
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.05% 39.02% -0.04%
==========================================
Files 2201 2203 +2
Lines 106169 106304 +135
Branches 15048 15057 +9
==========================================
+ Hits 41469 41489 +20
- Misses 61211 61326 +115
Partials 3489 3489 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dcalhoun
left a comment
dcalhoun
left a comment
There was a problem hiding this comment.
I tested successfully crating an application password, as well as failure when a network connection was unavailable. Both functioned as expected. Well done!
...fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt
Outdated
Show resolved
Hide resolved
...s/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogActivity.kt
Show resolved
Hide resolved
…est/wpapi/rs/WpApiClientProvider.kt Co-authored-by: David Calhoun <github@davidcalhoun.me>
…word-with-CookiesNonceAuthentication
…word-with-CookiesNonceAuthentication
|
adalpari
deleted the
feat/CMM-942-auto-generate-Application-Password-with-CookiesNonceAuthentication
branch
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