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

Add CustomerSessionClientSecret validation & remodel ElementsSessionManager response. #9373

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,10 @@ interface ErrorReporter {
),
CUSTOMER_SHEET_ATTACH_CALLED_WITH_CUSTOMER_SESSION(
partialEventName = "customersheet.customer_session.attach_called"
)
),
CUSTOMER_SESSION_ON_CUSTOMER_SHEET_ELEMENTS_SESSION_NO_CUSTOMER_FIELD(
partialEventName = "customersheet.customer_session.elements_session.no_customer_field"
),
;

override val eventName: String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,36 @@ import com.stripe.android.core.injection.IOContext
import com.stripe.android.customersheet.CustomerSheet
import com.stripe.android.customersheet.ExperimentalCustomerSheetApi
import com.stripe.android.model.ElementsSession
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.paymentsheet.ExperimentalCustomerSessionApi
import com.stripe.android.paymentsheet.PaymentSheet
import com.stripe.android.paymentsheet.PrefsRepository
import com.stripe.android.paymentsheet.model.SavedSelection
import com.stripe.android.paymentsheet.repositories.ElementsSessionRepository
import kotlinx.coroutines.withContext
import java.lang.IllegalArgumentException
import javax.inject.Inject
import javax.inject.Singleton
import kotlin.coroutines.CoroutineContext

internal interface CustomerSessionElementsSessionManager {
suspend fun fetchCustomerSessionEphemeralKey(): Result<CachedCustomerEphemeralKey.Available>

suspend fun fetchElementsSession(): Result<ElementsSession>
suspend fun fetchElementsSession(): Result<ElementsSessionWithCustomer>
}

internal data class ElementsSessionWithCustomer(
val elementsSession: ElementsSession,
val customer: ElementsSession.Customer,
)

@OptIn(ExperimentalCustomerSheetApi::class, ExperimentalCustomerSessionApi::class)
@Singleton
internal class DefaultCustomerSessionElementsSessionManager @Inject constructor(
private val elementsSessionRepository: ElementsSessionRepository,
private val prefsRepositoryFactory: @JvmSuppressWildcards (String) -> PrefsRepository,
private val customerSessionProvider: CustomerSheet.CustomerSessionProvider,
private val errorReporter: ErrorReporter,
private val timeProvider: () -> Long,
@IOContext private val workContext: CoroutineContext,
) : CustomerSessionElementsSessionManager {
Expand Down Expand Up @@ -55,7 +63,7 @@ internal class DefaultCustomerSessionElementsSessionManager @Inject constructor(
}
}

