8000 Fixed a dead-lock issue in authinfo (#3764) · Kitter/arangodb@7d95de4 · GitHub
[go: up one dir, main page]

< 8000 div class="position-relative header-wrapper js-header-wrapper "> Skip to content

Commit 7d95de4

Browse files
authored
Fixed a dead-lock issue in authinfo (arangodb#3764)
1 parent ce537af commit 7d95de4

File tree

3 files changed

+50
-5
lines changed

3 files changed

+50
-5
lines changed

arangod/RestHandler/RestUsersHandler.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,14 @@ void RestUsersHandler::generateDatabaseResult(AuthInfo* authInfo,
211211
data.add(c->name(), VPackValue("undefined"));
212212
}
213213
});
214-
lvl = authInfo->canUseCollection(user, vocbase->name(), "*");
214+
lvl = authInfo->canUseCollectionNoLock(user, vocbase->name(), "*");
215215
data.add("*", VPackValue(convertFromAuthLevel(lvl)));
216216
} else if (lvl != AuthLevel::NONE) { // hide db's without access
217217
data.add(vocbase->name(), VPackValue(str));
218218
}
219219
});
220220
if (full) {
221-
AuthLevel lvl = authInfo->canUseDatabase(user, "*");
221+
AuthLevel lvl = authInfo->canUseDatabaseNoLock(user, "*");
222222
data("*", VPackValue(VPackValueType::Object))(
223223
"permission", VPackValue(convertFromAuthLevel(lvl)))();
224224
}

arangod/VocBase/AuthInfo.cpp

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,9 @@ static VPackBuilder QueryUser(aql::QueryRegistry* queryRegistry,
208208
if (doc.isExternal()) {
209209
doc = doc.resolveExternals();
210210
}
211-
212-
return VPackBuilder(doc);
211+
VPackBuilder result;
212+
result.add(doc);
213+
return result;
213214
}
214215

215216
static void ConvertLegacyFormat(VPackSlice doc, VPackBuilder& result) {
@@ -680,6 +681,7 @@ Result AuthInfo::removeAllUsers() {
680681
Result res;
681682

682683
{
684+
MUTEX_LOCKER(locker, _loadFromDBLock);
683685
WRITE_LOCKER(guard, _authInfoLock);
684686

685687
for (auto const& pair : _authInfo) {
@@ -698,7 +700,6 @@ Result AuthInfo::removeAllUsers() {
698700

699701
// do not get into race conditions with loadFromDB
700702
{
701-
MUTEX_LOCKER(locker, _loadFromDBLock);
702703
_authInfo.clear();
703704
_authBasicCache.clear();
704705
_outdated = true;
@@ -912,6 +913,17 @@ AuthLevel AuthInfo::canUseDatabase(std::string const& username,
912913
return level;
913914
}
914915

916+
AuthLevel AuthInfo::canUseDatabaseNoLock(std::string const& username,
917+
std::string const& dbname) {
918+
AuthLevel level = configuredDatabaseAuthLevelInternal(username, dbname, 0);
919+
static_assert(AuthLevel::RO < AuthLevel::RW, "ro < rw");
920+
if (level > AuthLevel::RO && !ServerState::writeOpsEnabled()) {
921+
return AuthLevel::RO;
922+
}
923+
return level;
924+
}
925+
926+
915927
// internal method called by configuredCollectionAuthLevel
916928
// asserts that collection name is non-empty and already translated
917929
// from collection id to name
@@ -986,6 +998,29 @@ AuthLevel AuthInfo::canUseCollection(std::string const& username,
986998
return level;
987999
}
9881000

1001+
AuthLevel AuthInfo::canUseCollectionNoLock(std::string const& username,
1002+
std::string const& dbname,
1003+
std::string const& coll) {
1004+
if (coll.empty()) {
1005+
// no collection name given
1006+
return AuthLevel::NONE;
1007+
}
1008+
1009+
AuthLevel level;
1010+
if (coll[0] >= '0' && coll[0] <= '9') {
1011+
std::string tmpColl = DatabaseFeature::DATABASE->translateCollectionName(dbname, coll);
1012+
level = configuredCollectionAuthLevelInternal(username, dbname, tmpColl, 0);
1013+
} else {
1014+
level = configuredCollectionAuthLevelInternal(username, dbname, coll, 0);
1015+
}
1016+
1017+
static_assert(AuthLevel::RO < AuthLevel::RW, "ro < rw");
1018+
if (level > AuthLevel::RO && !ServerState::writeOpsEnabled()) {
1019+
return AuthLevel::RO;
1020+
}
1021+
return level;
1022+
}
1023+
9891024

9901025

9911026
// public called from HttpCommTask.cpp and VstCommTask.cpp

arangod/VocBase/AuthInfo.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ class AuthInfo {
126126
std::string const& dbname,
127127
std::string const& coll);
128128

129+
// No Lock variants of the above to be used in callbacks
130+
// Use with CARE! You need to make sure that the lock
131+
// is held from outside.
132+
AuthLevel canUseDatabaseNoLock(std::string const& username,
133+
std::string const& dbname);
134+
AuthLevel canUseCollectionNoLock(std::string const& username,
135+
std::string const& dbname,
136+
std::string const& coll);
137+
138+
129139
void setJwtSecret(std::string const&);
130140
std::string jwtSecret();
131141
std::string generateJwt(VPackBuilder const&);

0 commit comments

Comments
 (0)
0