`Trusted Entitlements`: add support for signing `POST` body by NachoSoto · Pull Request #2753 · RevenueCat/purchases-ios

NachoSoto · 2023-07-06T00:02:04Z

Changes:

HTTPClient has a new X-Post-Params-Hash request header
Replaced HTTPRequest.Method.post's Encodable value with a more specific HTTPRequestBody (like HTTPResponseBody)
HTTPRequestBody allows overriding contentForSignature (it defaults to []) which then Signing uses for the hash
Fixed LogInOperation not passing the VerificationResult to the result CustomerInfo. This is captured in unit and integration tests now.
Add support for signing POST requests in Path.logIn and Path.postReceiptData

Depends on https://github.com/RevenueCat/khepri/pull/6246

NachoSoto · 2023-07-06T00:15:20Z

Separating this sped up compilation from 4 seconds to less than 100ms (not sure exactly how long, but that was the threshold I set for the warning).

NachoSoto · 2023-07-07T20:07:46Z

+    func testLogInWithValidSignature() async throws {
+        let info = try await Purchases.shared.logIn(UUID().uuidString).customerInfo
+        expect(info.entitlements.verification) == .verified
+    }
+
+    func testLogInWithInvalidSignature() async throws {
+        self.invalidSignature = true
+
+        let info = try await Purchases.shared.logIn(UUID().uuidString).customerInfo
+        expect(info.entitlements.verification) == .failed
+    }


These exposed the fact that LogInOperation wasn't forwarding the verification result.

NachoSoto · 2023-07-07T20:08:51Z

        expect(completionCalled.value).toEventually(equal(2))
    }

