From 81c402ab9c182f99a1c5a0e2451b1d90d59e6966 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Thu, 6 Aug 2020 21:40:36 +1200 Subject: [PATCH] Code review --- domain-server/src/DomainGatekeeper.cpp | 21 ++++++++++--------- .../src/DomainServerSettingsManager.cpp | 12 +++++------ .../src/DomainServerSettingsManager.h | 4 ++-- interface/src/Application.cpp | 2 +- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index c77d188595..ead4002334 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -180,10 +180,11 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin if (!verifiedDomainUserName.isEmpty()) { auto userGroups = _domainGroupMemberships[verifiedDomainUserName]; foreach (QString userGroup, userGroups) { - // Domain groups may be specified as comma- and/or space-separated lists of group names. - // For example, "@silver @Gold, @platinum". - auto domainGroups = _server->_settingsManager.getDomainGroupNames() - .filter(QRegularExpression("^(.*[\\s,])?" + userGroup + "([\\s,].*)?$", + // A domain group is signified by a leading special character, "@". + // Multiple domain groups may be specified in one domain server setting as a comma- and/or space-separated lists of + // domain group names. For example, "@silver @Gold, @platinum". + auto domainGroups = _server->_settingsManager.getDomainServerGroupNames() + .filter(QRegularExpression("^(.*[\\s,])?" + QRegularExpression::escape(userGroup) + "([\\s,].*)?$", QRegularExpression::CaseInsensitiveOption)); foreach(QString domainGroup, domainGroups) { userPerms |= _server->_settingsManager.getPermissionsForGroup(domainGroup, QUuid()); // No rank for domain groups. @@ -193,7 +194,6 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin #endif } } - } if (verifiedUsername.isEmpty()) { @@ -301,10 +301,11 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin if (!verifiedDomainUserName.isEmpty()) { auto userGroups = _domainGroupMemberships[verifiedDomainUserName]; foreach(QString userGroup, userGroups) { - // Domain groups may be specified as comma- and/or space-separated lists of group names. - // For example, "@silver @Gold, @platinum". - auto domainGroups = _server->_settingsManager.getDomainBlacklistGroupNames() - .filter(QRegularExpression("^(.*[\\s,])?" + userGroup + "([\\s,].*)?$", + // A domain group is signified by a leading special character, "@". + // Multiple domain groups may be specified in one domain server setting as a comma- and/or space-separated lists of + // domain group names. For example, "@silver @Gold, @platinum". + auto domainGroups = _server->_settingsManager.getDomainServerBlacklistGroupNames() + .filter(QRegularExpression("^(.*[\\s,])?" + QRegularExpression::escape(userGroup) + "([\\s,].*)?$", QRegularExpression::CaseInsensitiveOption)); foreach(QString domainGroup, domainGroups) { userPerms &= ~_server->_settingsManager.getForbiddensForGroup(domainGroup, QUuid()); @@ -1277,7 +1278,7 @@ void DomainGatekeeper::requestDomainUserFinished() { QStringList domainUserGroups; auto userRoles = rootObject.value("roles").toArray(); foreach (auto role, userRoles) { - // Distinguish domain groups from metaverse groups by a leading special character. + // Distinguish domain groups from metaverse groups by adding a leading special character. domainUserGroups.append(DOMAIN_GROUP_CHAR + role.toString().toLower()); } _domainGroupMemberships[username] = domainUserGroups; diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index c03f26f0a1..95bf4adc65 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -1966,8 +1966,8 @@ void DomainServerSettingsManager::apiRefreshGroupInformation() { QStringList groupNames = getAllKnownGroupNames(); foreach (QString groupName, groupNames) { QString lowerGroupName = groupName.toLower(); - if (lowerGroupName.contains(DOMAIN_GROUP_CHAR)) { - // Ignore domain groups. + if (lowerGroupName.startsWith(DOMAIN_GROUP_CHAR)) { + // Ignore domain groups. (Assumption: metaverse group names can't start with a "@".) return; } if (_groupIDs.contains(lowerGroupName)) { @@ -2189,8 +2189,8 @@ QList DomainServerSettingsManager::getBlacklistGroupIDs() { return result.toList(); } -QStringList DomainServerSettingsManager::getDomainGroupNames() { - // Names as configured in domain server; not necessarily metaverse groups. +QStringList DomainServerSettingsManager::getDomainServerGroupNames() { + // All names as listed in the domain server settings; both metaverse groups and domain groups QSet result; foreach(NodePermissionsKey groupKey, _groupPermissions.keys()) { result += _groupPermissions[groupKey]->getID(); @@ -2198,8 +2198,8 @@ QStringList DomainServerSettingsManager::getDomainGroupNames() { return result.toList(); } -QStringList DomainServerSettingsManager::getDomainBlacklistGroupNames() { - // Names as configured in domain server; not necessarily mnetaverse groups. +QStringList DomainServerSettingsManager::getDomainServerBlacklistGroupNames() { + // All names as listed in the domain server settings; not necessarily mnetaverse groups. QSet result; foreach (NodePermissionsKey groupKey, _groupForbiddens.keys()) { result += _groupForbiddens[groupKey]->getID(); diff --git a/domain-server/src/DomainServerSettingsManager.h b/domain-server/src/DomainServerSettingsManager.h index 73aef07835..294e441ba4 100644 --- a/domain-server/src/DomainServerSettingsManager.h +++ b/domain-server/src/DomainServerSettingsManager.h @@ -105,8 +105,8 @@ public: QList getGroupIDs(); QList getBlacklistGroupIDs(); - QStringList getDomainGroupNames(); - QStringList getDomainBlacklistGroupNames(); + QStringList getDomainServerGroupNames(); + QStringList getDomainServerBlacklistGroupNames(); // these are used to locally cache the result of calling "api/v1/groups/.../is_member/..." on metaverse's api void clearGroupMemberships(const QString& name) { _groupMembership[name.toLower()].clear(); } diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 4dddcfb1cc..cc2aed7f53 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -7097,7 +7097,7 @@ void Application::updateWindowTitle() const { QString metaverseUsername = accountManager->getAccountInfo().getUsername(); QString domainUsername = domainAccountManager->getUsername(); - setCrashAnnotation("sentry[user][metaverseUsername]", metaverseUsername.toStdString()); + setCrashAnnotation("sentry[user][username]", metaverseUsername.toStdString()); QString currentPlaceName; if (isServerlessMode()) {