----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32979/#review81785 -----------------------------------------------------------
src/java/main/org/apache/zookeeper/Login.java <https://reviews.apache.org/r/32979/#comment132263> if className == null, LOG.error() about it. src/java/main/org/apache/zookeeper/LoginFactory.java <https://reviews.apache.org/r/32979/#comment132260> "In the reality it will be/should be.." : maybe change this to "In practice, it will be.." src/java/main/org/apache/zookeeper/LoginFactory.java <https://reviews.apache.org/r/32979/#comment132261> I'm not sure that we need this factory class - I don't see where we have any multiple logins in the test code (or in non-test code). If we do only need this factory in the test code, then it might be better under the test/ directory. src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java <https://reviews.apache.org/r/32979/#comment132262> Perhaps LOG.info("using user-defined authentication method: " + className) src/java/test/org/apache/zookeeper/test/auth/util/SimpleSaslServer.java <https://reviews.apache.org/r/32979/#comment132259> A comment would be good here; something like: "This SaslServer simply permits a client to authenticate according to whatever username they supply in their input response[]" - Eugene Koontz On April 8, 2015, 5:23 p.m., Yuliya Feldman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32979/ > ----------------------------------------------------------- > > (Updated April 8, 2015, 5:23 p.m.) > > > Review request for zookeeper, Eugene Koontz, Patrick Hunt, and Camille > Fournier. > > > Repository: zookeeper-git > > > Description > ------- > > Today SASLAuthenticationProvider is used for all SASL based authentications > which creates some "if/else" statements in ZookeeperSaslClient and > ZookeeperSaslServer code with just Kerberos and Digest. > We want to use yet another different SASL based authentication and adding one > more "if/else" with some code specific just to that new way does not make > much sense. > Proposal is to allow to plug custom SASL Authentication mechanism(s) without > further changes in Zookeeper code. > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/Login.java 44b0bdf > src/java/main/org/apache/zookeeper/LoginFactory.java PRE-CREATION > src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 53f33e8 > src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1235faa > src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 60711ee > src/java/main/org/apache/zookeeper/server/auth/AuthMethod.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/auth/AuthRegistry.java > PRE-CREATION > src/java/main/org/apache/zookeeper/server/auth/DigestAuthMethod.java > PRE-CREATION > src/java/main/org/apache/zookeeper/server/auth/KerberosAuthMethod.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/SaslPluggableAuthTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/auth/util/GenericLoginModule.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/auth/util/SimpleAuthMethod.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/auth/util/SimpleSaslClient.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/auth/util/SimpleSaslProvider.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/auth/util/SimpleSaslServer.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/32979/diff/ > > > Testing > ------- > > UnitTests passed, New Unit Tests added > tested Digest and Kerberos(with real KDC) > > > Thanks, > > Yuliya Feldman > >
