diff options
-rw-r--r-- | src/mongo/db/auth/authz_manager_external_state_local.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/auth/role_graph.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/auth/role_graph.h | 18 | ||||
-rw-r--r-- | src/mongo/db/auth/role_graph_test.cpp | 41 |
4 files changed, 62 insertions, 15 deletions
diff --git a/src/mongo/db/auth/authz_manager_external_state_local.cpp b/src/mongo/db/auth/authz_manager_external_state_local.cpp index ac5793a5f09..29baf0cdf1c 100644 --- a/src/mongo/db/auth/authz_manager_external_state_local.cpp +++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp @@ -433,8 +433,8 @@ Status AuthzManagerExternalStateLocal::_initializeRoleGraph(OperationContext* op } if (status.isOK()) { - _roleGraph = std::move(newRoleGraph); - _roleGraphState = std::move(newState); + _roleGraph.swap(newRoleGraph); + _roleGraphState = newState; } return status; } diff --git a/src/mongo/db/auth/role_graph.cpp b/src/mongo/db/auth/role_graph.cpp index 4fb6bc75262..15e8fc87646 100644 --- a/src/mongo/db/auth/role_graph.cpp +++ b/src/mongo/db/auth/role_graph.cpp @@ -43,6 +43,16 @@ namespace { PrivilegeVector emptyPrivilegeVector; } // namespace +RoleGraph::RoleGraph(){}; +RoleGraph::RoleGraph(const RoleGraph& other) + : _roleToSubordinates(other._roleToSubordinates), + _roleToIndirectSubordinates(other._roleToIndirectSubordinates), + _roleToMembers(other._roleToMembers), + _directPrivilegesForRole(other._directPrivilegesForRole), + _allPrivilegesForRole(other._allPrivilegesForRole), + _allRoles(other._allRoles) {} +RoleGraph::~RoleGraph(){}; + void RoleGraph::swap(RoleGraph& other) { using std::swap; swap(this->_roleToSubordinates, other._roleToSubordinates); @@ -53,6 +63,10 @@ void RoleGraph::swap(RoleGraph& other) { swap(this->_allRoles, other._allRoles); } +void swap(RoleGraph& lhs, RoleGraph& rhs) { + lhs.swap(rhs); +} + bool RoleGraph::roleExists(const RoleName& role) { _createBuiltinRoleIfNeeded(role); return _roleExistsDontCreateBuiltin(role); diff --git a/src/mongo/db/auth/role_graph.h b/src/mongo/db/auth/role_graph.h index 9a5c3d639cb..4d8334b7710 100644 --- a/src/mongo/db/auth/role_graph.h +++ b/src/mongo/db/auth/role_graph.h @@ -55,17 +55,7 @@ class OperationContext; * recomputePrivilegeData() on the object. */ class RoleGraph { - // Disallow copying - RoleGraph(const RoleGraph&) = delete; - RoleGraph& operator=(const RoleGraph&) = delete; - public: - RoleGraph() noexcept = default; - - // Explicitly make RoleGraph movable - RoleGraph(RoleGraph&&) noexcept = default; - RoleGraph& operator=(RoleGraph&&) noexcept = default; - /** * Adds to "privileges" the privileges associated with the named built-in role, and returns * true. Returns false if "role" does not name a built-in role, and does not modify @@ -74,6 +64,10 @@ public: */ static bool addPrivilegesForBuiltinRole(const RoleName& role, PrivilegeVector* privileges); + RoleGraph(); + RoleGraph(const RoleGraph& other); + ~RoleGraph(); + // Built-in roles for backwards compatibility with 2.2 and prior static const std::string BUILTIN_ROLE_V0_READ; static const std::string BUILTIN_ROLE_V0_READ_WRITE; @@ -315,8 +309,6 @@ private: std::set<RoleName> _allRoles; }; -inline void swap(RoleGraph& lhs, RoleGraph& rhs) { - lhs.swap(rhs); -} +void swap(RoleGraph& lhs, RoleGraph& rhs); } // namespace mongo diff --git a/src/mongo/db/auth/role_graph_test.cpp b/src/mongo/db/auth/role_graph_test.cpp index ff4c92f437a..c9024b22ea0 100644 --- a/src/mongo/db/auth/role_graph_test.cpp +++ b/src/mongo/db/auth/role_graph_test.cpp @@ -507,6 +507,47 @@ TEST(RoleGraphTest, ReAddRole) { ASSERT_FALSE(privileges[0].getActions().contains(ActionType::insert)); } +// Tests copy constructor and swap functionality. +TEST(RoleGraphTest, CopySwap) { + RoleName roleA("roleA", "dbA"); + RoleName roleB("roleB", "dbB"); + RoleName roleC("roleC", "dbC"); + + RoleGraph graph; + ASSERT_OK(graph.createRole(roleA)); + ASSERT_OK(graph.createRole(roleB)); + ASSERT_OK(graph.createRole(roleC)); + + ActionSet actions; + actions.addAction(ActionType::find); + ASSERT_OK(graph.addPrivilegeToRole(roleA, Privilege(dbAResource, actions))); + ASSERT_OK(graph.addPrivilegeToRole(roleB, Privilege(dbBResource, actions))); + ASSERT_OK(graph.addPrivilegeToRole(roleC, Privilege(dbCResource, actions))); + + ASSERT_OK(graph.addRoleToRole(roleA, roleB)); + + // Make a copy of the graph to do further modifications on. + RoleGraph tempGraph(graph); + ASSERT_OK(tempGraph.addRoleToRole(roleB, roleC)); + tempGraph.recomputePrivilegeData().transitional_ignore(); + + // Now swap the copy back with the original graph and make sure the original was updated + // properly. + swap(tempGraph, graph); + + RoleNameIterator it = graph.getDirectSubordinates(roleB); + ASSERT_TRUE(it.more()); + ASSERT_EQUALS(it.next().getFullName(), roleC.getFullName()); + ASSERT_FALSE(it.more()); + + graph.getAllPrivileges(roleA); // should have privileges from roleB *and* role C + PrivilegeVector privileges = graph.getAllPrivileges(roleA); + ASSERT_EQUALS(static_cast<size_t>(3), privileges.size()); + ASSERT_EQUALS(dbAResource, privileges[0].getResourcePattern()); + ASSERT_EQUALS(dbBResource, privileges[1].getResourcePattern()); + ASSERT_EQUALS(dbCResource, privileges[2].getResourcePattern()); +} + // Tests error handling TEST(RoleGraphTest, ErrorHandling) { RoleName roleA("roleA", "dbA"); |