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..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 @@ -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,56 @@ class ApplicationPasswordValidator @Inject constructor( } } + private fun classify(response: WpRequestResult): Outcome = when (response) { + is WpRequestResult.Success -> Outcome.Valid + + // 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 error envelope without an auth-rejection signal (e.g. a 5xx returned as + // a structured WpError). 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 isAuthStatusCode(statusCode: UInt): 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 new file mode 100644 index 000000000000..873f2619b794 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordValidatorTest.kt @@ -0,0 +1,275 @@ +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, statusCode: Int = 401) = WpRequestResult.WpError( + errorCode = code, + errorMessage = "msg", + statusCode = statusCode.toUInt(), + 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 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) + } + + // --- 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.toUInt(), + 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.toUInt(), + 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) + } +} 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(