You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Mohit Sabharwal <mo...@cloudera.com> on 2014/11/19 21:49:38 UTC

Review Request 28255: HIVE-8916 : Handle user@domain username under LDAP authentication

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

Review request for hive.


Bugs: HIVE-8916
    https://issues.apache.org/jira/browse/HIVE-8916


Repository: hive-git


Description
-------

HIVE-8916 : Handle user@domain username under LDAP authentication

If LDAP is configured with multiple domains for authentication, users can be in different domains.

Currently, LdapAuthenticationProviderImpl blindly appends the domain configured "hive.server2.authentication.ldap.Domain" to the username, which limits user to that domain. However, under multi-domain authentication, the username may already include the domain (ex: user@domain.foo.com). We should not append a domain if one is already present.

Also, if username already includes the domain, rest of Hive and authorization providers still expects the "short name" ("user" and not "user@domain.foo.com") for looking up privilege rules, etc. As such, any domain info in the username should be stripped off.


Diffs
-----

  service/src/java/org/apache/hive/service/ServiceUtils.java PRE-CREATION 
  service/src/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java d075761d079f8a18d7d317483783fe3b801e00d5 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 3a8ae70d8bd31c9958ea6ae00a2d01c315c80615 

Diff: https://reviews.apache.org/r/28255/diff/


Testing
-------

Configured HS2 for LDAP authentication:

<property>
  <name>hive.server2.authentication</name>
  <value>LDAP</value>
</property>
<property>    
  <name>hive.server2.authentication.ldap.url</name>
  <value>ldap://foo.ldap.server.com</value>
</property>
<property>
  <name>hive.server2.authentication.ldap.Domain</name>
  <value>foo.ldap.domain.com</value>
</property>

Ran beeline with user names with and without ldap domain, in both cases
authentication works. Before the change, authentication failed if
domain was present in username:

beeline -u jdbc:hive2://localhost:10000 -n user@foo.ldap.domain.com -p TestPassword --debug

beeline -u jdbc:hive2://localhost:10000 -n user -p TestPassword --debug


Thanks,

Mohit Sabharwal


Re: Review Request 28255: HIVE-8916 : Handle user@domain username under LDAP authentication

Posted by Mohit Sabharwal <mo...@cloudera.com>.

> On Nov. 20, 2014, 6:43 p.m., Prasad Mujumdar wrote:
> > Looks fine to me.
> > 
> > I would suggest creating a followup ticket to move the username format detection and replacement menthods to PasswdAuthenticationProvider and PlainServerCallbackHandler. This way the user name format can be managed by individual authentication handlers rather than high level HS2 RPC processor.

Thanks!, filed follow up JIRA HIVE-8928 to explore Prasad's and Szehon's suggestions.


- Mohit


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


On Nov. 19, 2014, 8:49 p.m., Mohit Sabharwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28255/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 8:49 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-8916
>     https://issues.apache.org/jira/browse/HIVE-8916
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-8916 : Handle user@domain username under LDAP authentication
> 
> If LDAP is configured with multiple domains for authentication, users can be in different domains.
> 
> Currently, LdapAuthenticationProviderImpl blindly appends the domain configured "hive.server2.authentication.ldap.Domain" to the username, which limits user to that domain. However, under multi-domain authentication, the username may already include the domain (ex: user@domain.foo.com). We should not append a domain if one is already present.
> 
> Also, if username already includes the domain, rest of Hive and authorization providers still expects the "short name" ("user" and not "user@domain.foo.com") for looking up privilege rules, etc. As such, any domain info in the username should be stripped off.
> 
> 
> Diffs
> -----
> 
>   service/src/java/org/apache/hive/service/ServiceUtils.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java d075761d079f8a18d7d317483783fe3b801e00d5 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 3a8ae70d8bd31c9958ea6ae00a2d01c315c80615 
> 
> Diff: https://reviews.apache.org/r/28255/diff/
> 
> 
> Testing
> -------
> 
> Configured HS2 for LDAP authentication:
> 
> <property>
>   <name>hive.server2.authentication</name>
>   <value>LDAP</value>
> </property>
> <property>    
>   <name>hive.server2.authentication.ldap.url</name>
>   <value>ldap://foo.ldap.server.com</value>
> </property>
> <property>
>   <name>hive.server2.authentication.ldap.Domain</name>
>   <value>foo.ldap.domain.com</value>
> </property>
> 
> Ran beeline with user names with and without ldap domain, in both cases
> authentication works. Before the change, authentication failed if
> domain was present in username:
> 
> beeline -u jdbc:hive2://localhost:10000 -n user@foo.ldap.domain.com -p TestPassword --debug
> 
> beeline -u jdbc:hive2://localhost:10000 -n user -p TestPassword --debug
> 
> 
> Thanks,
> 
> Mohit Sabharwal
> 
>


