-
Notifications
You must be signed in to change notification settings - Fork 50
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
Trusted entitlements: Support signing static endpoints #1119
Changes from all commits
accfeee
4c2db46
a8dc79f
d29e3a5
784f8d9
7d337ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import com.revenuecat.purchases.VerificationResult | |
import com.revenuecat.purchases.common.AppConfig | ||
import com.revenuecat.purchases.common.errorLog | ||
import com.revenuecat.purchases.common.networking.Endpoint | ||
import com.revenuecat.purchases.common.verboseLog | ||
import com.revenuecat.purchases.common.warnLog | ||
import com.revenuecat.purchases.strings.NetworkStrings | ||
import com.revenuecat.purchases.utils.Result | ||
|
@@ -69,7 +70,7 @@ class SigningManager( | |
} | ||
|
||
fun shouldVerifyEndpoint(endpoint: Endpoint): Boolean { | ||
return endpoint.supportsSignatureValidation && signatureVerificationMode.shouldVerify | ||
return endpoint.supportsSignatureVerification && signatureVerificationMode.shouldVerify | ||
} | ||
|
||
fun createRandomNonce(): String { | ||
|
@@ -142,6 +143,7 @@ class SigningManager( | |
) | ||
|
||
return if (verificationResult) { | ||
verboseLog(NetworkStrings.VERIFICATION_SUCCESS.format(urlPath)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 |
||
VerificationResult.VERIFIED | ||
} else { | ||
errorLog(NetworkStrings.VERIFICATION_ERROR.format(urlPath)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import io.mockk.Runs | |
import io.mockk.every | ||
import io.mockk.just | ||
import io.mockk.mockk | ||
import io.mockk.spyk | ||
import io.mockk.verify | ||
import org.assertj.core.api.Assertions.assertThat | ||
import org.junit.Assert.fail | ||
|
@@ -65,6 +66,14 @@ abstract class BaseBackendIntegrationTest { | |
|
||
@Before | ||
fun setUp() { | ||
setupTest() | ||
} | ||
|
||
abstract fun apiKey(): String | ||
|
||
protected fun setupTest( | ||
signatureVerificationMode: SignatureVerificationMode = SignatureVerificationMode.Disabled | ||
) { | ||
appConfig = mockk<AppConfig>().apply { | ||
every { baseURL } returns URL("https://api.revenuecat.com") | ||
every { store } returns Store.PLAY_STORE | ||
|
@@ -87,14 +96,12 @@ abstract class BaseBackendIntegrationTest { | |
every { edit() } returns sharedPreferencesEditor | ||
} | ||
eTagManager = ETagManager(sharedPreferences) | ||
signingManager = SigningManager(SignatureVerificationMode.Disabled, appConfig, apiKey()) | ||
signingManager = spyk(SigningManager(signatureVerificationMode, appConfig, apiKey())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I changed to a spy so we can see and verify the calls more easily, but it's using the actual implementation. Since there is no way to know that the offerings/product-entitlement mapping calls were verified or not, this made easier to make sure we were verifying those endpoints There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I checked that by using the "fake invalid signature" thing. Did you add a way to do that in Android too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I meant in the "success" case, we should make sure we are verifying the signature. But right now there is no way to detect whether a |
||
httpClient = HTTPClient(appConfig, eTagManager, diagnosticsTrackerIfEnabled = null, signingManager) | ||
backendHelper = BackendHelper(apiKey(), dispatcher, appConfig, httpClient) | ||
backend = Backend(appConfig, dispatcher, diagnosticsDispatcher, httpClient, backendHelper) | ||
} | ||
|
||
abstract fun apiKey(): String | ||
|
||
protected fun ensureBlockFinishes(block: (CountDownLatch) -> Unit) { | ||
val latch = CountDownLatch(1) | ||
block(latch) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,17 @@ import org.robolectric.annotation.Config | |
@Config(manifest = Config.NONE) | ||
class EndpointTest { | ||
|
||
private val allEndpoints = listOf( | ||
Endpoint.GetCustomerInfo("test-user-id"), | ||
Endpoint.LogIn, | ||
Endpoint.PostReceipt, | ||
Endpoint.GetOfferings("test-user-id"), | ||
Endpoint.GetProductEntitlementMapping, | ||
Endpoint.GetAmazonReceipt("test-user-id", "test-receipt-id"), | ||
Endpoint.PostAttributes("test-user-id"), | ||
Endpoint.PostDiagnostics, | ||
) | ||
|
||
@Test | ||
fun `GetCustomerInfo has correct path`() { | ||
val endpoint = Endpoint.GetCustomerInfo("test user-id") | ||
|
@@ -67,28 +78,73 @@ class EndpointTest { | |
} | ||
|
||
@Test | ||
fun `supportsSignatureValidation returns true for expected values`() { | ||
fun `supportsSignatureVerification returns true for expected values`() { | ||
val expectedSupportsValidationEndpoints = listOf( | ||
Endpoint.GetCustomerInfo("test-user-id"), | ||
Endpoint.LogIn, | ||
Endpoint.PostReceipt | ||
Endpoint.PostReceipt, | ||
Endpoint.GetOfferings("test-user-id"), | ||
Endpoint.GetProductEntitlementMapping, | ||
) | ||
for (endpoint in expectedSupportsValidationEndpoints) { | ||
assertThat(endpoint.supportsSignatureValidation).isTrue | ||
assertThat(endpoint.supportsSignatureVerification) | ||
.withFailMessage { "Endpoint $endpoint expected to support signature validation" } | ||
.isTrue | ||
} | ||
} | ||
|
||
@Test | ||
fun `supportsSignatureValidation returns false for expected values`() { | ||
fun `supportsSignatureVerification returns false for expected values`() { | ||
val expectedNotSupportsValidationEndpoints = listOf( | ||
Endpoint.GetAmazonReceipt("test-user-id", "test-receipt-id"), | ||
Endpoint.GetOfferings("test-user-id"), | ||
Endpoint.PostAttributes("test-user-id"), | ||
Endpoint.PostDiagnostics, | ||
Endpoint.GetProductEntitlementMapping | ||
) | ||
for (endpoint in expectedNotSupportsValidationEndpoints) { | ||
assertThat(endpoint.supportsSignatureValidation).isFalse | ||
assertThat(endpoint.supportsSignatureVerification) | ||
.withFailMessage { "Endpoint $endpoint expected to not support signature validation" } | ||
.isFalse | ||
} | ||
} | ||
|
||
@Test | ||
fun `verify needsNonceToPerformSigning is true only if supportsSignatureVerification is true`() { | ||
for (endpoint in allEndpoints) { | ||
if (!endpoint.supportsSignatureVerification) { | ||
assertThat(endpoint.needsNonceToPerformSigning) | ||
.withFailMessage { "Endpoint $endpoint requires nonce but does not support signature validation" } | ||
.isFalse | ||
} | ||
} | ||
} | ||
|
||
@Test | ||
fun `needsNonceToPerformSigning is true for expected values`() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These tests are great. |
||
val expectedEndpoints = listOf( | ||
Endpoint.GetCustomerInfo("test-user-id"), | ||
Endpoint.LogIn, | ||
Endpoint.PostReceipt, | ||
) | ||
for (endpoint in expectedEndpoints) { | ||
assertThat(endpoint.needsNonceToPerformSigning) | ||
.withFailMessage { "Endpoint $endpoint expected to require nonce for signing" } | ||
.isTrue | ||
} | ||
} | ||
|
||
@Test | ||
fun `needsNonceToPerformSigning is false for expected values`() { | ||
val expectedEndpoints = listOf( | ||
Endpoint.GetOfferings("test-user-id"), | ||
Endpoint.GetProductEntitlementMapping, | ||
Endpoint.GetAmazonReceipt("test-user-id", "test-receipt-id"), | ||
Endpoint.PostAttributes("test-user-id"), | ||
Endpoint.PostDiagnostics, | ||
) | ||
for (endpoint in expectedEndpoints) { | ||
assertThat(endpoint.needsNonceToPerformSigning) | ||
.withFailMessage { "Endpoint $endpoint expected to not require nonce for signing" } | ||
.isFalse | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great 👍🏻