From 399f6a2669afd822efea5483dd4934bf237d1450 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 25 May 2026 10:26:54 -0600 Subject: [PATCH 1/4] Tighten ApplicationPasswordValidator error classification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous classification was destructive on ambiguous outcomes: any WpRequestResult that wasn't Success, the timeout variant of RequestExecutionFailed, or — via `else` — any other variant fell to Outcome.Invalid, which the slice acts on by wiping the stored credentials and attempting a re-mint. That means a transient 5xx, a DeviceIsOfflineError, a parse error, or an unrecognized variant would all destroy working credentials. Invert the default. Outcome.Invalid is now returned only when there's positive evidence the credential was rejected — WpErrorCode.Unauthorized / Forbidden / ApplicationPasswordNotFound / NoAuthenticatedAppPassword, or RequestExecutionErrorReason.HttpAuthenticationRejectedError / HttpAuthenticationRequiredError / HttpForbiddenError. This mirrors the canonical PostRsErrorUtils.isAuthError pattern. Everything else maps to Outcome.NetworkUnavailable, which hides the card and retries next foreground. Also re-throw CancellationException before the generic Exception catch, so coroutine cancellation isn't silently converted to NetworkUnavailable. Adds 18 unit tests covering every relevant WpRequestResult variant and both exception paths — the class previously had none. --- .../ApplicationPasswordValidator.kt | 66 +++-- .../ApplicationPasswordValidatorTest.kt | 248 ++++++++++++++++++ 2 files changed, 295 insertions(+), 19 deletions(-) create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt index 6f4c0e7c3631..d16bb2caa2c9 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt @@ -1,11 +1,13 @@ package org.wordpress.android.ui.mysite.cards.applicationpassword +import kotlinx.coroutines.CancellationException import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.util.AppLog import rs.wordpress.api.kotlin.WpRequestResult import uniffi.wp_api.RequestExecutionErrorReason +import uniffi.wp_api.WpErrorCode import javax.inject.Inject /** @@ -13,11 +15,18 @@ import javax.inject.Inject * direct host. Uses [WpApiClientProvider.getApplicationPasswordClient] so the call exercises the * application password specifically — `getWpApiClient` would route WPCom-flagged sites through the * bearer-token path and would not catch a revoked password. + * + * Classification is intentionally asymmetric: a return of [Outcome.Invalid] cascades into a + * credential wipe + re-mint in the caller, so we only classify as Invalid when we have positive + * evidence the server rejected the credential (auth-specific [WpErrorCode] or + * [RequestExecutionErrorReason]). Everything ambiguous — 5xx, parse errors, offline, DNS — falls + * to [Outcome.NetworkUnavailable], which hides the card and lets the next foreground retry. */ class ApplicationPasswordValidator @Inject constructor( private val wpApiClientProvider: WpApiClientProvider, private val appLogWrapper: AppLogWrapper, ) { + @Suppress("ReturnCount") suspend fun validate(site: SiteModel): Outcome { appLogWrapper.d( AppLog.T.MAIN, @@ -27,25 +36,9 @@ class ApplicationPasswordValidator @Inject constructor( val client = wpApiClientProvider.getApplicationPasswordClient(site) val response = client.request { it.users().retrieveMeWithViewContext() } appLogWrapper.d(AppLog.T.MAIN, "A_P: Validation response: ${response::class.simpleName}") - when (response) { - is WpRequestResult.Success -> { - val user = response.response.data - appLogWrapper.d( - AppLog.T.MAIN, - "A_P: Validation Success returned user id=${user.id}, slug='${user.slug}', name='${user.name}'" - ) - Outcome.Valid - } - is WpRequestResult.WpError -> Outcome.Invalid - is WpRequestResult.UnknownError -> Outcome.Invalid - is WpRequestResult.RequestExecutionFailed -> - if (response.reason is RequestExecutionErrorReason.HttpTimeoutError) { - Outcome.NetworkUnavailable - } else { - Outcome.Invalid - } - else -> Outcome.Invalid - } + classify(response) + } catch (e: CancellationException) { + throw e } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { appLogWrapper.e( AppLog.T.MAIN, @@ -55,5 +48,40 @@ class ApplicationPasswordValidator @Inject constructor( } } + private fun classify(response: WpRequestResult): Outcome = when (response) { + is WpRequestResult.Success -> Outcome.Valid + + is WpRequestResult.WpError -> if (isAuthErrorCode(response.errorCode)) { + Outcome.Invalid + } else { + // Parseable WP error envelope that isn't an auth rejection (e.g. 5xx returned as a + // structured WpError, or a server-side plugin returning a custom code). Ambiguous — + // don't wipe creds. + Outcome.NetworkUnavailable + } + + is WpRequestResult.RequestExecutionFailed -> if (isAuthErrorReason(response.reason)) { + Outcome.Invalid + } else { + // Timeouts, offline, DNS, SSL, generic transport errors — all non-destructive. + Outcome.NetworkUnavailable + } + + // UnknownError (4xx/5xx without parseable WP error JSON), parse errors, and any other + // variants are all ambiguous. Default to non-destructive. + else -> Outcome.NetworkUnavailable + } + + private fun isAuthErrorCode(code: WpErrorCode): Boolean = + code is WpErrorCode.Unauthorized || + code is WpErrorCode.Forbidden || + code is WpErrorCode.ApplicationPasswordNotFound || + code is WpErrorCode.NoAuthenticatedAppPassword + + private fun isAuthErrorReason(reason: RequestExecutionErrorReason): Boolean = + reason is RequestExecutionErrorReason.HttpAuthenticationRejectedError || + reason is RequestExecutionErrorReason.HttpAuthenticationRequiredError || + reason is RequestExecutionErrorReason.HttpForbiddenError + enum class Outcome { Valid, Invalid, NetworkUnavailable } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt new file mode 100644 index 000000000000..e23f60e35235 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt @@ -0,0 +1,248 @@ +package org.wordpress.android.ui.mysite.cards.applicationpassword + +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider +import org.wordpress.android.fluxc.utils.AppLogWrapper +import rs.wordpress.api.kotlin.WpApiClient +import rs.wordpress.api.kotlin.WpRequestResult +import uniffi.wp_api.RequestExecutionErrorReason +import uniffi.wp_api.RequestMethod +import uniffi.wp_api.WpErrorCode + +@ExperimentalCoroutinesApi +class ApplicationPasswordValidatorTest : BaseUnitTest() { + @Mock + lateinit var wpApiClientProvider: WpApiClientProvider + + @Mock + lateinit var appLogWrapper: AppLogWrapper + + @Mock + lateinit var wpApiClient: WpApiClient + + private lateinit var validator: ApplicationPasswordValidator + private lateinit var site: SiteModel + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + validator = ApplicationPasswordValidator(wpApiClientProvider, appLogWrapper) + site = SiteModel().apply { + id = 1 + url = "https://example.com" + apiRestUsernamePlain = "user" + apiRestPasswordPlain = "pass" + } + whenever(wpApiClientProvider.getApplicationPasswordClient(site)).thenReturn(wpApiClient) + } + + @Suppress("UNCHECKED_CAST") + private suspend fun stubResponse(response: WpRequestResult<*>) { + whenever(wpApiClient.request(any())).thenReturn(response as WpRequestResult) + } + + private fun wpError(code: WpErrorCode) = WpRequestResult.WpError( + errorCode = code, + errorMessage = "msg", + statusCode = 401.toUShort(), + response = "", + requestUrl = "https://example.com", + requestMethod = RequestMethod.GET, + ) + + private fun requestFailed(reason: RequestExecutionErrorReason) = + WpRequestResult.RequestExecutionFailed( + statusCode = null, + redirects = null, + reason = reason, + requestUrl = "https://example.com", + requestMethod = RequestMethod.GET, + ) + + // --- Success --- + + @Test + fun `Success maps to Valid`() = runTest { + stubResponse(WpRequestResult.Success(response = Any())) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Valid) + } + + // --- WpError: auth-related codes map to Invalid --- + + @Test + fun `WpError Unauthorized maps to Invalid`() = runTest { + stubResponse(wpError(WpErrorCode.Unauthorized())) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `WpError Forbidden maps to Invalid`() = runTest { + stubResponse(wpError(WpErrorCode.Forbidden())) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `WpError ApplicationPasswordNotFound maps to Invalid`() = runTest { + stubResponse(wpError(WpErrorCode.ApplicationPasswordNotFound())) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `WpError NoAuthenticatedAppPassword maps to Invalid`() = runTest { + stubResponse(wpError(WpErrorCode.NoAuthenticatedAppPassword())) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + // --- WpError: non-auth codes must NOT wipe creds --- + + @Test + fun `WpError InvalidParam maps to NetworkUnavailable (do not wipe creds)`() = runTest { + stubResponse(wpError(WpErrorCode.InvalidParam())) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + // --- RequestExecutionFailed: auth-related reasons map to Invalid --- + + @Test + fun `RequestExecutionFailed HttpAuthenticationRejectedError maps to Invalid`() = runTest { + stubResponse( + requestFailed( + RequestExecutionErrorReason.HttpAuthenticationRejectedError( + hostname = "example.com", + method = null, + ) + ) + ) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `RequestExecutionFailed HttpAuthenticationRequiredError maps to Invalid`() = runTest { + stubResponse( + requestFailed( + RequestExecutionErrorReason.HttpAuthenticationRequiredError( + hostname = "example.com", + method = null, + ) + ) + ) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `RequestExecutionFailed HttpForbiddenError maps to Invalid`() = runTest { + stubResponse( + requestFailed(RequestExecutionErrorReason.HttpForbiddenError(hostname = "example.com")) + ) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + // --- RequestExecutionFailed: non-auth reasons must NOT wipe creds --- + + @Test + fun `RequestExecutionFailed HttpTimeoutError maps to NetworkUnavailable`() = runTest { + stubResponse(requestFailed(RequestExecutionErrorReason.HttpTimeoutError)) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test + fun `RequestExecutionFailed DeviceIsOfflineError maps to NetworkUnavailable`() = runTest { + stubResponse( + requestFailed(RequestExecutionErrorReason.DeviceIsOfflineError(errorMessage = "off")) + ) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test + fun `RequestExecutionFailed NonExistentSiteError maps to NetworkUnavailable`() = runTest { + stubResponse( + requestFailed( + RequestExecutionErrorReason.NonExistentSiteError( + errorMessage = null, + suggestedAction = null, + ) + ) + ) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test + fun `RequestExecutionFailed HttpError maps to NetworkUnavailable`() = runTest { + stubResponse(requestFailed(RequestExecutionErrorReason.HttpError(reason = "boom"))) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + // --- Other variants: all default to NetworkUnavailable --- + + @Test + fun `UnknownError (5xx without parseable JSON) maps to NetworkUnavailable`() = runTest { + stubResponse( + WpRequestResult.UnknownError( + statusCode = 503.toUShort(), + response = "Service Unavailable", + requestUrl = "https://example.com", + requestMethod = RequestMethod.GET, + ) + ) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test + fun `InvalidHttpStatusCode maps to NetworkUnavailable`() = runTest { + stubResponse( + WpRequestResult.InvalidHttpStatusCode( + statusCode = 999.toUShort(), + requestUrl = "https://example.com", + requestMethod = RequestMethod.GET, + ) + ) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test + fun `ResponseParsingError maps to NetworkUnavailable`() = runTest { + stubResponse( + WpRequestResult.ResponseParsingError( + reason = "bad json", + response = "not json", + requestUrl = "https://example.com", + requestMethod = RequestMethod.GET, + ) + ) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + // --- Exception handling --- + + @Test + fun `generic Exception maps to NetworkUnavailable`() = runTest { + whenever(wpApiClient.request(any())).thenThrow(RuntimeException("boom")) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test(expected = CancellationException::class) + fun `CancellationException is rethrown, not swallowed`() = runTest { + whenever(wpApiClient.request(any())).thenThrow(CancellationException("cancelled")) + validator.validate(site) + } +} From 9221d2e4cdcd1181bb5bc1ff3142f39f87fd1f7f Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 25 May 2026 10:28:37 -0600 Subject: [PATCH 2/4] Clear DB credential columns in deleteStoredApplicationPasswordCredentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, the method cleared only the encrypted SharedPreferences cache (`ApplicationPasswordsStore`) and left the SiteModel's `apiRest*` columns untouched. Combined with the slice's "validate → wipe → re-mint" loop, that meant: if validation returns Invalid and the subsequent mint fails (network down, server unreachable, etc.), the next buildCard reloads the site from DB, `decryptAPIRestCredentials` resurrects the stale plain fields, `siteHasBadCredentials` reports false, and validation runs again with the same revoked password — looping until either the mint eventually succeeds or the user re-authenticates via the banner. The XML-RPC rediscovery path (`ApplicationPasswordViewModelSlice.attemptXmlRpcRediscovery`) also reads `site.apiRestUsernamePlain` / `Password` directly and 401s when they're stale. Fix: also clear the SiteModel's plain + encrypted credential columns, using the same fetch-fresh-from-DB / mutate / save pattern as `removeApplicationPassword`, and emit `OnSiteChanged` so observers see the credential state change. `wpApiRestUrl` is deliberately preserved — it's reusable across credential rotations and clearing it would force a re-discovery on every revoke. Both clears happen: the encrypted prefs (so `ApplicationPasswordsManager` doesn't short-circuit the next mint by returning `Existing`) and the DB columns (so the validator and other readers see the cleared state). --- .../android/fluxc/store/SiteStore.kt | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index de9ecba91a4c..b73dcaf4dc71 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -2434,12 +2434,47 @@ open class SiteStore @Inject constructor( } /** - * Clears stored application-password credentials (encrypted prefs) so the next call to + * Clears stored application-password credentials so the next call to * [createApplicationPassword] mints a fresh one. Use this when a validation call against the * stored credentials has failed (e.g. 401, password revoked server-side). + * + * Both storage locations are cleared: + * - The encrypted SharedPreferences cache (`ApplicationPasswordsStore`), so + * [ApplicationPasswordsManager] doesn't short-circuit the next mint by returning + * `Existing` with the revoked password. + * - The SiteModel's credential columns (`apiRest*`), so subsequent reads — validation + * loops, XML-RPC rediscovery, etc. — don't see the stale credentials. + * + * `wpApiRestUrl` is intentionally preserved. Unlike [removeApplicationPassword] (full sign-out), + * the discovered REST URL is reusable across credential rotations; clearing it would force a + * re-discovery on every refresh after a revoke. */ fun deleteStoredApplicationPasswordCredentials(site: SiteModel) { applicationPasswordsManagerProvider.get().deleteLocalApplicationPassword(site) + emitChange(clearApplicationPasswordColumns(site)) + } + + @Suppress("SwallowedException") + private fun clearApplicationPasswordColumns(siteModel: SiteModel): OnSiteChanged { + return try { + val siteFromDB = getSiteByLocalId(siteModel.id) + ?: return OnSiteChanged(SiteError(SiteErrorType.INVALID_SITE)) + // Clear both Plain and Encrypted columns. `SiteSqlUtils.encryptAPIRestCredentials` + // short-circuits when the encrypted columns are non-empty, so clearing only the plain + // values would leave the stale ciphertext in place and `decryptAPIRestCredentials` + // would resurrect them on the next read. + siteFromDB.apply { + apiRestUsernamePlain = "" + apiRestPasswordPlain = "" + apiRestUsernameEncrypted = "" + apiRestPasswordEncrypted = "" + apiRestUsernameIV = "" + apiRestPasswordIV = "" + } + OnSiteChanged(siteSqlUtils.insertOrUpdateSite(siteFromDB)) + } catch (e: DuplicateSiteException) { + OnSiteChanged(SiteError(DUPLICATE_SITE)) + } } private fun persistApplicationPasswordCredentials( From 1ade16753976cf3c082a3b2e13b643744df9bd75 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 25 May 2026 10:55:03 -0600 Subject: [PATCH 3/4] Use HTTP status code, not just code name, for WpError auth detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous classifier matched a fixed allowlist of WpErrorCode variants — Unauthorized, Forbidden, ApplicationPasswordNotFound, NoAuthenticatedAppPassword — to decide whether a WpError represented an auth rejection. That missed the most common case in practice: WordPress returns `{"code": "incorrect_password", ...}` for a revoked application password (Basic auth rejection), and wordpress-rs parses that into `WpErrorCode.CustomError("incorrect_password")` via its untagged-string fallback. None of our name-based matchers caught CustomError, so the validator returned Outcome.NetworkUnavailable and the slice silently hid the My Site card instead of triggering the wipe + re-mint recovery flow. Reproduced on an Atomic site with a server-side-revoked application password: A_P: Validating application password for ... as user='jkmassel' A_P: Validation response: WpError A_P: Validation network error for ... (The "list" screen on the same site surfaces the same 401 as "The provided password is an invalid application password.") Fix: also classify by status code. Any parseable WpError with a 401 or 403 status is, by definition, an auth rejection. The error-code name remains a useful fast path for unusual cases (a Forbidden code without a 401/403 status, for example), but is no longer the only signal. This sidesteps the long tail of plugin-defined and untagged error codes WordPress can emit on auth failure. Adds 3 status-code-based tests and adjusts an existing fixture so the non-auth-code path isn't accidentally exercised with an auth status. --- .../ApplicationPasswordValidator.kt | 24 ++++++++++--- .../ApplicationPasswordValidatorTest.kt | 35 ++++++++++++++++--- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt index d16bb2caa2c9..b6aa80e9fb75 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt @@ -51,12 +51,20 @@ class ApplicationPasswordValidator @Inject constructor( private fun classify(response: WpRequestResult): Outcome = when (response) { is WpRequestResult.Success -> Outcome.Valid - is WpRequestResult.WpError -> if (isAuthErrorCode(response.errorCode)) { + // A WpError is the server returning a parseable error envelope. Treat any 401/403 as an + // auth rejection regardless of the WpErrorCode value — WordPress emits a wide variety of + // codes for credential failures (`incorrect_password`, `invalid_username`, + // `application_passwords_disabled_for_user`, plugin-defined codes, etc.) and many of them + // get parsed as `WpErrorCode.CustomError` via the library's untagged fallback. Status + // code is the reliable signal. Also include a few non-auth-status codes that we know + // mean the credential is unusable. + is WpRequestResult.WpError -> if ( + isAuthErrorCode(response.errorCode) || isAuthStatusCode(response.statusCode) + ) { Outcome.Invalid } else { - // Parseable WP error envelope that isn't an auth rejection (e.g. 5xx returned as a - // structured WpError, or a server-side plugin returning a custom code). Ambiguous — - // don't wipe creds. + // Parseable error envelope without an auth-rejection signal (e.g. a 5xx returned as + // a structured WpError). Ambiguous — don't wipe creds. Outcome.NetworkUnavailable } @@ -78,10 +86,18 @@ class ApplicationPasswordValidator @Inject constructor( code is WpErrorCode.ApplicationPasswordNotFound || code is WpErrorCode.NoAuthenticatedAppPassword + private fun isAuthStatusCode(statusCode: UShort): Boolean = + statusCode.toInt() == HTTP_UNAUTHORIZED || statusCode.toInt() == HTTP_FORBIDDEN + private fun isAuthErrorReason(reason: RequestExecutionErrorReason): Boolean = reason is RequestExecutionErrorReason.HttpAuthenticationRejectedError || reason is RequestExecutionErrorReason.HttpAuthenticationRequiredError || reason is RequestExecutionErrorReason.HttpForbiddenError + companion object { + private const val HTTP_UNAUTHORIZED = 401 + private const val HTTP_FORBIDDEN = 403 + } + enum class Outcome { Valid, Invalid, NetworkUnavailable } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt index e23f60e35235..b5e8ae311807 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt @@ -52,10 +52,10 @@ class ApplicationPasswordValidatorTest : BaseUnitTest() { whenever(wpApiClient.request(any())).thenReturn(response as WpRequestResult) } - private fun wpError(code: WpErrorCode) = WpRequestResult.WpError( + private fun wpError(code: WpErrorCode, statusCode: Int = 401) = WpRequestResult.WpError( errorCode = code, errorMessage = "msg", - statusCode = 401.toUShort(), + statusCode = statusCode.toUShort(), response = "", requestUrl = "https://example.com", requestMethod = RequestMethod.GET, @@ -107,8 +107,35 @@ class ApplicationPasswordValidatorTest : BaseUnitTest() { // --- WpError: non-auth codes must NOT wipe creds --- @Test - fun `WpError InvalidParam maps to NetworkUnavailable (do not wipe creds)`() = runTest { - stubResponse(wpError(WpErrorCode.InvalidParam())) + fun `WpError non-auth code with non-auth status maps to NetworkUnavailable`() = runTest { + // Non-401/403 status with an unrelated WpErrorCode: ambiguous, don't wipe creds. + stubResponse(wpError(WpErrorCode.InvalidParam(), statusCode = 400)) + assertThat(validator.validate(site)) + .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + } + + @Test + fun `WpError with 401 status maps to Invalid regardless of code`() = runTest { + // WordPress emits a wide range of WpErrorCodes for credential rejections (e.g. + // `incorrect_password`, `invalid_username`, plugin-defined codes). Many fall through + // to wordpress-rs's untagged-string fallback and aren't recognized as auth codes by + // name. The status code is the reliable signal — a parseable WpError with 401/403 is + // always an auth rejection regardless of which WpErrorCode it carries. + stubResponse(wpError(WpErrorCode.InvalidParam(), statusCode = 401)) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `WpError with 403 status maps to Invalid regardless of code`() = runTest { + stubResponse(wpError(WpErrorCode.InvalidParam(), statusCode = 403)) + assertThat(validator.validate(site)).isEqualTo(ApplicationPasswordValidator.Outcome.Invalid) + } + + @Test + fun `WpError with 500 status maps to NetworkUnavailable`() = runTest { + // A server returning a structured WpError on 5xx is unusual but possible. Without an + // auth-status signal and without an auth code, treat it as transient. + stubResponse(wpError(WpErrorCode.InvalidParam(), statusCode = 500)) assertThat(validator.validate(site)) .isEqualTo(ApplicationPasswordValidator.Outcome.NetworkUnavailable) } From e57042095696b301f3664df175ab55c47314a567 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 25 May 2026 11:23:39 -0600 Subject: [PATCH 4/4] Adapt application-password validator to wordpress-rs UInt status codes The wordpress-rs bump that landed on trunk (fc05d3bb3d9) changes WpError.statusCode / UnknownError.statusCode / InvalidHttpStatusCode.statusCode from `UShort` to `UInt`. Update the validator's `isAuthStatusCode` parameter and the test fixtures accordingly. No behavior change. --- .../applicationpassword/ApplicationPasswordValidator.kt | 2 +- .../applicationpassword/ApplicationPasswordValidatorTest.kt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt index b6aa80e9fb75..6a219ee37047 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidator.kt @@ -86,7 +86,7 @@ class ApplicationPasswordValidator @Inject constructor( code is WpErrorCode.ApplicationPasswordNotFound || code is WpErrorCode.NoAuthenticatedAppPassword - private fun isAuthStatusCode(statusCode: UShort): Boolean = + private fun isAuthStatusCode(statusCode: UInt): Boolean = statusCode.toInt() == HTTP_UNAUTHORIZED || statusCode.toInt() == HTTP_FORBIDDEN private fun isAuthErrorReason(reason: RequestExecutionErrorReason): Boolean = diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt index b5e8ae311807..873f2619b794 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt @@ -55,7 +55,7 @@ class ApplicationPasswordValidatorTest : BaseUnitTest() { private fun wpError(code: WpErrorCode, statusCode: Int = 401) = WpRequestResult.WpError( errorCode = code, errorMessage = "msg", - statusCode = statusCode.toUShort(), + statusCode = statusCode.toUInt(), response = "", requestUrl = "https://example.com", requestMethod = RequestMethod.GET, @@ -221,7 +221,7 @@ class ApplicationPasswordValidatorTest : BaseUnitTest() { fun `UnknownError (5xx without parseable JSON) maps to NetworkUnavailable`() = runTest { stubResponse( WpRequestResult.UnknownError( - statusCode = 503.toUShort(), + statusCode = 503.toUInt(), response = "Service Unavailable", requestUrl = "https://example.com", requestMethod = RequestMethod.GET, @@ -235,7 +235,7 @@ class ApplicationPasswordValidatorTest : BaseUnitTest() { fun `InvalidHttpStatusCode maps to NetworkUnavailable`() = runTest { stubResponse( WpRequestResult.InvalidHttpStatusCode( - statusCode = 999.toUShort(), + statusCode = 999.toUInt(), requestUrl = "https://example.com", requestMethod = RequestMethod.GET, )