Code review

This commit is contained in:
David Rowe 2020-08-06 21:40:36 +12:00
parent 5ba33a521f
commit 81c402ab9c
4 changed files with 20 additions and 19 deletions

View file

@ -180,10 +180,11 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin
if (!verifiedDomainUserName.isEmpty()) { if (!verifiedDomainUserName.isEmpty()) {
auto userGroups = _domainGroupMemberships[verifiedDomainUserName]; auto userGroups = _domainGroupMemberships[verifiedDomainUserName];
foreach (QString userGroup, userGroups) { foreach (QString userGroup, userGroups) {
// Domain groups may be specified as comma- and/or space-separated lists of group names. // A domain group is signified by a leading special character, "@".
// For example, "@silver @Gold, @platinum". // Multiple domain groups may be specified in one domain server setting as a comma- and/or space-separated lists of
auto domainGroups = _server->_settingsManager.getDomainGroupNames() // domain group names. For example, "@silver @Gold, @platinum".
.filter(QRegularExpression("^(.*[\\s,])?" + userGroup + "([\\s,].*)?$", auto domainGroups = _server->_settingsManager.getDomainServerGroupNames()
.filter(QRegularExpression("^(.*[\\s,])?" + QRegularExpression::escape(userGroup) + "([\\s,].*)?$",
QRegularExpression::CaseInsensitiveOption)); QRegularExpression::CaseInsensitiveOption));
foreach(QString domainGroup, domainGroups) { foreach(QString domainGroup, domainGroups) {
userPerms |= _server->_settingsManager.getPermissionsForGroup(domainGroup, QUuid()); // No rank for domain groups. userPerms |= _server->_settingsManager.getPermissionsForGroup(domainGroup, QUuid()); // No rank for domain groups.
@ -193,7 +194,6 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin
#endif #endif
} }
} }
} }
if (verifiedUsername.isEmpty()) { if (verifiedUsername.isEmpty()) {
@ -301,10 +301,11 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin
if (!verifiedDomainUserName.isEmpty()) { if (!verifiedDomainUserName.isEmpty()) {
auto userGroups = _domainGroupMemberships[verifiedDomainUserName]; auto userGroups = _domainGroupMemberships[verifiedDomainUserName];
foreach(QString userGroup, userGroups) { foreach(QString userGroup, userGroups) {
// Domain groups may be specified as comma- and/or space-separated lists of group names. // A domain group is signified by a leading special character, "@".
// For example, "@silver @Gold, @platinum". // Multiple domain groups may be specified in one domain server setting as a comma- and/or space-separated lists of
auto domainGroups = _server->_settingsManager.getDomainBlacklistGroupNames() // domain group names. For example, "@silver @Gold, @platinum".
.filter(QRegularExpression("^(.*[\\s,])?" + userGroup + "([\\s,].*)?$", auto domainGroups = _server->_settingsManager.getDomainServerBlacklistGroupNames()
.filter(QRegularExpression("^(.*[\\s,])?" + QRegularExpression::escape(userGroup) + "([\\s,].*)?$",
QRegularExpression::CaseInsensitiveOption)); QRegularExpression::CaseInsensitiveOption));
foreach(QString domainGroup, domainGroups) { foreach(QString domainGroup, domainGroups) {
userPerms &= ~_server->_settingsManager.getForbiddensForGroup(domainGroup, QUuid()); userPerms &= ~_server->_settingsManager.getForbiddensForGroup(domainGroup, QUuid());
@ -1277,7 +1278,7 @@ void DomainGatekeeper::requestDomainUserFinished() {
QStringList domainUserGroups; QStringList domainUserGroups;
auto userRoles = rootObject.value("roles").toArray(); auto userRoles = rootObject.value("roles").toArray();
foreach (auto role, userRoles) { 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()); domainUserGroups.append(DOMAIN_GROUP_CHAR + role.toString().toLower());
} }
_domainGroupMemberships[username] = domainUserGroups; _domainGroupMemberships[username] = domainUserGroups;

View file

@ -1966,8 +1966,8 @@ void DomainServerSettingsManager::apiRefreshGroupInformation() {
QStringList groupNames = getAllKnownGroupNames(); QStringList groupNames = getAllKnownGroupNames();
foreach (QString groupName, groupNames) { foreach (QString groupName, groupNames) {
QString lowerGroupName = groupName.toLower(); QString lowerGroupName = groupName.toLower();
if (lowerGroupName.contains(DOMAIN_GROUP_CHAR)) { if (lowerGroupName.startsWith(DOMAIN_GROUP_CHAR)) {
// Ignore domain groups. // Ignore domain groups. (Assumption: metaverse group names can't start with a "@".)
return; return;
} }
if (_groupIDs.contains(lowerGroupName)) { if (_groupIDs.contains(lowerGroupName)) {
@ -2189,8 +2189,8 @@ QList<QUuid> DomainServerSettingsManager::getBlacklistGroupIDs() {
return result.toList(); return result.toList();
} }
QStringList DomainServerSettingsManager::getDomainGroupNames() { QStringList DomainServerSettingsManager::getDomainServerGroupNames() {
// Names as configured in domain server; not necessarily metaverse groups. // All names as listed in the domain server settings; both metaverse groups and domain groups
QSet<QString> result; QSet<QString> result;
foreach(NodePermissionsKey groupKey, _groupPermissions.keys()) { foreach(NodePermissionsKey groupKey, _groupPermissions.keys()) {
result += _groupPermissions[groupKey]->getID(); result += _groupPermissions[groupKey]->getID();
@ -2198,8 +2198,8 @@ QStringList DomainServerSettingsManager::getDomainGroupNames() {
return result.toList(); return result.toList();
} }
QStringList DomainServerSettingsManager::getDomainBlacklistGroupNames() { QStringList DomainServerSettingsManager::getDomainServerBlacklistGroupNames() {
// Names as configured in domain server; not necessarily mnetaverse groups. // All names as listed in the domain server settings; not necessarily mnetaverse groups.
QSet<QString> result; QSet<QString> result;
foreach (NodePermissionsKey groupKey, _groupForbiddens.keys()) { foreach (NodePermissionsKey groupKey, _groupForbiddens.keys()) {
result += _groupForbiddens[groupKey]->getID(); result += _groupForbiddens[groupKey]->getID();

View file

@ -105,8 +105,8 @@ public:
QList<QUuid> getGroupIDs(); QList<QUuid> getGroupIDs();
QList<QUuid> getBlacklistGroupIDs(); QList<QUuid> getBlacklistGroupIDs();
QStringList getDomainGroupNames(); QStringList getDomainServerGroupNames();
QStringList getDomainBlacklistGroupNames(); QStringList getDomainServerBlacklistGroupNames();
// these are used to locally cache the result of calling "api/v1/groups/.../is_member/..." on metaverse's api // 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(); } void clearGroupMemberships(const QString& name) { _groupMembership[name.toLower()].clear(); }

View file

@ -7097,7 +7097,7 @@ void Application::updateWindowTitle() const {
QString metaverseUsername = accountManager->getAccountInfo().getUsername(); QString metaverseUsername = accountManager->getAccountInfo().getUsername();
QString domainUsername = domainAccountManager->getUsername(); QString domainUsername = domainAccountManager->getUsername();
setCrashAnnotation("sentry[user][metaverseUsername]", metaverseUsername.toStdString()); setCrashAnnotation("sentry[user][username]", metaverseUsername.toStdString());
QString currentPlaceName; QString currentPlaceName;
if (isServerlessMode()) { if (isServerlessMode()) {