zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Hunt" <ph...@apache.org>
Subject Re: Review Request: Allow server-side SASL login with JAAS configuration to be programmatically set (rather than only by reading JAAS configuration file)
Date Wed, 01 Aug 2012 21:30:34 GMT


> 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.
> 
> Patrick Hunt wrote:
>     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?
> 
> Eugene Koontz wrote:
>     Sure, that should work. Perhaps I should file a new JIRA for removing the various
sleep()s in the existing tests (and mentioning this JIRA too), and replace with watches?
> 
> Patrick Hunt wrote:
>     Sounds good. Although we should have a specific test that exercises this case for
ZOOKEEPER-1437, such a test is included in 1437?
> 
> Eugene Koontz wrote:
>     The latest patch for ZOOKEEPER-1437 simply remove the sleep()s from the tests but
do not do not add any watches for the SaslAuthenticated event. So I will add watches in a
new patch for ZK-1437.
>     
>

Eugene my previous comment about 1437, what I meant was that 1437 should exercise the code
path for the case where we don't wait (your comment about having to have the sleeps in the
current code). Seems like 1437 should have both test cases, 1) run the command w/o waiting,
2) wait for the watch and verify that code path works as well.


- Patrick


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6290/#review9703
-----------------------------------------------------------


On Aug. 1, 2012, 8:33 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, 8:33 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/Environment.java 1368201 
>   /src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 1368201 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1368201 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1368201 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1368201 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 1368201 
>   /src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 1368201

>   /src/java/test/org/apache/zookeeper/JaasConfiguration.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/ClientBase.java 1368201 
>   /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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message