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

Trusted entitlements: Do not use etags if cache version not requested and verification enabled #1114

Conversation

tonidero
Copy link
Contributor

@tonidero tonidero commented Jun 30, 2023

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.

@tonidero tonidero marked this pull request as ready for review June 30, 2023 09:18
@tonidero tonidero requested a review from a team June 30, 2023 09:18
@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Merging #1114 (589dbb8) into toniricodiez/sdk-3200-verify-intermediate-signature-android-3 (c5249fd) will decrease coverage by 0.01%.
The diff coverage is 87.50%.

@@                                        Coverage Diff                                        @@
##           toniricodiez/sdk-3200-verify-intermediate-signature-android-3    #1114      +/-   ##
=================================================================================================
- Coverage                                                          85.12%   85.11%   -0.01%     
=================================================================================================
  Files                                                                186      186              
  Lines                                                               6568     6572       +4     
  Branches                                                             932      934       +2     
=================================================================================================
+ Hits                                                                5591     5594       +3     
  Misses                                                               603      603              
- Partials                                                             374      375       +1     
Impacted Files Coverage Δ
...enuecat/purchases/common/networking/ETagManager.kt 95.77% <83.33%> (-1.25%) ⬇️
...java/com/revenuecat/purchases/common/HTTPClient.kt 90.00% <100.00%> (ø)

Copy link
Contributor

@NachoSoto NachoSoto left a comment

Choose a reason for hiding this comment

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

👏🏻
Yeah useful to add this logic here too as you said

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
Copy link
Contributor

Choose a reason for hiding this comment

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

I’ll add this comment too 👍🏻

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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏🏻

Base automatically changed from toniricodiez/sdk-3200-verify-intermediate-signature-android-3 to new-trusted-entitlements-signature-format July 3, 2023 10:55
@tonidero tonidero merged commit 089b669 into new-trusted-entitlements-signature-format Jul 3, 2023
2 checks passed
@tonidero tonidero deleted the toni/do-not-use-etags-if-cache-version-not-requested branch July 3, 2023 10:55
tonidero added a commit that referenced this pull request Jul 6, 2023
… 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.
tonidero added a commit that referenced this pull request Jul 7, 2023
… 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.
tonidero added a commit that referenced this pull request Jul 7, 2023
### Description
Integration branch for the changes in trusted entitlements. Includes
changes from:
- #1111 
- #1114 
- #1118 
- #1119 
- #1124
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants