Skip to content

Allow creating new groups with Browser Integration #2790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/browser/BrowserAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ QJsonObject BrowserAction::handleAction(const QJsonObject& json)
return handleLockDatabase(json, action);
} else if (action.compare("get-database-groups", Qt::CaseSensitive) == 0) {
return handleGetDatabaseGroups(json, action);
} else if (action.compare("create-new-group", Qt::CaseSensitive) == 0) {
return handleCreateNewGroup(json, action);
}

// Action was not recognized
Expand Down Expand Up @@ -407,6 +409,42 @@ QJsonObject BrowserAction::handleGetDatabaseGroups(const QJsonObject& json, cons
return buildResponse(action, message, newNonce);
}

QJsonObject BrowserAction::handleCreateNewGroup(const QJsonObject& json, const QString& action)
{
const QString hash = getDatabaseHash();
const QString nonce = json.value("nonce").toString();
const QString encrypted = json.value("message").toString();

QMutexLocker locker(&m_mutex);
if (!m_associated) {
return getErrorReply(action, ERROR_KEEPASS_ASSOCIATION_FAILED);
}

const QJsonObject decrypted = decryptMessage(encrypted, nonce);
if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
}

QString command = decrypted.value("action").toString();
if (command.isEmpty() || command.compare("create-new-group", Qt::CaseSensitive) != 0) {
return getErrorReply(action, ERROR_KEEPASS_INCORRECT_ACTION);
}

QString group = decrypted.value("groupName").toString();
const QJsonObject newGroup = m_browserService.createNewGroup(group);
if (newGroup.isEmpty() || newGroup["name"].toString().isEmpty() || newGroup["uuid"].toString().isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_CREATE_NEW_GROUP);
}

const QString newNonce = incrementNonce(nonce);

QJsonObject message = buildMessage(newNonce);
message["name"] = newGroup["name"];
message["uuid"] = newGroup["uuid"];

return buildResponse(action, message, newNonce);
}

QJsonObject BrowserAction::getErrorReply(const QString& action, const int errorCode) const
{
QJsonObject response;
Expand Down Expand Up @@ -468,6 +506,8 @@ QString BrowserAction::getErrorMessage(const int errorCode) const
return QObject::tr("No logins found");
case ERROR_KEEPASS_NO_GROUPS_FOUND:
return QObject::tr("No groups found");
case ERROR_KEEPASS_CANNOT_CREATE_NEW_GROUP:
return QObject::tr("Cannot create new group");
default:
return QObject::tr("Unknown error");
}
Expand Down
4 changes: 3 additions & 1 deletion src/browser/BrowserAction.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class BrowserAction : public QObject
ERROR_KEEPASS_EMPTY_MESSAGE_RECEIVED = 13,
ERROR_KEEPASS_NO_URL_PROVIDED = 14,
ERROR_KEEPASS_NO_LOGINS_FOUND = 15,
ERROR_KEEPASS_NO_GROUPS_FOUND = 16
ERROR_KEEPASS_NO_GROUPS_FOUND = 16,
ERROR_KEEPASS_CANNOT_CREATE_NEW_GROUP = 17
};

public:
Expand All @@ -66,6 +67,7 @@ class BrowserAction : public QObject
QJsonObject handleSetLogin(const QJsonObject& json, const QString& action);
QJsonObject handleLockDatabase(const QJsonObject& json, const QString& action);
QJsonObject handleGetDatabaseGroups(const QJsonObject& json, const QString& action);
QJsonObject handleCreateNewGroup(const QJsonObject& json, const QString& action);

QJsonObject buildMessage(const QString& nonce) const;
QJsonObject buildResponse(const QString& action, const QJsonObject& message, const QString& nonce);
Expand Down
80 changes: 77 additions & 3 deletions src/browser/BrowserService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ QString BrowserService::getDatabaseRecycleBinUuid()
return recycleBin->uuidToHex();
}

