Remove obsolete read only state from database. (#7324)

share/translations/keepassxc_en.ts

@@ -1342,10 +1342,6 @@ Do you want to delete the entry?
         <source>Error while reading the database: %1</source>
         <translation>Error while reading the database: %1</translation>
     </message>
-    <message>
-        <source>File cannot be written as it is opened in read-only mode.</source>
-        <translation>File cannot be written as it is opened in read-only mode.</translation>
-    </message>
     <message>
         <source>%1
 Backup database located at %2</source>
@@ -1355,10 +1351,6 @@ Backup database located at %2</source>
         <source>Could not save, database does not point to a valid file.</source>
         <translation type="unfinished"></translation>
     </message>
-    <message>
-        <source>Could not save, database file is read-only.</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>Database file has unmerged changes.</source>
         <translation type="unfinished"></translation>
@@ -2208,11 +2200,6 @@ This is definitely a bug, please report it to the developers.</translation>
         <comment>Database tab name modifier</comment>
         <translation>%1 [Locked]</translation>
     </message>
-    <message>
-        <source>%1 [Read-only]</source>
-        <comment>Database tab name modifier</comment>
-        <translation>%1 [Read-only]</translation>
-    </message>
     <message>
         <source>Failed to open %1. It either does not exist or is not accessible.</source>
         <translation type="unfinished"></translation>
@@ -2370,10 +2357,6 @@ Disable safe saves and try again?</translation>
         <source>Writing the database failed: %1</source>
         <translation type="unfinished">Writing the database failed: %1</translation>
     </message>
-    <message>
-        <source>This database is opened in read-only mode. Autosave is disabled.</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>Save database backup</source>
         <translation type="unfinished"></translation>

src/cli/Merge.cpp

@@ -80,7 +80,7 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
     } else {
         db2 = QSharedPointer<Database>::create();
         QString errorMessage;
-        if (!db2->open(fromDatabasePath, database->key(), &errorMessage, false)) {
+        if (!db2->open(fromDatabasePath, database->key(), &errorMessage)) {
             err << QObject::tr("Error reading merge file:\n%1").arg(errorMessage);
             return EXIT_FAILURE;
         }

src/cli/Utils.cpp

@@ -182,7 +182,7 @@ namespace Utils
 
         auto db = QSharedPointer<Database>::create();
         QString error;
-        if (db->open(databaseFilename, compositeKey, &error, false)) {
+        if (db->open(databaseFilename, compositeKey, &error)) {
             return db;
         } else {
             err << error << endl;

src/core/Database.cpp

@@ -96,17 +96,16 @@ QUuid Database::uuid() const
  * read-write mode and fall back to read-only if that is not possible.
  *
  * @param key composite key for unlocking the database
- * @param readOnly open in read-only mode
  * @param error error message in case of failure
  * @return true on success
  */
-bool Database::open(QSharedPointer<const CompositeKey> key, QString* error, bool readOnly)
+bool Database::open(QSharedPointer<const CompositeKey> key, QString* error)
 {
     Q_ASSERT(!m_data.filePath.isEmpty());
     if (m_data.filePath.isEmpty()) {
         return false;
     }
-    return open(m_data.filePath, std::move(key), error, readOnly);
+    return open(m_data.filePath, std::move(key), error);
 }
 
 /**
@@ -116,11 +115,10 @@ bool Database::open(QSharedPointer<const CompositeKey> key, QString* error, bool
  *
  * @param filePath path to the file
  * @param key composite key for unlocking the database
- * @param readOnly open in read-only mode
  * @param error error message in case of failure
  * @return true on success
  */
-bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error, bool readOnly)
+bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error)
 {
     QFile dbFile(filePath);
     if (!dbFile.exists()) {
@@ -154,7 +152,6 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
         return false;
     }
 
-    setReadOnly(readOnly);
     setFilePath(filePath);
     dbFile.close();
 
@@ -260,15 +257,6 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
     QMutexLocker locker(&m_saveMutex);
 
     if (filePath == m_data.filePath) {
-        // Disallow saving to the same file if read-only
-        if (m_data.isReadOnly) {
-            Q_ASSERT_X(false, "Database::saveAs", "Could not save, database file is read-only.");
-            if (error) {
-                *error = tr("Could not save, database file is read-only.");
-            }
-            return false;
-        }
-
         // Fail-safe check to make sure we don't overwrite underlying file changes
         // that have not yet triggered a file reload/merge operation.
         if (!m_fileWatcher->hasSameFileChecksum()) {
@@ -280,7 +268,6 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
     }
 
     // Clear read-only flag
-    setReadOnly(false);
     m_fileWatcher->stop();
 
     QFileInfo fileInfo(filePath);
@@ -402,14 +389,6 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
 
 bool Database::writeDatabase(QIODevice* device, QString* error)
 {
-    Q_ASSERT(!m_data.isReadOnly);
-    if (m_data.isReadOnly) {
-        if (error) {
-            *error = tr("File cannot be written as it is opened in read-only mode.");
-        }
-        return false;
-    }
-
     PasswordKey oldTransformedKey;
     if (m_data.key->isEmpty()) {
         oldTransformedKey.setHash(m_data.transformedDatabaseKey->rawKey());
@@ -556,16 +535,6 @@ bool Database::restoreDatabase(const QString& filePath, const QString& fromBacku
     return false;
 }
 
-bool Database::isReadOnly() const
-{
-    return m_data.isReadOnly;
-}
-
-void Database::setReadOnly(bool readOnly)
-{
-    m_data.isReadOnly = readOnly;
-}
-
 /**
  * Returns true if the database key exists, has subkeys, and the
  * root group exists
@@ -811,7 +780,6 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
                       bool updateTransformSalt,
                       bool transformKey)
 {
-    Q_ASSERT(!m_data.isReadOnly);
     m_keyError.clear();
 
     if (!key) {
@@ -870,14 +838,11 @@ const QVariantMap& Database::publicCustomData() const
 
 void Database::setPublicCustomData(const QVariantMap& customData)
 {
-    Q_ASSERT(!m_data.isReadOnly);
     m_data.publicCustomData = customData;
 }
 
 void Database::createRecycleBin()
 {
-    Q_ASSERT(!m_data.isReadOnly);
-
     auto recycleBin = new Group();
     recycleBin->setUuid(QUuid::createUuid());
     recycleBin->setParent(rootGroup());
@@ -891,7 +856,6 @@ void Database::createRecycleBin()
 
 void Database::recycleEntry(Entry* entry)
 {
-    Q_ASSERT(!m_data.isReadOnly);
     if (m_metadata->recycleBinEnabled()) {
         if (!m_metadata->recycleBin()) {
             createRecycleBin();
@@ -904,7 +868,6 @@ void Database::recycleEntry(Entry* entry)
 
 void Database::recycleGroup(Group* group)
 {
-    Q_ASSERT(!m_data.isReadOnly);
     if (m_metadata->recycleBinEnabled()) {
         if (!m_metadata->recycleBin()) {
             createRecycleBin();
@@ -917,7 +880,6 @@ void Database::recycleGroup(Group* group)
 
 void Database::emptyRecycleBin()
 {
-    Q_ASSERT(!m_data.isReadOnly);
     if (m_metadata->recycleBinEnabled() && m_metadata->recycleBin()) {
         // destroying direct entries of the recycle bin
         QList<Entry*> subEntries = m_metadata->recycleBin()->entries();
@@ -988,15 +950,12 @@ QSharedPointer<Kdf> Database::kdf() const
 
 void Database::setKdf(QSharedPointer<Kdf> kdf)
 {
-    Q_ASSERT(!m_data.isReadOnly);
     m_data.kdf = std::move(kdf);
     setFormatVersion(KeePass2Writer::kdbxVersionRequired(this, true, m_data.kdf.isNull()));
 }
 
 bool Database::changeKdf(const QSharedPointer<Kdf>& kdf)
 {
-    Q_ASSERT(!m_data.isReadOnly);
-
     kdf->randomizeSeed();
     QByteArray transformedDatabaseKey;
     if (!m_data.key) {

src/core/Database.h

@@ -74,11 +74,15 @@ public:
     explicit Database(const QString& filePath);
     ~Database() override;
 
-    bool open(QSharedPointer<const CompositeKey> key, QString* error = nullptr, bool readOnly = false);
+private:
-    bool open(const QString& filePath,
+    bool writeDatabase(QIODevice* device, QString* error = nullptr);
-              QSharedPointer<const CompositeKey> key,
+    bool backupDatabase(const QString& filePath, const QString& destinationFilePath);
-              QString* error = nullptr,
+    bool restoreDatabase(const QString& filePath, const QString& fromBackupFilePath);
-              bool readOnly = false);
+    bool performSave(const QString& filePath, SaveAction flags, const QString& backupFilePath, QString* error);
+
+public:
+    bool open(QSharedPointer<const CompositeKey> key, QString* error = nullptr);
+    bool open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error = nullptr);
     bool save(SaveAction action = Atomic, const QString& backupFilePath = QString(), QString* error = nullptr);
     bool saveAs(const QString& filePath,
                 SaveAction action = Atomic,
@@ -96,8 +100,6 @@ public:
     bool isInitialized() const;
     bool isModified() const;
     bool hasNonDataChanges() const;
-    bool isReadOnly() const;
-    void setReadOnly(bool readOnly);
     bool isSaving();
 
     QUuid uuid() const;
@@ -175,7 +177,6 @@ private:
     {
         quint32 formatVersion = 0;
         QString filePath;
-        bool isReadOnly = false;
         QUuid cipher = KeePass2::CIPHER_AES256;
         CompressionAlgorithm compressionAlgorithm = CompressionGZip;
 
@@ -213,10 +214,6 @@ private:
 
     void createRecycleBin();
 
-    bool writeDatabase(QIODevice* device, QString* error = nullptr);
-    bool backupDatabase(const QString& filePath, const QString& destinationFilePath);
-    bool restoreDatabase(const QString& filePath, const QString& fromBackupFilePath);
-    bool performSave(const QString& filePath, SaveAction flags, const QString& backupFilePath, QString* error);
     void startModifiedTimer();
     void stopModifiedTimer();
 

src/gui/DatabaseOpenWidget.cpp

@@ -202,7 +202,7 @@ void DatabaseOpenWidget::openDatabase()
     QApplication::setOverrideCursor(QCursor(Qt::WaitCursor));
     m_ui->passwordFormFrame->setEnabled(false);
     QCoreApplication::processEvents();
-    bool ok = m_db->open(m_filename, databaseKey, &error, false);
+    bool ok = m_db->open(m_filename, databaseKey, &error);
     QApplication::restoreOverrideCursor();
     m_ui->passwordFormFrame->setEnabled(true);
 

src/gui/DatabaseTabWidget.cpp

@@ -513,20 +513,6 @@ void DatabaseTabWidget::showDatabaseSettings()
     currentDatabaseWidget()->switchToDatabaseSettings();
 }
 
-bool DatabaseTabWidget::isReadOnly(int index) const
-{
-    if (count() == 0) {
-        return false;
-    }
-
-    if (index == -1) {
-        index = currentIndex();
-    }
-
-    auto db = databaseWidgetFromIndex(index)->database();
-    return db && db->isReadOnly();
-}
-
 bool DatabaseTabWidget::isModified(int index) const
 {
     if (count() == 0) {
@@ -543,7 +529,7 @@ bool DatabaseTabWidget::isModified(int index) const
 
 bool DatabaseTabWidget::canSave(int index) const
 {
-    return !isReadOnly(index) && isModified(index);
+    return isModified(index);
 }
 
 bool DatabaseTabWidget::hasLockableDatabases() const
@@ -601,10 +587,6 @@ QString DatabaseTabWidget::tabName(int index)
         tabName = tr("%1 [Locked]", "Database tab name modifier").arg(tabName);
     }
 
-    if (db->isReadOnly()) {
-        tabName = tr("%1 [Read-only]", "Database tab name modifier").arg(tabName);
-    }
-
     if (db->isModified()) {
         tabName.append("*");
     }

src/gui/DatabaseTabWidget.h

@@ -41,7 +41,6 @@ public:
     DatabaseWidget* currentDatabaseWidget();
     DatabaseWidget* databaseWidgetFromIndex(int index) const;
 
-    bool isReadOnly(int index = -1) const;
     bool canSave(int index = -1) const;
     bool isModified(int index = -1) const;
     bool hasLockableDatabases() const;

src/gui/DatabaseWidget.cpp

@@ -1201,10 +1201,6 @@ void DatabaseWidget::unlockDatabase(bool accepted)
         db = m_databaseOpenWidget->database();
     }
     replaceDatabase(db);
-    if (db->isReadOnly()) {
-        showMessage(
-            tr("This database is opened in read-only mode. Autosave is disabled."), MessageWidget::Warning, false, -1);
-    }
 
     restoreGroupEntryFocus(m_groupBeforeLock, m_entryBeforeLock);
     m_groupBeforeLock = QUuid();
@@ -1466,7 +1462,7 @@ void DatabaseWidget::onGroupChanged()
 
 void DatabaseWidget::onDatabaseModified()
 {
-    if (!m_blockAutoSave && config()->get(Config::AutoSaveAfterEveryChange).toBool() && !m_db->isReadOnly()) {
+    if (!m_blockAutoSave && config()->get(Config::AutoSaveAfterEveryChange).toBool()) {
         save();
     } else {
         // Only block once, then reset
@@ -1900,7 +1896,7 @@ bool DatabaseWidget::save()
     }
 
     // Read-only and new databases ask for filename
-    if (m_db->isReadOnly() || m_db->filePath().isEmpty()) {
+    if (m_db->filePath().isEmpty()) {
         return saveAs();
     }
 
@@ -2056,7 +2052,6 @@ bool DatabaseWidget::saveBackup()
 
         if (!newFilePath.isEmpty()) {
             // Ensure we don't recurse back into this function
-            m_db->setReadOnly(false);
             m_db->setFilePath(newFilePath);
             m_saveAttempts = 0;
 

tests/TestDatabase.cpp

@@ -144,11 +144,7 @@ void TestDatabase::testEmptyRecycleBinOnDisabled()
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("123"));
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
-
-    // Explicitly mark DB as read-write in case it was opened from a read-only drive.
-    // Prevents assertion failures on CI systems when the data dir is not writable
-    db->setReadOnly(false);
 
     QSignalSpy spyModified(db.data(), SIGNAL(modified()));
 
@@ -163,8 +159,7 @@ void TestDatabase::testEmptyRecycleBinOnNotCreated()
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("123"));
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
-    db->setReadOnly(false);
 
     QSignalSpy spyModified(db.data(), SIGNAL(modified()));
 
@@ -179,8 +174,7 @@ void TestDatabase::testEmptyRecycleBinOnEmpty()
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("123"));
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
-    db->setReadOnly(false);
 
     QSignalSpy spyModified(db.data(), SIGNAL(modified()));
 
@@ -195,8 +189,7 @@ void TestDatabase::testEmptyRecycleBinWithHierarchicalData()
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("123"));
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
-    db->setReadOnly(false);
 
     QFile originalFile(filename);
     qint64 initialSize = originalFile.size();

tests/TestKdbx3.cpp

@@ -112,7 +112,7 @@ void TestKdbx3::testNonAscii()
     key->addKey(QSharedPointer<PasswordKey>::create(QString::fromUtf8("\xce\x94\xc3\xb6\xd8\xb6")));
     KeePass2Reader reader;
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
     QVERIFY(db.data());
     QVERIFY(!reader.hasError());
     QCOMPARE(db->metadata()->name(), QString("NonAsciiTest"));
@@ -126,7 +126,7 @@ void TestKdbx3::testCompressed()
     key->addKey(QSharedPointer<PasswordKey>::create(""));
     KeePass2Reader reader;
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
     QVERIFY(db.data());
     QVERIFY(!reader.hasError());
     QCOMPARE(db->metadata()->name(), QString("Compressed"));
@@ -140,7 +140,7 @@ void TestKdbx3::testProtectedStrings()
     key->addKey(QSharedPointer<PasswordKey>::create("masterpw"));
     KeePass2Reader reader;
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(filename, key, nullptr, false));
+    QVERIFY(db->open(filename, key, nullptr));
     QVERIFY(db.data());
     QVERIFY(!reader.hasError());
     QCOMPARE(db->metadata()->name(), QString("Protected Strings Test"));
@@ -167,5 +167,5 @@ void TestKdbx3::testBrokenHeaderHash()
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create(""));
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(!db->open(filename, key, nullptr, false));
+    QVERIFY(!db->open(filename, key, nullptr));
 }

tests/TestKeys.cpp

@@ -104,7 +104,7 @@ void TestKeys::testFileKey()
     compositeKey->addKey(fileKey);
 
     auto db = QSharedPointer<Database>::create();
-    QVERIFY(db->open(dbFilename, compositeKey, nullptr, false));
+    QVERIFY(db->open(dbFilename, compositeKey, nullptr));
     QVERIFY(!reader.hasError());
     QCOMPARE(db->metadata()->name(), QString("%1 Database").arg(name));
 }

tests/gui/TestGui.cpp

@@ -1782,7 +1782,7 @@ void TestGui::checkDatabase(const QString& filePath, const QString& expectedDbNa
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("a"));
     auto dbSaved = QSharedPointer<Database>::create();
-    QVERIFY(dbSaved->open(filePath, key, nullptr, false));
+    QVERIFY(dbSaved->open(filePath, key, nullptr));
     QCOMPARE(dbSaved->metadata()->name(), expectedDbName);
 }