From f2dafbfcc13467d3b98a6152b16621a12f99749f Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 28 Mar 2023 13:33:16 +0900 Subject: [PATCH 1/3] Cache skus credential till we get valid subs credential fix https://github.com/brave/brave-browser/issues/29345 If we fail to get subs credential from guardian, we should keep skus credential till it's expired. --- .../brave_vpn/browser/brave_vpn_service.cc | 31 ++++++++++++++- .../browser/brave_vpn_service_helper.cc | 24 ++++++++++++ .../browser/brave_vpn_service_helper.h | 4 ++ .../brave_vpn/common/brave_vpn_constants.h | 1 + .../brave_vpn/common/brave_vpn_utils.cc | 38 +++++++++++++++++++ components/brave_vpn/common/brave_vpn_utils.h | 2 + 6 files changed, 98 insertions(+), 2 deletions(-) diff --git a/components/brave_vpn/browser/brave_vpn_service.cc b/components/brave_vpn/browser/brave_vpn_service.cc index 26411c805e43..936b25b6ffaa 100644 --- a/components/brave_vpn/browser/brave_vpn_service.cc +++ b/components/brave_vpn/browser/brave_vpn_service.cc @@ -89,6 +89,10 @@ void BraveVpnService::CheckInitialState() { ScheduleBackgroundRegionDataFetch(); #endif + } else if (HasValidSkusCredential(local_prefs_)) { + // If we have valid skus creds during the startup, we can try to get subs + // credential in advance. + ReloadPurchasedState(); } else { // Try to reload purchased state if cached credential is not valid because // it could be invalidated when not running. @@ -643,9 +647,28 @@ void BraveVpnService::LoadPurchasedState(const std::string& domain) { return; } + if (HasValidSkusCredential(local_prefs_)) { + // We can reach here if we fail to get subscriber credential from guardian. + VLOG(2) << __func__ + << " Try to get subscriber credential with valid cached skus " + "credential."; + + if (GetCurrentEnvironment() != requested_env) { + SetCurrentEnvironment(requested_env); + } + + api_request_.GetSubscriberCredentialV12( + base::BindOnce(&BraveVpnService::OnGetSubscriberCredentialV12, + base::Unretained(this), + GetExpirationTimeForSkusCredential(local_prefs_)), + GetSkusCredential(local_prefs_), + GetBraveVPNPaymentsEnv(GetCurrentEnvironment())); + return; + } + VLOG(2) << __func__ - << ": Checking purchased state as we doesn't have valid subscriber " - "credentials"; + << ": Checking purchased state as we doesn't have valid skus or " + "subscriber credentials"; ClearSubscriberCredential(local_prefs_); EnsureMojoConnected(); @@ -765,6 +788,8 @@ void BraveVpnService::OnPrepareCredentialsPresentation( return; } + SetSkusCredential(local_prefs_, credential, time); + if (GetCurrentEnvironment() != env) { // Change environment because we have successfully authorized with new one. SetCurrentEnvironment(env); @@ -794,6 +819,8 @@ void BraveVpnService::OnGetSubscriberCredentialV12( return; } + // Previously cached skus credential is cleared and fetched subscriber + // credential is cached. SetSubscriberCredential(local_prefs_, subscriber_credential, expiration_time); // Launch one-shot timer for refreshing subscriber_credential before it's diff --git a/components/brave_vpn/browser/brave_vpn_service_helper.cc b/components/brave_vpn/browser/brave_vpn_service_helper.cc index 8be06c631931..fad1c376cfa7 100644 --- a/components/brave_vpn/browser/brave_vpn_service_helper.cc +++ b/components/brave_vpn/browser/brave_vpn_service_helper.cc @@ -151,4 +151,28 @@ void ClearSubscriberCredential(PrefService* local_prefs) { local_prefs->ClearPref(prefs::kBraveVPNSubscriberCredential); } +void SetSkusCredential(PrefService* local_prefs, + const std::string& skus_credential, + const base::Time& expiration_time) { + base::Value::Dict cred_dict; + cred_dict.Set(kSkusCredentialKey, skus_credential); + cred_dict.Set(kSubscriberCredentialExpirationKey, + base::TimeToValue(expiration_time)); + local_prefs->SetDict(prefs::kBraveVPNSubscriberCredential, + std::move(cred_dict)); +} + +base::Time GetExpirationTimeForSkusCredential(PrefService* local_prefs) { + CHECK(HasValidSkusCredential(local_prefs)); + + const base::Value::Dict& sub_cred_dict = + local_prefs->GetDict(prefs::kBraveVPNSubscriberCredential); + + const base::Value* expiration_time_value = + sub_cred_dict.Find(kSubscriberCredentialExpirationKey); + + CHECK(expiration_time_value); + return *base::ValueToTime(expiration_time_value); +} + } // namespace brave_vpn diff --git a/components/brave_vpn/browser/brave_vpn_service_helper.h b/components/brave_vpn/browser/brave_vpn_service_helper.h index f23de201b8cd..def71958825e 100644 --- a/components/brave_vpn/browser/brave_vpn_service_helper.h +++ b/components/brave_vpn/browser/brave_vpn_service_helper.h @@ -40,6 +40,10 @@ void SetSubscriberCredential(PrefService* local_prefs, const std::string& subscriber_credential, const base::Time& expiration_time); void ClearSubscriberCredential(PrefService* local_prefs); +void SetSkusCredential(PrefService* local_prefs, + const std::string& skus_credential, + const base::Time& expiration_time); +base::Time GetExpirationTimeForSkusCredential(PrefService* local_prefs); } // namespace brave_vpn diff --git a/components/brave_vpn/common/brave_vpn_constants.h b/components/brave_vpn/common/brave_vpn_constants.h index cbcbb5bfc59a..fb3643496d72 100644 --- a/components/brave_vpn/common/brave_vpn_constants.h +++ b/components/brave_vpn/common/brave_vpn_constants.h @@ -43,6 +43,7 @@ constexpr char kCreateSubscriberCredentialV12[] = constexpr int kP3AIntervalHours = 24; constexpr char kSubscriberCredentialKey[] = "credential"; +constexpr char kSkusCredentialKey[] = "skus_credential"; constexpr char kSubscriberCredentialExpirationKey[] = "expiration"; #if !BUILDFLAG(IS_ANDROID) diff --git a/components/brave_vpn/common/brave_vpn_utils.cc b/components/brave_vpn/common/brave_vpn_utils.cc index 7887916076de..3da8e34b7012 100644 --- a/components/brave_vpn/common/brave_vpn_utils.cc +++ b/components/brave_vpn/common/brave_vpn_utils.cc @@ -182,4 +182,42 @@ std::string GetSubscriberCredential(PrefService* local_prefs) { return *cred; } +bool HasValidSkusCredential(PrefService* local_prefs) { + const base::Value::Dict& sub_cred_dict = + local_prefs->GetDict(prefs::kBraveVPNSubscriberCredential); + if (sub_cred_dict.empty()) { + return false; + } + + const std::string* skus_cred = sub_cred_dict.FindString(kSkusCredentialKey); + const base::Value* expiration_time_value = + sub_cred_dict.Find(kSubscriberCredentialExpirationKey); + + if (!skus_cred || !expiration_time_value) { + return false; + } + + if (skus_cred->empty()) { + return false; + } + + auto expiration_time = base::ValueToTime(expiration_time_value); + if (!expiration_time || expiration_time < base::Time::Now()) { + return false; + } + + return true; +} + +std::string GetSkusCredential(PrefService* local_prefs) { + CHECK(HasValidSkusCredential(local_prefs)) + << "Don't call when there is no valid skus credential."; + + const base::Value::Dict& sub_cred_dict = + local_prefs->GetDict(prefs::kBraveVPNSubscriberCredential); + const std::string* skus_cred = sub_cred_dict.FindString(kSkusCredentialKey); + DCHECK(skus_cred); + return *skus_cred; +} + } // namespace brave_vpn diff --git a/components/brave_vpn/common/brave_vpn_utils.h b/components/brave_vpn/common/brave_vpn_utils.h index 4711727d8515..6332518bc88a 100644 --- a/components/brave_vpn/common/brave_vpn_utils.h +++ b/components/brave_vpn/common/brave_vpn_utils.h @@ -32,6 +32,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); void RegisterAndroidProfilePrefs(PrefRegistrySimple* registry); bool HasValidSubscriberCredential(PrefService* local_prefs); std::string GetSubscriberCredential(PrefService* local_prefs); +bool HasValidSkusCredential(PrefService* local_prefs); +std::string GetSkusCredential(PrefService* local_prefs); } // namespace brave_vpn From 7b87061f32b0008271e0f35786484f2bc1acadf2 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 28 Mar 2023 16:59:52 +0900 Subject: [PATCH 2/3] Set purchase failed when we failed to subs credental from guardian --- components/brave_vpn/browser/brave_vpn_service.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/brave_vpn/browser/brave_vpn_service.cc b/components/brave_vpn/browser/brave_vpn_service.cc index 936b25b6ffaa..45364596a035 100644 --- a/components/brave_vpn/browser/brave_vpn_service.cc +++ b/components/brave_vpn/browser/brave_vpn_service.cc @@ -813,7 +813,7 @@ void BraveVpnService::OnGetSubscriberCredentialV12( if (subscriber_credential == kTokenNoLongerValid) { SetPurchasedState(GetCurrentEnvironment(), PurchasedState::INVALID); } else { - SetPurchasedState(GetCurrentEnvironment(), PurchasedState::NOT_PURCHASED); + SetPurchasedState(GetCurrentEnvironment(), PurchasedState::FAILED); } #endif return; From 40f722fac07f0fde2184e92ba4e9497b8e766c89 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 28 Mar 2023 17:16:37 +0900 Subject: [PATCH 3/3] Added test cases for skus credential caching --- .../browser/brave_vpn_service_unittest.cc | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/components/brave_vpn/browser/brave_vpn_service_unittest.cc b/components/brave_vpn/browser/brave_vpn_service_unittest.cc index 26e58f39af9d..a6f420c0c721 100644 --- a/components/brave_vpn/browser/brave_vpn_service_unittest.cc +++ b/components/brave_vpn/browser/brave_vpn_service_unittest.cc @@ -541,6 +541,29 @@ TEST_F(BraveVPNServiceTest, RegionDataTest) { EXPECT_EQ(regions()[0], device_region()); } +TEST_F(BraveVPNServiceTest, SkusCredentialCacheTest) { + std::string env = skus::GetDefaultEnvironment(); + std::string domain = skus::GetDomain("vpn", env); + + SetPurchasedState(env, PurchasedState::LOADING); + OnCredentialSummary( + domain, R"({ "active": true, "remaining_credential_count": 1 } )"); + EXPECT_EQ(PurchasedState::LOADING, GetPurchasedStateSync()); + OnPrepareCredentialsPresentation( + domain, "credential=abcdefghijk; Expires=Wed, 21 Oct 2050 07:28:00 GMT"); + EXPECT_TRUE(HasValidSkusCredential(&local_pref_service_)); + OnGetSubscriberCredentialV12("inalid", false); + EXPECT_EQ(PurchasedState::FAILED, GetPurchasedStateSync()); + + // Trying again with valid subscriber credential. + // Check cached skus credential is gone and we have valid subs credential + // instead. + SetPurchasedState(env, PurchasedState::LOADING); + OnGetSubscriberCredentialV12("valid-subs-credentials", true); + EXPECT_FALSE(HasValidSkusCredential(&local_pref_service_)); + EXPECT_TRUE(HasValidSubscriberCredential(&local_pref_service_)); +} + TEST_F(BraveVPNServiceTest, LoadPurchasedStateSessionExpiredTest) { std::string env = skus::GetDefaultEnvironment(); std::string domain = skus::GetDomain("vpn", env);