[#1352] Remove test fixture values ZIP321

- Addresses issue J from the security audit: Payment URI parser uses a test fixture address
- This also removes unused Request screen with its logic and tests
- Closes #1352
This commit is contained in:
Honza Rychnovský 2024-04-18 08:10:08 +02:00 committed by GitHub
parent 0e6b8f83a0
commit afd47e513e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 6 additions and 570 deletions

View File

@ -7,13 +7,14 @@ import cash.z.ecc.sdk.fixture.Zip321UriBuildFixture
import cash.z.ecc.sdk.fixture.Zip321UriParseFixture
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import org.junit.Ignore
import org.junit.Test
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
@Ignore("Disabled until the issue #397 is addressed")
class ZecRequestTest {
companion object {
private const val URI: String = "zcash:tmXuTnE11JojToagTqxXUn6KvdxDE3iLKbp?amount=1&message=Hello%20world!"

View File

@ -2,21 +2,20 @@ package cash.z.ecc.sdk.model
import cash.z.ecc.android.sdk.model.WalletAddress
import cash.z.ecc.android.sdk.model.Zatoshi
import cash.z.ecc.sdk.fixture.Zip321UriBuildFixture
import cash.z.ecc.sdk.fixture.Zip321UriParseFixture
data class ZecRequest(val address: WalletAddress.Unified, val amount: Zatoshi, val message: ZecRequestMessage) {
// TODO [#397]: Waiting for an implementation of Uri parser in SDK project
// TODO [#397]: https://github.com/Electric-Coin-Company/zcash-android-wallet-sdk/issues/397
suspend fun toUri(): String {
return Zip321UriBuildFixture.new(this)
TODO("Not implemented yet: Issue #397")
}
companion object {
// TODO [#397]: Waiting for an implementation of Uri parser in SDK project
// TODO [#397]: https://github.com/Electric-Coin-Company/zcash-android-wallet-sdk/issues/397
@Suppress("UNUSED_PARAMETER")
suspend fun fromUri(uriString: String): ZecRequest {
return Zip321UriParseFixture.new(uriString)
TODO("Not implemented yet: Issue #397")
}
}
}

View File

@ -41,7 +41,6 @@ android {
"src/main/res/ui/new_wallet_recovery",
"src/main/res/ui/onboarding",
"src/main/res/ui/receive",
"src/main/res/ui/request",
"src/main/res/ui/restore",
"src/main/res/ui/scan",
"src/main/res/ui/seed_recovery",

View File

@ -11,12 +11,10 @@ import org.junit.Test
import kotlin.test.assertTrue
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds
import kotlin.time.ExperimentalTime
import kotlin.time.TimeMark
import kotlin.time.TimeSource
class FlowExtTest {
@OptIn(ExperimentalTime::class)
@Test
@SmallTest
fun throttle_one_sec() =
@ -40,7 +38,6 @@ class FlowExtTest {
}
}
@OptIn(ExperimentalTime::class)
private fun raceConditionTest(duration: Duration): Boolean =
runBlocking {
val flow = (0..1000).asFlow().throttle(duration)

View File

@ -1,294 +0,0 @@
package co.electriccoin.zcash.ui.screen.request.view
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.junit4.ComposeContentTestRule
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performTextClearance
import androidx.compose.ui.test.performTextInput
import androidx.test.filters.MediumTest
import cash.z.ecc.android.sdk.fixture.WalletAddressFixture
import cash.z.ecc.android.sdk.model.MonetarySeparators
import cash.z.ecc.android.sdk.model.Zatoshi
import cash.z.ecc.sdk.fixture.ZecRequestFixture
import cash.z.ecc.sdk.model.ZecRequest
import cash.z.ecc.sdk.model.ZecRequestMessage
import co.electriccoin.zcash.test.UiTestPrerequisites
import co.electriccoin.zcash.ui.R
import co.electriccoin.zcash.ui.design.theme.ZcashTheme
import co.electriccoin.zcash.ui.test.getStringResource
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
import java.util.concurrent.atomic.AtomicInteger
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
class RequestViewTest : UiTestPrerequisites() {
@get:Rule
val composeTestRule = createComposeRule()
@Test
@MediumTest
fun create_button_disabled() {
@Suppress("UNUSED_VARIABLE")
val testSetup = TestSetup(composeTestRule)
composeTestRule.assertSendDisabled()
}
@Test
@MediumTest
@OptIn(ExperimentalCoroutinesApi::class)
fun create_request_no_message() =
runTest {
val testSetup = TestSetup(composeTestRule)
assertEquals(0, testSetup.getOnCreateCount())
assertEquals(null, testSetup.getLastCreateZecRequest())
composeTestRule.setValidAmount()
composeTestRule.clickCreateAndSend()
assertEquals(1, testSetup.getOnCreateCount())
testSetup.getLastCreateZecRequest().also {
assertNotNull(it)
assertEquals(WalletAddressFixture.unified(), it.address)
assertEquals(Zatoshi(12345600000), it.amount)
assertTrue(it.message.value.isEmpty())
}
}
@Test
@MediumTest
@OptIn(ExperimentalCoroutinesApi::class)
fun create_request_with_message() =
runTest {
val testSetup = TestSetup(composeTestRule)
assertEquals(0, testSetup.getOnCreateCount())
assertEquals(null, testSetup.getLastCreateZecRequest())
composeTestRule.setValidAmount()
composeTestRule.setValidMessage()
composeTestRule.clickCreateAndSend()
assertEquals(1, testSetup.getOnCreateCount())
testSetup.getLastCreateZecRequest().also {
assertNotNull(it)
assertEquals(WalletAddressFixture.unified(), it.address)
assertEquals(Zatoshi(12345600000), it.amount)
assertEquals(ZecRequestFixture.MESSAGE.value, it.message.value)
}
}
@Test
@MediumTest
fun check_regex_functionality_valid_inputs() {
val testSetup = TestSetup(composeTestRule)
val separators = MonetarySeparators.current()
assertEquals(0, testSetup.getOnCreateCount())
assertEquals(null, testSetup.getLastCreateZecRequest())
composeTestRule.setAmount("123")
composeTestRule.clickCreateAndSend()
assertEquals(1, testSetup.getOnCreateCount())
// e.g. 123,456
composeTestRule.setAmount("123${separators.grouping}456")
composeTestRule.clickCreateAndSend()
assertEquals(2, testSetup.getOnCreateCount())
// e.g. 123.
composeTestRule.setAmount("123${separators.decimal}")
composeTestRule.clickCreateAndSend()
assertEquals(3, testSetup.getOnCreateCount())
// e.g. 123,456.
composeTestRule.setAmount("123${separators.grouping}456${separators.decimal}")
composeTestRule.clickCreateAndSend()
assertEquals(4, testSetup.getOnCreateCount())
// e.g. 123,456.789
composeTestRule.setAmount("123${separators.grouping}456${separators.decimal}789")
composeTestRule.clickCreateAndSend()
assertEquals(5, testSetup.getOnCreateCount())
}
@Test
@MediumTest
fun check_regex_functionality_invalid_inputs() {
val testSetup = TestSetup(composeTestRule)
val separators = MonetarySeparators.current()
assertEquals(0, testSetup.getOnCreateCount())
assertEquals(null, testSetup.getLastCreateZecRequest())
composeTestRule.setAmount("aaa")
composeTestRule.clickCreateAndSend()
assertEquals(0, testSetup.getOnCreateCount())
composeTestRule.setAmount("123aaa")
composeTestRule.clickCreateAndSend()
assertEquals(0, testSetup.getOnCreateCount())
// e.g. ,.
composeTestRule.setAmount("${separators.grouping}${separators.decimal}")
composeTestRule.clickCreateAndSend()
assertEquals(0, testSetup.getOnCreateCount())
// e.g. 123,.
composeTestRule.setAmount("123${separators.grouping}${separators.decimal}")
composeTestRule.clickCreateAndSend()
assertEquals(0, testSetup.getOnCreateCount())
// e.g. 1,2,3
composeTestRule.setAmount("1${separators.grouping}2${separators.grouping}3")
composeTestRule.clickCreateAndSend()
assertEquals(0, testSetup.getOnCreateCount())
// e.g. 1.2.3
composeTestRule.setAmount("1${separators.decimal}2${separators.decimal}3")
composeTestRule.clickCreateAndSend()
assertEquals(0, testSetup.getOnCreateCount())
}
@Test
@MediumTest
@OptIn(ExperimentalCoroutinesApi::class)
fun max_message_length() =
runTest {
val testSetup = TestSetup(composeTestRule)
composeTestRule.setValidAmount()
composeTestRule.setMessage(
buildString {
repeat(ZecRequestMessage.MAX_MESSAGE_LENGTH + 1) { number ->
append("$number")
}
}
)
composeTestRule.clickCreateAndSend()
assertEquals(1, testSetup.getOnCreateCount())
testSetup.getLastCreateZecRequest().also {
assertNotNull(it)
assertEquals(WalletAddressFixture.unified(), it.address)
assertEquals(Zatoshi(12345600000), it.amount)
assertTrue(it.message.value.isEmpty())
}
}
@Test
@MediumTest
fun back() {
val testSetup = TestSetup(composeTestRule)
assertEquals(0, testSetup.getOnBackCount())
composeTestRule.clickBack()
assertEquals(1, testSetup.getOnBackCount())
}
private class TestSetup(private val composeTestRule: ComposeContentTestRule) {
private val onBackCount = AtomicInteger(0)
private val onCreateCount = AtomicInteger(0)
@Volatile
private var onCreateZecRequest: ZecRequest? = null
fun getOnBackCount(): Int {
composeTestRule.waitForIdle()
return onBackCount.get()
}
fun getOnCreateCount(): Int {
composeTestRule.waitForIdle()
return onCreateCount.get()
}
fun getLastCreateZecRequest(): ZecRequest? {
composeTestRule.waitForIdle()
return onCreateZecRequest
}
init {
composeTestRule.setContent {
ZcashTheme {
Request(
myAddress = runBlocking { WalletAddressFixture.unified() },
goBack = {
onBackCount.incrementAndGet()
},
onCreateAndSend = {
onCreateCount.incrementAndGet()
onCreateZecRequest = it
}
)
}
}
}
}
}
private fun ComposeContentTestRule.clickBack() {
onNodeWithContentDescription(getStringResource(R.string.request_back_content_description)).also {
it.performClick()
}
}
private fun ComposeContentTestRule.setValidAmount() {
onNodeWithText(getStringResource(R.string.request_amount)).also {
val separators = MonetarySeparators.current()
it.performTextClearance()
it.performTextInput("123${separators.decimal}456")
}
}
private fun ComposeContentTestRule.setAmount(amount: String) {
onNodeWithText(getStringResource(R.string.request_amount)).also {
it.performTextClearance()
it.performTextInput(amount)
}
}
private fun ComposeContentTestRule.setValidMessage() {
onNodeWithText(getStringResource(R.string.request_message)).also {
it.performTextClearance()
it.performTextInput(ZecRequestFixture.MESSAGE.value)
}
}
private fun ComposeContentTestRule.setMessage(message: String) {
onNodeWithText(getStringResource(R.string.request_message)).also {
it.performTextClearance()
it.performTextInput(message)
}
}
private fun ComposeContentTestRule.clickCreateAndSend() {
onNodeWithText(getStringResource(R.string.request_create), ignoreCase = true).also {
it.performClick()
}
}
private fun ComposeContentTestRule.assertSendDisabled() {
onNodeWithText(getStringResource(R.string.request_create), ignoreCase = true).also {
it.assertIsNotEnabled()
}
}

View File

@ -20,7 +20,6 @@ import co.electriccoin.zcash.ui.NavigationTargets.ADVANCED_SETTINGS
import co.electriccoin.zcash.ui.NavigationTargets.CHOOSE_SERVER
import co.electriccoin.zcash.ui.NavigationTargets.EXPORT_PRIVATE_DATA
import co.electriccoin.zcash.ui.NavigationTargets.HOME
import co.electriccoin.zcash.ui.NavigationTargets.REQUEST
import co.electriccoin.zcash.ui.NavigationTargets.SCAN
import co.electriccoin.zcash.ui.NavigationTargets.SEED_RECOVERY
import co.electriccoin.zcash.ui.NavigationTargets.SEND_CONFIRMATION
@ -38,7 +37,6 @@ import co.electriccoin.zcash.ui.screen.advancedsettings.WrapAdvancedSettings
import co.electriccoin.zcash.ui.screen.chooseserver.WrapChooseServer
import co.electriccoin.zcash.ui.screen.exportdata.WrapExportPrivateData
import co.electriccoin.zcash.ui.screen.home.WrapHome
import co.electriccoin.zcash.ui.screen.request.WrapRequest
import co.electriccoin.zcash.ui.screen.scan.WrapScanValidator
import co.electriccoin.zcash.ui.screen.seedrecovery.WrapSeedRecovery
import co.electriccoin.zcash.ui.screen.send.ext.toSerializableAddress
@ -157,9 +155,6 @@ internal fun MainActivity.Navigation() {
},
)
}
composable(REQUEST) {
WrapRequest(goBack = { navController.popBackStackJustOnce(REQUEST) })
}
composable(SUPPORT) {
// Pop back stack won't be right if we deep link into support
WrapSupport(goBack = { navController.popBackStackJustOnce(SUPPORT) })
@ -268,7 +263,6 @@ object NavigationTargets {
const val EXPORT_PRIVATE_DATA = "export_private_data"
const val HOME = "home"
const val CHOOSE_SERVER = "choose_server"
const val REQUEST = "request"
const val SCAN = "scan"
const val SEED_RECOVERY = "seed_recovery"
const val SEND_CONFIRMATION = "send_confirmation"

View File

@ -1,63 +0,0 @@
@file:Suppress("ktlint:standard:filename")
package co.electriccoin.zcash.ui.screen.request
import android.content.Context
import android.content.Intent
import androidx.activity.ComponentActivity
import androidx.activity.viewModels
import androidx.compose.runtime.Composable
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import cash.z.ecc.sdk.model.ZecRequest
import co.electriccoin.zcash.ui.MainActivity
import co.electriccoin.zcash.ui.R
import co.electriccoin.zcash.ui.common.viewmodel.WalletViewModel
import co.electriccoin.zcash.ui.design.component.CircularScreenProgressIndicator
import co.electriccoin.zcash.ui.screen.request.view.Request
import kotlinx.coroutines.runBlocking
@Composable
internal fun MainActivity.WrapRequest(goBack: () -> Unit) {
WrapRequest(this, goBack)
}
@Composable
private fun WrapRequest(
activity: ComponentActivity,
goBack: () -> Unit
) {
val walletViewModel by activity.viewModels<WalletViewModel>()
val walletAddresses = walletViewModel.addresses.collectAsStateWithLifecycle().value
if (null == walletAddresses) {
// TODO [#1146]: Consider moving CircularScreenProgressIndicator from Android layer to View layer
// TODO [#1146]: Improve this by allowing screen composition and updating it after the data is available
// TODO [#1146]: https://github.com/Electric-Coin-Company/zashi-android/issues/1146
CircularScreenProgressIndicator()
} else {
Request(
walletAddresses.unified,
goBack = goBack,
onCreateAndSend = {
val chooserIntent =
Intent.createChooser(
it.newShareIntent(activity.applicationContext),
null
)
activity.startActivity(chooserIntent)
goBack()
}
)
}
}
private fun ZecRequest.newShareIntent(context: Context) =
runBlocking {
Intent().apply {
action = Intent.ACTION_SEND
putExtra(Intent.EXTRA_TEXT, context.getString(R.string.request_template_format, toUri()))
type = "text/plain"
}
}

View File

@ -1,171 +0,0 @@
package co.electriccoin.zcash.ui.screen.request.view
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.text.KeyboardOptions
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.filled.ArrowBack
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton
import androidx.compose.material3.Scaffold
import androidx.compose.material3.Text
import androidx.compose.material3.TextField
import androidx.compose.material3.TopAppBar
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.input.KeyboardType
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import cash.z.ecc.android.sdk.fixture.WalletAddressFixture
import cash.z.ecc.android.sdk.model.MonetarySeparators
import cash.z.ecc.android.sdk.model.WalletAddress
import cash.z.ecc.android.sdk.model.Zatoshi
import cash.z.ecc.android.sdk.model.ZecString
import cash.z.ecc.android.sdk.model.ZecStringExt
import cash.z.ecc.android.sdk.model.fromZecString
import cash.z.ecc.sdk.model.ZecRequest
import cash.z.ecc.sdk.model.ZecRequestMessage
import co.electriccoin.zcash.ui.R
import co.electriccoin.zcash.ui.design.MINIMAL_WEIGHT
import co.electriccoin.zcash.ui.design.component.GradientSurface
import co.electriccoin.zcash.ui.design.component.PrimaryButton
import co.electriccoin.zcash.ui.design.theme.ZcashTheme
import kotlinx.coroutines.runBlocking
import java.util.Locale
@Preview("Request")
@Composable
private fun PreviewRequest() {
ZcashTheme(forceDarkMode = false) {
GradientSurface {
Request(
myAddress = runBlocking { WalletAddressFixture.unified() },
goBack = {},
onCreateAndSend = {}
)
}
}
}
/**
* @param myAddress The address that ZEC should be sent to.
*/
@Composable
fun Request(
myAddress: WalletAddress.Unified,
goBack: () -> Unit,
onCreateAndSend: (ZecRequest) -> Unit
) {
Scaffold(topBar = {
RequestTopAppBar(onBack = goBack)
}) { paddingValues ->
RequestMainContent(
myAddress = myAddress,
onCreateAndSend = onCreateAndSend,
modifier =
Modifier.padding(
top = paddingValues.calculateTopPadding() + ZcashTheme.dimens.spacingDefault,
bottom = paddingValues.calculateTopPadding(),
start = ZcashTheme.dimens.screenHorizontalSpacingRegular,
end = ZcashTheme.dimens.screenHorizontalSpacingRegular
)
)
}
}
@Composable
@OptIn(ExperimentalMaterial3Api::class)
private fun RequestTopAppBar(onBack: () -> Unit) {
TopAppBar(
title = { Text(text = stringResource(id = R.string.request_title)) },
navigationIcon = {
IconButton(
onClick = onBack
) {
Icon(
imageVector = Icons.AutoMirrored.Filled.ArrowBack,
contentDescription = stringResource(R.string.request_back_content_description)
)
}
}
)
}
// TODO [#215]: Need to add some UI to explain to the user if a request is invalid
// TODO [#217]: Need to handle changing of Locale after user input, but before submitting the button.
@Composable
private fun RequestMainContent(
myAddress: WalletAddress.Unified,
onCreateAndSend: (ZecRequest) -> Unit,
modifier: Modifier = Modifier
) {
val context = LocalContext.current
// TODO [#1171]: Remove default MonetarySeparators locale
// TODO [#1171]: https://github.com/Electric-Coin-Company/zashi-android/issues/1171
val monetarySeparators = MonetarySeparators.current(Locale.US)
val allowedCharacters = ZecString.allowedCharacters(monetarySeparators)
var amountZecString by rememberSaveable { mutableStateOf("") }
var message by rememberSaveable { mutableStateOf("") }
Column(
modifier = modifier,
horizontalAlignment = Alignment.CenterHorizontally
) {
// TODO [#289]: Crash occurs while typed more than some acceptable amount to this field.
TextField(
value = amountZecString,
onValueChange = { newValue ->
if (!ZecStringExt.filterContinuous(context, monetarySeparators, newValue)) {
return@TextField
}
amountZecString = newValue.filter { allowedCharacters.contains(it) }
},
keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number),
label = { Text(stringResource(id = R.string.request_amount)) }
)
Spacer(Modifier.size(8.dp))
TextField(value = message, onValueChange = {
if (it.length <= ZecRequestMessage.MAX_MESSAGE_LENGTH) {
message = it
}
}, label = { Text(stringResource(id = R.string.request_message)) })
Spacer(
modifier =
Modifier
.fillMaxHeight()
.weight(MINIMAL_WEIGHT)
)
Spacer(modifier = Modifier.height(ZcashTheme.dimens.spacingSmall))
val zatoshi = Zatoshi.fromZecString(context, amountZecString, monetarySeparators)
PrimaryButton(
onClick = {
onCreateAndSend(ZecRequest(myAddress, zatoshi!!, ZecRequestMessage(message)))
},
text = stringResource(id = R.string.request_create),
enabled = null != zatoshi,
modifier = Modifier.fillMaxWidth()
)
Spacer(modifier = Modifier.height(ZcashTheme.dimens.spacingHuge))
}
}

View File

@ -1,12 +0,0 @@
<resources xmlns:tools="http://schemas.android.com/tools" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
<string name="request_title">Request ZEC</string>
<string name="request_back_content_description">Back</string>
<string name="request_from">Who would you like to request ZEC from?</string>
<string name="request_amount">How much?</string>
<string name="request_message">Message</string>
<string name="request_create">Create and Send</string>
<string name="request_template_format" formatted="true">Someone is requesting ZEC from you. Please click the link: <xliff:g id="link" example="zcash:asdf">%1$s</xliff:g></string>
</resources>

View File

@ -416,20 +416,6 @@ private fun settingsScreenshots(
ScreenshotTest.takeScreenshot(tag, "Settings 1")
}
// This screen is not currently navigable from the app
@Suppress("UnusedPrivateMember")
private fun requestZecScreenshots(
resContext: Context,
tag: String,
composeTestRule: ComposeTestRule
) {
composeTestRule.onNode(hasText(resContext.getString(R.string.request_title))).also {
it.assertExists()
}
ScreenshotTest.takeScreenshot(tag, "Request 1")
}
private fun receiveZecScreenshots(
resContext: Context,
tag: String,