> On Aug. 1, 2012, 6:38 p.m., Patrick Hunt wrote: > > /src/java/test/org/apache/zookeeper/test/SaslAuthDesignatedServerTest.java, > > line 77 > > <https://reviews.apache.org/r/6290/diff/1/?file=132325#file132325line77> > > > > why is this sleep here? > > Eugene Koontz wrote: > These sleep()s are present in other SASL-related tests (for example > SaslAuthTest) as a work-around for ZOOKEEPER-1437: without the sleep()s, the > client may be denied permission because the SASL authentication process has > not completed yet. In the patch for ZOOKEEPER-1437, these sleeps are removed, > and thus could be removed also here, too.
Ah. Thanks Eugene. However given we know this is a SASL connection can't we just look for the KeeperState.SaslAuthenticated to come to the watcher? - Patrick ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6290/#review9703 ----------------------------------------------------------- On Aug. 1, 2012, 6:20 p.m., Matteo Bertozzi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6290/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2012, 6:20 p.m.) > > > Review request for zookeeper, Patrick Hunt and Eugene Koontz. > > > Description > ------- > > Currently the CnxnFactory checks for "java.security.auth.login.config" to > decide whether or not enable SASL. > - zookeeper/server/NIOServerCnxnFactory.java > - zookeeper/server/NettyServerCnxnFactory.java > - configure() checks for "java.security.auth.login.config" > - If present start the new Login("Server", > SaslServerCallbackHandler(conf)) > > But since the SaslServerCallbackHandler does the right thing just checking if > getAppConfigurationEntry() is empty, we can allow SASL with JAAS > configuration to be programmatically just checking weather or not a > configuration entry is present instead of "java.security.auth.login.config". > (Something quite similar was done for the SaslClient in ZOOKEEPER-1373) > > > This addresses bug ZOOKEEPER-1497. > https://issues.apache.org/jira/browse/ZOOKEEPER-1497 > > > Diffs > ----- > > /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java > 1360369 > /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 1360369 > /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1360369 > /src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 1360369 > > /src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java > 1360369 > /src/java/test/org/apache/zookeeper/JaasConfiguration.java PRE-CREATION > /src/java/test/org/apache/zookeeper/test/SaslAuthDesignatedServerTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/6290/diff/ > > > Testing > ------- > > New testcase added SaslAuthDesignatedServerTest to check if > ZooKeeperSaslServer.LOGIN_CONTEXT_NAME_KEY is used. > (A new JaasConfiguration class was added to wrap the jaas.conf) > > +Manual testing for HBASE-4791 > > > Thanks, > > Matteo Bertozzi > >
