8000 Fix isAdminUser. by neunhoef · Pull Request #11326 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

Fix isAdminUser. #11326

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
merged 11 commits into from
Mar 24, 2020
3 changes: 2 additions & 1 deletion arangod/Actions/RestActionHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "RestActionHandler.h"

#include "Actions/actions.h"
#include "Basics/StaticStrings.h"
#include "Basics/StringUtils.h"
#include "Statistics/RequestStatistics.h"
#include "VocBase/vocbase.h"
Expand All @@ -40,7 +41,7 @@ RestActionHandler::RestActionHandler(application_features::ApplicationServer& se

RestStatus RestActionHandler::execute() {
// check the request path
if (_request->databaseName() == TRI_VOC_SYSTEM_DATABASE) {
if (_request->databaseName() == StaticStrings::SystemDatabase) {
if (StringUtils::isPrefix(_request->requestPath(), "/_admin/aardvark")) {
RequestStatistics::SET_IGNORE(_statistics);
}
Expand Down
2 changes: 1 addition & 1 deletion arangod/Auth/User.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ auth::Level auth::User::collectionAuthLevel(std::string const& dbname,
bool isSystem = cname[0] == '_';
if (isSystem) {
// disallow access to _system/_users for everyone
if (dbname == TRI_VOC_SYSTEM_DATABASE && cname == TRI_COL_NAME_USERS) {
if (dbname == StaticStrings::SystemDatabase && cname == TRI_COL_NAME_USERS) {
return auth::Level::NONE;
} else if (cname == "_queues") {
return auth::Level::RO;
Expand Down
3 changes: 2 additions & 1 deletion arangod/Cluster/ClusterInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "Basics/MutexLocker.h"
#include "Basics/NumberUtils.h"
#include "Basics/RecursiveLocker.h"
#include "Basics/StaticStrings.h"
#include "Basics/StringUtils.h"
#include "Basics/VelocyPackHelper.h"
#include "Basics/WriteLocker.h"
Expand Down Expand Up @@ -1759,7 +1760,7 @@ Result ClusterInfo::dropDatabaseCoordinator( // drop database
double timeout // request timeout
) {
TRI_ASSERT(ServerState::instance()->isCoordinator());
if (name == TRI_VOC_SYSTEM_DATABASE) {
if (name == StaticStrings::SystemDatabase) {
return Result(TRI_ERROR_FORBIDDEN);
}

Expand Down
3 changes: 2 additions & 1 deletion arangod/MMFiles/MMFilesEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "Basics/MutexLocker.h"
#include "Basics/MutexUnlocker.h"
#include "Basics/ReadLocker.h"
#include "Basics/StaticStrings.h"
#include "Basics/StringUtils.h"
#include "Basics/VelocyPackHelper.h"
#include "Basics/WriteLocker.h"
Expand Down Expand Up @@ -264,7 +265,7 @@ void MMFilesEngine::start() {
if (names.empty()) {
// no databases found, i.e. there is no system database!
// create a database for the system database
int res = createDatabaseDirectory(TRI_NewTickServer(), TRI_VOC_SYSTEM_DATABASE);
int res = createDatabaseDirectory(TRI_NewTickServer(), StaticStrings::SystemDatabase);

if (res != TRI_ERROR_NO_ERROR) {
LOG_TOPIC("982c7", ERR, arangodb::Logger::ENGINES)
Expand Down
3 changes: 2 additions & 1 deletion arangod/Replication/GlobalInitialSyncer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "ApplicationFeatures/ApplicationServer.h"
#include "Basics/Result.h"
#include "Basics/StaticStrings.h"
#include "Basics/StringUtils.h"
#include "Basics/VelocyPackHelper.h"
#include "Replication/DatabaseInitialSyncer.h"
Expand All @@ -50,7 +51,7 @@ using namespace arangodb::rest;
GlobalInitialSyncer::GlobalInitialSyncer(ReplicationApplierConfiguration const& configuration)
: InitialSyncer(configuration) {
// has to be set here, otherwise broken
_state.databaseName = TRI_VOC_SYSTEM_DATABASE;
_state.databaseName = StaticStrings::SystemDatabase;
}

GlobalInitialSyncer::~GlobalInitialSyncer() {
Expand Down
3 changes: 2 additions & 1 deletion arangod/Replication/GlobalTailingSyncer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
////////////////////////////////////////////////////////////////////////////////

#include "GlobalTailingSyncer.h"
#include "Basics/StaticStrings.h"
#include "Basics/Thread.h"
#include "Logger/LogMacros.h"
#include "Logger/Logger.h"
Expand All @@ -43,7 +44,7 @@ GlobalTailingSyncer::GlobalTailingSyncer(ReplicationApplierConfiguration const&
configuration, initialTick, useTick, barrierId),
_queriedTranslations(false) {
_ignoreDatabaseMarkers = false;
_state.databaseName = TRI_VOC_SYSTEM_DATABASE;
_state.databaseName = StaticStrings::SystemDatabase;
}

std::string GlobalTailingSyncer::tailingBaseUrl(std::string const& command) {
Expand Down
4 changes: 2 additions & 2 deletions arangod/Replication/TailingSyncer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ Result TailingSyncer::processDBMarker(TRI_r 10000 eplication_operation_e type,

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

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

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

Expand Down
29 changes: 19 additions & 10 deletions arangod/RestHandler/RestAdminClusterHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,10 @@ std::string const RestAdminClusterHandler::RemoveServer = "removeServer";
std::string const RestAdminClusterHandler::RebalanceShards = "rebalanceShards";

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

auto const& suffixes = request()->suffixes();
size_t const len = suffixes.size();
Expand Down Expand Up @@ -965,6 +965,11 @@ RestStatus RestAdminClusterHandler::handleShardDistribution() {
return RestStatus::DONE;
}

if (!ExecContext::current().isAdminUser()) {
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
return RestStatus::DONE;
}

if (request()->requestType() != rest::RequestType::GET) {
generateError(rest::ResponseCode::METHOD_NOT_ALLOWED, TRI_ERROR_HTTP_METHOD_NOT_ALLOWED);
return RestStatus::DONE;
Expand Down Expand Up @@ -1014,6 +1019,11 @@ RestStatus RestAdminClusterHandler::handleCollectionShardDistribution() {
return RestStatus::DONE;
}

if (!ExecContext::current().isAdminUser()) {
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
return RestStatus::DONE;
}

switch (request()->requestType()) {
case rest::RequestType::GET:
return handleGetCollectionShardDistribution(
Expand Down Expand Up @@ -1392,7 +1402,8 @@ RestStatus RestAdminClusterHandler::handlePutNumberOfServers() {
}

RestStatus RestAdminClusterHandler::handleNumberOfServers() {
if (!ServerState::instance()->isCoordinator()) {
if (!ServerState::instance()->isCoordinator() ||
!ExecContext::current().isAdminUser()) {
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN,
"only allowed on coordinators");
return RestStatus::DONE;
Expand All @@ -1410,11 +1421,9 @@ RestStatus RestAdminClusterHandler::handleNumberOfServers() {
}

RestStatus RestAdminClusterHandler::handleHealth() {
if (!ExecContext::current().isAdminUser()) {
generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_HTTP_FORBIDDEN);
return RestStatus::DONE;
}

// We allow this API whenever one is authenticated in some way. There used
// to be a check for isAdminUser here. However, we want that the UI with
// the cluster health dashboard works for every authenticated user.
if (request()->requestType() != rest::RequestType::GET) {
generateError(rest::ResponseCode::METHOD_NOT_ALLOWED, TRI_ERROR_HTTP_METHOD_NOT_ALLOWED);
return RestStatus::DONE;
Expand Down
3 changes: 2 additions & 1 deletion arangod/RestHandler/RestAdminServerHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "Actions/RestActionHandler.h"
#include "ApplicationFeatures/ApplicationServer.h"
#include "Basics/StaticStrings.h"
#include "GeneralServer/AuthenticationFeature.h"
#include "GeneralServer/GeneralServerFeature.h"
#include "GeneralServer/SslServerFeature.h"
Expand Down Expand Up @@ -159,7 +160,7 @@ void RestAdminServerHandler::handleMode() {
if (af->isActive() && !_request->user().empty()) {
auth::Level lvl;
if (af->userManager() != nullptr) {
lvl = af->userManager()->databaseAuthLevel(_request->user(), TRI_VOC_SYSTEM_DATABASE,
lvl = af->userManager()->databaseAuthLevel(_request->user(), StaticStrings::SystemDatabase,
/*configured*/ true);
} else {
lvl = auth::Level::RW;
Expand Down
13 changes: 10 additions & 3 deletions arangod/RestHandler/RestReplicationHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,9 @@ Result RestReplicationHandler::processRestoreCollection(VPackSlice const& collec
return Result();
}

ExecContextSuperuserScope escope(ExecContext::current().isAdminUser());
ExecContextSuperuserScope escope(
ExecContext::current().isSuperuser() ||
(ExecContext::current().isAdminUser() && !ServerState::readOnly()));

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

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

ExecContextSuperuserScope escope(ExecContext::current().isAdminUser());
ExecContextSuperuserScope escope(
ExecContext::current().isSuperuser() ||
(ExecContext::current().isAdminUser() && !ServerState::readOnly()));

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

Result fres;

ExecContextSuperuserScope escope(ExecContext::current().isAdminUser());
ExecContextSuperuserScope escope(
ExecContext::current().isSuperuser() ||
(ExecContext::current().isAdminUser() && !ServerState::readOnly()));

READ_LOCKER(readLocker, _vocbase._inventoryLock);

// look up the collection
Expand Down
9 changes: 5 additions & 4 deletions arangod/RestServer/DatabaseFeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "Basics/FileUtils.h"
#include "Basics/MutexLocker.h"
#include "Basics/NumberUtils.h"
#include "Basics/StaticStrings.h"
#include "Basics/StringUtils.h"
#include "Basics/VelocyPackHelper.h"
#include "Basics/WriteLocker.h"
Expand Down Expand Up @@ -377,7 +378,7 @@ void DatabaseFeature::start() {
FATAL_ERROR_EXIT();
}

if (!lookupDatabase(TRI_VOC_SYSTEM_DATABASE)) {
if (!lookupDatabase(StaticStrings::SystemDatabase)) {
LOG_TOPIC("97e7c", FATAL, arangodb::Logger::FIXME)
<< "No _system database found in database directory. Cannot start!";
FATAL_ERROR_EXIT();
Expand Down Expand Up @@ -740,7 +741,7 @@ Result DatabaseFeature::createDatabase(CreateDatabaseInfo&& info, TRI_vocbase_t*
/// @brief drop database
int DatabaseFeature::dropDatabase(std::string const& name, bool waitForDeletion,
bool removeAppsDirectory) {
if (name == TRI_VOC_SYSTEM_DATABASE) {
if (name == StaticStrings::SystemDatabase) {
// prevent deletion of system database
events::DropDatabase(name, TRI_ERROR_FORBIDDEN);
return TRI_ERROR_FORBIDDEN;
Expand Down Expand Up @@ -892,7 +893,7 @@ std::vector<TRI_voc_tick_t> DatabaseFeature::getDatabaseIds(bool includeSystem)
if (vocbase->isDropped()) {
continue;
}
if (includeSystem || vocbase->name() != TRI_VOC_SYSTEM_DATABASE) {
if (includeSystem || vocbase->name() != StaticStrings::SystemDatabase) {
ids.emplace_back(vocbase->id());
}
}
Expand Down Expand Up @@ -1095,7 +1096,7 @@ void DatabaseFeature::updateContexts() {
return;
}

auto* vocbase = useDatabase(TRI_VOC_SYSTEM_DATABASE);
auto* vocbase = useDatabase(StaticStrings::SystemDatabase);
TRI_ASSERT(vocbase);

auto queryRegistry = QueryRegistryFeature::registry();
Expand Down
3 changes: 2 additions & 1 deletion arangod/RestServer/SystemDatabaseFeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "SystemDatabaseFeature.h"

#include "ApplicationFeatures/ApplicationServer.h"
#include "Basics/StaticStrings.h"
#include "Basics/application-exit.h"
#include "Logger/LogMacros.h"
#include "RestServer/DatabaseFeature.h"
Expand Down Expand Up @@ -55,7 +56,7 @@ SystemDatabaseFeature::SystemDatabaseFeature(application_features::ApplicationSe
void SystemDatabaseFeature::start() {
if (server().hasFeature<arangodb::DatabaseFeature>()) {
auto& feature = server().getFeature<arangodb::DatabaseFeature>();
_vocbase.store(feature.lookupDatabase(TRI_VOC_SYSTEM_DATABASE));
_vocbase.store(feature.lookupDatabase(StaticStrings::SystemDatabase));

return;
}
Expand Down
37 changes: 23 additions & 14 deletions arangod/RestServer/VocbaseContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
////////////////////////////////////////////////////////////////////////////////

#include "VocbaseContext.h"
#include "Basics/StaticStrings.h"
#include "Cluster/ServerState.h"
#include "GeneralServer/AuthenticationFeature.h"
#include "Logger/LogMacros.h"
Expand All @@ -35,8 +36,8 @@ namespace arangodb {

VocbaseContext::VocbaseContext(GeneralRequest& req, TRI_vocbase_t& vocbase,
ExecContext::Type type, auth::Level systemLevel,
auth::Level dbLevel)
: ExecContext(type, req.user(), req.databaseName(), systemLevel, dbLevel),
auth::Level dbLevel, bool isAdminUser)
: ExecContext(type, req.user(), req.databaseName(), systemLevel, dbLevel, isAdminUser),
_vocbase(vocbase) {
// _vocbase has already been refcounted for us
TRI_ASSERT(!_vocbase.isDangling());
Expand All @@ -58,7 +59,7 @@ VocbaseContext* VocbaseContext::create(GeneralRequest& req, TRI_vocbase_t& vocba
if (isSuperUser) {
return new VocbaseContext(req, vocbase, ExecContext::Type::Internal,
/*sysLevel*/ auth::Level::RW,
/*dbLevel*/ auth::Level::RW);
/*dbLevel*/ auth::Level::RW, true);
}

AuthenticationFeature* auth = AuthenticationFeature::instance();
Expand All @@ -68,19 +69,21 @@ VocbaseContext* VocbaseContext::create(GeneralRequest& req, TRI_vocbase_t& vocba
// special read-only case
return new VocbaseContext(req, vocbase, ExecContext::Type::Internal,
/*sysLevel*/ auth::Level::RO,
/*dbLevel*/ auth::Level::RO);
/*dbLevel*/ auth::Level::RO, true);
}
return new VocbaseContext(req, vocbase, req.user().empty() ?
ExecContext::Type::Internal : ExecContext::Type::Default,
/*sysLevel*/ auth::Level::RW,
/*dbLevel*/ auth::Level::RW);
/*dbLevel*/ auth::Level::RW, true);
}

if (!req.authenticated()) {
return new VocbaseContext(req, vocbase, ExecContext::Type::Default,
/*sysLevel*/ auth::Level::NONE,
/*dbLevel*/ auth::Level::NONE);
} else if (req.user().empty()) {
/*dbLevel*/ auth::Level::NONE, false);
}

if (req.user().empty()) {
std::string msg = "only jwt can be used to authenticate as superuser";
LOG_TOPIC("2d0f6", WARN, Logger::AUTHENTICATION) << msg;
THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, msg);
Expand All @@ -93,26 +96,32 @@ VocbaseContext* VocbaseContext::create(GeneralRequest& req, TRI_vocbase_t& vocba
return nullptr;
}

auth::Level dbLvl = um->databaseAuthLevel(req.user(), req.databaseName());
auth::Level dbLvl = um->databaseAuthLevel(req.user(), req.databaseName(), false);
auth::Level sysLvl = dbLvl;
if (req.databaseName() != TRI_VOC_SYSTEM_DATABASE) {
sysLvl = um->databaseAuthLevel(req.user(), TRI_VOC_SYSTEM_DATABASE);
if (req.databaseName() != StaticStrings::SystemDatabase) {
sysLvl = um->databaseAuthLevel(req.user(), StaticStrings::SystemDatabase, false);
}
bool isAdminUser = (sysLvl == auth::Level::RW);
if (!isAdminUser && ServerState::readOnly()) {
// in case we are in read-only mode, we need to re-check the original permissions
isAdminUser = um->databaseAuthLevel(req.user(), StaticStrings::SystemDatabase, true) ==
auth::Level::RW;
}

return new VocbaseContext(req, vocbase, ExecContext::Type::Default,
/*sysLevel*/ sysLvl,
/*dbLevel*/ dbLvl);
/*dbLevel*/ dbLvl, isAdminUser);
}

void VocbaseContext::forceSuperuser() {
TRI_ASSERT(_type != ExecContext::Type::Internal || _user.empty());
_type = ExecContext::Type::Internal;
if (ServerState::readOnly()) {
_systemDbAuthLevel = auth::Level::RO;
_databaseAuthLevel = auth::Level::RO;
forceReadOnly();
} else {
_type = ExecContext::Type::Internal;
_systemDbAuthLevel = auth::Level::RW;
_databaseAuthLevel = auth::Level::RW;
_isAdminUser = true;
}
}

Expand Down
2 changes: 1 addition & 1 deletion arangod/RestServer/VocbaseContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class VocbaseContext : public arangodb::ExecContext {
TRI_vocbase_t& _vocbase;

VocbaseContext(GeneralRequest& req, TRI_vocbase_t& vocbase, ExecContext::Type type,
auth::Level systemLevel, auth::Level dbLevel);
auth::Level systemLevel, auth::Level dbLevel, bool isAdminUser);
VocbaseContext(VocbaseContext const&) = delete;
VocbaseContext& operator=(VocbaseContext const&) = delete;
};
Expand Down
Loading
0