Add support for multiple URL's in entry

[varjolintu]

Type of change

• ✅ New feature (non-breaking change which adds functionality)

Description and Context

Adds support for multiple URL's per entry. The new Browser Integration entry settings page has a list view with any additional URL's. These URL's are added to the entry attributes with KP2A_URL_<counter>, which means those are directly compatible with Keepass2Android.

Please note that #3444 (Browser Integration releated entry settings) must be merged before this PR. This extends that feature. Alternatively, that PR can be dismissed totally and all the features will be included in this one.

Fixes #398.

Screenshots

The Browser Integration page shows the following list:

The URL's will be also added to the attributes page:

Testing strategy

Manually. GUI tests are also included.

Checklist:

• ✅ I have read the CONTRIBUTING document. [REQUIRED]
• ✅ My code follows the code style of this project. [REQUIRED]
• ✅ All new and existing tests passed. [REQUIRED]
• ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
• ✅ I have added tests to cover my changes.

[droidmonkey]

Quick review, can't wait to try it out

[varjolintu]

@droidmonkey Fixes made.

[yan12125]

Thanks for this PR! Just a minor note.

[yan12125]

An untested diff for my previous comment:

diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp
index eb4c32b3..edcd793e 100644
--- a/src/browser/BrowserService.cpp
+++ b/src/browser/BrowserService.cpp
@@ -320,7 +320,7 @@ QString BrowserService::storeKey(const QString& key)
             return {};
         }

-        contains = db->metadata()->customData()->contains(QLatin1String(ASSOCIATE_KEY_PREFIX.toUtf8()) + id);
+        contains = db->metadata()->customData()->contains(ASSOCIATE_KEY_PREFIX + id);
         if (contains) {
             dialogResult = MessageBox::warning(nullptr,
                                                tr("KeePassXC: Overwrite existing key?"),
@@ -333,7 +333,7 @@ QString BrowserService::storeKey(const QString& key)
     } while (contains && dialogResult == MessageBox::Cancel);

     hideWindow();
-    db->metadata()->customData()->set(QLatin1String(ASSOCIATE_KEY_PREFIX.toUtf8()) + id, key);
+    db->metadata()->customData()->set(ASSOCIATE_KEY_PREFIX + id, key);
     return id;
 }

@@ -344,7 +344,7 @@ QString BrowserService::getKey(const QString& id)
         return {};
     }

