SERVER-7572 Change AuthorizationManager::checkAuthorization to return…

… bool instead of unused Principal*.
mongodb · Dec 11, 2012 · 071f37b · 071f37b
1 parent f728de6
commit 071f37b
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 46 deletions.
diff --git a/src/mongo/db/auth/authorization_manager.cpp b/src/mongo/db/auth/authorization_manager.cpp
@@ -44,7 +44,6 @@ namespace mongo {
     const std::string AuthorizationManager::CLUSTER_RESOURCE_NAME = "$CLUSTER";
 
     namespace {
-        Principal specialAdminPrincipal("special", "local");
         const std::string ADMIN_DBNAME = "admin";
         const std::string LOCAL_DBNAME = "local";
         const std::string WILDCARD_DBNAME = "*";
@@ -336,44 +335,33 @@ namespace mongo {
         return Status::OK();
     }
 
-    const Principal* AuthorizationManager::checkAuthorization(const std::string& resource,
-                                                              ActionType action) const {
+    bool AuthorizationManager::checkAuthorization(const std::string& resource,
+                                                  ActionType action) const {
 
         if (_externalState->shouldIgnoreAuthChecks()) {
-            return &specialAdminPrincipal;
+            return true;
         }
 
-        const AcquiredPrivilege* privilege;
-        privilege = _acquiredPrivileges.getPrivilegeForAction(nsToDatabase(resource), action);
-        if (privilege) {
-            return privilege->getPrincipal();
-        }
-        privilege = _acquiredPrivileges.getPrivilegeForAction(WILDCARD_DBNAME, action);
-        if (privilege) {
-            return privilege->getPrincipal();
-        }
-
-        return NULL; // Not authorized
+        if (_acquiredPrivileges.getPrivilegeForAction(nsToDatabase(resource), action))
+            return true;
+        if (_acquiredPrivileges.getPrivilegeForAction(WILDCARD_DBNAME, action))
+            return true;
+        return false;
     }
 
-    const Principal* AuthorizationManager::checkAuthorization(const std::string& resource,
-                                                              ActionSet actions) const {
+    bool AuthorizationManager::checkAuthorization(const std::string& resource,
+                                                  ActionSet actions) const {
 
         if (_externalState->shouldIgnoreAuthChecks()) {
-            return &specialAdminPrincipal;
+            return true;
         }
 
-        const AcquiredPrivilege* privilege;
-        privilege = _acquiredPrivileges.getPrivilegeForActions(nsToDatabase(resource), actions);
-        if (privilege) {
-            return privilege->getPrincipal();
-        }
-        privilege = _acquiredPrivileges.getPrivilegeForActions(WILDCARD_DBNAME, actions);
-        if (privilege) {
-            return privilege->getPrincipal();
-        }
+        if (_acquiredPrivileges.getPrivilegeForActions(nsToDatabase(resource), actions))
+            return true;
+        if (_acquiredPrivileges.getPrivilegeForActions(WILDCARD_DBNAME, actions))
+            return true;
 
-        return NULL; // Not authorized
+        return false;
     }
 
     Status AuthorizationManager::checkAuthForQuery(const std::string& ns) {

diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
@@ -84,14 +84,12 @@ namespace mongo {
 
         // Checks if this connection has the privileges required to perform the given action
         // on the given resource.  Contains all the authorization logic including handling things
-        // like the localhost exception.  If it is authorized, returns the principal that granted
-        // the needed privilege.  Returns NULL if not authorized.  If the action is authorized but
-        // not because of a standard user Principal but for a special reason such as the localhost
-        // exception, it returns a pointer to specialAdminPrincipal.
-        const Principal* checkAuthorization(const std::string& resource, ActionType action) const;
-        // Same as above but takes an ActionSet instead of a single ActionType.  The one principal
-        // returned must be able to perform all the actions in the ActionSet on the given resource.
-        const Principal* checkAuthorization(const std::string& resource, ActionSet actions) const;
+        // like the localhost exception.  Returns true if the action may proceed on the resource.
+        bool checkAuthorization(const std::string& resource, ActionType action) const;
+
+        // Same as above but takes an ActionSet instead of a single ActionType.  Returns true if
+        // all of the actions may proceed on the resource.
+        bool checkAuthorization(const std::string& resource, ActionSet actions) const;
 
         // Parses the privilege documents and acquires all privileges that the privilege document
         // grants

diff --git a/src/mongo/db/auth/authorization_manager_test.cpp b/src/mongo/db/auth/authorization_manager_test.cpp
@@ -39,28 +39,26 @@ namespace {
         AuthExternalStateMock* externalState = new AuthExternalStateMock();
         AuthorizationManager authManager(externalState);
 
-        ASSERT_NULL(authManager.checkAuthorization("test", ActionType::insert));
+        ASSERT_FALSE(authManager.checkAuthorization("test", ActionType::insert));
         externalState->setReturnValueForShouldIgnoreAuthChecks(true);
-        ASSERT_EQUALS("special",
-                      authManager.checkAuthorization("test", ActionType::insert)->getName());
+        ASSERT_TRUE(authManager.checkAuthorization("test", ActionType::insert));
         externalState->setReturnValueForShouldIgnoreAuthChecks(false);
-        ASSERT_NULL(authManager.checkAuthorization("test", ActionType::insert));
+        ASSERT_FALSE(authManager.checkAuthorization("test", ActionType::insert));
 
         ASSERT_EQUALS(ErrorCodes::UserNotFound,
                       authManager.acquirePrivilege(writePrivilege).code());
         authManager.addAuthorizedPrincipal(principal);
         ASSERT_OK(authManager.acquirePrivilege(writePrivilege));
-        ASSERT_EQUALS(principal, authManager.checkAuthorization("test", ActionType::insert));
+        ASSERT_TRUE(authManager.checkAuthorization("test", ActionType::insert));
 
-        ASSERT_NULL(authManager.checkAuthorization("otherDb", ActionType::insert));
+        ASSERT_FALSE(authManager.checkAuthorization("otherDb", ActionType::insert));
         ASSERT_OK(authManager.acquirePrivilege(allDBsWritePrivilege));
-        ASSERT_EQUALS(principal, authManager.checkAuthorization("otherDb", ActionType::insert));
+        ASSERT_TRUE(authManager.checkAuthorization("otherDb", ActionType::insert));
         // Auth checks on a collection should be applied to the database name.
-        ASSERT_EQUALS(principal, authManager.checkAuthorization("otherDb.collectionName",
-                                                                ActionType::insert));
+        ASSERT_TRUE(authManager.checkAuthorization("otherDb.collectionName", ActionType::insert));
 
         authManager.logoutDatabase("test");
-        ASSERT_NULL(authManager.checkAuthorization("test", ActionType::insert));
+        ASSERT_FALSE(authManager.checkAuthorization("test", ActionType::insert));
     }
 
     TEST(AuthorizationManagerTest, GetPrivilegesFromPrivilegeDocument) {