From e466cad0f1defd379beb43c0ecb79e07962d02d7 Mon Sep 17 00:00:00 2001 From: brave-builds Date: Fri, 2 Feb 2024 03:54:49 +0000 Subject: [PATCH 1/2] Uplift of #21847 (squashed) to release --- .../browser/rs/lib/src/sdk/credentials/fetch.rs | 15 ++++++++------- components/skus/browser/rs/lib/src/storage/kv.rs | 1 + 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs b/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs index 37cf373decbc..8e2ed19c73ba 100644 --- a/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs +++ b/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs @@ -132,13 +132,6 @@ where let blinded_creds: Vec = match self.client.get_time_limited_v2_creds(&item.id).await? { Some(item_creds) => { - // overwrite our request id if creds are already present - request_id = match item_creds.request_id { - Some(request_id) => request_id, - // use the item id as the request id if it was not persisted - None => item_id.to_string(), - }; - // are we almost expired let almost_expired = self .last_matching_time_limited_v2_credential(&item.id) @@ -155,6 +148,14 @@ where match item_creds.state { CredentialState::GeneratedCredentials | CredentialState::SubmittedCredentials => { + // overwrite our request id if creds are already present + // that we need to resubmit + request_id = match item_creds.request_id { + Some(request_id) => request_id, + // use the item id as the request id if it was not persisted + None => item_id.to_string(), + }; + // we have generated, or performed submission, reuse the // creds we created for signing. creds diff --git a/components/skus/browser/rs/lib/src/storage/kv.rs b/components/skus/browser/rs/lib/src/storage/kv.rs index 566614f0088c..7e7b734a40e1 100644 --- a/components/skus/browser/rs/lib/src/storage/kv.rs +++ b/components/skus/browser/rs/lib/src/storage/kv.rs @@ -302,6 +302,7 @@ where // don't overwrite the existing unblinded_creds, we just want to add the new blinded // tokens to the item_credentials.creds for signing request if let Credentials::TimeLimitedV2(item_credentials) = item_credentials { + item_credentials.request_id = Some(request_id.to_string()); item_credentials.creds = creds; // update state to generated credentials item_credentials.state = CredentialState::GeneratedCredentials; From efd12f2e8751380b7b9844446a464fadfae4d81c Mon Sep 17 00:00:00 2001 From: eV <8796196+evq@users.noreply.github.com> Date: Tue, 6 Feb 2024 21:01:46 +0000 Subject: [PATCH 2/2] SKUs SDK: regenerate request id on submit conflict from server (#21887) * SKUs SDK: regenerate request id on submit conflict from server * add error if item type is not time limited v2 --- .../rs/lib/src/sdk/credentials/fetch.rs | 16 ++++++++++ .../skus/browser/rs/lib/src/storage/kv.rs | 30 +++++++++++++++++++ .../skus/browser/rs/lib/src/storage/mod.rs | 5 ++++ 3 files changed, 51 insertions(+) diff --git a/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs b/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs index 8e2ed19c73ba..b5ac6d603627 100644 --- a/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs +++ b/components/skus/browser/rs/lib/src/sdk/credentials/fetch.rs @@ -211,6 +211,22 @@ where match resp.status() { http::StatusCode::OK => Ok(()), + http::StatusCode::CONFLICT => { + // On conflict we need to regenerate our request id since + // this indicates a different set of credentials were + // already submitted for the existing id + // NOTE: this should only happen in one of two cases: + // 1. we upgraded from a browser version that did not + // persist request ids and there are multiple devices + // 2. we accidentally re-used a request id due to + // https://github.com/brave/brave-browser/issues/35742 + self.client.upsert_time_limited_v2_item_creds_request_id( + &item.id, + &Uuid::new_v4().to_string(), + ) + .await?; + Err(resp.into()) + }, _ => Err(resp.into()), } }, diff --git a/components/skus/browser/rs/lib/src/storage/kv.rs b/components/skus/browser/rs/lib/src/storage/kv.rs index 7e7b734a40e1..ac679ba16ffd 100644 --- a/components/skus/browser/rs/lib/src/storage/kv.rs +++ b/components/skus/browser/rs/lib/src/storage/kv.rs @@ -330,6 +330,36 @@ where store.set_state(&state) } + #[instrument] + async fn upsert_time_limited_v2_item_creds_request_id( + &self, + item_id: &str, + request_id: &str, + ) -> Result<(), InternalError> { + let mut store = self.get_store()?; + let mut state: KVState = store.get_state()?; + + if state.credentials.is_none() { + state.credentials = Some(CredentialsState::default()); + } + + if let Some(credentials) = state.credentials.as_mut() { + // check and see if we already have this item initialized + if let Some(item_credentials) = credentials.items.get_mut(item_id) { + // overwrite the request_id with the new value + if let Credentials::TimeLimitedV2(item_credentials) = item_credentials { + item_credentials.request_id = Some(request_id.to_string()); + } else { + return Err(InternalError::StorageWriteFailed( + "Item is not time limited v2".to_string(), + )); + } + } + } + + store.set_state(&state) + } + #[instrument] async fn init_single_use_item_creds( &self, diff --git a/components/skus/browser/rs/lib/src/storage/mod.rs b/components/skus/browser/rs/lib/src/storage/mod.rs index 03790d37d4cb..ff25b180df1a 100644 --- a/components/skus/browser/rs/lib/src/storage/mod.rs +++ b/components/skus/browser/rs/lib/src/storage/mod.rs @@ -39,6 +39,11 @@ pub trait StorageClient { request_id: &str, creds: Vec, ) -> Result<(), InternalError>; + async fn upsert_time_limited_v2_item_creds_request_id( + &self, + item_id: &str, + request_id: &str, + ) -> Result<(), InternalError>; async fn append_time_limited_v2_item_unblinded_creds( &self, item_id: &str,