This is an automated email from the ASF dual-hosted git repository.

hanm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 1af3dcc  ZOOKEEPER-3561: Generalize target authentication scheme for 
ZooKeeper authentication enforcement.
1af3dcc is described below

commit 1af3dcc633d4829864da74ca6b030428448fcc16
Author: Mohammad Arshad <ars...@apache.org>
AuthorDate: Tue Oct 20 19:41:38 2020 -0700

    ZOOKEEPER-3561: Generalize target authentication scheme for ZooKeeper 
authentication enforcement.
    
    Added enforce.auth.enabled and enforce.auth.scheme to enforce any 
authentication scheme.
    
    Author: Mohammad Arshad <ars...@apache.org>
    
    Reviewers: Michael Han <h...@apache.org>, Damien Diederen 
<d...@crosstwine.com>, Enrico Olivelli <eolive...@gmail.com>, Andor Molnár 
<an...@apache.org>
    
    Closes #1500 from arshadmohammad/ZOOKEEPER-3561-master
---
 .../zookeeper-client-c/include/zookeeper.h         |   2 +-
 .../zookeeper-client-c/tests/zkServer.sh           |   2 +-
 .../src/main/resources/markdown/zookeeperAdmin.md  |  34 ++-
 .../java/org/apache/zookeeper/KeeperException.java |   5 +-
 .../zookeeper/server/AuthenticationHelper.java     | 141 +++++++++++
 .../apache/zookeeper/server/ZooKeeperServer.java   |  29 +--
 .../zookeeper/EnforceAuthenticationTest.java       | 266 +++++++++++++++++++++
 .../server/quorum/QuorumPeerTestBase.java          |  24 +-
 .../java/org/apache/zookeeper/test/ClientBase.java |  14 +-
 .../test/SaslAuthRequiredFailNoSASLTest.java       |  16 +-
 10 files changed, 493 insertions(+), 40 deletions(-)