-    return db->metadata()->customData()->value(QLatin1String(ASSOCIATE_KEY_PREFIX.toUtf8()) + id);
+    return db->metadata()->customData()->value(ASSOCIATE_KEY_PREFIX + id);
 }

 QJsonArray BrowserService::findMatchingEntries(const QString& id,
@@ -651,7 +651,7 @@ QList<Entry*> BrowserService::searchEntries(const QString& url, const StringPair
                     // Check if database is connected with KeePassXC-Browser
                     for (const auto& keyPair : keyList) {
                         QString key =
-                            db->metadata()->customData()->value(QLatin1String(ASSOCIATE_KEY_PREFIX.toUtf8()) + keyPair.first);
+                            db->metadata()->customData()->value(ASSOCIATE_KEY_PREFIX + keyPair.first);
                         if (!key.isEmpty() && keyPair.second == key) {
                             databases << db;
                         }
@@ -741,12 +741,9 @@ void BrowserService::convertAttributesToCustomData(const QSharedPointer<Database
         return;
     }

-    const QString keePassBrowserGroupName = QLatin1String(KEEPASSXCBROWSER_GROUP_NAME.toUtf8());
-    const QString keePassHttpGroupName = QLatin1String(KEEPASSHTTP_GROUP_NAME.toUtf8());
-
     for (auto* g : rootGroup->groupsRecursive(true)) {
-        if (g->name() == keePassHttpGroupName) {
-            g->setName(keePassBrowserGroupName);
+        if (g->name() == KEEPASSHTTP_GROUP_NAME) {
+            g->setName(KEEPASSXCBROWSER_GROUP_NAME);
             break;
         }
     }
@@ -919,17 +916,15 @@ Group* BrowserService::getDefaultEntryGroup(const QSharedPointer<Database>& sele
         return nullptr;
     }

-    const QString groupName = QLatin1String(KEEPASSXCBROWSER_GROUP_NAME.toUtf8());
-
     for (auto* g : rootGroup->groupsRecursive(true)) {
-        if (g->name() == groupName && !g->isRecycled()) {
+        if (g->name() == KEEPASSXCBROWSER_GROUP_NAME && !g->isRecycled()) {
             return db->rootGroup()->findGroupByUuid(g->uuid());
         }
     }

     auto* group = new Group();
     group->setUuid(QUuid::createUuid());
-    group->setName(groupName);
+    group->setName(KEEPASSXCBROWSER_GROUP_NAME);
     group->setIcon(KEEPASSXCBROWSER_DEFAULT_ICON);
     group->setParent(rootGroup);
     return group;
@@ -1100,8 +1095,8 @@ int BrowserService::moveKeysToCustomData(Entry* entry, const QSharedPointer<Data
             publicKey.remove(LEGACY_ASSOCIATE_KEY_PREFIX);

             // Add key to database custom data
-            if (db && !db->metadata()->customData()->contains(QLatin1String(ASSOCIATE_KEY_PREFIX.toUtf8()) + publicKey)) {
-                db->metadata()->customData()->set(QLatin1String(ASSOCIATE_KEY_PREFIX.toUtf8()) + publicKey,
+            if (db && !db->metadata()->customData()->contains(ASSOCIATE_KEY_PREFIX + publicKey)) {
+                db->metadata()->customData()->set(ASSOCIATE_KEY_PREFIX + publicKey,
                                                   entry->attributes()->value(key));
                 ++keyCounter;
             }

By the way, could you rebase this PR against the develop branch?

[droidmonkey]

I rebased, this could use a bit of testing before merge. Please give me thumbs up when ready.

[phoerious]

Could we integrate this with the main edit entry widget? I think it makes more sense to support multiple URLs in general than to support it only for browser integration.

Also, I don't really like where this is going with all the vendor prefixes for custom attributes. Should we be initiating a joint effort with other programs to agree on an extended set of attributes with a common prefix? Otherwise this will get very chaotic. There's a reason why CSS got rid of vendor prefixes.

[varjolintu]

@phoerious We could. If we are going to do that, do we maintain the current UI or how are we going to display/add/modify/delete the additional URL's in the main edit entry widget?

I agree that a common prefix with other programs would be a much better choice. AFAIK the only application supporting multiple URL's at this point is KP2A, so it makes sense (at least for me) to use that prefix for now, even if it's a custom one.

[yan12125]

AFAIK the only application supporting multiple URL's at this point is KP2A

Another one that supports multiple URLs in an entry is Kee (formerly KeeFox). During the last time I checked (two years ago), it uses a quite different, JSON-based format as I described in #398 (comment).

[varjolintu]

@yan12125 Thanks. Good to know. It seems a bit complex, if the main function is just to provide an additional URL('s).

[phoerious]

No JSON, please. The KDBX format is very flexible in what attributes it allows and the only difference between "main" attributes and "custom" attributes is that the GUI knows how to interpret the "main" ones. We should make use of that.

[droidmonkey]

So are we good with this for now? We can always work with KeePass2Android to define a more common method. Ideally this would also be a starting point to a better entry template format as well.

[varjolintu]

Rebased.

[AlexanderAmelkin]

Great! Very good you made it compatible with KP2A! Unfortunately, editing the URLs via the 'Browser integration' tab doesn't work for me. I always end up with a <Пустой URL> (Empty URL in Russian) after editing it. I can successfully edit the KP2A_URL_* additional fields though and the edited content is then properly displayed in the "Browser integration" tab.

[opqriu]

While using Appimage in Debian I found an issue where Multiple URLs were not possible. So I came here. Do I need to compile and install keepassxreboot to use this version?

[varjolintu]

@opqriu What version are you using? This is introduced in 2.5.0.

[opqriu]

Thanks. I solved I found it. Good day.

[stratozero]

Is it possible to use wildcard or regular expressions in additional URLs? Or should I add every additional URL as an entry?
e.g.: using https://login.domain.com as URL, then https://*.domain.com as custom URL to match every other, so that when I click on the entry i get redirected to the login page, but also other website with the same second level domain are matched by the browser pugin?

[varjolintu]

Is it possible to use wildcard or regular expressions in additional URLs? Or should I add every additional URL as an entry? e.g.: using https://login.domain.com as URL, then https://*.domain.com as custom URL to match every other, so that when I click on the entry i get redirected to the login page, but also other website with the same second level domain are matched by the browser pugin?

#9835