From 5d96ac8aebb0333e0d40d7ceaf829b3151b6fc1e Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 17 Jun 2026 23:09:08 -0700 Subject: [PATCH 1/2] fix(piggycards): harden token re-login (mirror PR #1492 CTX fixes) Apply the same auth hardening to PiggyCards that PR #1492 applied to CTX: - Move PiggyCardsAuthenticator.tokenMutex to the companion object so it is shared across every instance (the OkHttp retry path and the factory-built instance used by the repository), serializing concurrent re-logins against the single shared token store instead of racing on separate per-instance locks. - Funnel all re-logins through PiggyCardsAuthenticator.reLogin(): the repository's performAutoLogin() now delegates to it, so the repo and OkHttp paths share the same lock and token persistence. - Preserve the cached token on transient failures; only a genuine HTTP 401 (credential rejection, surfaced as CTXSpendException) clears it. Previously any exception wiped the access token. - Add a responseCount guard to authenticate() to avoid retry loops. Note: Problem 1 from PR #1492 (verifyEmail mis-reporting success as "invalid code") is already fixed for PiggyCards by that PR, since the routing lives in the shared DashSpendUserAuthFragment. Adds PiggyCardsAuthenticatorTest covering serialized re-login, token preservation on transient failure, and token clearing on a genuine 401. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../network/PiggyCardsRemoteDataSource.kt | 2 +- .../authenticator/PiggyCardsAuthenticator.kt | 84 ++++++---- .../repository/DashSpendRepositoryFactory.kt | 5 +- .../repository/PiggyCardsRepository.kt | 36 +---- .../PiggyCardsAuthenticatorTest.kt | 143 ++++++++++++++++++ 5 files changed, 210 insertions(+), 60 deletions(-) create mode 100644 features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt diff --git a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/PiggyCardsRemoteDataSource.kt b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/PiggyCardsRemoteDataSource.kt index 866426d879..0cd6a1a354 100644 --- a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/PiggyCardsRemoteDataSource.kt +++ b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/PiggyCardsRemoteDataSource.kt @@ -58,7 +58,7 @@ class PiggyCardsRemoteDataSource @Inject constructor( .create(api) } - private fun buildTokenApi(): PiggyCardsTokenApi { + fun buildTokenApi(): PiggyCardsTokenApi { return Retrofit.Builder() .baseUrl( if (walletData.networkParameters.id == NetworkParameters.ID_MAINNET) { diff --git a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/authenticator/PiggyCardsAuthenticator.kt b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/authenticator/PiggyCardsAuthenticator.kt index 4c6b1d3867..add44c742c 100644 --- a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/authenticator/PiggyCardsAuthenticator.kt +++ b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/authenticator/PiggyCardsAuthenticator.kt @@ -24,8 +24,8 @@ import okhttp3.Request import okhttp3.Response import okhttp3.Route import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.LoginRequest -import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.LoginResponse import org.dash.wallet.features.exploredash.network.service.piggycards.PiggyCardsTokenApi +import org.dash.wallet.features.exploredash.repository.CTXSpendException import org.dash.wallet.features.exploredash.utils.PiggyCardsConfig import org.slf4j.LoggerFactory import java.time.LocalDateTime @@ -39,53 +39,79 @@ class PiggyCardsAuthenticator @Inject constructor( companion object { private val log = LoggerFactory.getLogger(PiggyCardsAuthenticator::class.java) - } - // For multiple call to refresh token sync - private val tokenMutex = Mutex() + // Shared across every PiggyCardsAuthenticator instance (the OkHttp retry path plus the + // factory-built instance used by the repository) so concurrent re-logins are serialized + // against the single shared token store, instead of racing on separate per-instance locks. + private val tokenMutex = Mutex() + } override fun authenticate(route: Route?, response: Response): Request? { + if (response.responseCount >= 2) { + return null + } + return runBlocking { - tokenMutex.withLock { - try { - val loginResponse = refreshToken() - if (loginResponse != null) { - handleLoginResponse(loginResponse) - response.request.newBuilder() - .header("Authorization", "Bearer ${loginResponse.accessToken}") - .build() - } else { - null - } - } catch (e: Exception) { - log.error("Failed to refresh token: ${e.message}", e) - config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, "") - null - } + reLogin()?.let { accessToken -> + response.request.newBuilder() + .header("Authorization", "Bearer $accessToken") + .build() } } } - private suspend fun refreshToken(): LoginResponse? { + /** + * Re-authenticates with the stored credentials under a process-wide lock and persists the new + * access token, so the OkHttp retry path and [org.dash.wallet.features.exploredash.repository.PiggyCardsRepository] + * never issue overlapping logins. + * + * @return the new access token, or null when no credentials are stored or the login failed. + * Stored credentials are preserved on transient failures so the session survives; only a + * genuine rejection (HTTP 401) clears the cached access token. + */ + suspend fun reLogin(): String? = tokenMutex.withLock { val userId = config.getSecuredData(PiggyCardsConfig.PREFS_KEY_USER_ID) val password = config.getSecuredData(PiggyCardsConfig.PREFS_KEY_PASSWORD) - return if (!userId.isNullOrBlank() && !password.isNullOrBlank()) { - tokenApi.login(LoginRequest(userId = userId, password = password)) - } else { + if (userId.isNullOrBlank() || password.isNullOrBlank()) { + return@withLock null + } + + try { + val response = tokenApi.login(LoginRequest(userId = userId, password = password)) + persistLogin(response.accessToken, response.expiresIn) + response.accessToken.takeIf { it.isNotBlank() } + } catch (e: Exception) { + log.error("Failed to refresh token: ${e.message}", e) + // Only a genuine credential rejection drops the cached token. The login endpoint + // returns HTTP 401 for that case, surfaced by ErrorHandlingInterceptor as a + // CTXSpendException; any other failure is transient, so the token is kept and the + // next request retries instead of forcing the user to sign in again. + if (e is CTXSpendException && e.errorCode == 401) { + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, "") + } null } } - private suspend fun handleLoginResponse( - response: LoginResponse - ) { - config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, response.accessToken) + private suspend fun persistLogin(accessToken: String, expiresIn: Int) { + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, accessToken) - val expiresAt = LocalDateTime.now().plusSeconds(response.expiresIn.toLong()) + val expiresAt = LocalDateTime.now().plusSeconds(expiresIn.toLong()) config.setSecuredData( PiggyCardsConfig.PREFS_KEY_TOKEN_EXPIRES_AT, expiresAt.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME) ) } + + private val Response.responseCount: Int + get() { + var result = 1 + var priorResponse = this.priorResponse + while (priorResponse != null) { + result++ + priorResponse = priorResponse.priorResponse + } + return result + } } diff --git a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.kt b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.kt index c85e5a0067..c2ba0a0da1 100644 --- a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.kt +++ b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.kt @@ -21,6 +21,7 @@ import org.dash.wallet.common.WalletDataProvider import org.dash.wallet.features.exploredash.data.dashspend.GiftCardProviderType import org.dash.wallet.features.exploredash.network.PiggyCardsRemoteDataSource import org.dash.wallet.features.exploredash.network.RemoteDataSource +import org.dash.wallet.features.exploredash.network.authenticator.PiggyCardsAuthenticator import org.dash.wallet.features.exploredash.network.authenticator.TokenAuthenticator import org.dash.wallet.features.exploredash.network.service.ctxspend.CTXSpendApi import org.dash.wallet.features.exploredash.network.service.ctxspend.CTXSpendTokenApi @@ -55,7 +56,9 @@ class DashSpendRepositoryFactory @Inject constructor( private fun createPiggyCardsRepository(): PiggyCardsRepository { val remoteDataSource = PiggyCardsRemoteDataSource(piggyCardsConfig, walletDataProvider) val api = remoteDataSource.buildApi(PiggyCardsApi::class.java) + val tokenApi = remoteDataSource.buildTokenApi() + val authenticator = PiggyCardsAuthenticator(tokenApi, piggyCardsConfig) - return PiggyCardsRepository(api, piggyCardsConfig) + return PiggyCardsRepository(api, piggyCardsConfig, authenticator) } } diff --git a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/PiggyCardsRepository.kt b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/PiggyCardsRepository.kt index f0944deeb7..a4913ed8dc 100644 --- a/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/PiggyCardsRepository.kt +++ b/features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/PiggyCardsRepository.kt @@ -31,8 +31,6 @@ import org.dash.wallet.features.exploredash.data.dashspend.model.UpdatedMerchant import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.Brand import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.ExchangeRateResult import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.Giftcard -import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.LoginRequest -import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.LoginResponse import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.Order import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.OrderRequest import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.OrderResponse @@ -40,6 +38,7 @@ import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.Sign import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.User import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.UserMetadata import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.VerifyOtpRequest +import org.dash.wallet.features.exploredash.network.authenticator.PiggyCardsAuthenticator import org.dash.wallet.features.exploredash.network.service.piggycards.PiggyCardsApi import org.dash.wallet.features.exploredash.ui.dashspend.GiftCardShoppingCart import org.dash.wallet.features.exploredash.utils.PiggyCardsConfig @@ -52,7 +51,8 @@ import kotlin.math.max class PiggyCardsRepository @Inject constructor( private val api: PiggyCardsApi, - private val config: PiggyCardsConfig + private val config: PiggyCardsConfig, + private val authenticator: PiggyCardsAuthenticator ) : DashSpendRepository { companion object { const val DEFAULT_COUNTRY = "US" @@ -101,32 +101,10 @@ class PiggyCardsRepository @Inject constructor( } private suspend fun performAutoLogin(): Boolean { - return try { - val userId = config.getSecuredData(PiggyCardsConfig.PREFS_KEY_USER_ID) - val password = config.getSecuredData(PiggyCardsConfig.PREFS_KEY_PASSWORD) - - if (userId != null && password != null) { - val response = api.login(LoginRequest(userId = userId, password = password)) - handleLoginResponse(response) - } else { - false - } - } catch (e: Exception) { - log.error("Failed to perform auto login: ${e.message}", e) - false - } - } - - private suspend fun handleLoginResponse(response: LoginResponse): Boolean { - config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, response.accessToken) - - val expiresAt = LocalDateTime.now().plusSeconds(response.expiresIn.toLong()) - config.setSecuredData( - PiggyCardsConfig.PREFS_KEY_TOKEN_EXPIRES_AT, - expiresAt.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME) - ) - - return response.accessToken.isNotEmpty() + // Delegate to the authenticator so this shares the same process-wide lock and token + // persistence as the OkHttp retry path, avoiding overlapping logins that could leave + // the cached token in an inconsistent state. + return authenticator.reLogin() != null } override suspend fun isUserSignedIn(): Boolean { diff --git a/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt b/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt new file mode 100644 index 0000000000..2b4888788f --- /dev/null +++ b/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2026. Dash Core Group. + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.dash.wallet.features.exploredash + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.runBlocking +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import org.dash.wallet.common.WalletDataProvider +import org.dash.wallet.common.util.security.EncryptionProvider +import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.LoginRequest +import org.dash.wallet.features.exploredash.data.dashspend.piggycards.model.LoginResponse +import org.dash.wallet.features.exploredash.network.authenticator.PiggyCardsAuthenticator +import org.dash.wallet.features.exploredash.network.service.piggycards.PiggyCardsTokenApi +import org.dash.wallet.features.exploredash.repository.CTXSpendException +import org.dash.wallet.features.exploredash.utils.PiggyCardsConfig +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.mock +import org.robolectric.RobolectricTestRunner +import java.io.IOException +import java.util.concurrent.atomic.AtomicInteger +import kotlin.concurrent.thread +import kotlin.math.max + +/** + * Verifies the PiggyCards auth hardening that mirrors PR #1492's CTX fixes: + * - re-logins are serialized by a process-wide (companion) lock across instances, and + * - a transient failure keeps the cached token while a genuine 401 clears it. + */ +@RunWith(RobolectricTestRunner::class) +class PiggyCardsAuthenticatorTest { + + private val identityEncryption = object : EncryptionProvider { + override fun encrypt(keyAlias: String, textToEncrypt: String): ByteArray = textToEncrypt.toByteArray() + override fun decrypt(keyAlias: String, encryptedData: ByteArray): String = String(encryptedData) + override fun deleteKey(keyAlias: String) {} + } + + private fun realConfig(): PiggyCardsConfig { + val context = ApplicationProvider.getApplicationContext() + return PiggyCardsConfig(context, mock(), identityEncryption) + } + + private fun unauthorizedResponse(): Response = + Response.Builder() + .request(Request.Builder().url("https://example.com/").build()) + .protocol(Protocol.HTTP_1_1) + .code(401) + .message("Unauthorized") + .build() + + @Test + fun reLogin_acrossInstances_isSerialized_byProcessWideLock() { + val config = realConfig() + runBlocking { + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_USER_ID, "user-1") + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_PASSWORD, "pw-1") + } + + val concurrent = AtomicInteger(0) + val maxConcurrent = AtomicInteger(0) + + val tokenApi = object : PiggyCardsTokenApi { + override suspend fun login(loginRequest: LoginRequest): LoginResponse { + val now = concurrent.incrementAndGet() + maxConcurrent.updateAndGet { max(it, now) } + Thread.sleep(200) + concurrent.decrementAndGet() + return LoginResponse(accessToken = "fresh-token", tokenType = "Bearer", expiresIn = 3600) + } + } + val authenticatorA = PiggyCardsAuthenticator(tokenApi, config) + val authenticatorB = PiggyCardsAuthenticator(tokenApi, config) + val response = unauthorizedResponse() + + val threadA = thread { authenticatorA.authenticate(null, response) } + val threadB = thread { authenticatorB.authenticate(null, response) } + threadA.join() + threadB.join() + + // The shared companion lock prevents overlapping re-logins across separate instances. + assertEquals(1, maxConcurrent.get()) + } + + @Test + fun reLogin_onTransientFailure_preservesToken() = runBlocking { + val config = realConfig() + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_USER_ID, "user-1") + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_PASSWORD, "pw-1") + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, "existing-token") + + val tokenApi = object : PiggyCardsTokenApi { + override suspend fun login(loginRequest: LoginRequest): LoginResponse = + throw IOException("transient network failure") // not a 401 + } + val authenticator = PiggyCardsAuthenticator(tokenApi, config) + + val result = authenticator.reLogin() + + assertNull(result) + // Transient failure must NOT wipe the cached token: the session survives and retries. + assertEquals("existing-token", config.getSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN)) + } + + @Test + fun reLogin_onGenuine401_clearsToken() = runBlocking { + val config = realConfig() + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_USER_ID, "user-1") + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_PASSWORD, "pw-1") + config.setSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN, "existing-token") + + val tokenApi = object : PiggyCardsTokenApi { + override suspend fun login(loginRequest: LoginRequest): LoginResponse = + throw CTXSpendException("unauthorized", errorCode = 401) // genuine credential rejection + } + val authenticator = PiggyCardsAuthenticator(tokenApi, config) + + val result = authenticator.reLogin() + + assertNull(result) + // A genuine 401 means the credentials are no longer valid, so the cached token is cleared. + assertEquals("", config.getSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN)) + } +} \ No newline at end of file From d3afad741f15a1ad031ed4e9d3bede96f043ef4a Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 17 Jun 2026 23:10:23 -0700 Subject: [PATCH 2/2] style: ktlint --- .../wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt b/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt index 2b4888788f..615fa760e2 100644 --- a/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt +++ b/features/exploredash/src/test/java/org/dash/wallet/features/exploredash/PiggyCardsAuthenticatorTest.kt @@ -140,4 +140,4 @@ class PiggyCardsAuthenticatorTest { // A genuine 401 means the credentials are no longer valid, so the cached token is cleared. assertEquals("", config.getSecuredData(PiggyCardsConfig.PREFS_KEY_ACCESS_TOKEN)) } -} \ No newline at end of file +}