diff --git a/zookeeper-client/zookeeper-client-c/include/zookeeper.h 
b/zookeeper-client/zookeeper-client-c/include/zookeeper.h
index 8bd1d95..467a857 100644
--- a/zookeeper-client/zookeeper-client-c/include/zookeeper.h
+++ b/zookeeper-client/zookeeper-client-c/include/zookeeper.h
@@ -140,7 +140,7 @@ enum ZOO_ERRORS {
   ZEPHEMERALONLOCALSESSION = -120, /*!< Attempt to create ephemeral node on a 
local session */
   ZNOWATCHER = -121, /*!< The watcher couldn't be found */
   ZRECONFIGDISABLED = -123, /*!< Attempts to perform a reconfiguration 
operation when reconfiguration feature is disabled */
-  ZSESSIONCLOSEDREQUIRESASLAUTH = -124, /*!< The session has been closed by 
server because server requires client to do SASL authentication, but client is 
not configured with SASL authentication or configuted with SASL but failed 
(i.e. wrong credential used.). */
+  ZSESSIONCLOSEDREQUIRESASLAUTH = -124, /*!< The session has been closed by 
server because server requires client to do authentication via configured 
authentication scheme at server, but client is not configured with required 
authentication scheme or configured but failed (i.e. wrong credential used.). */
   ZTHROTTLEDOP = -127 /*!< Operation was throttled and not executed at all. 
please, retry! */
 
   /* when adding/changing values here also update zerror(int) to return 
correct error message */
diff --git a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh 
b/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
index 99b716a..f98b405 100755
--- a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
+++ b/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
@@ -128,9 +128,9 @@ PROPERTIES="$EXTRA_JVM_ARGS 
-Dzookeeper.extendedTypesEnabled=true -Dznode.contai
 if [ "x$1" == "xstartRequireSASLAuth" ]
 then
     PROPERTIES="-Dzookeeper.sessionRequireClientSASLAuth=true $PROPERTIES"
+    PROPERTIES="$PROPERTIES 
-Dzookeeper.authProvider.1=org.apache.zookeeper.server.auth.SASLAuthenticationProvider"
     if [ "x$2" != "x" ]
     then
-        PROPERTIES="$PROPERTIES 
-Dzookeeper.authProvider.1=org.apache.zookeeper.server.auth.SASLAuthenticationProvider"
         PROPERTIES="$PROPERTIES -Djava.security.auth.login.config=$2"
     fi
     if [ "x$3" != "x" ]
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md 
b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 2150c5a..a3a1c6c 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1498,8 +1498,8 @@ and [SASL authentication for 
ZooKeeper](https://cwiki.apache.org/confluence/disp
     If the credential is not in the list, the connection request will be 
refused.
     This prevents a client accidentally connecting to a wrong ensemble.
 
-* *zookeeper.sessionRequireClientSASLAuth* :
-    (Java system property only: **zookeeper.sessionRequireClientSASLAuth**)
+* *sessionRequireClientSASLAuth* :
+    (Java system property: **zookeeper.sessionRequireClientSASLAuth**)
     **New in 3.6.0:**
     When set to **true**, ZooKeeper server will only accept connections and 
requests from clients
     that have authenticated with server via SASL. Clients that are not 
configured with SASL
@@ -1508,13 +1508,41 @@ and [SASL authentication for 
ZooKeeper](https://cwiki.apache.org/confluence/disp
     in such case, both Java and C client will close the session with server 
thereafter,
     without further attempts on retrying to reconnect.
 
+    This configuration is short hand for **enforce.auth.enabled=true** and 
**enforce.auth.scheme=sasl**
+
     By default, this feature is disabled. Users who would like to opt-in can 
enable the feature
-    by setting **zookeeper.sessionRequireClientSASLAuth** to **true**.
+    by setting **sessionRequireClientSASLAuth** to **true**.
 
     This feature overrules the <emphasis 
role="bold">zookeeper.allowSaslFailedClients</emphasis> option, so even if 
server is
     configured to allow clients that fail SASL authentication to login, client 
will not be able to
     establish a session with server if this feature is enabled.
 
+* *enforce.auth.enabled* :
+    (Java system property : **zookeeper.enforce.auth.enabled**)
+    **New in 3.7.0:**
+    When set to **true**, ZooKeeper server will only accept connections and 
requests from clients
+    that have authenticated with server via configured auth scheme. 
Authentication schemes
+    can be configured using property enforce.auth.schemes. Clients that are not
+    configured with the any of the auth scheme configured at server or 
configured but failed authentication (i.e. with invalid credential)
+    will not be able to establish a session with server. A typed error code 
(-124) will be delivered
+    in such case, both Java and C client will close the session with server 
thereafter,
+    without further attempts on retrying to reconnect.
+
+    By default, this feature is disabled. Users who would like to opt-in can 
enable the feature
+    by setting **enforce.auth.enabled** to **true**.
+
+    When **enforce.auth.enabled=true** and **enforce.auth.schemes=sasl** then 
+    <emphasis role="bold">zookeeper.allowSaslFailedClients</emphasis> 
configuration is overruled. So even if server is
+    configured to allow clients that fail SASL authentication to login, client 
will not be able to
+    establish a session with server if this feature is enabled with sasl as 
authentication scheme.
+
+* *enforce.auth.schemes* :
+    (Java system property : **zookeeper.enforce.auth.schemes**)
+    **New in 3.7.0:**
+    Comma separated list of authentication schemes. Clients must be 
authenticated with at least one
+    authentication scheme before doing any zookeeper operations. 
+    This property is used only when **enforce.auth.enabled** is to **true**.
+
 * *sslQuorum* :
     (Java system property: **zookeeper.sslQuorum**)
     **New in 3.5.5:**
diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java 
b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
index c8b33b7..9469d64 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
@@ -403,8 +403,9 @@ public abstract class KeeperException extends Exception {
         REQUESTTIMEOUT(-122),
         /** Attempts to perform a reconfiguration operation when 
reconfiguration feature is disabled. */
         RECONFIGDISABLED(-123),
-        /** The session has been closed by server because server requires 
client to do SASL authentication,
-         *  but client is not configured with SASL authentication or 
configuted with SASL but failed
+        /** The session has been closed by server because server requires 
client to do authentication
+         *  with configured authentication scheme at the server, but client is 
not configured with
+         *  required  authentication scheme or configured but authentication 
failed
          *  (i.e. wrong credential used.). */
         SESSIONCLOSEDREQUIRESASLAUTH(-124),
         /** Operation was throttled and not executed at all. This error code 
indicates that zookeeper server
diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/AuthenticationHelper.java
 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/AuthenticationHelper.java
new file mode 100644
index 0000000..61aebb3
--- /dev/null
+++ 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/AuthenticationHelper.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.server;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.proto.ReplyHeader;
+import org.apache.zookeeper.server.auth.ProviderRegistry;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Contains helper methods to enforce authentication
+ */
+public class AuthenticationHelper {
+    private static final Logger LOG = 
LoggerFactory.getLogger(AuthenticationHelper.class);
+
+    public static final String ENFORCE_AUTH_ENABLED = 
"zookeeper.enforce.auth.enabled";
+    public static final String ENFORCE_AUTH_SCHEMES = 
"zookeeper.enforce.auth.schemes";
+    public static final String SESSION_REQUIRE_CLIENT_SASL_AUTH =
+        "zookeeper.sessionRequireClientSASLAuth";
+    public static final String SASL_AUTH_SCHEME = "sasl";
+
+    private boolean enforceAuthEnabled;
+    private List<String> enforceAuthSchemes = new ArrayList<>();
+    private boolean saslAuthRequired;
+
+    public AuthenticationHelper() {
+        initConfigurations();
+    }
+
+    private void initConfigurations() {
+        if 
(Boolean.parseBoolean(System.getProperty(SESSION_REQUIRE_CLIENT_SASL_AUTH, 
"false"))) {
+            enforceAuthEnabled = true;
+            enforceAuthSchemes.add(SASL_AUTH_SCHEME);
+        } else {
+            enforceAuthEnabled =
+                Boolean.parseBoolean(System.getProperty(ENFORCE_AUTH_ENABLED, 
"false"));
+            String enforceAuthSchemesProp = 
System.getProperty(ENFORCE_AUTH_SCHEMES);
+            if (enforceAuthSchemesProp != null) {
+                Arrays.stream(enforceAuthSchemesProp.split(",")).forEach(s -> {
+                    enforceAuthSchemes.add(s.trim());
+                });
+            }
+        }
+        LOG.info("{} = {}", ENFORCE_AUTH_ENABLED, enforceAuthEnabled);
+        LOG.info("{} = {}", ENFORCE_AUTH_SCHEMES, enforceAuthSchemes);
+        validateConfiguredProperties();
+        saslAuthRequired = enforceAuthEnabled && 
enforceAuthSchemes.contains(SASL_AUTH_SCHEME);
+    }
+
+    private void validateConfiguredProperties() {
+        if (enforceAuthEnabled) {
+            if (enforceAuthSchemes.isEmpty()) {
+                String msg =
+                    ENFORCE_AUTH_ENABLED + " is true " + ENFORCE_AUTH_SCHEMES 
+ " must be  "
+                        + "configured.";
+                LOG.error(msg);
+                throw new IllegalArgumentException(msg);
+            }
+            enforceAuthSchemes.forEach(scheme -> {
+                if (ProviderRegistry.getProvider(scheme) == null) {
+                    String msg = "Authentication scheme " + scheme + " is not 
available.";
+                    LOG.error(msg);
+                    throw new IllegalArgumentException(msg);
+                }
+            });
+        }
+    }
+
+    /**
+     * Checks if connection is authenticated or not.
+     *
+     * @param cnxn server connection
+     * @return boolean
+     */
+    private boolean isCnxnAuthenticated(ServerCnxn cnxn) {
+        for (Id id : cnxn.getAuthInfo()) {
+            if (enforceAuthSchemes.contains(id.getScheme())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public boolean isEnforceAuthEnabled() {
+        return enforceAuthEnabled;
+    }
+
+    /**
+     * Returns true when authentication enforcement was success otherwise 
returns false
+     * also closes the connection
+     *
+     * @param connection server connection
+     * @param xid        current operation xid
+     * @return true when authentication enforcement is success otherwise false
+     */
+    public boolean enforceAuthentication(ServerCnxn connection, int xid) 
throws IOException {
+        if (isEnforceAuthEnabled() && !isCnxnAuthenticated(connection)) {
+            //Un authenticated connection, lets inform user with response and 
then close the session
+            LOG.error("Client authentication scheme(s) {} does not match with 
any of the expected "
+                    + "authentication scheme {}, closing session.", 
getAuthSchemes(connection),
+                enforceAuthSchemes);
+            ReplyHeader replyHeader = new ReplyHeader(xid, 0,
+                KeeperException.Code.SESSIONCLOSEDREQUIRESASLAUTH.intValue());
+            connection.sendResponse(replyHeader, null, "response");
+            connection.sendCloseSession();
+            connection.disableRecv();
+            return false;
+        }
+        return true;
+    }
+
+    private List<String> getAuthSchemes(ServerCnxn connection) {
+        return 
connection.getAuthInfo().stream().map(Id::getScheme).collect(Collectors.toList());
+    }
+
+    public boolean isSaslAuthRequired() {
+        return saslAuthRequired;
+    }
+}
diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
index b016724..00550be 100644
--- 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
+++ 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
@@ -107,9 +107,6 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
     static final boolean skipACL;
 
     public static final String ALLOW_SASL_FAILED_CLIENTS = 
"zookeeper.allowSaslFailedClients";
-    public static final String SESSION_REQUIRE_CLIENT_SASL_AUTH = 
"zookeeper.sessionRequireClientSASLAuth";
-    public static final String SASL_AUTH_SCHEME = "sasl";
-
     public static final String ZOOKEEPER_DIGEST_ENABLED = 
"zookeeper.digest.enabled";
     private static boolean digestEnabled;
 
@@ -284,6 +281,8 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
 
     private final AtomicInteger currentLargeRequestBytes = new 
AtomicInteger(0);
 
+    private AuthenticationHelper authHelper;
+
     void removeCnxn(ServerCnxn cnxn) {
         zkDb.removeCnxn(cnxn);
     }
@@ -298,6 +297,7 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
         listener = new ZooKeeperServerListenerImpl(this);
         serverStats = new ServerStats(this);
         this.requestPathMetricsCollector = new RequestPathMetricsCollector();
+        this.authHelper = new AuthenticationHelper();
     }
 
     /**
@@ -339,6 +339,8 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
 
         this.initLargeRequestThrottlingSettings();
 
+        this.authHelper = new AuthenticationHelper();
+
         LOG.info(
             "Created server with"
                 + " tickTime {}"
@@ -1623,11 +1625,10 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
         } else if (h.getType() == OpCode.sasl) {
             processSasl(incomingBuffer, cnxn, h);
         } else {
-            if (shouldRequireClientSaslAuth() && 
!hasCnxSASLAuthenticated(cnxn)) {
-                ReplyHeader replyHeader = new ReplyHeader(h.getXid(), 0, 
Code.SESSIONCLOSEDREQUIRESASLAUTH.intValue());
-                cnxn.sendResponse(replyHeader, null, "response");
-                cnxn.sendCloseSession();
-                cnxn.disableRecv();
+            if (!authHelper.enforceAuthentication(cnxn, h.getXid())) {
+                // Authentication enforcement is failed
+                // Already sent response to user about failure and closed the 
session, lets return
+                return;
             } else {
                 Request si = new Request(cnxn, cnxn.getSessionId(), 
h.getXid(), h.getType(), incomingBuffer, cnxn.getAuthInfo());
                 int length = incomingBuffer.limit();
@@ -1646,14 +1647,6 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
         return Boolean.getBoolean(ALLOW_SASL_FAILED_CLIENTS);
     }
 
-    private static boolean shouldRequireClientSaslAuth() {
-        return Boolean.getBoolean(SESSION_REQUIRE_CLIENT_SASL_AUTH);
-    }
-
-    private boolean hasCnxSASLAuthenticated(ServerCnxn cnxn) {
-        return cnxn.getAuthInfo().stream().anyMatch(id -> 
id.getScheme().equals(SASL_AUTH_SCHEME));
-    }
-
     private void processSasl(ByteBuffer incomingBuffer, ServerCnxn cnxn, 
RequestHeader requestHeader) throws IOException {
         LOG.debug("Responding to client SASL token.");
         GetSASLRequest clientTokenRecord = new GetSASLRequest();
@@ -1679,11 +1672,11 @@ public class ZooKeeperServer implements SessionExpirer, 
ServerStats.Provider {
                 }
             } catch (SaslException e) {
                 LOG.warn("Client {} failed to SASL authenticate: {}", 
cnxn.getRemoteSocketAddress(), e);
-                if (shouldAllowSaslFailedClientsConnect() && 
!shouldRequireClientSaslAuth()) {
+                if (shouldAllowSaslFailedClientsConnect() && 
!authHelper.isSaslAuthRequired()) {
                     LOG.warn("Maintaining client connection despite SASL 
authentication failure.");
                 } else {
                     int error;
-                    if (shouldRequireClientSaslAuth()) {
+                    if (authHelper.isSaslAuthRequired()) {
                         LOG.warn(
                             "Closing client connection due to server requires 
client SASL authenticaiton,"
                                 + "but client SASL authentication has failed, 
or client is not configured with SASL "
diff --git 
a/zookeeper-server/src/test/java/org/apache/zookeeper/EnforceAuthenticationTest.java
 
b/zookeeper-server/src/test/java/org/apache/zookeeper/EnforceAuthenticationTest.java
new file mode 100644
index 0000000..ef66562
--- /dev/null
+++ 
b/zookeeper-server/src/test/java/org/apache/zookeeper/EnforceAuthenticationTest.java
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper;
+
+import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.server.AuthenticationHelper;
+import org.apache.zookeeper.server.ServerConfig;
+import org.apache.zookeeper.server.ZooKeeperServerMain;
+import org.apache.zookeeper.server.quorum.QuorumPeerTestBase;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class EnforceAuthenticationTest extends QuorumPeerTestBase {
+    protected static final Logger LOG = 
LoggerFactory.getLogger(EnforceAuthenticationTest.class);
+    private Servers servers;
+    private int clientPort;
+
+    @Before
+    public void setUp() {
+        System.setProperty("zookeeper.admin.enableServer", "false");
+        System.setProperty("zookeeper.4lw.commands.whitelist", "*");
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_ENABLED);
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_SCHEMES);
+    }
+
+    @After
+    public void tearDown() throws InterruptedException {
+        if (servers != null) {
+            servers.shutDownAllServers();
+        }
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_ENABLED);
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_SCHEMES);
+    }
+
+    /**
+     * When AuthenticationHelper.ENFORCE_AUTH_ENABLED is not set or set to 
false, behaviour should
+     * be same as the old ie. clients without authentication are allowed to do 
operations
+     */
+    @Test
+    public void testEnforceAuthenticationOldBehaviour() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        startServer(prop);
+        testEnforceAuthOldBehaviour(false);
+    }
+
+    @Test
+    public void testEnforceAuthenticationOldBehaviourWithNetty() throws 
Exception {
+        Map<String, String> prop = new HashMap<>();
+        //setting property false should give the same behaviour as when 
property is not set
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), 
"false");
+        prop.put("serverCnxnFactory", 
"org.apache.zookeeper.server.NettyServerCnxnFactory");
+        startServer(prop);
+        testEnforceAuthOldBehaviour(true);
+    }
+
+    private void testEnforceAuthOldBehaviour(boolean netty) throws Exception {
+        ZKClientConfig config = new ZKClientConfig();
+        if (netty) {
+            config.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
+                "org.apache.zookeeper.ClientCnxnSocketNetty");
+        }
+        ZooKeeper client = ClientBase
+            .createZKClient("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, 
CONNECTION_TIMEOUT,
+                config);
+        String path = "/defaultAuth" + System.currentTimeMillis();
+        String data = "someData";
+        client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, 
CreateMode.PERSISTENT);
+        byte[] data1 = client.getData(path, false, null);
+        assertEquals(data, new String(data1));
+        client.close();
+    }
+
+    /**
+     * Server start should fail when ZooKeeperServer.ENFORCE_AUTH_ENABLED is 
set to true  but
+     * AuthenticationHelper.ENFORCE_AUTH_SCHEME is not configured
+     */
+    @Test
+    public void 
testServerStartShouldFailWhenEnforceAuthSchemeIsNotConfigured() throws 
Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), 
"true");
+        testServerCannotStart(prop);
+    }
+
+    /**
+     * Server start should fail when AuthenticationHelper.ENFORCE_AUTH_ENABLED 
is set to true,
+     * AuthenticationHelper.ENFORCE_AUTH_SCHEME is configured but 
authentication provider is not
+     * configured.
+     */
+    @Test
+    public void testServerStartShouldFailWhenAuthProviderIsNotConfigured() 
throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), 
"true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), 
"sasl");
+        testServerCannotStart(prop);
+    }
+
+    private void testServerCannotStart(Map<String, String> prop)
+        throws Exception {
+        File confFile = getConfFile(prop);
+        ServerConfig config = new ServerConfig();
+        config.parse(confFile.toString());
+        ZooKeeperServerMain serverMain = new ZooKeeperServerMain();
+        try {
+            serverMain.runFromConfig(config);
+            fail("IllegalArgumentException is expected.");
+        } catch (IllegalArgumentException e) {
+            //do nothing
+        }
+    }
+
+    @Test
+    public void testEnforceAuthenticationNewBehaviour() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), 
"true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), 
"digest");
+        //digest auth provider is started by default, so no need to
+        //prop.put("authProvider.1", 
DigestAuthenticationProvider.class.getName());
+        startServer(prop);
+        testEnforceAuthNewBehaviour(false);
+    }
+
+    @Test
+    public void testEnforceAuthenticationNewBehaviourWithNetty() throws 
Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), 
"true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), 
"digest");
+        prop.put("serverCnxnFactory", 
"org.apache.zookeeper.server.NettyServerCnxnFactory");
+        startServer(prop);
+        testEnforceAuthNewBehaviour(true);
+    }
+
+    /**
+     * Client operations are allowed only after the authentication is done
+     */
+    private void testEnforceAuthNewBehaviour(boolean netty) throws Exception {
+        ZKClientConfig config = new ZKClientConfig();
+        if (netty) {
+            config.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
+                "org.apache.zookeeper.ClientCnxnSocketNetty");
+        }
+        CountDownLatch countDownLatch = new CountDownLatch(1);
+        ZooKeeper client =
+            new ZooKeeper("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, 
getWatcher(countDownLatch),
+                config);
+        countDownLatch.await();
+        String path = "/newAuth" + System.currentTimeMillis();
+        String data = "someData";
+
+        //try without authentication
+        try {
+            client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, 
CreateMode.PERSISTENT);
+            fail("SessionClosedRequireAuthException is expected.");
+        } catch (KeeperException.SessionClosedRequireAuthException e) {
+            //do nothing
+        }
+        client.close();
+        countDownLatch = new CountDownLatch(1);
+        client =
+            new ZooKeeper("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, 
getWatcher(countDownLatch),
+                config);
+        countDownLatch.await();
+
+        // try operations after authentication
+        String idPassword = "user1:pass1";
+        client.addAuthInfo("digest", idPassword.getBytes());
+        client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, 
CreateMode.PERSISTENT);
+        byte[] data1 = client.getData(path, false, null);
+        assertEquals(data, new String(data1));
+        client.close();
+    }
+
+    @Test
+    public void testEnforceAuthenticationWithMultipleAuthSchemes() throws 
Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), 
"true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), 
"digest,ip");
+        startServer(prop);
+        ZKClientConfig config = new ZKClientConfig();
+        CountDownLatch countDownLatch = new CountDownLatch(1);
+        ZooKeeper client =
+            new ZooKeeper("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, 
getWatcher(countDownLatch),
+                config);
+        countDownLatch.await();
+        // try operation without adding auth info, it should be success as ip 
auth info is
+        // added automatically by server
+        String path = "/newAuth" + System.currentTimeMillis();
+        String data = "someData";
+        client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, 
CreateMode.PERSISTENT);
+        byte[] data1 = client.getData(path, false, null);
+        assertEquals(data, new String(data1));
+        client.close();
+    }
+
+    private String removeZooKeeper(String prop) {
+        return prop.replace("zookeeper.", "");
+    }
+
+    private File getConfFile(Map<String, String> additionalProp) throws 
IOException {
+        clientPort = PortAssignment.unique();
+        StringBuilder sb = new StringBuilder();
+        sb.append("standaloneEnabled=true" + "\n");
+        if (null != additionalProp) {
+            for (Map.Entry<String, String> entry : additionalProp.entrySet()) {
+                sb.append(entry.getKey());
+                sb.append("=");
+                sb.append(entry.getValue());
+                sb.append("\n");
+            }
+        }
+        String currentQuorumCfgSection = sb.toString();
+        return new MainThread(1, clientPort, currentQuorumCfgSection, 
false).getConfFile();
+    }
+
+    private void startServer(Map<String, String> additionalProp) throws 
Exception {
+        additionalProp.put("standaloneEnabled", "true");
+        servers = LaunchServers(1, additionalProp);
+        clientPort = servers.clientPorts[0];
+    }
+
+    private Watcher getWatcher(CountDownLatch countDownLatch) {
+        return event -> {
+            Event.EventType type = event.getType();
+            if (type == Event.EventType.None) {
+                Event.KeeperState state = event.getState();
+                if (state == Event.KeeperState.SyncConnected) {
+                    LOG.info("Event.KeeperState.SyncConnected");
+                    countDownLatch.countDown();
+                } else if (state == Event.KeeperState.Expired) {
+                    LOG.info("Event.KeeperState.Expired");
+                } else if (state == Event.KeeperState.Disconnected) {
+                    LOG.info("Event.KeeperState.Disconnected");
+                } else if (state == Event.KeeperState.AuthFailed) {
+                    LOG.info("Event.KeeperState.AuthFailed");
+                }
+            }
+        };
+    }
+}
diff --git 
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
 
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
index 953f4ab..2a116dc 100644
--- 
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
+++ 
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
@@ -406,7 +406,7 @@ public class QuorumPeerTestBase extends ZKTestCase 
implements Watcher {
 
         MainThread[] mt;
         ZooKeeper[] zk;
-        int[] clientPorts;
+        public int[] clientPorts;
 
         public void shutDownAllServers() throws InterruptedException {
             for (MainThread t : mt) {
@@ -470,23 +470,35 @@ public class QuorumPeerTestBase extends ZKTestCase 
implements Watcher {
     }
 
     protected Servers LaunchServers(int numServers) throws IOException, 
InterruptedException {
-        return LaunchServers(numServers, null);
+        return LaunchServers(numServers, (Integer) null);
+    }
+
+    protected Servers LaunchServers(int numServers, Map<String, String> 
otherConfigs)
+        throws IOException, InterruptedException {
+        return LaunchServers(numServers, 0, null, otherConfigs);
     }
 
     protected Servers LaunchServers(int numServers, Integer tickTime) throws 
IOException, InterruptedException {
         return LaunchServers(numServers, 0, tickTime);
     }
 
+    protected Servers LaunchServers(int numServers, int numObservers, Integer 
tickTime)
+        throws IOException, InterruptedException {
+        return LaunchServers(numServers, numObservers, tickTime, new 
HashMap<>());
+    }
+
     /** * This is a helper function for launching a set of servers
      *
      * @param numServers the number of participant servers
-     * @param numObserver the number of observer servers
+     * @param numObservers the number of observer servers
      * @param tickTime A ticktime to pass to MainThread
+     * @param otherConfigs any zoo.cfg configuration
      * @return
      * @throws IOException
      * @throws InterruptedException
      */
-    protected Servers LaunchServers(int numServers, int numObservers, Integer 
tickTime) throws IOException, InterruptedException {
+    protected Servers LaunchServers(int numServers, int numObservers, Integer 
tickTime,
+        Map<String, String> otherConfigs) throws IOException, 
InterruptedException {
         int SERVER_COUNT = numServers + numObservers;
         QuorumPeerMainTest.Servers svrs = new QuorumPeerMainTest.Servers();
         svrs.clientPorts = new int[SERVER_COUNT];
@@ -504,9 +516,9 @@ public class QuorumPeerTestBase extends ZKTestCase 
implements Watcher {
         svrs.zk = new ZooKeeper[SERVER_COUNT];
         for (int i = 0; i < SERVER_COUNT; i++) {
             if (tickTime != null) {
-                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], 
quorumCfgSection, new HashMap<String, String>(), tickTime);
+                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], 
quorumCfgSection, otherConfigs, tickTime);
             } else {
-                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], 
quorumCfgSection);
+                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], 
quorumCfgSection, otherConfigs);
             }
             svrs.mt[i].start();
             svrs.restartClient(i, this);