Re: Review Request 28255: HIVE-8916 : Handle user@domain username under LDAP authentication

Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28255/#review62378
-----------------------------------------------------------


Looks fine to me.

I would suggest creating a followup ticket to move the username format detection and replacement menthods to PasswdAuthenticationProvider and PlainServerCallbackHandler. This way the user name format can be managed by individual authentication handlers rather than high level HS2 RPC processor.

- Prasad Mujumdar


On Nov. 19, 2014, 8:49 p.m., Mohit Sabharwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28255/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 8:49 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-8916
>     https://issues.apache.org/jira/browse/HIVE-8916
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-8916 : Handle user@domain username under LDAP authentication
> 
> If LDAP is configured with multiple domains for authentication, users can be in different domains.
> 
> Currently, LdapAuthenticationProviderImpl blindly appends the domain configured "hive.server2.authentication.ldap.Domain" to the username, which limits user to that domain. However, under multi-domain authentication, the username may already include the domain (ex: user@domain.foo.com). We should not append a domain if one is already present.
> 
> Also, if username already includes the domain, rest of Hive and authorization providers still expects the "short name" ("user" and not "user@domain.foo.com") for looking up privilege rules, etc. As such, any domain info in the username should be stripped off.
> 
> 
> Diffs
> -----
> 
>   service/src/java/org/apache/hive/service/ServiceUtils.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java d075761d079f8a18d7d317483783fe3b801e00d5 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 3a8ae70d8bd31c9958ea6ae00a2d01c315c80615 
> 
> Diff: https://reviews.apache.org/r/28255/diff/
> 
> 
> Testing
> -------
> 
> Configured HS2 for LDAP authentication:
> 
> <property>
>   <name>hive.server2.authentication</name>
>   <value>LDAP</value>
> </property>
> <property>    
>   <name>hive.server2.authentication.ldap.url</name>
>   <value>ldap://foo.ldap.server.com</value>
> </property>
> <property>
>   <name>hive.server2.authentication.ldap.Domain</name>
>   <value>foo.ldap.domain.com</value>
> </property>
> 
> Ran beeline with user names with and without ldap domain, in both cases
> authentication works. Before the change, authentication failed if
> domain was present in username:
> 
> beeline -u jdbc:hive2://localhost:10000 -n user@foo.ldap.domain.com -p TestPassword --debug
> 
> beeline -u jdbc:hive2://localhost:10000 -n user -p TestPassword --debug
> 
> 
> Thanks,
> 
> Mohit Sabharwal
> 
>


Re: Review Request 28255: HIVE-8916 : Handle user@domain username under LDAP authentication

Posted by Brock Noland <br...@cloudera.com>.

> On Nov. 19, 2014, 9:32 p.m., Szehon Ho wrote:
> > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java, line 307
> > <https://reviews.apache.org/r/28255/diff/1/?file=770245#file770245line307>
> >
> >     Will be it simpler to use a regex like [^\@]+ to find this?

>From my side I feel the approaches are equivalent.


- Brock


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


