----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53204/#review157049 -----------------------------------------------------------
service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 90) <https://reviews.apache.org/r/53204/#comment227431> Its not a good idea to log the name of the group the user belongs to whether it succeeds or fails. So perhaps just log the fact that authentication succeeded based on group filter? service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 124) <https://reviews.apache.org/r/53204/#comment227433> Remove group name from the log record. service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 130) <https://reviews.apache.org/r/53204/#comment227436> anonymize the log record, should have no indication of the configuration service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 138) <https://reviews.apache.org/r/53204/#comment227432> Same as above. Remove the group name from the log record. service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 143) <https://reviews.apache.org/r/53204/#comment227437> same here service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line 261) <https://reviews.apache.org/r/53204/#comment227440> Please add javadoc comments as to what this test is supposed to test and how its being accomplished. service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line 282) <https://reviews.apache.org/r/53204/#comment227441> add javadoc comments for the test service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line 299) <https://reviews.apache.org/r/53204/#comment227442> Add javadoc comments for the test service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 32) <https://reviews.apache.org/r/53204/#comment227445> can the usage of hamcrest APIs be replaced by JDK regex ? service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 108) <https://reviews.apache.org/r/53204/#comment227446> Add javadocs service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 119) <https://reviews.apache.org/r/53204/#comment227447> Add javadoc comments for the test service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 134) <https://reviews.apache.org/r/53204/#comment227449> Add javadoc comments for the test service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 149) <https://reviews.apache.org/r/53204/#comment227450> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 212) <https://reviews.apache.org/r/53204/#comment227452> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 225) <https://reviews.apache.org/r/53204/#comment227453> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 235) <https://reviews.apache.org/r/53204/#comment227454> Add javadoc comments for the test service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 246) <https://reviews.apache.org/r/53204/#comment227455> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 264) <https://reviews.apache.org/r/53204/#comment227456> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 279) <https://reviews.apache.org/r/53204/#comment227457> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 81) <https://reviews.apache.org/r/53204/#comment227458> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 90) <https://reviews.apache.org/r/53204/#comment227459> Add javadoc comments for the test. service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 96) <https://reviews.apache.org/r/53204/#comment227460> Add javadoc comments for the test. - Naveen Gangam On Oct. 28, 2016, 12:59 p.m., Illya Yalovyy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53204/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2016, 12:59 p.m.) > > > Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon > Ho. > > > Repository: hive-git > > > Description > ------- > > HIVE-15076 Improve scalability of LDAP authentication provider group filter > > https://issues.apache.org/jira/browse/HIVE-15076 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 > service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java > 152c4b2 > service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea > service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 > service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java > e9172d3 > > service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java > cd62935 > > service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java > 4fad755 > > service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java > acde8c1 > service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java > 0cc2ead > service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java > 499b624 > service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java > 3054e33 > service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION > service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION > > Diff: https://reviews.apache.org/r/53204/diff/ > > > Testing > ------- > > Build succeeded. > > Test results: > > Tests run: 149, Failures: 0, Errors: 0, Skipped: 0 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 03:14 min > [INFO] Finished at: 2016-10-26T13:53:15-07:00 > [INFO] Final Memory: 36M/1091M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Illya Yalovyy > >
