You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Abraham Elmahrek <ab...@cloudera.com> on 2014/11/22 02:20:03 UTC
Review Request 28359: SQOOP-1775: Sqoop2: Use username during simple
auth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-1775
https://issues.apache.org/jira/browse/SQOOP-1775
Repository: sqoop-sqoop2
Description
-------
commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Fri Nov 21 17:13:55 2014 -0800
SQOOP-1775: Use username during simple auth
:100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
:100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
:100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
Diffs
-----
client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
Diff: https://reviews.apache.org/r/28359/diff/
Testing
-------
Manual
Thanks,
Abraham Elmahrek
Re: Review Request 28359: SQOOP-1775: Sqoop2: Use username during
simple auth
Posted by Jarek Cecho <ja...@apache.org>.
> On Nov. 22, 2014, 1:22 a.m., Jarek Cecho wrote:
> > server/src/main/java/org/apache/sqoop/server/RequestContext.java, line 129
> > <https://reviews.apache.org/r/28359/diff/1/?file=772925#file772925line129>
> >
> > Considering that this code is executed on server side and not user side, it means that we return here the server user as a default case when no user name is given by the request. That seems quite weird to me.
> >
> > The original method allowed returning null (if the header was unspecified), so I'm wondering if we should preserve the same "default" behaviour? Or perhaps define a special "default user" (that will be different then the user running sqoop2 server).
>
> Abraham Elmahrek wrote:
> Good point Jarcec. Either way works for me. Let's just go with null until we better understand how user names will be managed?
+1
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/#review62708
-----------------------------------------------------------
On Nov. 22, 2014, 1:20 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28359/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2014, 1:20 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1775
> https://issues.apache.org/jira/browse/SQOOP-1775
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Fri Nov 21 17:13:55 2014 -0800
>
> SQOOP-1775: Use username during simple auth
>
> :100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
> :100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
> :100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
> common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
> server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
>
> Diff: https://reviews.apache.org/r/28359/diff/
>
>
> Testing
> -------
>
> Manual
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 28359: SQOOP-1775: Sqoop2: Use username during
simple auth
Posted by richard zhou <yi...@gmail.com>.
> On Nov. 22, 2014, 1:22 a.m., Jarek Cecho wrote:
> > server/src/main/java/org/apache/sqoop/server/RequestContext.java, line 129
> > <https://reviews.apache.org/r/28359/diff/1/?file=772925#file772925line129>
> >
> > Considering that this code is executed on server side and not user side, it means that we return here the server user as a default case when no user name is given by the request. That seems quite weird to me.
> >
> > The original method allowed returning null (if the header was unspecified), so I'm wondering if we should preserve the same "default" behaviour? Or perhaps define a special "default user" (that will be different then the user running sqoop2 server).
>
> Abraham Elmahrek wrote:
> Good point Jarcec. Either way works for me. Let's just go with null until we better understand how user names will be managed?
>
> Jarek Cecho wrote:
> +1
Neither System.getProperty("user.name"), nor request.getParameter(PseudoAuthenticator.USER_NAME) is a good way to obtain user name.
The user name should be from AuthenticationHandler, after the verification of the token sent from the http request.
After SQOOP-1776 (Delegation Token support for Authentication) finished, it could be get from ugi.
For the time being, since it has blocked the integration test, please revert this patch or change to hardcode (return "sqoop";) in function getUserName.
- richard
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/#review62708
-----------------------------------------------------------
On Nov. 22, 2014, 1:28 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28359/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2014, 1:28 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1775
> https://issues.apache.org/jira/browse/SQOOP-1775
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Fri Nov 21 17:13:55 2014 -0800
>
> SQOOP-1775: Use username during simple auth
>
> :100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
> :100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
> :100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
> common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
> server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
>
> Diff: https://reviews.apache.org/r/28359/diff/
>
>
> Testing
> -------
>
> Manual
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 28359: SQOOP-1775: Sqoop2: Use username during
simple auth
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Nov. 22, 2014, 1:22 a.m., Jarek Cecho wrote:
> > server/src/main/java/org/apache/sqoop/server/RequestContext.java, line 129
> > <https://reviews.apache.org/r/28359/diff/1/?file=772925#file772925line129>
> >
> > Considering that this code is executed on server side and not user side, it means that we return here the server user as a default case when no user name is given by the request. That seems quite weird to me.
> >
> > The original method allowed returning null (if the header was unspecified), so I'm wondering if we should preserve the same "default" behaviour? Or perhaps define a special "default user" (that will be different then the user running sqoop2 server).
Good point Jarcec. Either way works for me. Let's just go with null until we better understand how user names will be managed?
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/#review62708
-----------------------------------------------------------
On Nov. 22, 2014, 1:20 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28359/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2014, 1:20 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1775
> https://issues.apache.org/jira/browse/SQOOP-1775
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Fri Nov 21 17:13:55 2014 -0800
>
> SQOOP-1775: Use username during simple auth
>
> :100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
> :100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
> :100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
> common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
> server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
>
> Diff: https://reviews.apache.org/r/28359/diff/
>
>
> Testing
> -------
>
> Manual
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 28359: SQOOP-1775: Sqoop2: Use username during
simple auth
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/#review62708
-----------------------------------------------------------
server/src/main/java/org/apache/sqoop/server/RequestContext.java
<https://reviews.apache.org/r/28359/#comment104784>
Considering that this code is executed on server side and not user side, it means that we return here the server user as a default case when no user name is given by the request. That seems quite weird to me.
The original method allowed returning null (if the header was unspecified), so I'm wondering if we should preserve the same "default" behaviour? Or perhaps define a special "default user" (that will be different then the user running sqoop2 server).
- Jarek Cecho
On Nov. 22, 2014, 1:20 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28359/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2014, 1:20 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1775
> https://issues.apache.org/jira/browse/SQOOP-1775
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Fri Nov 21 17:13:55 2014 -0800
>
> SQOOP-1775: Use username during simple auth
>
> :100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
> :100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
> :100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
> common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
> server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
>
> Diff: https://reviews.apache.org/r/28359/diff/
>
>
> Testing
> -------
>
> Manual
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 28359: SQOOP-1775: Sqoop2: Use username during
simple auth
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/#review62714
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Nov. 22, 2014, 1:28 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28359/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2014, 1:28 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1775
> https://issues.apache.org/jira/browse/SQOOP-1775
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Fri Nov 21 17:13:55 2014 -0800
>
> SQOOP-1775: Use username during simple auth
>
> :100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
> :100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
> :100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
> common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
> server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
>
> Diff: https://reviews.apache.org/r/28359/diff/
>
>
> Testing
> -------
>
> Manual
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 28359: SQOOP-1775: Sqoop2: Use username during
simple auth
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28359/
-----------------------------------------------------------
(Updated Nov. 22, 2014, 1:28 a.m.)
Review request for Sqoop.
Bugs: SQOOP-1775
https://issues.apache.org/jira/browse/SQOOP-1775
Repository: sqoop-sqoop2
Description
-------
commit 40ae562f30fc3ef8969e5d45a243ed0baadf83d8
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Fri Nov 21 17:13:55 2014 -0800
SQOOP-1775: Use username during simple auth
:100644 100644 db16107... 3c4a4d2... M client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java
:100644 100644 d08c855... 8f4f709... M common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java
:100644 100644 54e9cca... 9ecb2bc... M server/src/main/java/org/apache/sqoop/server/RequestContext.java
Diffs (updated)
-----
client/src/main/java/org/apache/sqoop/client/request/ResourceRequest.java db16107
common/src/main/java/org/apache/sqoop/common/SqoopProtocolConstants.java d08c855
server/src/main/java/org/apache/sqoop/server/RequestContext.java 54e9cca
Diff: https://reviews.apache.org/r/28359/diff/
Testing
-------
Manual
Thanks,
Abraham Elmahrek