Fix handling of users with multiple identities

ROS users were identified by the pair (identity, auth_url), and in the v10 port
this was translated to (user_id, provider_type), but that isn't actually how
Atlas users work. An App is the equivalent of the old auth_url, and within an
App user_id uniquely identifies a user. Users don't actually have a
"provider type" at all: users have one or more identities, each of which has a
provider type, and the same SyncUser should be used regardless of which
identity was used to log in.

This had surprisingly mild consequences, but was visibly broken in a few ways.
Logging into the same user using two different identities resulted in two
SyncUsers with the same user id. Opening the same partition (or any flx Realm)
with both users resulted in both using whichever SyncUser happened to open it
first, and the second would not create a SyncSession. This meant that most of
the potential bad things (such as creating two session for one file) didn't
actually happen, but the second user's list of sessions would be "incorrect",
and removing one of the users would remove the incorrect set of local files.

Linking identities to anonymous users resulted in the user still being treated
as anonymous. The primary negative effect of this was that linking an identity,
logging out, then logging back in would result in the user being removed
entirely in between, forcing the re-download of all Realms (and the loss of any
un-uploaded data).

This bumps the schema version of the metadata Realm primarily for the sake of
recovery on metadata Realms in invalid states: the migration block handles the
case of multiple users with the same user id and unifies them. Since this needs
a migration anyway, it fixes some incidental problems with the metadata Realm's
schema which weren't fixable without a migration: the marked_for_deletion
column was redundant with the Removed state, identity objects were leaked
due to not being embedded, and the local_uuid field is no longer used for
anything other than opening files written by early v10 versions, so there's no
need to populate it for new users.

Author: tgoyne

CHANGELOG.md

@@ -5,13 +5,21 @@
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
-* None.
+* Logging into a single user using multiple auth providers created a separate SyncUser per auth provider. This mostly worked, but had some quirks:
+  - Sync sessions would not necessarily be associated with the specific SyncUser used to create them. As a result, querying a user for its sessions could give incorrect results, and logging one user out could close the wrong sessions.
+  - Existing local synchronized Realm files created using version of Realm from August - November 2020 would sometimes not be opened correctly and would instead be redownloaded.
+  - Removing one of the SyncUsers would delete all local Realm files for all SyncUsers for that user.
+  - Deleting the server-side user via one of the SyncUsers left the other SyncUsers in an invalid state.
+  - A SyncUser which was originally created via anonymous login and then linked to an identity would still be treated as an anonymous users and removed entirely on logout.
+  (since v10.0.0)
 
 ### Breaking changes
-* None.
+* SyncUser::provider_type() and realm_user_get_auth_provider() have been removed. Users don't have provider types; identities do.
+* SyncUser no longer has a `local_identity()`. `identity()` has been guaranteed to be unique per App ever since v10.
 
 ### Compatibility
 * Fileformat: Generates files with format v23. Reads and automatically upgrade from fileformat v5.
+* The metadata Realm used to store sync users has had its schema version bumped. It is automatically migrated to the new version on first open. Downgrading to older version of Realm after upgrading will discard stored user tokens and require logging back in.
 
 -----------
 

src/realm.h

@@ -3232,13 +3232,9 @@ RLM_API realm_user_state_e realm_user_get_state(const realm_user_t* user) RLM_AP
 RLM_API bool realm_user_get_all_identities(const realm_user_t* user, realm_user_identity_t* out_identities,
                                            size_t capacity, size_t* out_n);
 
-RLM_API const char* realm_user_get_local_identity(const realm_user_t*) RLM_API_NOEXCEPT;
-
 // returned pointer must be manually released with realm_free()
 RLM_API char* realm_user_get_device_id(const realm_user_t*) RLM_API_NOEXCEPT;
 