override suspend fun fetchElementsSession(): Result<ElementsSession> {
override suspend fun fetchElementsSession(): Result<ElementsSessionWithCustomer> {
return withContext(workContext) {
runCatching {
val intentConfiguration = intentConfiguration
Expand All @@ -67,6 +75,8 @@ internal class DefaultCustomerSessionElementsSessionManager @Inject constructor(
.providesCustomerSessionClientSecret()
.getOrThrow()

validateCustomerSessionClientSecret(customerSessionClientSecret.clientSecret)

val prefsRepository = prefsRepositoryFactory(customerSessionClientSecret.customerId)

val savedSelection = prefsRepository.getSavedSelection(
Expand All @@ -87,16 +97,59 @@ internal class DefaultCustomerSessionElementsSessionManager @Inject constructor(
clientSecret = customerSessionClientSecret.clientSecret,
),
externalPaymentMethods = listOf(),
).onSuccess { elementsSession ->
elementsSession.customer?.session?.run {
cachedCustomerEphemeralKey = CachedCustomerEphemeralKey.Available(
customerId = customerId,
ephemeralKey = apiKey,
expiresAt = apiKeyExpiry,
).mapCatching { elementsSession ->
val customer = elementsSession.customer ?: run {
errorReporter.report(
ErrorReporter
.UnexpectedErrorEvent
.CUSTOMER_SESSION_ON_CUSTOMER_SHEET_ELEMENTS_SESSION_NO_CUSTOMER_FIELD
)

throw IllegalStateException(
"`customer` field should be available when using `CustomerSession` in elements/session!"
)
}

ElementsSessionWithCustomer(
elementsSession = elementsSession,
customer = customer
)
}.onSuccess { elementsSessionWithCustomer ->
val customerSession = elementsSessionWithCustomer.customer.session

cachedCustomerEphemeralKey = CachedCustomerEphemeralKey.Available(
customerId = customerSession.customerId,
ephemeralKey = customerSession.apiKey,
expiresAt = customerSession.apiKeyExpiry,
)
}.getOrThrow()
}
}
}

private fun validateCustomerSessionClientSecret(customerSessionClientSecret: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a way we can share this code with payment sheet rather than re-implementing it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a CustomerSessionClientSecretProvider that does the actual validation. Error messages are still different between both implementations given the context in how the errors are produced is different between both PaymentSheet and CustomerSheet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

great, thanks!

val error = when {
customerSessionClientSecret.isBlank() -> {
"The 'customerSessionClientSecret' cannot be an empty string."
}
customerSessionClientSecret.startsWith(EPHEMERAL_KEY_SECRET_PREFIX) -> {
"Provided secret looks like an Ephemeral Key secret, but expecting a CustomerSession client " +
"secret. See CustomerSession API: https://docs.stripe.com/api/customer_sessions/create"
}
!customerSessionClientSecret.startsWith(CUSTOMER_SESSION_CLIENT_SECRET_KEY_PREFIX) -> {
"Provided secret does not look like a CustomerSession client secret. " +
"See CustomerSession API: https://docs.stripe.com/api/customer_sessions/create"
}
else -> null
}

error?.let {
throw IllegalArgumentException(it)
}
}

private companion object {
const val EPHEMERAL_KEY_SECRET_PREFIX = "ek_"
const val CUSTOMER_SESSION_CLIENT_SECRET_KEY_PREFIX = "cuss_"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,8 @@ internal class CustomerSessionPaymentMethodDataSource @Inject constructor(
) : CustomerSheetPaymentMethodDataSource {
override suspend fun retrievePaymentMethods(): CustomerSheetDataResult<List<PaymentMethod>> {
return withContext(workContext) {
elementsSessionManager.fetchElementsSession().mapCatching { elementsSession ->
elementsSession.customer?.paymentMethods
?: throw IllegalStateException(
"`CustomerSession` should contain payment methods in `elements/session` response!"
)
elementsSessionManager.fetchElementsSession().mapCatching { elementsSessionWithCustomer ->
elementsSessionWithCustomer.customer.paymentMethods
}.toCustomerSheetDataResult()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ package com.stripe.android.customersheet.data

import com.google.common.truth.Truth.assertThat
import com.stripe.android.isInstanceOf
import com.stripe.android.model.ElementsSession
import com.stripe.android.model.PaymentMethod
import com.stripe.android.model.PaymentMethodUpdateParams
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.paymentsheet.repositories.CustomerRepository
import com.stripe.android.testing.FakeErrorReporter
import com.stripe.android.testing.PaymentMethodFactory
import com.stripe.android.testing.SetupIntentFactory
import com.stripe.android.utils.FakeCustomerRepository
import kotlinx.coroutines.test.runTest
import org.junit.Test
Expand All @@ -34,38 +32,6 @@ class CustomerSessionPaymentMethodDataSourceTest {
assertThat(returnedPaymentMethods).containsExactlyElementsIn(paymentMethods)
}

@Test
fun `on fetch payment methods, should fail if elements session does not have payment methods`() = runTest {
val paymentMethodDataSource = createPaymentMethodDataSource(
elementsSessionManager = FakeCustomerSessionElementsSessionManager(
elementsSession = Result.success(
ElementsSession(
linkSettings = null,
paymentMethodSpecs = null,
stripeIntent = SetupIntentFactory.create(),
merchantCountry = null,
isGooglePayEnabled = true,
sessionsError = null,
externalPaymentMethodData = null,
customer = null,
cardBrandChoice = null,
)
)
),
)

val paymentMethodsResult = paymentMethodDataSource.retrievePaymentMethods()

assertThat(paymentMethodsResult).isInstanceOf<CustomerSheetDataResult.Failure<List<PaymentMethod>>>()

val failedResult = paymentMethodsResult.asFailure()

assertThat(failedResult.cause).isInstanceOf<IllegalStateException>()
assertThat(failedResult.cause.message).isEqualTo(
"`CustomerSession` should contain payment methods in `elements/session` response!"
)
}

@Test
fun `on fetch payment methods, should fail if elements session fetch fails`() = runTest {
val exception = IllegalStateException("Failed to load!")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import com.stripe.android.customersheet.ExperimentalCustomerSheetApi
import com.stripe.android.customersheet.utils.FakeCustomerSessionProvider
import com.stripe.android.isInstanceOf
import com.stripe.android.model.ElementsSession
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.paymentsheet.ExperimentalCustomerSessionApi
import com.stripe.android.paymentsheet.FakePrefsRepository
import com.stripe.android.paymentsheet.PaymentSheet
import com.stripe.android.paymentsheet.model.SavedSelection
import com.stripe.android.paymentsheet.repositories.ElementsSessionRepository
import com.stripe.android.testing.FakeErrorReporter
import com.stripe.android.testing.PaymentIntentFactory
import com.stripe.android.testing.SetupIntentFactory
import com.stripe.android.utils.FakeElementsSessionRepository
import kotlinx.coroutines.test.runTest
import org.junit.Test
Expand Down Expand Up @@ -264,6 +267,78 @@ class DefaultCustomerSessionElementsSessionManagerTest {
)
}

@Test
fun `on fetch elements session, should fail & report if 'customer' field is empty`() = runTest {
val errorReporter = FakeErrorReporter()
val elementsSessionManager = createElementsSessionManager(
elementsSessionRepository = FakeElementsSessionRepository(
stripeIntent = SetupIntentFactory.create(),
error = null,
linkSettings = null,
sessionsCustomer = null,
),
errorReporter = errorReporter,
)

val elementsSessionResult = elementsSessionManager.fetchElementsSession()

assertThat(elementsSessionResult.isFailure).isTrue()
assertThat(elementsSessionResult.exceptionOrNull()?.message).isEqualTo(
"`customer` field should be available when using `CustomerSession` in elements/session!"
)

assertThat(errorReporter.getLoggedErrors()).containsExactly(
ErrorReporter
.UnexpectedErrorEvent
.CUSTOMER_SESSION_ON_CUSTOMER_SHEET_ELEMENTS_SESSION_NO_CUSTOMER_FIELD
.eventName
)
}

@Test
fun `on fetch elements session, should fail if client secret is empty`() = runClientValidationErrorTest(
invalidClientSecret = "",
errorMessage = "The 'customerSessionClientSecret' cannot be an empty string."
)

@Test
fun `on fetch elements session, should fail if client secret is legacy ephemeral key`() =
runClientValidationErrorTest(
invalidClientSecret = "ek_123",
errorMessage = "Provided secret looks like an Ephemeral Key secret, but expecting a CustomerSession " +
"client secret. See CustomerSession API: https://docs.stripe.com/api/customer_sessions/create"
)

@Test
fun `on fetch elements session, should fail if client secret is not in expected customer session format`() =
runClientValidationErrorTest(
invalidClientSecret = "cutt_123",
errorMessage = "Provided secret does not look like a CustomerSession client secret. " +
"See CustomerSession API: https://docs.stripe.com/api/customer_sessions/create"
)

private fun runClientValidationErrorTest(
invalidClientSecret: String,
errorMessage: String,
) = runTest {
val manager = createElementsSessionManager(
customerSessionClientSecret = Result.success(
CustomerSheet.CustomerSessionClientSecret.create(
customerId = "cus_1",
clientSecret = invalidClientSecret
)
)
)

val result = manager.fetchElementsSession()

assertThat(result.isFailure).isTrue()

val exception = result.exceptionOrNull()

assertThat(exception?.message).isEqualTo(errorMessage)
}

private suspend fun createElementsSessionManagerWithCustomer(
customerId: String = "cus_1",
apiKey: String = "ek_123",
Expand Down Expand Up @@ -311,6 +386,7 @@ class DefaultCustomerSessionElementsSessionManagerTest {
error = null,
linkSettings = null,
),
errorReporter: ErrorReporter = FakeErrorReporter(),
intentConfiguration: Result<CustomerSheet.IntentConfiguration> =
Result.success(CustomerSheet.IntentConfiguration.Builder().build()),
onIntentConfiguration: () -> Result<CustomerSheet.IntentConfiguration> = { intentConfiguration },
Expand All @@ -331,6 +407,7 @@ class DefaultCustomerSessionElementsSessionManagerTest {
): CustomerSessionElementsSessionManager {
return DefaultCustomerSessionElementsSessionManager(
elementsSessionRepository = elementsSessionRepository,
errorReporter = errorReporter,
prefsRepositoryFactory = {
FakePrefsRepository().apply {
setSavedSelection(savedSelection = savedSelection)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,43 @@ internal class FakeCustomerSessionElementsSessionManager(
ElementsSession.Customer.Components.CustomerSheet.Enabled(
isPaymentMethodRemoveEnabled = true,
),
private val elementsSession: Result<ElementsSession> = Result.success(
ElementsSession(
linkSettings = null,
paymentMethodSpecs = null,
stripeIntent = SetupIntentFactory.create(),
merchantCountry = null,
isGooglePayEnabled = true,
sessionsError = null,
externalPaymentMethodData = null,
customer = ElementsSession.Customer(
session = ElementsSession.Customer.Session(
id = "cuss_1",
customerId = "cus_1",
apiKey = "ek_123",
apiKeyExpiry = 999999,
components = ElementsSession.Customer.Components(
mobilePaymentElement = ElementsSession.Customer.Components.MobilePaymentElement.Disabled,
customerSheet = customerSheetComponent,
),
liveMode = false,
),
defaultPaymentMethod = null,
paymentMethods = paymentMethods,
private val customer: ElementsSession.Customer = ElementsSession.Customer(
session = ElementsSession.Customer.Session(
id = "cuss_1",
customerId = "cus_1",
apiKey = "ek_123",
apiKeyExpiry = 999999,
components = ElementsSession.Customer.Components(
mobilePaymentElement = ElementsSession.Customer.Components.MobilePaymentElement.Disabled,
customerSheet = customerSheetComponent,
),
liveMode = false,
),
defaultPaymentMethod = null,
paymentMethods = paymentMethods,
),
private val elementsSession: Result<ElementsSessionWithCustomer> = Result.success(
ElementsSessionWithCustomer(
elementsSession = ElementsSession(
linkSettings = null,
paymentMethodSpecs = null,
stripeIntent = SetupIntentFactory.create(),
merchantCountry = null,
isGooglePayEnabled = true,
sessionsError = null,
externalPaymentMethodData = null,
customer = customer,
cardBrandChoice = null,
),
cardBrandChoice = null,
customer = customer,
)
)
) : CustomerSessionElementsSessionManager {
override suspend fun fetchCustomerSessionEphemeralKey(): Result<CachedCustomerEphemeralKey.Available> {
return ephemeralKey
}

override suspend fun fetchElementsSession(): Result<ElementsSession> {
override suspend fun fetchElementsSession(): Result<ElementsSessionWithCustomer> {
return elementsSession
}
}
Loading