Skip to content

Commit

Permalink
Trusted entitlements: Do not use etags if cache version not requested…
Browse files Browse the repository at this point in the history 
… and verification enabled (#1114)

### Description
Added additional checks to not use cached etags when cached result is
NOT_REQUESTED and verification is enabled. This will be used for other
signed requests aside from the customer info/post receipt/login
endpoints.
  • Loading branch information
@tonidero
tonidero committed Jul 6, 2023
1 parent 04fb5f8 commit 7002d5a
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 19 deletions.
5 changes: 3 additions & 2 deletions purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTPClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ internal class HTTPClient(
val fullURL = URL(baseURL, urlPathWithVersion)

nonce = if (shouldSignResponse) signingManager.createRandomNonce() else null
val headers = getHeaders(requestHeaders, urlPathWithVersion, refreshETag, nonce)
val headers = getHeaders(requestHeaders, urlPathWithVersion, refreshETag, nonce, shouldSignResponse)

val httpRequest = HTTPRequest(fullURL, headers, jsonBody)

Expand Down Expand Up @@ -237,6 +237,7 @@ internal class HTTPClient(
urlPath: String,
refreshETag: Boolean,
nonce: String?,
shouldSignResponse: Boolean,
): Map<String, String> {
return mapOf(
"Content-Type" to "application/json",
Expand All @@ -252,7 +253,7 @@ internal class HTTPClient(
"X-Nonce" to nonce,
)
.plus(authenticationHeaders)
.plus(eTagManager.getETagHeaders(urlPath, refreshETag))
.plus(eTagManager.getETagHeaders(urlPath, shouldSignResponse, refreshETag))
.filterNotNullValues()
}

Expand Down
17 changes: 12 additions & 5 deletions purchases/src/main/kotlin/com/revenuecat/purchases/common/networking/ETagManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ internal class ETagManager(

internal fun getETagHeaders(
path: String,
verificationRequested: Boolean,
refreshETag: Boolean = false,
): Map<String, String?> {
val eTagData = if (refreshETag) null else getETagData(path)
val storedResult = if (refreshETag) null else getStoredResultSavedInSharedPreferences(path)
val eTagData = storedResult?.eTagData?.takeIf { shouldUseETag(storedResult, verificationRequested) }
return mapOf(
HTTPRequest.ETAG_HEADER_NAME to eTagData?.eTag.orEmpty(),
HTTPRequest.ETAG_LAST_REFRESH_NAME to eTagData?.lastRefreshTime?.time?.toString(),
Expand Down Expand Up @@ -146,17 +148,22 @@ internal class ETagManager(
}
}

private fun getETagData(path: String): ETagData? {
return getStoredResultSavedInSharedPreferences(path)?.eTagData
}

private fun shouldStoreBackendResult(resultFromBackend: HTTPResult): Boolean {
val responseCode = resultFromBackend.responseCode
return responseCode != RCHTTPStatusCodes.NOT_MODIFIED &&
responseCode < RCHTTPStatusCodes.ERROR &&
resultFromBackend.verificationResult != VerificationResult.FAILED
}

private fun shouldUseETag(storedResult: HTTPResultWithETag, verificationRequested: Boolean): Boolean {
return when (storedResult.httpResult.verificationResult) {
VerificationResult.VERIFIED -> true
VerificationResult.NOT_REQUESTED -> !verificationRequested
// Should never happen since we don't store these verification results in the cache
VerificationResult.FAILED, VerificationResult.VERIFIED_ON_DEVICE -> false
}
}

companion object {
fun initializeSharedPreferences(context: Context): SharedPreferences =
context.getSharedPreferences(
Expand Down
2 changes: 1 addition & 1 deletion purchases/src/test/java/com/revenuecat/purchases/common/BaseHTTPClientTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ internal abstract class BaseHTTPClientTest {

protected val mockETagManager = mockk<ETagManager>().also {
every {
it.getETagHeaders(any(), any())
it.getETagHeaders(any(), any(), any())
} answers {
mapOf(HTTPRequest.ETAG_HEADER_NAME to "")
}
Expand Down
8 changes: 4 additions & 4 deletions purchases/src/test/java/com/revenuecat/purchases/common/HTTPClientTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ internal class HTTPClientTest: BaseHTTPClientTest() {
val endpoint = Endpoint.LogIn

every {
mockETagManager.getETagHeaders(any(), any())
mockETagManager.getETagHeaders(any(), any(), any())
} answers {
mapOf(
HTTPRequest.ETAG_HEADER_NAME to "mock-etag",
Expand All @@ -222,7 +222,7 @@ internal class HTTPClientTest: BaseHTTPClientTest() {
val endpoint = Endpoint.LogIn

every {
mockETagManager.getETagHeaders(any(), any())
mockETagManager.getETagHeaders(any(), any(), any())
} answers {
mapOf(
HTTPRequest.ETAG_HEADER_NAME to "mock-etag",
Expand Down Expand Up @@ -365,10 +365,10 @@ internal class HTTPClientTest: BaseHTTPClientTest() {
server.takeRequest()

verify(exactly = 1) {
mockETagManager.getETagHeaders(any(), false)
mockETagManager.getETagHeaders(any(), any(), refreshETag = false)
}
verify(exactly = 1) {
mockETagManager.getETagHeaders(any(), true)
mockETagManager.getETagHeaders(any(), any(), refreshETag = true)
}
assertThat(result.payload).isEqualTo(expectedResult.payload)
assertThat(result.responseCode).isEqualTo(expectedResult.responseCode)
Expand Down
90 changes: 83 additions & 7 deletions purchases/src/test/java/com/revenuecat/purchases/common/networking/ETagManagerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ETagManagerTest {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(expectedETag = null, path = path)

val eTagHeaders = underTest.getETagHeaders(path)
val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isNotNull
assertThat(eTagHeader).isBlank
Expand All @@ -67,7 +67,7 @@ class ETagManagerTest {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(expectedETag = null, path = path)

val eTagHeaders = underTest.getETagHeaders(path)
val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val lastRefreshTimeHeader = eTagHeaders[HTTPRequest.ETAG_LAST_REFRESH_NAME]
assertThat(lastRefreshTimeHeader).isNull()
}
Expand All @@ -77,7 +77,7 @@ class ETagManagerTest {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(expectedETag = "etag", expectedLastRefreshTime = null, path = path)

val eTagHeaders = underTest.getETagHeaders(path)
val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isEqualTo("etag")

Expand All @@ -91,7 +91,7 @@ class ETagManagerTest {
val expectedETag = "etag"
mockCachedHTTPResult(expectedETag, path)

val eTagHeaders = underTest.getETagHeaders(path)
val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isNotNull
assertThat(eTagHeader).isEqualTo(expectedETag)
Expand All @@ -102,7 +102,7 @@ class ETagManagerTest {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(expectedETag = "etag", expectedLastRefreshTime = testDate, path = path)

val eTagHeaders = underTest.getETagHeaders(path)
val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val lastRefreshTimeHeader = eTagHeaders[HTTPRequest.ETAG_LAST_REFRESH_NAME]
assertThat(lastRefreshTimeHeader).isEqualTo("1675954145")
}
Expand All @@ -112,10 +112,86 @@ class ETagManagerTest {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(expectedETag = "etag", expectedLastRefreshTime = testDate, path = path)

val eTagHeaders = underTest.getETagHeaders(path)
val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
assertThat(eTagHeaders.size).isEqualTo(2)
}

// region ETag headers usage verification tests

@Test
fun `ETag headers are added if cached result is verified and verification not requested`() {
val path = "/v1/subscribers/appUserID"
val expectedETag = "etag"
mockCachedHTTPResult(
expectedETag,
path,
httpResult = HTTPResult.createResult(origin = HTTPResult.Origin.CACHE, verificationResult = VERIFIED),
)

val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isEqualTo(expectedETag)
}

@Test
fun `ETag headers are added if cached result is not requested and verification not requested`() {
val path = "/v1/subscribers/appUserID"
val expectedETag = "etag"
mockCachedHTTPResult(
expectedETag,
path,
httpResult = HTTPResult.createResult(origin = HTTPResult.Origin.CACHE, verificationResult = NOT_REQUESTED),
)

val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isEqualTo(expectedETag)
}

@Test
fun `ETag headers are not added if cached result is not requested and verification requested`() {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(
"etag",
path,
httpResult = HTTPResult.createResult(origin = HTTPResult.Origin.CACHE, verificationResult = NOT_REQUESTED),
)

val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = true)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isEqualTo("")
}

@Test
fun `ETag headers are not added if cached result errored`() {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(
"etag",
path,
httpResult = HTTPResult.createResult(origin = HTTPResult.Origin.CACHE, verificationResult = FAILED),
)

val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isEqualTo("")
}

@Test
fun `ETag headers are not added if cached result verified on device`() {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(
"etag",
path,
httpResult = HTTPResult.createResult(origin = HTTPResult.Origin.CACHE, verificationResult = VERIFIED_ON_DEVICE),
)

val eTagHeaders = underTest.getETagHeaders(path, verificationRequested = false)
val eTagHeader = eTagHeaders[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isEqualTo("")
}

// endregion ETag headers usage verification tests

@Test
fun `If response code is 304, cached version should be used`() {
val shouldUse = underTest.shouldUseCachedVersion(RCHTTPStatusCodes.NOT_MODIFIED)
Expand Down Expand Up @@ -256,7 +332,7 @@ class ETagManagerTest {
val path = "/v1/subscribers/appUserID"
mockCachedHTTPResult(expectedETag = null, path = path)

val requestWithETagHeader = underTest.getETagHeaders(path, refreshETag = true)
val requestWithETagHeader = underTest.getETagHeaders(path, verificationRequested = false, refreshETag = true)
val eTagHeader = requestWithETagHeader[HTTPRequest.ETAG_HEADER_NAME]
assertThat(eTagHeader).isNotNull
assertThat(eTagHeader).isBlank
Expand Down

0 comments on commit 7002d5a

Please sign in to comment.