10000 Fix isAdminUser. (#11326) · arangodb/arangodb@d2a17a2 · GitHub
[go: up one dir, main page]
Skip to content

Commit d2a17a2

Browse files
neunhoefneunhoef
authored
Fix isAdminUser. (#11326)
1 parent 7d53f2d commit d2a17a2

34 files changed

+457
-131
lines changed

‎arangod/Actions/RestActionHandler.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "RestActionHandler.h"
2525

2626
#include "Actions/actions.h"
27+
#include "Basics/StaticStrings.h"
2728
#include "Basics/StringUtils.h"
2829
#include "Statistics/RequestStatistics.h"
2930
#include "VocBase/vocbase.h"
@@ -40,7 +41,7 @@ RestActionHandler::RestActionHandler(application_features::ApplicationServer& se
4041

4142
RestStatus RestActionHandler::execute() {
4243
// check the request path
43-
if (_request->databaseName() == TRI_VOC_SYSTEM_DATABASE) {
44+
if (_request->databaseName() == StaticStrings::SystemDatabase) {
4445
if (StringUtils::isPrefix(_request->requestPath(), "/_admin/aardvark")) {
4546
RequestStatistics::SET_IGNORE(_statistics);
4647
}

‎arangod/Auth/User.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ auth::Level auth::User::collectionAuthLevel(std::string const& dbname,
559559
bool isSystem = cname[0] == '_';
560560
if (isSystem) {
561561
// disallow access to _system/_users for everyone
562-
if (dbname == TRI_VOC_SYSTEM_DATABASE && cname == TRI_COL_NAME_USERS) {
562+
if (dbname == StaticStrings::SystemDatabase && cname == TRI_COL_NAME_USERS) {
563563
return auth::Level::NONE;
564564
} else if (cname == "_queues") {
565565
return auth::Level::RO;

‎arangod/Cluster/ClusterInfo.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "Basics/MutexLocker.h"
3333
#include "Basics/NumberUtils.h"
3434
#include "Basics/RecursiveLocker.h"
35+
#include "Basics/StaticStrings.h"
3536
#include "Basics/StringUtils.h"
3637
#include "Basics/VelocyPackHelper.h"
3738
#include "Basics/WriteLocker.h"
@@ -1759,7 +1760,7 @@ Result ClusterInfo::dropDatabaseCoordinator( // drop database
17591760
double timeout // request timeout
17601761
) {
17611762
TRI_ASSERT(ServerState::instance()->isCoordinator());
1762-
if (name == TRI_VOC_SYSTEM_DATABASE) {
1763+
if (name == StaticStrings::SystemDatabase) {
17631764
return Result(TRI_ERROR_FORBIDDEN);
17641765
}
17651766

‎arangod/MMFiles/MMFilesEngine.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "Basics/MutexLocker.h"
3030
#include "Basics/MutexUnlocker.h"
3131
#include "Basics/ReadLocker.h"
32+
#include "Basics/StaticStrings.h"
3233
#include "Basics/StringUtils.h"
3334
#include "Basics/VelocyPackHelper.h"
3435
#include "Basics/WriteLocker.h"
@@ -264,7 +265,7 @@ void MMFilesEngine::start() {
264265
if (names.empty()) {
265266
// no databases found, i.e. there is no system database!
266267
// create a database for the system database
267-
int res = createDatabaseDirectory(TRI_NewTickServer(), TRI_VOC_SYSTEM_DATABASE);
268+
int res = createDatabaseDirectory(TRI_NewTickServer(), StaticStrings::SystemDatabase);
268269

269270
if (res != TRI_ERROR_NO_ERROR) {
270271
LOG_TOPIC("982c7", ERR, arangodb::Logger::ENGINES)

‎arangod/Replication/GlobalInitialSyncer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include "ApplicationFeatures/ApplicationServer.h"
2727
#include "Basics/Result.h"
28+
#include "Basics/StaticStrings.h"
2829
#include "Basics/StringUtils.h"
2930
#include "Basics/VelocyPackHelper.h"
3031
#include "Replication/DatabaseInitialSyncer.h"
@@ -50,7 +51,7 @@ using namespace arangodb::rest;
5051
GlobalInitialSyncer::GlobalInitialSyncer(ReplicationApplierConfiguration const& configuration)
5152
: InitialSyncer(configuration) {
5253
// has to be set here, otherwise broken
53-
_state.databaseName = TRI_VOC_SYSTEM_DATABASE;
54+
_state.databaseName = StaticStrings::SystemDatabase;
5455
}
5556

5657
GlobalInitialSyncer::~GlobalInitialSyncer() {

‎arangod/Replication/GlobalTailingSyncer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
////////////////////////////////////////////////////////////////////////////////
2323

2424
#include "GlobalTailingSyncer.h"
25+
#include "Basics/StaticStrings.h"
2526
#include "Basics/Thread.h"
2627
#include "Logger/LogMacros.h"
2728
#include "Logger/Logger.h"
@@ -43,7 +44,7 @@ GlobalTailingSyncer::GlobalTailingSyncer(ReplicationApplierConfiguration const&
4344
configuration, initialTick, useTick, barrierId),
4445
_queriedTranslations(false) {
4546
_ignoreDatabaseMarkers = false;
46-
_state.databaseName = TRI_VOC_SYSTEM_DATABASE;
47+
_state.databaseName = StaticStrings::SystemDatabase;
4748
}
4849

4950
std::string GlobalTailingSyncer::tailingBaseUrl(std::string const& command) {

‎arangod/Replication/TailingSyncer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ Result TailingSyncer::processDBMarker(TRI_replication_operation_e type,
334334

335335
TRI_vocbase_t* vocbase = DatabaseFeature::DATABASE->lookupDatabase(name);
336336

337-
if (vocbase != nullptr && name != TRI_VOC_SYSTEM_DATABASE) {
337+
if (vocbase != nullptr && name != StaticStrings::SystemDatabase) {
338338
LOG_TOPIC("0a3a4", WARN, Logger::REPLICATION)
339339
<< "seeing database creation marker "
340340
<< "for an already existing db. Dropping db...";
@@ -357,7 +357,7 @@ Result TailingSyncer::processDBMarker(TRI_replication_operation_e type,
357357
} else if (type == REPLICATION_DATABASE_DROP) {
358358
TRI_vocbase_t* vocbase = DatabaseFeature::DATABASE->lookupDatabase(name);
359359

360-
if (vocbase != nullptr && name != TRI_VOC_SYSTEM_DATABASE) {
360+
if (vocbase != nullptr && name != StaticStrings::SystemDatabase) {
361361
// abort all ongoing transactions for the database to be dropped
362362
abortOngoingTransactions(name);
363363

‎arangod/RestHandler/RestAdminClusterHandler.cpp

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,10 @@ std::string const RestAdminClusterHandler::RemoveServer = "removeServer";
272272
std::string const RestAdminClusterHandler::RebalanceShards = "rebalanceShards";
273273

274274
RestStatus RestAdminClusterHandler::execute() {
275-
if (!ExecContext::current().isAdminUser()) {
276-
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
277-
return RestStatus::DONE;
278-
}
275+
// No more check for admin rights here, since we handle this in every individual
276+
// method below. Some of them do no longer require admin access
277+
// (e.g. /_admin/cluster/health). If you add a new API below here, please
278+
// make sure to check for permissions!
279279

280280
auto const& suffixes = request()->suffixes();
281281
size_t const len = suffixes.size();
@@ -965,6 +965,11 @@ RestStatus RestAdminClusterHandler::handleShardDistribution() {
965965
return RestStatus::DONE;
966966
}
967967

968+
if (!ExecContext::current().isAdminUser()) {
969+
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
970+
return RestStatus::DONE;
971+
}
972+
968973
if (request()->requestType() != rest::RequestType::GET) {
969974
generateError(rest::ResponseCode::METHOD_NOT_ALLOWED, TRI_ERROR_HTTP_METHOD_NOT_ALLOWED);
970975
return RestStatus::DONE;
@@ -1014,6 +1019,11 @@ RestStatus RestAdminClusterHandler::handleCollectionShardDistribution() {
10141019
return RestStatus::DONE;
10151020
}
10161021

1022+
if (!ExecContext::current().isAdminUser()) {
1023+
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
1024+
return RestStatus::DONE;
1025+
}
1026+
10171027
switch (request()->requestType()) {
10181028
case rest::RequestType::GET:
10191029
return handleGetCollectionShardDistribution(
@@ -1392,7 +1402,8 @@ RestStatus RestAdminClusterHandler::handlePutNumberOfServers() {
13921402
}
13931403

13941404
RestStatus RestAdminClusterHandler::handleNumberOfServers() {
1395-
if (!ServerState::instance()->isCoordinator()) {
1405+
if (!ServerState::instance()->isCoordinator() ||
1406+
!ExecContext::current().isAdminUser()) {
13961407
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN,
13971408
"only allowed on coordinators");
13981409
return RestStatus::DONE;
@@ -1410,11 +1421,9 @@ RestStatus RestAdminClusterHandler::handleNumberOfServers() {
14101421
}
14111422

14121423
RestStatus RestAdminClusterHandler::handleHealth() {
1413-
if (!ExecContext::current().isAdminUser()) {
1414-
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
1415-
return RestStatus::DONE;
1416-
}
1417-
1424+
// We allow this API whenever one is authenticated in some way. There used
1425+
// to be a check for isAdminUser here. However, we want that the UI with
1426+
// the cluster health dashboard works for every authenticated user.
14181427
if (request()->requestType() != rest::RequestType::GET) {
14191428
generateError(rest::ResponseCode::METHOD_NOT_ALLOWED, TRI_ERROR_HTTP_METHOD_NOT_ALLOWED);
14201429
return RestStatus::DONE;

‎arangod/RestHandler/RestAdminServerHandler.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include "Actions/RestActionHandler.h"
2727
#include "ApplicationFeatures/ApplicationServer.h"
28+
#include "Basics/StaticStrings.h"
2829
#include "GeneralServer/AuthenticationFeature.h"
2930
#include "GeneralServer/GeneralServerFeature.h"
3031
#include "GeneralServer/SslServerFeature.h"
@@ -159,7 +160,7 @@ void RestAdminServerHandler::handleMode() {
159160
if (af->isActive() && !_request->user().empty()) {
160161
auth::Level lvl;
161162
if (af->userManager() != nullptr) {
162-
lvl = af->userManager()->databaseAuthLevel(_request->user(), TRI_VOC_SYSTEM_DATABASE,
163+
lvl = af->userManager()->databaseAuthLevel(_request->user(), StaticStrings::SystemDatabase,
163164
/*configured*/ true);
164165
} else {
165166
lvl = auth::Level::RW;

‎arangod/RestHandler/RestReplicationHandler.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,9 @@ Result RestReplicationHandler::processRestoreCollection(VPackSlice const& collec
10491049
return Result();
10501050
}
10511051

1052-
ExecContextSuperuserScope escope(ExecContext::current().isAdminUser());
1052+
ExecContextSuperuserScope escope(
1053+
ExecContext::current().isSuperuser() ||
1054+
(ExecContext::current().isAdminUser() && !ServerState::readOnly()));
10531055

10541056
auto* col = _vocbase.lookupCollection(name).get();
10551057

@@ -1427,7 +1429,9 @@ Result RestReplicationHandler::processRestoreData(std::string const& colName) {
14271429
bool generateNewRevisionIds =
14281430
!_request->parsedValue(StaticStrings::PreserveRevisionIds, false);
14291431

1430-
ExecContextSuperuserScope escope(ExecContext::current().isAdminUser());
1432+
ExecContextSuperuserScope escope(
1433+
ExecContext::current().isSuperuser() ||
1434+
(ExecContext::current().isAdminUser() && !ServerState::readOnly()));
14311435

14321436
if (colName == TRI_COL_NAME_USERS) {
14331437
// We need to handle the _users in a special way
@@ -1898,7 +1902,10 @@ Result RestReplicationHandler::processRestoreIndexes(VPackSlice const& collectio
18981902

18991903
Result fres;
19001904

1901-
ExecContextSuperuserScope escope(ExecContext::current().isAdminUser());
1905+
ExecContextSuperuserScope escope(
1906+
ExecContext::current().isSuperuser() ||
1907+
(ExecContext::current().isAdminUser() && !ServerState::readOnly()));
1908+
19021909
READ_LOCKER(readLocker, _vocbase._inventoryLock);
19031910

19041911
// look up the collection

‎arangod/RestServer/DatabaseFeature.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include < 10000 span class="pl-s">"Basics/FileUtils.h"
3434
#include "Basics/MutexLocker.h"
3535
#include "Basics/NumberUtils.h"
36+
#include "Basics/StaticStrings.h"
3637
#include "Basics/StringUtils.h"
3738
#include "Basics/VelocyPackHelper.h"
3839
#include "Basics/WriteLocker.h"
@@ -377,7 +378,7 @@ void DatabaseFeature::start() {
377378
FATAL_ERROR_EXIT();
378379
}
379380

380-
if (!lookupDatabase(TRI_VOC_SYSTEM_DATABASE)) {
381+
if (!lookupDatabase(StaticStrings::SystemDatabase)) {
381382
LOG_TOPIC("97e7c", FATAL, arangodb::Logger::FIXME)
382383
<< "No _system database found in database directory. Cannot start!";
383384
FATAL_ERROR_EXIT();
@@ -740,7 +741,7 @@ Result DatabaseFeature::createDatabase(CreateDatabaseInfo&& info, TRI_vocbase_t*
740741
/// @brief drop database
741742
int DatabaseFeature::dropDatabase(std::string const& name, bool waitForDeletion,
742743
bool removeAppsDirectory) {
743-
if (name == TRI_VOC_SYSTEM_DATABASE) {
744+
if (name == StaticStrings::SystemDatabase) {
744745
// prevent deletion of system database
745746
events::DropDatabase(name, TRI_ERROR_FORBIDDEN);
746747
return TRI_ERROR_FORBIDDEN;
@@ -892,7 +893,7 @@ std::vector<TRI_voc_tick_t> DatabaseFeature::getDatabaseIds(bool includeSystem)
892893
if (vocbase->isDropped()) {
893894
continue;
894895
}
895-
if (includeSystem || vocbase->name() != TRI_VOC_SYSTEM_DATABASE) {
896+
if (includeSystem || vocbase->name() != StaticStrings::SystemDatabase) {
896897
ids.emplace_back(vocbase->id());
897898
}
898899
}
@@ -1095,7 +1096,7 @@ void DatabaseFeature::updateContexts() {
10951096
return;
10961097
}
10971098

1098-
auto* vocbase = useDatabase(TRI_VOC_SYSTEM_DATABASE);
1099+
auto* vocbase = useDatabase(StaticStrings::SystemDatabase);
10991100
TRI_ASSERT(vocbase);
11001101

11011102
auto queryRegistry = QueryRegistryFeature::registry();

‎arangod/RestServer/SystemDatabaseFeature.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "SystemDatabaseFeature.h"
2525

2626
#include "ApplicationFeatures/ApplicationServer.h"
27+
#include "Basics/StaticStrings.h"
2728
#include "Basics/application-exit.h"
2829
#include "Logger/LogMacros.h"
2930
#include "RestServer/DatabaseFeature.h"
@@ -55,7 +56,7 @@ SystemDatabaseFeature::SystemDatabaseFeature(application_features::ApplicationSe
5556
void SystemDatabaseFeature::start() {
5657
if (server().hasFeature<arangodb::DatabaseFeature>()) {
5758
auto& feature = server().getFeature<arangodb::DatabaseFeature>();
58-
_vocbase.store(feature.lookupDatabase(TRI_VOC_SYSTEM_DATABASE));
59+
_vocbase.store(feature.lookupDatabase(StaticStrings::SystemDatabase));
5960

6061
return;
6162
}

‎arangod/RestServer/VocbaseContext.cpp

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
////////////////////////////////////////////////////////////////////////////////
2323

2424
#include "VocbaseContext.h"
25+
#include "Basics/StaticStrings.h"
2526
#include "Cluster/ServerState.h"
2627
#include "GeneralServer/AuthenticationFeature.h"
2728
#include "Logger/LogMacros.h"
@@ -35,8 +36,8 @@ namespace arangodb {
3536

3637
VocbaseContext::VocbaseContext(GeneralRequest& req, TRI_vocbase_t& vocbase,
3738
ExecContext::Type type, auth::Level systemLevel,
38-
auth::Level dbLevel)
39-
: ExecContext(type, req.user(), req.databaseName(), systemLevel, dbLevel),
39+
auth::Level dbLevel, bool isAdminUser)
40+
: ExecContext(type, req.user(), req.databaseName(), systemLevel, dbLevel, isAdminUser),
4041
_vocbase(vocbase) {
4142
// _vocbase has already been refcounted for us
4243
TRI_ASSERT(!_vocbase.isDangling());
@@ -58,7 +59,7 @@ VocbaseContext* VocbaseContext::create(GeneralRequest& req, TRI_vocbase_t& vocba
5859
if (isSuperUser) {
5960
return new VocbaseContext(req, vocbase, ExecContext::Type::Internal,
6061
/*sysLevel*/ auth::Level::RW,
61-
/*dbLevel*/ auth::Level::RW);
62+
/*dbLevel*/ auth::Level::RW, true);
6263
}
6364

6465
AuthenticationFeature* auth = AuthenticationFeature::instance();
@@ -68,19 +69,21 @@ VocbaseContext* VocbaseContext::create(GeneralRequest& req, TRI_vocbase_t& vocba
6869
// special read-only case
6970
return new VocbaseContext(req, vocbase, ExecContext::Type::Internal,
7071
/*sysLevel*/ auth::Level::RO,
71-
/*dbLevel*/ auth::Level::RO);
72+
/*dbLevel*/ auth::Level::RO, true);
7273
}
7374
return new VocbaseContext(req, vocbase, req.user().empty() ?
7475
ExecContext::Type::Internal : ExecContext::Type::Default,
7576
/*sysLevel*/ auth::Level::RW,
76-
/*dbLevel*/ auth::Level::RW);
77+
/*dbLevel*/ auth::Level::RW, true);
7778
}
7879

7980
if (!req.authenticated()) {
8081
return new VocbaseContext(req, vocbase, ExecContext::Type::Default,
8182
/*sysLevel*/ auth::Level::NONE,
82-
/*dbLevel*/ auth::Level::NONE);
83-
} else if (req.user().empty()) {
83+
/*dbLevel*/ auth::Level::NONE, false);
84+
}
85+
86+
if (req.user().empty()) {
8487
std::string msg = "only jwt can be used to authenticate as superuser";
8588
LOG_TOPIC("2d0f6", WARN, Logger::AUTHENTICATION) << msg;
8689
THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, msg);
@@ -93,26 +96,32 @@ VocbaseContext* VocbaseContext::create(GeneralRequest& req, TRI_vocbase_t& vocba
9396
return nullptr;
9497
}
9598

96-
auth::Level dbLvl = um->databaseAuthLevel(req.user(), req.databaseName());
99+
auth::Level dbLvl = um->databaseAuthLevel(req.user(), req.databaseName(), false);
97100
auth::Level sysLvl = dbLvl;
98-
if (req.databaseName() != TRI_VOC_SYSTEM_DATABASE) {
99-
sysLvl = um->databaseAuthLevel(req.user(), TRI_VOC_SYSTEM_DATABASE);
101+
if (req.databaseName() != StaticStrings::SystemDatabase) {
102+
sysLvl = um->databaseAuthLevel(req.user(), StaticStrings::SystemDatabase, false);
103+
}
104+
bool isAdminUser = (sysLvl == auth::Level::RW);
105+
if (!isAdminUser && ServerState::readOnly()) {
106+
// in case we are in read-only mode, we need to re-check the original permissions
107+
isAdminUser = um->databaseAuthLevel(req.user(), StaticStrings::SystemDatabase, true) ==
108+
auth::Level::RW;
100109
}
101110

102111
return new VocbaseContext(req, vocbase, ExecContext::Type::Default,
103112
/*sysLevel*/ sysLvl,
104-
/*dbLevel*/ dbLvl);
113+
/*dbLevel*/ dbLvl, isAdminUser);
105114
}
106115

107116
void VocbaseContext::forceSuperuser() {
108117
TRI_ASSERT(_type != ExecContext::Type::Internal || _user.empty());
109-
_type = ExecContext::Type::Internal;
110118
if (ServerState::readOnly()) {
111-
_systemDbAuthLevel = auth::Level::RO;
112-
_databaseAuthLevel = auth::Level::RO;
119+
forceReadOnly();
113120
} else {
121+
_type = ExecContext::Type::Internal;
114122
_systemDbAuthLevel = auth::Level::RW;
115123
_databaseAuthLevel = auth::Level::RW;
124+
_isAdminUser = true;
116125
}
117126
}
118127

‎arangod/RestServer/VocbaseContext.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class VocbaseContext : public arangodb::ExecContext {
5252
TRI_vocbase_t& _vocbase;
5353

5454
VocbaseContext(GeneralRequest& req, TRI_vocbase_t& vocbase, ExecContext::Type type,
55-
auth::Level systemLevel, auth::Level dbLevel);
55+
auth::Level systemLevel, auth::Level dbLevel, bool isAdminUser);
5656
VocbaseContext(VocbaseContext const&) = delete;
5757
VocbaseContext& operator=(VocbaseContext const&) = delete;
5858
};

0 commit comments

Comments
 (0)
0