SKUs SDK Changes to Support Receipt Submission #13820
Conversation
bsclifton
force-pushed
the
bh-skus-sdk-submit-receipt
branch
from
7f8706a
to
8eb1baf
Compare
bsclifton
force-pushed
the
bh-skus-sdk-submit-receipt
branch
2 times, most recently
from
cce3eed
to
92d22fb
Compare
|
We don't have an integration using this... yet! But #13387 will definitely be using this code 😄 See more details in https://github.com/brave/security/issues/933#issuecomment-1169200217 Code LGTM - but would leave to @evq or another person to review also (since I helped 😄). @husobee there were some comments by @spylogsster in case you missed them above |
bsclifton
force-pushed
the
bh-skus-sdk-submit-receipt
branch
from
92d22fb
to
641462c
Compare
|
I don't love all the repeated mangling we're doing at the moment to convert errors into something that wasm-bindgen is happy with, I think we should look into whether the resolution linked at the bottom of this issue helps clean things up [1] |
husobee
requested review from
a team,
samartnik,
goodov,
iefremov and
simonhong
as code owners
bsclifton
force-pushed
the
bh-skus-sdk-submit-receipt
branch
from
c8ede13
to
28f4708
Compare
Partial implementation for brave/brave-browser#23167
bsclifton
force-pushed
the
bh-skus-sdk-submit-receipt
branch
from
28f4708
to
114f1b2
Compare
husobee
force-pushed
the
bh-skus-sdk-submit-receipt
branch
from
114f1b2
to
a2f535c
Compare
Correcting serde unwrap in SDK. Utilizing APIError structure in InternalError implementation Propogate endpoint specific error conditions through wasm removing ExternalPurchase errors in favor of generic api errors fixing cxx rust errors to match sdk
husobee
force-pushed
the
bh-skus-sdk-submit-receipt
branch
from
a2f535c
to
3e13292
Compare
| } | ||
| ffi::SkusResult::UnhandledStatus => { | ||
| skus::errors::InternalError::UnhandledStatus(skus::models::APIError { | ||
| code: 499, |
There was a problem hiding this comment.
This used to be 699, not 499 like BadRequest. Is that correct?
| Ok(None) => callback.0(callback_state.into_raw(), ffi::SkusResult::Ok, ""), | ||
| Err(e) => callback.0(callback_state.into_raw(), e, ""), | ||
| } | ||
| } | ||
|
|
||
| #[repr(transparent)] | ||
| pub struct CredentialSummaryCallback( | ||
| pub extern "C" fn( | ||
| pub extern "C" fn( |
There was a problem hiding this comment.
https://github.com/rust-lang/rust.vim is the plugin I have been using, and it keeps adding this one particular whitespace in ;)
I'll disable the formatting on save to change this.
There was a problem hiding this comment.
Wild, I've never seen that! I filed rust-lang/rustfmt#5525
| builder.uri(format!("{}/v1/orders/{}/submit-receipt", self.base_url, order_id)); | ||
|
|
||
| let receipt_bytes: Vec<u8> = receipt.as_bytes().to_vec(); | ||
| let req = builder.body(receipt_bytes).unwrap(); |
There was a problem hiding this comment.
I don't like the unwrap() calls here. They're fine for static input, which I guess is the case here, but you have to read the http crate source to determine that. This should be mapped to an error.
However, since the pattern isn't new to this patch, I'm ok with this as-is if you file a follow-up issue about addressing it.
| name = "zeroize" | ||
| version = "1.4.3" | ||
| version = "1.3.0" |
There was a problem hiding this comment.
Downgrade to match challenge-bypass-ristretto and curve25519-dalek because of api drift in 1.4. Ok.
Implementation of generic receipt submission client for SKUs SDK, including WASM bindings which will support various vendor (google, apple) payment confirmations for in-app purchases.
Resolves brave/brave-browser#23167
Security / privacy review: https://github.com/brave/security/issues/933
Submitter Checklist:
QA/YesorQA/No;release-notes/includeorrelease-notes/exclude;OS/...) to the associated issuenpm run test -- brave_browser_tests,npm run test -- brave_unit_tests,npm run lint,npm run gn_check,npm run tslintgit rebase master(if needed)Reviewer Checklist:
gnAfter-merge Checklist:
changes has landed on
Test Plan: