Merge pull request #20 from Psiphon-Inc/v2.3.1

v2.3.1

psicash.cpp

@@ -532,7 +532,7 @@ inline bool IsServerError(int code) {
 }
 
 // Creates the metadata JSON that should be included with requests.
-// This method MUST be called rather than calling UserData::GetRequestMetdata directly.
+// This method MUST be called rather than calling UserData::GetRequestMetadata directly.
 // If `attempt` is 0 it will be omitted from the metadata object.
 json PsiCash::GetRequestMetadata(int attempt) const {
     auto req_metadata = user_data_->GetRequestMetadata();

userdata.cpp

@@ -74,7 +74,9 @@ const char* const kAccountTokenType = "account";
 const char* const kLogoutTokenType = "logout";
 
 
-UserData::UserData() {
+UserData::UserData()
+    : stashed_request_metadata_(json::object())
+{
 }
 
 UserData::~UserData() {
@@ -147,6 +149,9 @@ error::Error UserData::Clear() {
 }
 
 error::Error UserData::DeleteUserData(bool isLoggedOutAccount) {
+    // We're about to delete the request metadata, so now is the time to stash it.
+    SetStashedRequestMetadata(GetRequestMetadata());
+
     Transaction transaction(*this);
     // Not checking return values, since writing is paused.
     (void)datastore_.Set(kUserPtr, json::object());
@@ -268,6 +273,13 @@ error::Error UserData::SetAuthTokens(const AuthTokens& v, bool is_account, const
     (void)datastore_.Set(kAuthTokensPtr, json_tokens);
     (void)datastore_.Set(kIsAccountPtr, is_account);
     (void)datastore_.Set(kAccountUsernamePtr, utf8_username);
+
+    // We may have request metadata that we stashed when the user data was deleted.
+    // Setting auth tokens means we have user data once again, so we should restore that
+    // request metadata. GetRequestMetadata automatically incorporates the stashed
+    // metadata, so we're just going to get it and store it.
+    datastore_.Set(kRequestMetadataPtr, GetRequestMetadata());
+
     return PassError(transaction.Commit()); // write
 }
 
@@ -458,11 +470,18 @@ error::Error UserData::SetLastTransactionID(const TransactionID& v) {
 
 json UserData::GetRequestMetadata() const {
     auto j = datastore_.Get<json>(kRequestMetadataPtr);
-    if (!j) {
-        return json::object();
+    auto stored = json::object();
+    if (j) {
+        stored = *j;
     }
 
-    return *j;
+    // We might have stashed request metadata. We'll merge the stash into the stored
+    // metadata. Either might be empty, or neither. Stored metadata will take precedence.
+    // A side-effect of this is that metadata items that the caller might no longer want
+    // will be retained. But we know that in our use of it this doesn't happen.
+    stored.update(GetStashedRequestMetadata());
+
+    return stored;
 }
 
 std::string UserData::GetLocale() const {
@@ -477,5 +496,15 @@ error::Error UserData::SetLocale(const std::string& v) {
     return PassError(datastore_.Set(kLocalePtr, v));
 }
 
+json UserData::GetStashedRequestMetadata() const {
+    SYNCHRONIZE(stashed_request_metadata_mutex_);
+    auto stashed = stashed_request_metadata_;
+    return stashed;
+}
+
+void UserData::SetStashedRequestMetadata(const json& j) {
+    SYNCHRONIZE(stashed_request_metadata_mutex_);
+    stashed_request_metadata_ = j;
+}
 
 } // namespace psicash

userdata.hpp

@@ -152,8 +152,18 @@ class UserData {
     /// Modifies the purchases in the argument.
     void UpdatePurchasesLocalTimeExpiry(Purchases& purchases) const;
 
+    nlohmann::json GetStashedRequestMetadata() const;
+    void SetStashedRequestMetadata(const nlohmann::json& j);
+
 private:
     Datastore datastore_;
+
+    /// In-memory stash of request metadata. When DeleteUserData is called, the request
+    /// metadata is lost. But we want that data available when making a Login request and
+    /// we want to restore it after login (so that the clients don't have to set it again).
+    /// This _must_ be accessed through the mutex. Use Get/SetStashedRequestMetadata.
+    nlohmann::json stashed_request_metadata_;
+    mutable std::recursive_mutex stashed_request_metadata_mutex_;
 };
 
 } // namespace psicash

userdata_test.cpp

@@ -151,6 +151,23 @@ TEST_F(TestUserData, Persistence)
     }
 }
 
+TEST_F(TestUserData, DeleteUserData)
+{
+    UserData ud;
+    auto err = ud.Init(GetTempDir().c_str(), dev);
+    ASSERT_FALSE(err);
+
+    ASSERT_EQ(ud.GetBalance(), 0);
+    ASSERT_FALSE(ud.GetIsLoggedOutAccount());
+
+    ASSERT_FALSE(ud.SetBalance(1234L));
+    ASSERT_EQ(ud.GetBalance(), 1234L);
+
+    ASSERT_FALSE(ud.DeleteUserData(/*is_logged_out_account=*/true));
+    ASSERT_EQ(ud.GetBalance(), 0);
+    ASSERT_TRUE(ud.GetIsLoggedOutAccount());
+}
+
 TEST_F(TestUserData, GetInstanceID)
 {
     UserData ud;
@@ -630,6 +647,90 @@ TEST_F(TestUserData, Metadata)
     ASSERT_TRUE(v["k"].is_null());
 }
 
+TEST_F(TestUserData, StashMetadata)
+{
+    auto ds_dir = GetTempDir();
+
+    {
+        UserData ud;
+        auto err = ud.Init(ds_dir.c_str(), dev);
+        ASSERT_FALSE(err);
+
+        ASSERT_EQ(ud.GetBalance(), 0);
+        ASSERT_FALSE(ud.GetIsLoggedOutAccount());
+        ASSERT_TRUE(ud.GetRequestMetadata().empty());
+
+        ASSERT_FALSE(ud.SetBalance(1234L));
+        ASSERT_EQ(ud.GetBalance(), 1234L);
+        ASSERT_FALSE(ud.SetRequestMetadataItem("key1", "val1"));
+        auto val1 = ud.GetRequestMetadata().at("key1").get<string>();
+        ASSERT_EQ(val1, "val1");
+        ASSERT_FALSE(ud.DeleteUserData(/*is_logged_out_account=*/true));
+
+        ASSERT_EQ(ud.GetBalance(), 0);
+        ASSERT_TRUE(ud.GetIsLoggedOutAccount());
+        // We still have stashed metadata that should be accessible
+        val1 = ud.GetRequestMetadata().at("key1").get<string>();
+        ASSERT_EQ(val1, "val1");
+
+        // Adding new metadata should result in the new item stored but not the stash
+        ASSERT_FALSE(ud.SetRequestMetadataItem("key2", "val2"));
+        auto val2 = ud.GetRequestMetadata().at("key2").get<string>();
+        ASSERT_EQ(val2, "val2");
+        // Ensure the original value is still available
+        val1 = ud.GetRequestMetadata().at("key1").get<string>();
+        ASSERT_EQ(val1, "val1");
+    }
+
+    // Close and reopen the datastore
+    {
+        UserData ud;
+        auto err = ud.Init(ds_dir.c_str(), dev);
+        ASSERT_FALSE(err);
+
+        ASSERT_EQ(ud.GetBalance(), 0);
+        ASSERT_TRUE(ud.GetIsLoggedOutAccount());
+
+        // Our stashed metadata is gone, but the stored metadata is retained
+        auto val2 = ud.GetRequestMetadata().at("key2").get<string>();
+        ASSERT_EQ(val2, "val2");
+        ASSERT_FALSE(ud.GetRequestMetadata().contains("val1"));
+
+        ASSERT_FALSE(ud.SetBalance(1234L));
+        ASSERT_EQ(ud.GetBalance(), 1234L);
+        ASSERT_FALSE(ud.SetRequestMetadataItem("key1", "val1"));
+        auto val1 = ud.GetRequestMetadata().at("key1").get<string>();
+        ASSERT_EQ(val1, "val1");
+
+        // Delete the user data again
+        ASSERT_FALSE(ud.DeleteUserData(/*is_logged_out_account=*/true));
+
+        ASSERT_EQ(ud.GetBalance(), 0);
+        ASSERT_TRUE(ud.GetIsLoggedOutAccount());
+        // We still have stashed metadata that should be accessible
+        val1 = ud.GetRequestMetadata().at("key1").get<string>();
+        ASSERT_EQ(val1, "val1");
+        val2 = ud.GetRequestMetadata().at("key2").get<string>();
+        ASSERT_EQ(val2, "val2");
+
+        // Setting the auth tokens should make the stashed metadata permanent
+        err = ud.SetAuthTokens({{"k1", {"v1", datetime::DateTime::Now()}}}, false, "");
+    }
+
+    // Close and reopen the datastore again
+    {
+        UserData ud;
+        auto err = ud.Init(ds_dir.c_str(), dev);
+        ASSERT_FALSE(err);
+
+        // This time our request metadata should have persisted
+        auto val1 = ud.GetRequestMetadata().at("key1").get<string>();
+        ASSERT_EQ(val1, "val1");
+        auto val2 = ud.GetRequestMetadata().at("key2").get<string>();
+        ASSERT_EQ(val2, "val2");
+    }
+}
+
 TEST_F(TestUserData, Locale)
 {
     UserData ud;