Fixed ZOOKEEPER-48 - auth_id now handled correctly when no auth ids present

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@681465 13f79535-47bb-0310-9956-ffa450edef68

src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java

@@ -417,7 +417,7 @@ public class NIOServerCnxn implements Watcher, ServerCnxn {
                     LOG.error("No authentication provider for scheme: "
                             + scheme);
                 else
-                    LOG.error("Authentication failed for scheme: "
+                    LOG.debug("Authentication failed for scheme: "
                             + scheme);
                 // send a response...
                 ReplyHeader rh = new ReplyHeader(h.getXid(), 0,
@@ -427,7 +427,7 @@ public class NIOServerCnxn implements Watcher, ServerCnxn {
                 sendBuffer(NIOServerCnxn.closeConn);
                 disableRecv();
             } else {
-                LOG.error("Authentication succeeded for scheme: "
+                LOG.debug("Authentication succeeded for scheme: "
                         + scheme);
                 ReplyHeader rh = new ReplyHeader(h.getXid(), 0,
                         KeeperException.Code.Ok);

src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java

@@ -65,6 +65,9 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
     static boolean skipACL;
     static {
         skipACL = System.getProperty("zookeeper.skipACL", "no").equals("yes");
+        if (skipACL) {
+            LOG.info("zookeeper.skipACL==\"yes\", ACL checks will be skipped");
+        }
     }
 
     LinkedBlockingQueue<Request> submittedRequests = new LinkedBlockingQueue<Request>();
@@ -291,11 +294,11 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
                         .getNextZxid(), zks.getTime(), OpCode.setACL);
                 zks.sessionTracker.checkSession(request.sessionId);
                 SetACLRequest setAclRequest = new SetACLRequest();
+                ZooKeeperServer.byteBuffer2Record(request.request,
+                        setAclRequest);
                 if (!fixupACL(request.authInfo, setAclRequest.getAcl())) {
                     throw new KeeperException.InvalidACLException();
                 }
-                ZooKeeperServer.byteBuffer2Record(request.request,
-                        setAclRequest);
                 path = setAclRequest.getPath();
                 nodeRecord = getRecordForPath(path);
                 checkACL(zks, nodeRecord.acl, ZooDefs.Perms.ADMIN,
@@ -382,6 +385,9 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
     }
 
     /**
+     * This method checks out the acl making sure it isn't null or empty,
+     * it has valid schemes and ids, and expanding any relative ids that
+     * depend on the requestor's authentication information.
      *
      * @param authInfo list of ACL IDs associated with the client connection
      * @param acl list of ACLs being assigned to the node (create or setACL operation)
@@ -401,19 +407,26 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
             Id id = a.getId();
             if (id.getScheme().equals("world") && id.getId().equals("anyone")) {
             } else if (id.getScheme().equals("auth")) {
+                // This is the "auth" id, so we have to expand it to the
+                // authenticated ids of the requestor
                 it.remove();
                 if (toAdd == null) {
                     toAdd = new LinkedList<ACL>();
                 }
+                boolean authIdValid = false;
                 for (Id cid : authInfo) {
                     AuthenticationProvider ap = ProviderRegistry.getProvider(cid.getScheme());
                     if (ap == null) {
                         LOG.error("Missing AuthenticationProvider for "
                                 + cid.getScheme());
                     } else if (ap.isAuthenticated()) {
+                        authIdValid = true;
                         toAdd.add(new ACL(a.getPerms(), cid));
                     }
                 }
+                if (!authIdValid) {
+                    return false;
+                }
             } else {
                 AuthenticationProvider ap = ProviderRegistry.getProvider(id
                         .getScheme());
@@ -430,7 +443,7 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
                 acl.add(a);
             }
         }
-        return true;
+        return acl.size() > 0;
     }
 
     public void processRequest(Request request) {

src/java/test/org/apache/zookeeper/test/ClientTest.java

@@ -31,10 +31,15 @@ import java.util.concurrent.TimeUnit;
 import org.junit.Test;
 
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.InvalidACLException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.ZooDefs.CreateFlags;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooDefs.Perms;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.proto.WatcherEvent;
 
@@ -102,6 +107,52 @@ public class ClientTest extends ClientBase implements Watcher {
         performClientTest(true);
     }
 
+    @Test
+    public void testACLs() throws Exception {
+        ZooKeeper zk = null;
+        try {
+            zk = createClient(this);
+            try {
+                zk.create("/acltest", new byte[0], Ids.CREATOR_ALL_ACL, 0);
+                fail("Should have received an invalid acl error");
+            } catch(InvalidACLException e) {
+            }
+            try {
+                ArrayList<ACL> testACL = new ArrayList<ACL>();
+                testACL.add(new ACL(Perms.ALL | Perms.ADMIN, Ids.AUTH_IDS));
+                testACL.add(new ACL(Perms.ALL | Perms.ADMIN, new Id("ip", "127.0.0.1/8")));
+                zk.create("/acltest", new byte[0], testACL, 0);
+                fail("Should have received an invalid acl error");
+            } catch(InvalidACLException e) {
+            }
+            zk.addAuthInfo("digest", "ben:passwd".getBytes());
+            zk.create("/acltest", new byte[0], Ids.CREATOR_ALL_ACL, 0);
+            zk.close();
+            zk = createClient(this);
+            zk.addAuthInfo("digest", "ben:passwd2".getBytes());
+            try {
+                zk.getData("/acltest", false, new Stat());
+                fail("Should have received a permission error");
+            } catch (KeeperException e) {
+                assertEquals(Code.NoAuth, e.getCode());
+            }
+            zk.addAuthInfo("digest", "ben:passwd".getBytes());
+            zk.getData("/acltest", false, new Stat());
+            zk.setACL("/acltest", Ids.OPEN_ACL_UNSAFE, -1);
+            zk.close();
+            zk = createClient(this);
+            zk.getData("/acltest", false, new Stat());
+            List<ACL> acls = zk.getACL("/acltest", new Stat());
+            assertEquals(1, acls.size());
+            assertEquals(Ids.OPEN_ACL_UNSAFE, acls);
+            zk.close();
+        } finally {
+            if (zk != null) {
+                zk.close();
+            }
+        }
+    }
+
     private void performClientTest(boolean withWatcherObj) throws IOException,
             InterruptedException, KeeperException {
         ZooKeeper zk = null;