diff options
author | Shreyas Kalyan <shreyas.kalyan@10gen.com> | 2019-02-28 13:41:40 -0500 |
---|---|---|
committer | Shreyas Kalyan <shreyas.kalyan@10gen.com> | 2019-03-04 15:44:09 -0500 |
commit | 447847d93d6e0a21b018d5df45528e815c7c13d8 (patch) | |
tree | 02b7857e766fa85730a61b4b189d4cc7d48d8bfd /src/mongo | |
parent | bede0e9b7afee4bb30025b1ffc3e22763aec9e93 (diff) | |
download | mongo-447847d93d6e0a21b018d5df45528e815c7c13d8.tar.gz |
SERVER-39058 Synchronize user set modification in AuthorizationSession with Clientr3.4.20-rc0r3.4.20
(cherry picked from commit a9277e874039f32ce0d848fcdfb10de705c96fd9)
Diffstat (limited to 'src/mongo')
-rw-r--r-- | src/mongo/db/auth/authorization_session.cpp | 19 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session.h | 20 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session_test.cpp | 9 | ||||
-rw-r--r-- | src/mongo/db/commands/authentication_commands.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/commands/current_op.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/commands/kill_op.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/dbwebserver.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/index_builder.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/index_rebuilder.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/bgsync.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/master_slave.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_sync.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/sync_tail.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/task_runner.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/s/migration_destination_manager.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/ttl.cpp | 2 |
16 files changed, 56 insertions, 30 deletions
diff --git a/src/mongo/db/auth/authorization_session.cpp b/src/mongo/db/auth/authorization_session.cpp index c4f219c15de..bc41c9ab310 100644 --- a/src/mongo/db/auth/authorization_session.cpp +++ b/src/mongo/db/auth/authorization_session.cpp @@ -48,6 +48,7 @@ #include "mongo/db/client.h" #include "mongo/db/jsobj.h" #include "mongo/db/namespace_string.h" +#include "mongo/db/operation_context.h" #include "mongo/db/pipeline/pipeline.h" #include "mongo/util/assert_util.h" #include "mongo/util/log.h" @@ -125,7 +126,8 @@ User* AuthorizationSession::lookupUser(const UserName& name) { return _authenticatedUsers.lookup(name); } -void AuthorizationSession::logoutDatabase(const std::string& dbname) { +void AuthorizationSession::logoutDatabase(OperationContext* opCtx, const std::string& dbname) { + stdx::lock_guard<Client> lk(*opCtx->getClient()); User* removedUser = _authenticatedUsers.removeByDBName(dbname); if (removedUser) { getAuthorizationManager().releaseUser(removedUser); @@ -153,11 +155,20 @@ std::string AuthorizationSession::getAuthenticatedUserNamesToken() { return ret; } -void AuthorizationSession::grantInternalAuthorization() { +void AuthorizationSession::grantInternalAuthorization(Client* client) { + stdx::lock_guard<Client> lk(*client); _authenticatedUsers.add(internalSecurity.user); _buildAuthenticatedRolesVector(); } +/** + * Overloaded function - takes in the opCtx of the current AuthSession + * and calls the function above. + */ +void AuthorizationSession::grantInternalAuthorization(OperationContext* opCtx) { + grantInternalAuthorization(opCtx->getClient()); +} + PrivilegeVector AuthorizationSession::getDefaultPrivileges() { PrivilegeVector defaultPrivileges; @@ -748,6 +759,8 @@ void AuthorizationSession::_refreshUserInfoAsNeeded(OperationContext* txn) { User* updatedUser; Status status = authMan.acquireUser(txn, name, &updatedUser); + stdx::lock_guard<Client> lk(*txn->getClient()); + switch (status.code()) { case ErrorCodes::OK: { // Success! Replace the old User object with the updated one. @@ -835,7 +848,7 @@ void AuthorizationSession::setImpersonatedUserData(std::vector<UserName> usernam _impersonationFlag = true; } -bool AuthorizationSession::isCoauthorizedWithClient(Client* opClient) { +bool AuthorizationSession::isCoauthorizedWithClient(Client* opClient, WithLock opClientLock) { auto getUserNames = [](AuthorizationSession* authSession) { if (authSession->isImpersonating()) { return authSession->getImpersonatedUserNames(); diff --git a/src/mongo/db/auth/authorization_session.h b/src/mongo/db/auth/authorization_session.h index 2cd46a8d59f..96352b2a560 100644 --- a/src/mongo/db/auth/authorization_session.h +++ b/src/mongo/db/auth/authorization_session.h @@ -42,6 +42,7 @@ #include "mongo/db/auth/user_name.h" #include "mongo/db/auth/user_set.h" #include "mongo/db/namespace_string.h" +#include "mongo/util/concurrency/with_lock.h" namespace mongo { @@ -130,12 +131,15 @@ public: std::string getAuthenticatedUserNamesToken(); // Removes any authenticated principals whose authorization credentials came from the given - // database, and revokes any privileges that were granted via that principal. - void logoutDatabase(const std::string& dbname); + // database, and revokes any privileges that were granted via that principal. This function + // modifies state. Synchronizes with the Client lock. + void logoutDatabase(OperationContext* opCtx, const std::string& dbname); // Adds the internalSecurity user to the set of authenticated users. - // Used to grant internal threads full access. - void grantInternalAuthorization(); + // Used to grant internal threads full access. Takes in the Client + // as a parameter so it can take out a lock on the client. + void grantInternalAuthorization(Client* client); + void grantInternalAuthorization(OperationContext* opCtx); // Generates a vector of default privileges that are granted to any user, // regardless of which roles that user does or does not possess. @@ -268,8 +272,12 @@ public: // authenticated user. If either object has impersonated users, // those users will be considered as 'authenticated' for the purpose of this check. // - // The existence of 'opClient' must be guaranteed through locks taken by the caller. - bool isCoauthorizedWithClient(Client* opClient); + // The existence of 'opClient' must be guaranteed through locks taken by the caller, + // as demonstrated by opClientLock which must be a lock taken on opClient. + // + // Returns true if the current auth session and the opClient's auth session have users + // in common. + bool isCoauthorizedWithClient(Client* opClient, WithLock opClientLock); // Tells whether impersonation is active or not. This state is set when // setImpersonatedUserData is called and cleared when clearImpersonatedUserData is diff --git a/src/mongo/db/auth/authorization_session_test.cpp b/src/mongo/db/auth/authorization_session_test.cpp index 0c242b6c13d..29f9966c08d 100644 --- a/src/mongo/db/auth/authorization_session_test.cpp +++ b/src/mongo/db/auth/authorization_session_test.cpp @@ -40,6 +40,7 @@ #include "mongo/db/json.h" #include "mongo/db/namespace_string.h" #include "mongo/db/operation_context_noop.h" +#include "mongo/db/service_context_noop.h" #include "mongo/stdx/memory.h" #include "mongo/unittest/unittest.h" #include "mongo/util/map_util.h" @@ -76,8 +77,10 @@ private: class AuthorizationSessionTest : public ::mongo::unittest::Test { public: + std::unique_ptr<ServiceContextNoop> _service = stdx::make_unique<ServiceContextNoop>(); + ServiceContext::UniqueClient _client = _service.get()->makeClient("test"); FailureCapableAuthzManagerExternalStateMock* managerState; - OperationContextNoop _txn; + OperationContextNoop _txn{_client.get(), 0}; AuthzSessionExternalStateMock* sessionState; std::unique_ptr<AuthorizationManager> authzManager; std::unique_ptr<AuthorizationSessionForTest> authzSession; @@ -203,7 +206,7 @@ TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) { ASSERT_TRUE( authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::insert)); - authzSession->logoutDatabase("test"); + authzSession->logoutDatabase(&_txn, "test"); ASSERT_TRUE( authzSession->isAuthorizedForActionsOnResource(otherFooCollResource, ActionType::insert)); ASSERT_TRUE( @@ -211,7 +214,7 @@ TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) { ASSERT_FALSE( authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::collMod)); - authzSession->logoutDatabase("admin"); + authzSession->logoutDatabase(&_txn, "admin"); ASSERT_FALSE( authzSession->isAuthorizedForActionsOnResource(otherFooCollResource, ActionType::insert)); ASSERT_FALSE( diff --git a/src/mongo/db/commands/authentication_commands.cpp b/src/mongo/db/commands/authentication_commands.cpp index 68a483b24f9..852c8ab56ed 100644 --- a/src/mongo/db/commands/authentication_commands.cpp +++ b/src/mongo/db/commands/authentication_commands.cpp @@ -346,7 +346,7 @@ Status CmdAuthenticate::_authenticateX509(OperationContext* txn, "authentication. The current configuration does not allow " "x.509 cluster authentication, check the --clusterAuthMode flag"); } - authorizationSession->grantInternalAuthorization(); + authorizationSession->grantInternalAuthorization(client); } // Handle normal client authentication, only applies to client-server connections else { @@ -386,14 +386,14 @@ public: string& errmsg, BSONObjBuilder& result) { AuthorizationSession* authSession = AuthorizationSession::get(Client::getCurrent()); - authSession->logoutDatabase(dbname); + authSession->logoutDatabase(txn, dbname); if (Command::testCommandsEnabled && dbname == "admin") { // Allows logging out as the internal user against the admin database, however // this actually logs out of the local database as well. This is to // support the auth passthrough test framework on mongos (since you can't use the // local database on a mongos, so you can't logout as the internal user // without this). - authSession->logoutDatabase("local"); + authSession->logoutDatabase(txn, "local"); } return true; } diff --git a/src/mongo/db/commands/current_op.cpp b/src/mongo/db/commands/current_op.cpp index 0c5572ea385..9f67c4baee7 100644 --- a/src/mongo/db/commands/current_op.cpp +++ b/src/mongo/db/commands/current_op.cpp @@ -124,7 +124,8 @@ public: stdx::lock_guard<Client> lk(*client); if (ownOpsOnly && - !AuthorizationSession::get(txn->getClient())->isCoauthorizedWithClient(client)) { + !AuthorizationSession::get(txn->getClient()) + ->isCoauthorizedWithClient(client, lk)) { continue; } diff --git a/src/mongo/db/commands/kill_op.cpp b/src/mongo/db/commands/kill_op.cpp index f96f3692ecd..1e11fb67466 100644 --- a/src/mongo/db/commands/kill_op.cpp +++ b/src/mongo/db/commands/kill_op.cpp @@ -93,7 +93,7 @@ public: if (opCtx && opCtx->getOpID() == opId) { if (authzSession->isAuthorizedForActionsOnResource( ResourcePattern::forClusterResource(), ActionType::killop) || - authzSession->isCoauthorizedWithClient(opClient)) { + authzSession->isCoauthorizedWithClient(opClient, lk)) { return {std::make_tuple(std::move(lk), opCtx)}; } break; diff --git a/src/mongo/db/dbwebserver.cpp b/src/mongo/db/dbwebserver.cpp index 50fa68a8176..26483994632 100644 --- a/src/mongo/db/dbwebserver.cpp +++ b/src/mongo/db/dbwebserver.cpp @@ -495,7 +495,7 @@ bool DbWebServer::_allowed(OperationContext* txn, } if (from.isLocalHost() && !_webUsers->haveAdminUsers(txn)) { - authSess->grantInternalAuthorization(); + authSess->grantInternalAuthorization(txn); return true; } diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp index 59e83d268f1..1627cc20c9d 100644 --- a/src/mongo/db/index_builder.cpp +++ b/src/mongo/db/index_builder.cpp @@ -86,7 +86,7 @@ void IndexBuilder::run() { OperationContext& txn = *txnPtr; txn.lockState()->setShouldConflictWithSecondaryBatchApplication(false); - AuthorizationSession::get(txn.getClient())->grantInternalAuthorization(); + AuthorizationSession::get(txn.getClient())->grantInternalAuthorization(txnPtr.get()); { stdx::lock_guard<Client> lk(*txn.getClient()); diff --git a/src/mongo/db/index_rebuilder.cpp b/src/mongo/db/index_rebuilder.cpp index de00ef920d1..b2857b65c0e 100644 --- a/src/mongo/db/index_rebuilder.cpp +++ b/src/mongo/db/index_rebuilder.cpp @@ -150,7 +150,7 @@ void forceRestartInProgressIndexesOnCollection(OperationContext* opCtx, const Na } void restartInProgressIndexesFromLastShutdown(OperationContext* txn) { - AuthorizationSession::get(txn->getClient())->grantInternalAuthorization(); + AuthorizationSession::get(txn->getClient())->grantInternalAuthorization(txn); std::vector<std::string> dbNames; diff --git a/src/mongo/db/repl/bgsync.cpp b/src/mongo/db/repl/bgsync.cpp index ac49485ede0..551cf74ac59 100644 --- a/src/mongo/db/repl/bgsync.cpp +++ b/src/mongo/db/repl/bgsync.cpp @@ -183,7 +183,7 @@ bool BackgroundSync::_inShutdown_inlock() const { void BackgroundSync::_run() { Client::initThread("rsBackgroundSync"); - AuthorizationSession::get(cc())->grantInternalAuthorization(); + AuthorizationSession::get(cc())->grantInternalAuthorization(&cc()); while (!inShutdown()) { try { diff --git a/src/mongo/db/repl/master_slave.cpp b/src/mongo/db/repl/master_slave.cpp index 6929d5be297..82233f36753 100644 --- a/src/mongo/db/repl/master_slave.cpp +++ b/src/mongo/db/repl/master_slave.cpp @@ -1315,7 +1315,8 @@ static void replMasterThread() { // when things are idle. const ServiceContext::UniqueOperationContext txnPtr = cc().makeOperationContext(); OperationContext& txn = *txnPtr; - AuthorizationSession::get(txn.getClient())->grantInternalAuthorization(); + AuthorizationSession::get(txn.getClient())->grantInternalAuthorization(txnPtr.get()); + Lock::GlobalWrite globalWrite(txn.lockState(), 1); if (globalWrite.isLocked()) { @@ -1341,7 +1342,7 @@ static void replSlaveThread() { const ServiceContext::UniqueOperationContext txnPtr = cc().makeOperationContext(); OperationContext& txn = *txnPtr; - AuthorizationSession::get(txn.getClient())->grantInternalAuthorization(); + AuthorizationSession::get(txn.getClient())->grantInternalAuthorization(txnPtr.get()); DisableDocumentValidation validationDisabler(&txn); while (1) { @@ -1368,7 +1369,7 @@ void startMasterSlave(OperationContext* txn) { if (!replSettings.isSlave() && !replSettings.isMaster()) return; - AuthorizationSession::get(txn->getClient())->grantInternalAuthorization(); + AuthorizationSession::get(txn->getClient())->grantInternalAuthorization(txn); { ReplSource temp(txn); // Ensures local.me is populated diff --git a/src/mongo/db/repl/rs_sync.cpp b/src/mongo/db/repl/rs_sync.cpp index b04972c29e4..dd5dd99752e 100644 --- a/src/mongo/db/repl/rs_sync.cpp +++ b/src/mongo/db/repl/rs_sync.cpp @@ -65,7 +65,7 @@ void RSDataSync::join() { void RSDataSync::_run() { Client::initThread("rsSync"); - AuthorizationSession::get(cc())->grantInternalAuthorization(); + AuthorizationSession::get(cc())->grantInternalAuthorization(&cc()); // Overwrite prefetch index mode in BackgroundSync if ReplSettings has a mode set. auto&& replSettings = _replCoord->getSettings(); diff --git a/src/mongo/db/repl/sync_tail.cpp b/src/mongo/db/repl/sync_tail.cpp index dfd1d564f90..ac75166052d 100644 --- a/src/mongo/db/repl/sync_tail.cpp +++ b/src/mongo/db/repl/sync_tail.cpp @@ -148,7 +148,7 @@ ServerStatusMetricField<TimerStats> displayOpBatchesApplied("repl.apply.batches" void initializePrefetchThread() { if (!Client::getCurrent()) { Client::initThreadIfNotAlready(); - AuthorizationSession::get(cc())->grantInternalAuthorization(); + AuthorizationSession::get(cc())->grantInternalAuthorization(&cc()); } } @@ -461,7 +461,7 @@ void initializeWriterThread() { // Only do this once per thread if (!Client::getCurrent()) { Client::initThreadIfNotAlready(); - AuthorizationSession::get(cc())->grantInternalAuthorization(); + AuthorizationSession::get(cc())->grantInternalAuthorization(&cc()); } } diff --git a/src/mongo/db/repl/task_runner.cpp b/src/mongo/db/repl/task_runner.cpp index 134160bbdf1..668f6cd7fc3 100644 --- a/src/mongo/db/repl/task_runner.cpp +++ b/src/mongo/db/repl/task_runner.cpp @@ -136,7 +136,7 @@ void TaskRunner::_runTasks() { Client::initThreadIfNotAlready(); Client* client = &cc(); if (AuthorizationManager::get(client->getServiceContext())->isAuthEnabled()) { - AuthorizationSession::get(client)->grantInternalAuthorization(); + AuthorizationSession::get(client)->grantInternalAuthorization(client); } ServiceContext::UniqueOperationContext txn; diff --git a/src/mongo/db/s/migration_destination_manager.cpp b/src/mongo/db/s/migration_destination_manager.cpp index 3c62381a1f8..ee38f8f39fe 100644 --- a/src/mongo/db/s/migration_destination_manager.cpp +++ b/src/mongo/db/s/migration_destination_manager.cpp @@ -479,7 +479,7 @@ void MigrationDestinationManager::_migrateThread(BSONObj min, if (getGlobalAuthorizationManager()->isAuthEnabled()) { ShardedConnectionInfo::addHook(); - AuthorizationSession::get(opCtx->getClient())->grantInternalAuthorization(); + AuthorizationSession::get(opCtx->getClient())->grantInternalAuthorization(&cc()); } try { diff --git a/src/mongo/db/ttl.cpp b/src/mongo/db/ttl.cpp index 9688f661578..09785151326 100644 --- a/src/mongo/db/ttl.cpp +++ b/src/mongo/db/ttl.cpp @@ -85,7 +85,7 @@ public: virtual void run() { Client::initThread(name().c_str()); - AuthorizationSession::get(cc())->grantInternalAuthorization(); + AuthorizationSession::get(cc())->grantInternalAuthorization(&cc()); while (!inShutdown()) { { |