Review Request 19801: review request for HIVE-6738, HiveServer2 secure Thrift/HTTP needs to accept doAs parameter from proxying intermediary
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/ --- Review request for hive, Thejas Nair and Vaibhav Gumashta. Bugs: HIVE-6738 https://issues.apache.org/jira/browse/HIVE-6738 Repository: hive-git Description --- see the jira HIVE-6738 Diffs - service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 58f3e3b service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java c579db5 Diff: https://reviews.apache.org/r/19801/diff/ Testing --- see the attachment to Jira HIVE-6738 https://issues.apache.org/jira/secure/attachment/12637059/hive-6738-req-impl-verify.md Thanks, dilli dorai
Re: Review Request 19801: review request for HIVE-6738, HiveServer2 secure Thrift/HTTP needs to accept doAs parameter from proxying intermediary
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/#review38968 --- service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java https://reviews.apache.org/r/19801/#comment71348 Can we use proxyUserNameParam for doAsQueryParam? doAs in hive world is different than proxy user like we discussed offline. service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java https://reviews.apache.org/r/19801/#comment71349 Similar to the previous comment: I think it'll be good to change the method name to getProxyUserNameParam from getDoAsQueryParam. - Vaibhav Gumashta On March 28, 2014, 9:08 p.m., dilli dorai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/ --- (Updated March 28, 2014, 9:08 p.m.) Review request for hive, Thejas Nair and Vaibhav Gumashta. Bugs: HIVE-6738 https://issues.apache.org/jira/browse/HIVE-6738 Repository: hive-git Description --- see the jira HIVE-6738 Diffs - service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 58f3e3b service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java c579db5 Diff: https://reviews.apache.org/r/19801/diff/ Testing --- see the attachment to Jira HIVE-6738 https://issues.apache.org/jira/secure/attachment/12637059/hive-6738-req-impl-verify.md Thanks, dilli dorai
Re: Review Request 19801: review request for HIVE-6738, HiveServer2 secure Thrift/HTTP needs to accept doAs parameter from proxying intermediary
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/#review38974 --- service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java https://reviews.apache.org/r/19801/#comment71355 I think we should set proxyUser from the threadlocal only when HS2 is running in http mode - this will avoid any future confusion while development. This is the block you can use: if (hiveConf.getVar(HiveConf.ConfVars.HIVE_SERVER2_TRANSPORT_MODE).equalsIgnoreCase(http)) { } - Vaibhav Gumashta On March 28, 2014, 9:08 p.m., dilli dorai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/ --- (Updated March 28, 2014, 9:08 p.m.) Review request for hive, Thejas Nair and Vaibhav Gumashta. Bugs: HIVE-6738 https://issues.apache.org/jira/browse/HIVE-6738 Repository: hive-git Description --- see the jira HIVE-6738 Diffs - service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 58f3e3b service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java c579db5 Diff: https://reviews.apache.org/r/19801/diff/ Testing --- see the attachment to Jira HIVE-6738 https://issues.apache.org/jira/secure/attachment/12637059/hive-6738-req-impl-verify.md Thanks, dilli dorai
Re: Review Request 19801: review request for HIVE-6738, HiveServer2 secure Thrift/HTTP needs to accept doAs parameter from proxying intermediary
On March 28, 2014, 11:43 p.m., Vaibhav Gumashta wrote: service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java, line 82 https://reviews.apache.org/r/19801/diff/1/?file=539920#file539920line82 Can we use proxyUserNameParam for doAsQueryParam? doAs in hive world is different than proxy user like we discussed offline. First, thanks for the review. I think the query parameter name has to be doAs, case does not matter. That is the interface used by other HTTP services of Hadoop echo system. On March 28, 2014, 11:43 p.m., Vaibhav Gumashta wrote: service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java, line 309 https://reviews.apache.org/r/19801/diff/1/?file=539920#file539920line309 Similar to the previous comment: I think it'll be good to change the method name to getProxyUserNameParam from getDoAsQueryParam. See the comment above. - dilli --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/#review38968 --- On March 28, 2014, 9:08 p.m., dilli dorai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/ --- (Updated March 28, 2014, 9:08 p.m.) Review request for hive, Thejas Nair and Vaibhav Gumashta. Bugs: HIVE-6738 https://issues.apache.org/jira/browse/HIVE-6738 Repository: hive-git Description --- see the jira HIVE-6738 Diffs - service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 58f3e3b service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java c579db5 Diff: https://reviews.apache.org/r/19801/diff/ Testing --- see the attachment to Jira HIVE-6738 https://issues.apache.org/jira/secure/attachment/12637059/hive-6738-req-impl-verify.md Thanks, dilli dorai
Re: Review Request 19801: review request for HIVE-6738, HiveServer2 secure Thrift/HTTP needs to accept doAs parameter from proxying intermediary
On March 29, 2014, 12:08 a.m., Vaibhav Gumashta wrote: service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java, line 543 https://reviews.apache.org/r/19801/diff/1/?file=539919#file539919line543 I think we should set proxyUser from the threadlocal only when HS2 is running in http mode - this will avoid any future confusion while development. This is the block you can use: if (hiveConf.getVar(HiveConf.ConfVars.HIVE_SERVER2_TRANSPORT_MODE).equalsIgnoreCase(http)) { } The value is set at the higher layer only if the request was over http and we got a query parameter. So, I think this check would be superfluous. If Hive code later adds a different code path to set the value without http mode, that should be an intentional design choice at that time. - dilli --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/#review38974 --- On March 28, 2014, 9:08 p.m., dilli dorai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19801/ --- (Updated March 28, 2014, 9:08 p.m.) Review request for hive, Thejas Nair and Vaibhav Gumashta. Bugs: HIVE-6738 https://issues.apache.org/jira/browse/HIVE-6738 Repository: hive-git Description --- see the jira HIVE-6738 Diffs - service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 58f3e3b service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java c579db5 Diff: https://reviews.apache.org/r/19801/diff/ Testing --- see the attachment to Jira HIVE-6738 https://issues.apache.org/jira/secure/attachment/12637059/hive-6738-req-impl-verify.md Thanks, dilli dorai