Revert "SERVER-29910 Make RoleGraph non-copyable (but movable)"

This reverts commit 41cd527620d94a11362f2a5a1aa86643be22d36e.

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");