diff --git 
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java 
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java
index 5904a37..c628e4f 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java
@@ -51,6 +51,7 @@ import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.Watcher.Event.KeeperState;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.common.IOUtils;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.common.X509Exception.SSLContextException;
@@ -351,6 +352,9 @@ public abstract class ClientBase extends ZKTestCase {
     }
 
     static File createTmpDir(File parentDir, boolean createInitFile) throws 
IOException {
+        if (!parentDir.exists()) {
+            parentDir.mkdir();
+        }
         File tmpFile = File.createTempFile("test", ".junit", parentDir);
         // don't delete tmpFile - this ensures we don't attempt to create
         // a tmpDir with a duplicate name
@@ -738,9 +742,15 @@ public abstract class ClientBase extends ZKTestCase {
         return createZKClient(cxnString, sessionTimeout, CONNECTION_TIMEOUT);
     }
 
-    public static ZooKeeper createZKClient(String cxnString, int 
sessionTimeout, long connectionTimeout) throws IOException {
+    public static ZooKeeper createZKClient(String cxnString, int 
sessionTimeout,
+        long connectionTimeout) throws IOException {
+        return createZKClient(cxnString, sessionTimeout, connectionTimeout, 
new ZKClientConfig());
+    }
+
+    public static ZooKeeper createZKClient(String cxnString, int 
sessionTimeout,
+        long connectionTimeout, ZKClientConfig config) throws IOException {
         CountdownWatcher watcher = new CountdownWatcher();
-        ZooKeeper zk = new ZooKeeper(cxnString, sessionTimeout, watcher);
+        ZooKeeper zk = new ZooKeeper(cxnString, sessionTimeout, watcher, 
config);
         try {
             watcher.waitForConnected(connectionTimeout);
         } catch (InterruptedException | TimeoutException e) {
diff --git 
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
 
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
index 51dbbb5..f5b99b9 100644
--- 
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
+++ 
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
@@ -24,20 +24,22 @@ import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.ZooKeeper;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 public class SaslAuthRequiredFailNoSASLTest extends ClientBase {
 
-    @BeforeEach
-    public void setup() {
+    @BeforeAll
+    public static void setup() {
         System.setProperty(SaslTestUtil.requireSASLAuthProperty, "true");
+        System.setProperty(SaslTestUtil.authProviderProperty, 
SaslTestUtil.authProvider);
     }
 
-    @AfterEach
-    public void tearDown() throws Exception {
+    @AfterAll
+    public static void clearSetup() {
         System.clearProperty(SaslTestUtil.requireSASLAuthProperty);
+        System.clearProperty(SaslTestUtil.authProviderProperty);
     }
 
     @Test
@@ -46,7 +48,7 @@ public class SaslAuthRequiredFailNoSASLTest extends 
ClientBase {
         CountdownWatcher watcher = new CountdownWatcher();
         try {
             zk = createClient(watcher);
-            zk.create("/foo", null, Ids.CREATOR_ALL_ACL, 
CreateMode.PERSISTENT);
+            zk.create("/foo", null, Ids.OPEN_ACL_UNSAFE, 
CreateMode.PERSISTENT);
             fail("Client is not configured with SASL authentication, so 
zk.create operation should fail.");
         } catch (KeeperException e) {
             assertTrue(e.code() == 
KeeperException.Code.SESSIONCLOSEDREQUIRESASLAUTH);

Reply via email to