diff --git a/src/jamidht/archive_account_manager.cpp b/src/jamidht/archive_account_manager.cpp index 6509a0cfb77e0df496c6f82f37b9f1d27c0f2420..f372bb82eb72af7c6cfc5a8787930dbf59513f2e 100644 --- a/src/jamidht/archive_account_manager.cpp +++ b/src/jamidht/archive_account_manager.cpp @@ -361,13 +361,16 @@ ArchiveAccountManager::migrateAccount(AuthContext& ctx) updateArchive(archive); if (updateCertificates(archive, ctx.credentials->updateIdentity)) { + // because updateCertificates already regenerate a device, we do not need to + // regenerate one in onArchiveLoaded onArchiveLoaded(ctx, std::move(archive)); } else ctx.onFailure(AuthError::UNKNOWN, ""); } void -ArchiveAccountManager::onArchiveLoaded(AuthContext& ctx, AccountArchive&& a) +ArchiveAccountManager::onArchiveLoaded(AuthContext& ctx, + AccountArchive&& a) { auto ethAccount = dev::KeyPair(dev::Secret(a.eth_key)).address().hex(); fileutils::check_dir(path_.c_str(), 0700); @@ -378,30 +381,55 @@ ArchiveAccountManager::onArchiveLoaded(AuthContext& ctx, AccountArchive&& a) if (not a.id.second->isCA()) { JAMI_ERR("[Auth] trying to sign a certificate with a non-CA."); } - JAMI_WARN("[Auth] creating new device certificate"); - auto request = ctx.request.get(); - if (not request->verify()) { - JAMI_ERR("[Auth] Invalid certificate request."); - ctx.onFailure(AuthError::INVALID_ARGUMENTS, ""); - return; + std::shared_ptr<dht::crypto::Certificate> deviceCertificate; + std::unique_ptr<ContactList> contacts; + auto usePreviousIdentity = false; + // If updateIdentity got a valid certificate, there is no need for a new cert + if (auto oldId = ctx.credentials->updateIdentity.second) { + contacts = std::make_unique<ContactList>(oldId, path_, onChange_); + if (contacts->isValidAccountDevice(*oldId) && ctx.credentials->updateIdentity.first) { + deviceCertificate = oldId; + usePreviousIdentity = true; + JAMI_WARN("[Auth] Using previously generated certificate %s", + deviceCertificate->getLongId().toString().c_str()); + } else { + contacts.reset(); + } + } + + // Generate a new device if needed + if (!deviceCertificate) { + JAMI_WARN("[Auth] creating new device certificate"); + auto request = ctx.request.get(); + if (not request->verify()) { + JAMI_ERR("[Auth] Invalid certificate request."); + ctx.onFailure(AuthError::INVALID_ARGUMENTS, ""); + return; + } + deviceCertificate = std::make_shared<dht::crypto::Certificate>( + dht::crypto::Certificate::generate(*request, a.id)); + JAMI_WARN("[Auth] created new device: %s", + deviceCertificate->getLongId().toString().c_str()); } - auto deviceCertificate = std::make_shared<dht::crypto::Certificate>( - dht::crypto::Certificate::generate(*request, a.id)); auto receipt = makeReceipt(a.id, *deviceCertificate, ethAccount); auto receiptSignature = a.id.first->sign({receipt.first.begin(), receipt.first.end()}); auto info = std::make_unique<AccountInfo>(); - info->identity.first = ctx.key.get(); + auto pk = usePreviousIdentity ? ctx.credentials->updateIdentity.first : ctx.key.get(); + auto sharedPk = pk->getSharedPublicKey(); + info->identity.first = pk; info->identity.second = deviceCertificate; info->accountId = a.id.second->getId().toString(); - info->devicePk = info->identity.first->getSharedPublicKey(); + info->devicePk = sharedPk; info->deviceId = info->devicePk->getLongId().toString(); if (ctx.deviceName.empty()) ctx.deviceName = info->deviceId.substr(8); - info->contacts = std::make_unique<ContactList>(a.id.second, path_, onChange_); + if (!contacts) + contacts = std::make_unique<ContactList>(a.id.second, path_, onChange_); + info->contacts = std::move(contacts); info->contacts->setContacts(a.contacts); info->contacts->foundAccountDevice(deviceCertificate, ctx.deviceName, clock::now()); info->ethAccount = ethAccount; @@ -410,7 +438,6 @@ ArchiveAccountManager::onArchiveLoaded(AuthContext& ctx, AccountArchive&& a) ConversationModule::saveConvRequestsToPath(path_, a.conversationsRequests); info_ = std::move(info); - JAMI_WARN("[Auth] created new device: %s", info_->deviceId.c_str()); ctx.onSuccess(*info_, std::move(a.config), std::move(receipt.first), diff --git a/src/jamidht/archive_account_manager.h b/src/jamidht/archive_account_manager.h index 689d34bdda124a03fb5d49c95fe389424a0bf816..4441e98a73919fdf6fb8672a9deeec02c0b8f030 100644 --- a/src/jamidht/archive_account_manager.h +++ b/src/jamidht/archive_account_manager.h @@ -110,7 +110,8 @@ private: void loadFromFile(AuthContext& ctx); void loadFromDHT(const std::shared_ptr<AuthContext>& ctx); - void onArchiveLoaded(AuthContext& ctx, AccountArchive&& a); + void onArchiveLoaded(AuthContext& ctx, + AccountArchive&& a); OnExportConfig onExportConfig_; std::string archivePath_; diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index e7fd873319f9099863614fdc2de2656cc8f4480d..e429f1ff582921a46999282861647d0a4a3eba00 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1244,17 +1244,16 @@ JamiAccount::loadAccount(const std::string& archive_password, fDeviceKey, ip_utils::getDeviceName(), std::move(creds), - [this, fDeviceKey, migrating](const AccountInfo& info, - const std::map<std::string, std::string>& config, - std::string&& receipt, - std::vector<uint8_t>&& receipt_signature) { - JAMI_WARN("[Account %s] Auth success !", getAccountID().c_str()); + [this, migrating](const AccountInfo& info, + const std::map<std::string, std::string>& config, + std::string&& receipt, + std::vector<uint8_t>&& receipt_signature) { + JAMI_INFO("[Account %s] Auth success!", getAccountID().c_str()); fileutils::check_dir(idPath_.c_str(), 0700); // save the chain including CA auto id = info.identity; - id.first = std::move(fDeviceKey.get()); std::tie(tlsPrivateKeyFile_, tlsCertificateFile_) = saveIdentity(id, idPath_, DEVICE_ID_PATH); diff --git a/test/unitTest/account_archive/migration.cpp b/test/unitTest/account_archive/migration.cpp index c0d8f15b65e03630a00edfcad70c2e01952835fb..7cb5098db18a18e08e7caf6bfa8c734b09d53d8d 100644 --- a/test/unitTest/account_archive/migration.cpp +++ b/test/unitTest/account_archive/migration.cpp @@ -22,6 +22,7 @@ #include <condition_variable> #include <string> +#include <filesystem> #include <fstream> #include <streambuf> #include <fmt/format.h> @@ -62,9 +63,11 @@ public: private: void testLoadExpiredAccount(); + void testMigrationAfterRevokation(); CPPUNIT_TEST_SUITE(MigrationTest); CPPUNIT_TEST(testLoadExpiredAccount); + CPPUNIT_TEST(testMigrationAfterRevokation); CPPUNIT_TEST_SUITE_END(); }; @@ -73,7 +76,7 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(MigrationTest, MigrationTest::name()); void MigrationTest::setUp() { - auto actors = load_actors_and_wait_for_announcement("actors/alice-bob-password.yml"); + auto actors = load_actors_and_wait_for_announcement("actors/alice-bob.yml"); aliceId = actors["alice"]; bobId = actors["bob"]; } @@ -81,6 +84,9 @@ MigrationTest::setUp() void MigrationTest::tearDown() { + auto bobArchive = std::filesystem::current_path().string() + "/bob.gz"; + std::remove(bobArchive.c_str()); + if (!bob2Id.empty()) wait_for_removal_of({aliceId, bob2Id, bobId}); else @@ -91,6 +97,8 @@ void MigrationTest::testLoadExpiredAccount() { auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId); + auto aliceUri = aliceAccount->getUsername(); + auto aliceDevice = std::string(aliceAccount->currentDeviceId()); // Get alice's expiration auto archivePath = fmt::format("{}/{}/archive.gz", @@ -143,6 +151,76 @@ MigrationTest::testLoadExpiredAccount() accountExpiration = archive.id.second->getExpiration(); CPPUNIT_ASSERT(newDeviceExpiration < deviceExpiration && newAccountExpiration < accountExpiration); + CPPUNIT_ASSERT(aliceUri == aliceAccount->getUsername()); + CPPUNIT_ASSERT(aliceDevice == aliceAccount->currentDeviceId()); +} + +void +MigrationTest::testMigrationAfterRevokation() +{ + auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId); + auto bobUri = bobAccount->getUsername(); + + // Generate bob2 + std::mutex mtx; + std::unique_lock<std::mutex> lk {mtx}; + std::condition_variable cv; + + // Add second device for Bob + std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers; + auto bobArchive = std::filesystem::current_path().string() + "/bob.gz"; + std::remove(bobArchive.c_str()); + bobAccount->exportArchive(bobArchive); + + std::map<std::string, std::string> details = DRing::getAccountTemplate("RING"); + details[ConfProperties::TYPE] = "RING"; + details[ConfProperties::DISPLAYNAME] = "BOB2"; + details[ConfProperties::ALIAS] = "BOB2"; + details[ConfProperties::UPNP_ENABLED] = "true"; + details[ConfProperties::ARCHIVE_PASSWORD] = ""; + details[ConfProperties::ARCHIVE_PIN] = ""; + details[ConfProperties::ARCHIVE_PATH] = bobArchive; + + auto deviceRevoked = false; + confHandlers.insert( + DRing::exportable_callback<DRing::ConfigurationSignal::DeviceRevocationEnded>( + [&](const std::string& accountId, const std::string&, int status) { + if (accountId == bobId && status == 0) + deviceRevoked = true; + cv.notify_one(); + })); + auto bobMigrated = false; + confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::MigrationEnded>( + [&](const std::string& accountId, const std::string& state) { + if (accountId == bob2Id && state == "SUCCESS") { + bobMigrated = true; + } + cv.notify_one(); + })); + auto knownChanged = false; + confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::KnownDevicesChanged>( + [&](const std::string& accountId, auto devices) { + if (accountId == bobId && devices.size() == 2) + knownChanged = true; + cv.notify_one(); + })); + DRing::registerSignalHandlers(confHandlers); + + bob2Id = Manager::instance().addAccount(details); + auto bob2Account = Manager::instance().getAccount<JamiAccount>(bob2Id); + + CPPUNIT_ASSERT(cv.wait_for(lk, 20s, [&]() { return knownChanged; })); + + // Revoke bob2 + auto bob2Device = std::string(bob2Account->currentDeviceId()); + bobAccount->revokeDevice("", bob2Device); + CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return deviceRevoked; })); + + // Check migration is triggered and expiration updated + bob2Account->forceReloadAccount(); + CPPUNIT_ASSERT(cv.wait_for(lk, 15s, [&]() { return bobMigrated; })); + // Because the device was revoked, a new ID must be generated there + CPPUNIT_ASSERT(bob2Device != bob2Account->currentDeviceId()); } } // namespace test