On Nov. 19, 2014, 8:49 p.m., Mohit Sabharwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28255/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 8:49 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-8916
>     https://issues.apache.org/jira/browse/HIVE-8916
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-8916 : Handle user@domain username under LDAP authentication
> 
> If LDAP is configured with multiple domains for authentication, users can be in different domains.
> 
> Currently, LdapAuthenticationProviderImpl blindly appends the domain configured "hive.server2.authentication.ldap.Domain" to the username, which limits user to that domain. However, under multi-domain authentication, the username may already include the domain (ex: user@domain.foo.com). We should not append a domain if one is already present.
> 
> Also, if username already includes the domain, rest of Hive and authorization providers still expects the "short name" ("user" and not "user@domain.foo.com") for looking up privilege rules, etc. As such, any domain info in the username should be stripped off.
> 
> 
> Diffs
> -----
> 
>   service/src/java/org/apache/hive/service/ServiceUtils.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java d075761d079f8a18d7d317483783fe3b801e00d5 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 3a8ae70d8bd31c9958ea6ae00a2d01c315c80615 
> 
> Diff: https://reviews.apache.org/r/28255/diff/
> 
> 
> Testing
> -------
> 
> Configured HS2 for LDAP authentication:
> 
> <property>
>   <name>hive.server2.authentication</name>
>   <value>LDAP</value>
> </property>
> <property>    
>   <name>hive.server2.authentication.ldap.url</name>
>   <value>ldap://foo.ldap.server.com</value>
> </property>
> <property>
>   <name>hive.server2.authentication.ldap.Domain</name>
>   <value>foo.ldap.domain.com</value>
> </property>
> 
> Ran beeline with user names with and without ldap domain, in both cases
> authentication works. Before the change, authentication failed if
> domain was present in username:
> 
> beeline -u jdbc:hive2://localhost:10000 -n user@foo.ldap.domain.com -p TestPassword --debug
> 
> beeline -u jdbc:hive2://localhost:10000 -n user -p TestPassword --debug
> 
> 
> Thanks,
> 
> Mohit Sabharwal
> 
>


Re: Review Request 28255: HIVE-8916 : Handle user@domain username under LDAP authentication

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28255/#review62235
-----------------------------------------------------------


Hi Mohit, looks great, just one suggestion on rb for your consideration.


service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
<https://reviews.apache.org/r/28255/#comment104245>

    Will be it simpler to use a regex like [^\@]+ to find this?


- Szehon Ho


On Nov. 19, 2014, 8:49 p.m., Mohit Sabharwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28255/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 8:49 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-8916
>     https://issues.apache.org/jira/browse/HIVE-8916
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-8916 : Handle user@domain username under LDAP authentication
> 
> If LDAP is configured with multiple domains for authentication, users can be in different domains.
> 
> Currently, LdapAuthenticationProviderImpl blindly appends the domain configured "hive.server2.authentication.ldap.Domain" to the username, which limits user to that domain. However, under multi-domain authentication, the username may already include the domain (ex: user@domain.foo.com). We should not append a domain if one is already present.
> 
> Also, if username already includes the domain, rest of Hive and authorization providers still expects the "short name" ("user" and not "user@domain.foo.com") for looking up privilege rules, etc. As such, any domain info in the username should be stripped off.
> 
> 
> Diffs
> -----
> 
>   service/src/java/org/apache/hive/service/ServiceUtils.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java d075761d079f8a18d7d317483783fe3b801e00d5 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 3a8ae70d8bd31c9958ea6ae00a2d01c315c80615 
> 
> Diff: https://reviews.apache.org/r/28255/diff/
> 
> 
> Testing
> -------
> 
> Configured HS2 for LDAP authentication:
> 
> <property>
>   <name>hive.server2.authentication</name>
>   <value>LDAP</value>
> </property>
> <property>    
>   <name>hive.server2.authentication.ldap.url</name>
>   <value>ldap://foo.ldap.server.com</value>
> </property>
> <property>
>   <name>hive.server2.authentication.ldap.Domain</name>
>   <value>foo.ldap.domain.com</value>
> </property>
> 
> Ran beeline with user names with and without ldap domain, in both cases
> authentication works. Before the change, authentication failed if
> domain was present in username:
> 
> beeline -u jdbc:hive2://localhost:10000 -n user@foo.ldap.domain.com -p TestPassword --debug
> 
> beeline -u jdbc:hive2://localhost:10000 -n user -p TestPassword --debug
> 
> 
> Thanks,
> 
> Mohit Sabharwal
> 
>