Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create PaymentIntent.ClientSecret and SetupIntent.ClientSecret for validation #1958

Merged
merged 1 commit into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions stripe/src/main/java/com/stripe/android/Stripe.kt
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class Stripe internal constructor(
fun authenticatePayment(activity: Activity, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(activity),
clientSecret,
PaymentIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand All @@ -192,7 +192,7 @@ class Stripe internal constructor(
fun handleNextActionForPayment(activity: Activity, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(activity),
clientSecret,
PaymentIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand All @@ -211,7 +211,7 @@ class Stripe internal constructor(
fun authenticatePayment(fragment: Fragment, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(fragment),
clientSecret,
PaymentIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand All @@ -230,7 +230,7 @@ class Stripe internal constructor(
fun handleNextActionForPayment(fragment: Fragment, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(fragment),
clientSecret,
PaymentIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand Down Expand Up @@ -273,7 +273,7 @@ class Stripe internal constructor(
@WorkerThread
fun retrievePaymentIntentSynchronous(clientSecret: String): PaymentIntent? {
return stripeRepository.retrievePaymentIntent(
clientSecret,
PaymentIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand Down Expand Up @@ -353,7 +353,7 @@ class Stripe internal constructor(
fun authenticateSetup(activity: Activity, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(activity),
clientSecret,
SetupIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand All @@ -370,7 +370,7 @@ class Stripe internal constructor(
fun handleNextActionForSetupIntent(activity: Activity, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(activity),
clientSecret,
SetupIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand All @@ -388,7 +388,7 @@ class Stripe internal constructor(
fun authenticateSetup(fragment: Fragment, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(fragment),
clientSecret,
SetupIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand All @@ -405,7 +405,7 @@ class Stripe internal constructor(
fun handleNextActionForSetupIntent(fragment: Fragment, clientSecret: String) {
paymentController.startAuth(
AuthActivityStarter.Host.create(fragment),
clientSecret,
SetupIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand Down Expand Up @@ -447,7 +447,7 @@ class Stripe internal constructor(
@WorkerThread
fun retrieveSetupIntentSynchronous(clientSecret: String): SetupIntent? {
return stripeRepository.retrieveSetupIntent(
clientSecret,
SetupIntent.ClientSecret(clientSecret).value,
ApiRequest.Options(publishableKey, stripeAccountId)
)
}
Expand Down
19 changes: 10 additions & 9 deletions stripe/src/main/java/com/stripe/android/StripeApiRepository.kt
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ internal class StripeApiRepository @JvmOverloads internal constructor(
options: ApiRequest.Options
): PaymentIntent? {
val params = networkUtils.paramsWithUid(confirmPaymentIntentParams.toParamMap())
val paymentIntentId =
PaymentIntent.parseIdFromClientSecret(confirmPaymentIntentParams.clientSecret)
val apiUrl = getConfirmPaymentIntentUrl(paymentIntentId)
val apiUrl = getConfirmPaymentIntentUrl(
PaymentIntent.ClientSecret(confirmPaymentIntentParams.clientSecret).paymentIntentId
)

try {
fireFingerprintRequest()
Expand Down Expand Up @@ -114,8 +114,9 @@ internal class StripeApiRepository @JvmOverloads internal constructor(
clientSecret: String,
options: ApiRequest.Options
): PaymentIntent? {
val paymentIntentId = PaymentIntent.parseIdFromClientSecret(clientSecret)
val apiUrl = getRetrievePaymentIntentUrl(paymentIntentId)
val apiUrl = getRetrievePaymentIntentUrl(
PaymentIntent.ClientSecret(clientSecret).paymentIntentId
)

try {
fireFingerprintRequest()
Expand Down Expand Up @@ -178,9 +179,9 @@ internal class StripeApiRepository @JvmOverloads internal constructor(
): SetupIntent? {
val params = networkUtils.paramsWithUid(confirmSetupIntentParams.toParamMap())

val setupIntentId =
SetupIntent.parseIdFromClientSecret(confirmSetupIntentParams.clientSecret)
val apiUrl = getConfirmSetupIntentUrl(setupIntentId)
val apiUrl = getConfirmSetupIntentUrl(
SetupIntent.ClientSecret(confirmSetupIntentParams.clientSecret).setupIntentId
)

try {
fireFingerprintRequest()
Expand Down Expand Up @@ -219,7 +220,7 @@ internal class StripeApiRepository @JvmOverloads internal constructor(
clientSecret: String,
options: ApiRequest.Options
): SetupIntent? {
val setupIntentId = SetupIntent.parseIdFromClientSecret(clientSecret)
val setupIntentId = SetupIntent.ClientSecret(clientSecret).setupIntentId
val apiUrl = getRetrieveSetupIntentUrl(setupIntentId)

try {
Expand Down
17 changes: 17 additions & 0 deletions stripe/src/main/java/com/stripe/android/model/PaymentIntent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.stripe.android.model

import android.net.Uri
import com.stripe.android.model.parsers.PaymentIntentJsonParser
import java.util.regex.Pattern
import kotlinx.android.parcel.IgnoredOnParcel
import kotlinx.android.parcel.Parcelize
import kotlinx.android.parcel.RawValue
Expand Down Expand Up @@ -252,6 +253,22 @@ data class PaymentIntent internal constructor(
}
}

internal data class ClientSecret(internal val value: String) {
internal val paymentIntentId: String =
value.split("_secret".toRegex())
.dropLastWhile { it.isEmpty() }.toTypedArray()[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not entirely sure how this works but wondering if there is a case where we end up with an empty array here? Or do we ensure an empty array will never be the case with the isEmpty()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if there are any edge cases this isn't handling, but given that it's the existing logic for obtaining an ID from a client secret, I'm going to leave it as is.


init {
require(PATTERN.matcher(value).matches()) {
"Invalid client secret: $value"
}
}

private companion object {
private val PATTERN = Pattern.compile("^pi_(\\w)+_secret_(\\w)+$")
}
}

enum class CancellationReason(private val code: String) {
Duplicate("duplicate"),
Fraudulent("fraudulent"),
Expand Down
22 changes: 17 additions & 5 deletions stripe/src/main/java/com/stripe/android/model/SetupIntent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.stripe.android.model

import android.net.Uri
import com.stripe.android.model.parsers.SetupIntentJsonParser
import java.util.regex.Pattern
import kotlinx.android.parcel.Parcelize
import kotlinx.android.parcel.RawValue
import org.json.JSONObject
Expand Down Expand Up @@ -199,6 +200,22 @@ data class SetupIntent internal constructor(
}
}

internal data class ClientSecret(internal val value: String) {
internal val setupIntentId: String =
value.split("_secret".toRegex())
.dropLastWhile { it.isEmpty() }.toTypedArray()[0]

init {
require(PATTERN.matcher(value).matches()) {
"Invalid client secret: $value"
}
}

private companion object {
private val PATTERN = Pattern.compile("^seti_(\\w)+_secret_(\\w)+$")
}
}

enum class CancellationReason(private val code: String) {
Duplicate("duplicate"),
RequestedByCustomer("requested_by_customer"),
Expand All @@ -214,11 +231,6 @@ data class SetupIntent internal constructor(
companion object {
private const val FIELD_NEXT_ACTION_TYPE = "type"

internal fun parseIdFromClientSecret(clientSecret: String): String {
return clientSecret.split("_secret".toRegex())
.dropLastWhile { it.isEmpty() }.toTypedArray()[0]
}

@JvmStatic
fun fromJson(jsonObject: JSONObject?): SetupIntent? {
return jsonObject?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ class StripeApiRepositoryTest {

@Test
fun confirmPaymentIntent_withSourceData_canSuccessfulConfirm() {
val clientSecret = "temporarily put a private key here simulate the backend"
// put a private key here to simulate the backend
val clientSecret = "pi_12345_secret_fake"

`when`(stripeApiRequestExecutor.execute(any()))
.thenReturn(
Expand Down
18 changes: 7 additions & 11 deletions stripe/src/test/java/com/stripe/android/StripeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1085,20 +1085,16 @@ public void run() throws Throwable {
}

@Test
public void retrievePaymentIntent_withInvalidClientSecretInGermanyLocale_shouldReturnLocalizedMessage() {
public void retrievePaymentIntent_withInvalidClientSecret_shouldThrowException() {
Locale.setDefault(Locale.GERMANY);

// This card is missing quite a few numbers.
final Stripe stripe = createStripe();
final InvalidRequestException exception = assertThrows(
InvalidRequestException.class,
new ThrowingRunnable() {
@Override
public void run() throws Throwable {
stripe.retrievePaymentIntentSynchronous("invalid");
}
});
assertEquals("Keine solche payment_intent: invalid", exception.getStripeError().getMessage());
assertThrows(IllegalArgumentException.class, new ThrowingRunnable() {
@Override
public void run() throws Throwable {
stripe.retrievePaymentIntentSynchronous("invalid");
}
});
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.stripe.android.model
import com.stripe.android.model.parsers.PaymentIntentJsonParser
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
Expand Down Expand Up @@ -50,8 +51,8 @@ class PaymentIntentTest {
@Test
fun parseIdFromClientSecret_parsesCorrectly() {
val clientSecret = "pi_1CkiBMLENEVhOs7YMtUehLau_secret_s4O8SDh7s6spSmHDw1VaYPGZA"
val id = PaymentIntent.parseIdFromClientSecret(clientSecret)
assertEquals("pi_1CkiBMLENEVhOs7YMtUehLau", id)
val paymentIntentId = PaymentIntent.ClientSecret(clientSecret).paymentIntentId
assertEquals("pi_1CkiBMLENEVhOs7YMtUehLau", paymentIntentId)
}

@Test
Expand Down Expand Up @@ -120,6 +121,25 @@ class PaymentIntentTest {
PaymentIntentFixtures.CANCELLED.canceledAt)
}

@Test
fun clientSecret_withInvalidKeys_throwsException() {
assertFailsWith<IllegalArgumentException> {
PaymentIntent.ClientSecret("pi_12345")
}

assertFailsWith<IllegalArgumentException> {
PaymentIntent.ClientSecret("pi_12345_secret_")
}
}

@Test
fun clientSecret_withValidKeys_throwsException() {
assertEquals(
"pi_a1b2c3_secret_x7y8z9",
PaymentIntent.ClientSecret("pi_a1b2c3_secret_x7y8z9").value
)
}

private companion object {
private val PARSER = PaymentIntentJsonParser()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.stripe.android.model

import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
Expand All @@ -13,8 +14,9 @@ class SetupIntentTest {

@Test
fun parseIdFromClientSecret_correctIdParsed() {
val id = SetupIntent.parseIdFromClientSecret(
"seti_1Eq5kyGMT9dGPIDGxiSp4cce_secret_FKlHb3yTI0YZWe4iqghS8ZXqwwMoMmy")
val id = SetupIntent.ClientSecret(
"seti_1Eq5kyGMT9dGPIDGxiSp4cce_secret_FKlHb3yTI0YZWe4iqghS8ZXqwwMoMmy"
).setupIntentId
assertEquals("seti_1Eq5kyGMT9dGPIDGxiSp4cce", id)
}

Expand Down Expand Up @@ -67,4 +69,23 @@ class SetupIntentTest {
assertEquals(SetupIntent.CancellationReason.Abandoned,
SetupIntentFixtures.CANCELLED.cancellationReason)
}

@Test
fun clientSecret_withInvalidKeys_throwsException() {
assertFailsWith<IllegalArgumentException> {
SetupIntent.ClientSecret("seti_12345")
}

assertFailsWith<IllegalArgumentException> {
SetupIntent.ClientSecret("seti_12345_secret_")
}
}

@Test
fun clientSecret_withValidKeys_throwsException() {
assertEquals(
"seti_a1b2c3_secret_x7y8z9",
SetupIntent.ClientSecret("seti_a1b2c3_secret_x7y8z9").value
)
}
}