You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Naveen Gangam <ng...@cloudera.com> on 2016/11/28 17:09:10 UTC
Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP
authentication provider group filter
-----------------------------------------------------------
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
>
>