-    func testGetsEntitlementsWithVerifiedResponse() {


The diff here is horribly hard to read for no reason.
I simply moved 2 tests into a separate class, which is this:

class BackendPostReceiptWithSignatureVerificationTests: BaseBackendPostReceiptDataTests { override var verificationMode: Configuration.EntitlementVerificationMode { .informational } func testGetsEntitlementsWithVerifiedResponse() { self.httpClient.mock( requestPath: .postReceiptData, response: .init(statusCode: .success, response: Self.validCustomerResponse, verificationResult: .verified) ) let result = waitUntilValue { completed in self.backend.post(receiptData: Self.receiptData, productData: nil, transactionData: .init( appUserID: Self.userID, presentedOfferingID: nil, unsyncedAttributes: nil, storefront: nil, source: .init(isRestore: false, initiationSource: .purchase) ), observerMode: false, completion: { result in completed(result) }) } expect(result).to(beSuccess()) if #available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.2, *) { expect(result?.value?.entitlements.verification) == .verified } } func testGetsEntitlementsWithFailedVerification() { self.httpClient.mock( requestPath: .postReceiptData, response: .init(statusCode: .success, response: Self.validCustomerResponse, verificationResult: .failed) ) let result = waitUntilValue { completed in self.backend.post(receiptData: Self.receiptData2, productData: nil, transactionData: .init( appUserID: Self.userID, presentedOfferingID: nil, unsyncedAttributes: nil, storefront: nil, source: .init(isRestore: false, initiationSource: .purchase) ), observerMode: false, completion: { result in completed(result) }) } expect(result).to(beSuccess()) if #available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.2, *) { expect(result?.value?.entitlements.verification) == .failed } } }

thanks for the heads up!

NachoSoto · 2023-07-07T20:09:35Z

+        expect(headers?.keys).toNot(contain(HTTPClient.RequestHeader.postParameters.rawValue))
+    }
+
+    func testPostRequestWithDisabledSignatureVerificationDoesNotContainPostParametersHeader() {


A test WITH the header is in SignatureVerificationHTTPClientTests below.

NachoSoto · 2023-07-07T20:19:08Z

            .map { response in
-                (response.body, created: response.statusCode == .createdSuccess)
+                (
+                    response.body.copy(with: response.verificationResult),


⚠️ this was missing! And we had no unit/integration tests for it. We do now.

NachoSoto · 2023-07-07T20:51:32Z

I had to refactor these using #2769 because of the change to use HTTPRequestBody

aboedo

approving for now but let's remember to update the signatures of tests with the keys that don't have expiration set

aboedo · 2023-07-07T20:43:44Z

nitpick: reading this without knowing how we do signature verification, this seems like it'd be confusing and hard to understand why we'd need a specific header for post parameters. maybe something like postParametersHeaderForSigning(with body:? (also changed to "with" since the others use "with" too

aboedo · 2023-07-07T21:02:20Z

        expect(completionCalled.value).toEventually(equal(2))
    }

-    func testGetsEntitlementsWithVerifiedResponse() {


thanks for the heads up!

NachoSoto · 2023-07-07T22:22:12Z

approving for now but let's remember to update the signatures of tests with the keys that don't have expiration set

Doing that in a separate PR 👍🏻

codecov · 2023-07-07T22:22:16Z

Codecov Report

Merging #2753 (99ed6be) into main (1e12136) will decrease coverage by 0.01%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##             main    #2753      +/-   ##
==========================================
- Coverage   86.51%   86.50%   -0.01%     
==========================================
  Files         214      216       +2     
  Lines       15396    15482      +86     
==========================================
+ Hits        13320    13393      +73     
- Misses       2076     2089      +13

Impacted Files	Coverage Δ
...ources/Networking/HTTPClient/HTTPRequestBody.swift	`0.00% <0.00%> (ø)`
...urces/Networking/HTTPClient/HTTPResponseBody.swift	`100.00% <ø> (ø)`
...king/Operations/GetIntroEligibilityOperation.swift	`100.00% <ø> (ø)`
...king/Operations/PostAdServicesTokenOperation.swift	`90.90% <ø> (ø)`
...king/Operations/PostAttributionDataOperation.swift	`88.88% <ø> (ø)`
...king/Operations/PostOfferForSigningOperation.swift	`95.89% <ø> (ø)`
...Operations/PostSubscriberAttributesOperation.swift	`90.69% <ø> (ø)`
Sources/Networking/HTTPClient/HTTPClient.swift	`97.88% <82.35%> (-0.66%)`	⬇️
Sources/Security/HTTPRequestBody+Signing.swift	`91.89% <91.89%> (ø)`
Sources/FoundationExtensions/Data+Extensions.swift	`72.22% <100.00%> (+0.79%)`	⬆️
... and 5 more

... and 5 files with indirect coverage changes

NachoSoto · 2023-07-07T22:58:28Z

Verified integration tests are working with the canary 🎉

NachoSoto · 2023-07-13T14:45:04Z

Ship it!

@NachoSoto

**This is an automatic release.** ### Dependency Updates * Bump fastlane from 2.213.0 to 2.214.0 (#2824) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `MainThreadMonitor`: don't crash if there is no test in progress (#2838) via NachoSoto (@NachoSoto) * `CI`: fixed Fastlane APITester lanes (#2836) via NachoSoto (@NachoSoto) * `Integration Tests`: workaround Swift runtime crash (#2826) via NachoSoto (@NachoSoto) * `@EnsureNonEmptyArrayDecodable` (#2831) via NachoSoto (@NachoSoto) * `iOS 17`: added tests for simulating cancellations (#2597) via NachoSoto (@NachoSoto) * `CI`: make all `Codecov` jobs `informational` (#2828) via NachoSoto (@NachoSoto) * `MainThreadMonitor`: check deadlocks only ever N seconds (#2820) via NachoSoto (@NachoSoto) * New `@NonEmptyStringDecodable` (#2819) via NachoSoto (@NachoSoto) * `MockDeviceCache`: avoid using real `UserDefaults` (#2814) via NachoSoto (@NachoSoto) * `throwAssertion`: fixed Xcode 15 compilation (#2813) via NachoSoto (@NachoSoto) * `CustomEntitlementsComputation`: fixed API testers (#2815) via NachoSoto (@NachoSoto) * `PackageTypeTests`: fixed iOS 12 (#2807) via NachoSoto (@NachoSoto) * `Tests`: avoid race-condition in leak detection (#2806) via NachoSoto (@NachoSoto) * Revert "`Unit Tests`: removed leak detection" (#2805) via NachoSoto (@NachoSoto) * `PackageType: Codable` implementation (#2797) via NachoSoto (@NachoSoto) * `SystemInfo.init` no longer `throws` (#2803) via NachoSoto (@NachoSoto) * `Trusted Entitlements`: add support for signing `POST` body (#2753) via NachoSoto (@NachoSoto) * `Tests`: unified default timeouts (#2801) via NachoSoto (@NachoSoto) * `Tests`: removed forced-unwrap (#2799) via NachoSoto (@NachoSoto) * `Tests`: added missing `super.setUp()` (#2804) via NachoSoto (@NachoSoto) * Replaced `FatalErrorUtil` with `Nimble` (#2802) via NachoSoto (@NachoSoto) * `Tests`: fixed another flaky test (#2795) via NachoSoto (@NachoSoto) * `TimingUtil`: improved tests by using `Clock` (#2794) via NachoSoto (@NachoSoto) * `IgnoreDecodeErrors`: log decoding error (#2778) via NachoSoto (@NachoSoto) * `TestLogHandler`: changed all tests to explicitly deinitialize it (#2784) via NachoSoto (@NachoSoto) * `LocalReceiptParserStoreKitTests`: fixed flaky test failure (#2785) via NachoSoto (@NachoSoto) * `Unit Tests`: removed leak detection (#2792) via NachoSoto (@NachoSoto) * `Tests`: fixed another flaky failure with asynchronous check (#2786) via NachoSoto (@NachoSoto) --------- Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>

NachoSoto added the refactor label Jul 6, 2023

NachoSoto requested a review from a team July 6, 2023 00:02

NachoSoto commented Jul 6, 2023

View reviewed changes

NachoSoto force-pushed the nacho/signature-post-params branch 4 times, most recently from e6d030e to d2eb42b Compare July 7, 2023 20:06

NachoSoto commented Jul 7, 2023

View reviewed changes

NachoSoto changed the title ~~[WIP] Trusted Entitlements: add support for signing POST body~~ Trusted Entitlements: add support for signing POST body Jul 7, 2023

NachoSoto marked this pull request as ready for review July 7, 2023 20:17

NachoSoto commented Jul 7, 2023

View reviewed changes

NachoSoto force-pushed the nacho/signature-post-params branch from fc3fdeb to 5f1cde7 Compare July 7, 2023 20:21

NachoSoto commented Jul 7, 2023

View reviewed changes

aboedo approved these changes Jul 7, 2023

View reviewed changes

NachoSoto force-pushed the nacho/signature-post-params branch from b0941c0 to 208d482 Compare July 7, 2023 22:21

NachoSoto enabled auto-merge (squash) July 7, 2023 22:23

NachoSoto disabled auto-merge July 7, 2023 22:24

NachoSoto force-pushed the nacho/signature-post-params branch from a73bd1e to 9caa66d Compare July 10, 2023 14:24

NachoSoto added the blocked label Jul 10, 2023

NachoSoto force-pushed the nacho/signature-post-params branch from 9caa66d to 49eee02 Compare July 11, 2023 20:09

NachoSoto and others added 5 commits July 12, 2023 21:50

WIP

168c238

Updated tests

fe48d93

Finished implementation

d9ec841

Snapshots

8ed90e5

Fixed test

fd98140

NachoSoto added 2 commits July 12, 2023 21:50

Fixed tests

740e895

Improved method name

99ed6be

NachoSoto force-pushed the nacho/signature-post-params branch from 49eee02 to 99ed6be Compare July 13, 2023 04:50

NachoSoto removed the blocked label Jul 13, 2023

NachoSoto merged commit f7e36fb into main Jul 13, 2023

NachoSoto deleted the nacho/signature-post-params branch July 13, 2023 14:45

RCGitBot mentioned this pull request Jul 19, 2023

[AUTOMATIC] Release/4.25.1 #2842

Merged

Conversation

NachoSoto commented Jul 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aboedo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Jul 7, 2023

Uh oh!

codecov Bot commented Jul 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NachoSoto commented Jul 7, 2023

Uh oh!

NachoSoto commented Jul 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented Jul 6, 2023 •

edited

Loading

codecov Bot commented Jul 7, 2023 •

edited

Loading