-RLM_API realm_auth_provider_e realm_user_get_auth_provider(const realm_user_t*) RLM_API_NOEXCEPT;
-
 /**
  * Log out the user and mark it as logged out.
  *

src/realm/collection.hpp

@@ -19,7 +19,7 @@ struct CollectionIterator;
 /// Collections are bound to particular properties of an object. In a
 /// collection's public interface, the implementation must take care to keep the
 /// object consistent with the persisted state, mindful of the fact that the
-/// state may have changed as a consquence of modifications from other instances
+/// state may have changed as a consequence of modifications from other instances
 /// referencing the same persisted state.
 class CollectionBase {
 public:

src/realm/object-store/c_api/app.cpp

@@ -642,11 +642,6 @@ RLM_API bool realm_user_get_all_identities(const realm_user_t* user, realm_user_
     });
 }
 
-RLM_API const char* realm_user_get_local_identity(const realm_user_t* user) noexcept
-{
-    return (*user)->local_identity().c_str();
-}
-
 RLM_API char* realm_user_get_device_id(const realm_user_t* user) noexcept
 {
     if ((*user)->has_device_id()) {
@@ -656,11 +651,6 @@ RLM_API char* realm_user_get_device_id(const realm_user_t* user) noexcept
     return nullptr;
 }
 
-RLM_API realm_auth_provider_e realm_user_get_auth_provider(const realm_user_t* user) noexcept
-{
-    return realm_auth_provider_e(enum_from_provider_type((*user)->provider_type()));
-}
-
 RLM_API bool realm_user_log_out(realm_user_t* user)
 {
     return wrap_err([&] {

src/realm/object-store/sync/app.cpp

@@ -644,7 +644,7 @@ void App::log_in_with_credentials(
     // is already an anonymous session active, reuse it
     if (credentials.provider() == AuthProvider::ANONYMOUS) {
         for (auto&& user : m_sync_manager->all_users()) {
-            if (user->provider_type() == credentials.provider_as_string() && user->is_logged_in()) {
+            if (user->is_anonymous()) {
                 completion(switch_user(user), util::none);
                 return;
             }
@@ -686,8 +686,7 @@ void App::log_in_with_credentials(
                 else {
                     sync_user = self->m_sync_manager->get_user(
                         get<std::string>(json, "user_id"), get<std::string>(json, "refresh_token"),
-                        get<std::string>(json, "access_token"), credentials.provider_as_string(),
-                        get<std::string>(json, "device_id"));
+                        get<std::string>(json, "access_token"), get<std::string>(json, "device_id"));
                 }
             }
             catch (const AppError& e) {
@@ -758,11 +757,7 @@ std::shared_ptr<SyncUser> App::switch_user(const std::shared_ptr<SyncUser>& user
     if (!user || user->state() != SyncUser::State::LoggedIn) {
         throw AppError(ErrorCodes::ClientUserNotLoggedIn, "User is no longer valid or is logged out");
     }
-
-    auto users = m_sync_manager->all_users();
-    auto it = std::find(users.begin(), users.end(), user);
-
-    if (it == users.end()) {
+    if (!verify_user_present(user)) {
         throw AppError(ErrorCodes::ClientUserNotFound, "User does not exist");
     }
 

src/realm/object-store/sync/impl/sync_file.cpp

@@ -295,11 +295,11 @@ bool SyncFileManager::copy_realm_file(const std::string& old_path, const std::st
     return true;
 }
 
-bool SyncFileManager::remove_realm(const std::string& user_identity, const std::string& local_identity,
+bool SyncFileManager::remove_realm(const std::string& user_identity,
+                                   const std::vector<std::string>& legacy_user_identities,
                                    const std::string& raw_realm_path, const std::string& partition) const
 {
-    util::Optional<std::string> existing =
-        get_existing_realm_file_path(user_identity, local_identity, raw_realm_path, partition);
+    auto existing = get_existing_realm_file_path(user_identity, legacy_user_identities, raw_realm_path, partition);
     if (existing) {
         return remove_realm(*existing);
     }
@@ -327,10 +327,10 @@ static bool try_file_remove(const std::string& path) noexcept
     }
 }
 
-util::Optional<std::string> SyncFileManager::get_existing_realm_file_path(const std::string& user_identity,
-                                                                          const std::string& local_user_identity,
-                                                                          const std::string& realm_file_name,
-                                                                          const std::string& partition) const
+util::Optional<std::string>
+SyncFileManager::get_existing_realm_file_path(const std::string& user_identity,
+                                              const std::vector<std::string>& legacy_user_identities,
+                                              const std::string& realm_file_name, const std::string& partition) const
 {
     std::string preferred_name = preferred_realm_path_without_suffix(user_identity, realm_file_name);
     if (try_file_exists(preferred_name)) {
@@ -365,14 +365,14 @@ util::Optional<std::string> SyncFileManager::get_existing_realm_file_path(const
         }
     }
 
-    if (!local_user_identity.empty()) {
+    for (auto& legacy_identity : legacy_user_identities) {
         // retain support for legacy paths
-        std::string old_path = legacy_realm_file_path(local_user_identity, realm_file_name);
+        std::string old_path = legacy_realm_file_path(legacy_identity, realm_file_name);
         if (try_file_exists(old_path)) {
             return old_path;
         }
         // retain support for legacy local identity paths
-        std::string old_local_identity_path = legacy_local_identity_path(local_user_identity, partition);
+        std::string old_local_identity_path = legacy_local_identity_path(legacy_identity, partition);
         if (try_file_exists(old_local_identity_path)) {
             return old_local_identity_path;
         }
@@ -381,11 +381,12 @@ util::Optional<std::string> SyncFileManager::get_existing_realm_file_path(const
     return util::none;
 }
 
-std::string SyncFileManager::realm_file_path(const std::string& user_identity, const std::string& local_user_identity,
+std::string SyncFileManager::realm_file_path(const std::string& user_identity,
+                                             const std::vector<std::string>& legacy_user_identities,
                                              const std::string& realm_file_name, const std::string& partition) const
 {
-    util::Optional<std::string> existing_path =
-        get_existing_realm_file_path(user_identity, local_user_identity, realm_file_name, partition);
+    auto existing_path =
+        get_existing_realm_file_path(user_identity, legacy_user_identities, realm_file_name, partition);
     if (existing_path) {
         return *existing_path;
     }

src/realm/object-store/sync/impl/sync_file.hpp

@@ -69,15 +69,16 @@ class SyncFileManager {
     static bool try_file_exists(const std::string& path) noexcept;
 
     util::Optional<std::string> get_existing_realm_file_path(const std::string& user_identity,
-                                                             const std::string& local_user_identity,
+                                                             const std::vector<std::string>& legacy_user_identities,
                                                              const std::string& realm_file_name,
                                                              const std::string& partition) const;
     /// Return the path for a given Realm, creating the user directory if it does not already exist.
-    std::string realm_file_path(const std::string& user_identity, const std::string& local_user_identity,
+    std::string realm_file_path(const std::string& user_identity,
+                                const std::vector<std::string>& legacy_user_identities,
                                 const std::string& realm_file_name, const std::string& partition) const;
 
     /// Remove the Realm at a given path for a given user. Returns `true` if the remove operation fully succeeds.
-    bool remove_realm(const std::string& user_identity, const std::string& local_identity,
+    bool remove_realm(const std::string& user_identity, const std::vector<std::string>& legacy_user_identities,
                       const std::string& realm_file_name, const std::string& partition) const;
 
     /// Remove the Realm whose primary Realm file is located at `absolute_path`. Returns `true` if the remove