QJsonArray BrowserService::addChildrenToGroup(Group* group)
QJsonArray BrowserService::getChildrenFromGroup(Group* group)
{
QJsonArray groupList;

Expand All @@ -166,7 +166,7 @@ QJsonArray BrowserService::addChildrenToGroup(Group* group)
QJsonObject jsonGroup;
jsonGroup["name"] = c->name();
jsonGroup["uuid"] = Tools::uuidToHex(c->uuid());
jsonGroup["children"] = addChildrenToGroup(c);
jsonGroup["children"] = getChildrenFromGroup(c);
groupList.push_back(jsonGroup);
}
return groupList;
Expand All @@ -187,7 +187,7 @@ QJsonObject BrowserService::getDatabaseGroups()
QJsonObject root;
root["name"] = rootGroup->name();
root["uuid"] = Tools::uuidToHex(rootGroup->uuid());
root["children"] = addChildrenToGroup(rootGroup);
root["children"] = getChildrenFromGroup(rootGroup);

QJsonArray groups;
groups.push_back(root);
Expand All @@ -198,6 +198,80 @@ QJsonObject BrowserService::getDatabaseGroups()
return result;
}

QJsonObject BrowserService::createNewGroup(const QString& groupName)
{
QJsonObject result;
if (thread() != QThread::currentThread()) {
QMetaObject::invokeMethod(this, "createNewGroup", Qt::BlockingQueuedConnection,
Q_RETURN_ARG(QJsonObject, result), Q_ARG(QString, groupName));
return result;
}

auto db = getDatabase();
if (!db) {
return {};
}

Group* rootGroup = db->rootGroup();
if (!rootGroup) {
return {};
}

auto group = rootGroup->findGroupByPath(groupName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only issue here is that groupName is from user-input data. We should do some basic sanitization and normalization of slashes. Is there a function in the group edit process that checks the name of a group for disallowed characters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any functions that checks input data, even when creating new groups manually.


// Group already exists
if (group) {
result["name"] = group->name();
result["uuid"] = Tools::uuidToHex(group->uuid());
return result;
}

auto dialogResult = MessageBox::warning(nullptr,
tr("KeePassXC: Create a new group"),
tr("A request for creating a new group \"%1\" has been received.\n"
"Do you want to create this group?\n").arg(groupName),
MessageBox::Yes | MessageBox::No);

if (dialogResult != MessageBox::Yes) {
return result;
}

QString name, uuid;
Group* previousGroup = rootGroup;
auto groups = groupName.split("/");

// Returns the group name based on depth
auto getGroupName = [&](int depth) {
QString gName;
for (int i = 0; i < depth+1; ++i) {
gName.append((i == 0 ? "" : "/") + groups[i]);
}
return gName;
};

// Create new group(s) always when the path is not found
for (int i = 0; i < groups.length(); ++i) {
QString gName = getGroupName(i);
auto tempGroup = rootGroup->findGroupByPath(gName);
if (!tempGroup) {
Group* newGroup = new Group();
newGroup->setName(groups[i]);
newGroup->setUuid(QUuid::createUuid());
newGroup->setParent(previousGroup);
name = newGroup->name();
uuid = Tools::uuidToHex(newGroup->uuid());
previousGroup = newGroup;
continue;
}

previousGroup = tempGroup;
}

result["name"] = name;
result["uuid"] = uuid;
return result;
}

QString BrowserService::storeKey(const QString& key)
{
QString id;
Expand Down
3 changes: 2 additions & 1 deletion src/browser/BrowserService.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class BrowserService : public QObject
QString getDatabaseRootUuid();
QString getDatabaseRecycleBinUuid();
QJsonObject getDatabaseGroups();
QJsonObject createNewGroup(const QString& groupName);
QString getKey(const QString& id);
void addEntry(const QString& id,
const QString& login,
Expand Down Expand Up @@ -121,7 +122,7 @@ public slots:
QString baseDomain(const QString& url) const;
QSharedPointer<Database> getDatabase();
QSharedPointer<Database> selectedDatabase();
QJsonArray addChildrenToGroup(Group* group);
QJsonArray getChildrenFromGroup(Group* group);
bool moveSettingsToCustomData(Entry* entry, const QString& name) const;
int moveKeysToCustomData(Entry* entry, const QSharedPointer<Database>& db) const;
bool checkLegacySettings();
Expand Down