Re: Review Request 66549: Lens Server: SPNEGO authentication

2018-05-04 Thread Barun Kumar 


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
> > Line 1516 (original)
> > 
> >
> > revert this

This is a duplicate method.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java
> > Line 164 (original), 169 (patched)
> > 
> >
> > the ! condition here is intentional?

yes.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/auth/LensSecurityContext.java
> > Lines 63 (patched)
> > 
> >
> > The purpose of this class isn't clear. All the methods return null

fixed.


- Barun


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


On May 4, 2018, 12:48 p.m., Barun Kumar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66549/
> ---
> 
> (Updated May 4, 2018, 12:48 p.m.)
> 
> 
> Review request for lens, Ankit Kailaswar, Puneet Gupta, and Rajitha R.
> 
> 
> Bugs: LENS-1509
> https://issues.apache.org/jira/browse/LENS-1509
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Currently authentication in lens works by taking username/password while 
> opening a session and validating it.
> 
> This change will add support of SPNEGO support in lens server and clinet so 
> that it can negotiate the authentication scheme. Currently only 
> Negotiate(Kerberos) scheme is implemented but others like Basic/Digest etc 
> can be added as well later.
> 
> 
> Diffs
> -
> 
>   contrib/clients/python/lens/client/auth.py PRE-CREATION 
>   contrib/clients/python/lens/client/session.py a1ccc4ba3 
>   contrib/clients/python/setup.py de59d32a9 
>   lens-api/src/main/java/org/apache/lens/api/auth/AuthScheme.java 
> PRE-CREATION 
>   lens-client/src/main/java/org/apache/lens/client/LensClientConfig.java 
> b703e132d 
>   lens-client/src/main/java/org/apache/lens/client/LensConnectionParams.java 
> 3a5dcdb82 
>   lens-client/src/main/java/org/apache/lens/client/SpnegoClientFilter.java 
> PRE-CREATION 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java 
> e83eacb11 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  f14ae44af 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 
> b5248f3d5 
>   lens-server/src/main/java/org/apache/lens/server/LensApplication.java 
> c3f99529a 
>   lens-server/src/main/java/org/apache/lens/server/auth/Authenticate.java 
> PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/auth/LensSecurityContext.java
>  PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
>  PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/error/NotAuthorizedExceptionMapper.java
>  PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> 08a5cff4f 
>   
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 
> 63eea6313 
> 
> 
> Diff: https://reviews.apache.org/r/66549/diff/2/
> 
> 
> Testing
> ---
> 
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules . SUCCESS [1.458s]
> [INFO] Lens .. SUCCESS [3.567s]
> [INFO] Lens API .. SUCCESS [19.185s]
> [INFO] Lens API for server and extensions  SUCCESS [17.395s]
> [INFO] Lens Cube . SUCCESS [3:00.221s]
> [INFO] Lens DB storage ... SUCCESS [13.920s]
> [INFO] Lens Query Library  SUCCESS [11.039s]
> [INFO] Lens Hive Driver .. SUCCESS [1:06.122s]
> [INFO] Lens Driver for JDBC .. SUCCESS [42.723s]
> [INFO] Lens Elastic Search Driver  SUCCESS [17.640s]
> [INFO] Lens Server ... SUCCESS 
> [10:18.231s]
> [INFO] Lens client ... SUCCESS [1:27.686s]
> [INFO] Lens CLI .. SUCCESS [1:32.599s]
> [INFO] Lens Examples . SUCCESS [8.707s]
> [INFO] Lens Ship Jars to Distributed Cache ... SUCCESS [0.691s]
> [INFO] Lens Distribution . SUCCESS [8.002s]
> [INFO]

Re: Review Request 66549: Lens Server: SPNEGO authentication

2018-05-04 Thread Barun Kumar 

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

(Updated May 4, 2018, 12:48 p.m.)


Review request for lens, Ankit Kailaswar, Puneet Gupta, and Rajitha R.


Bugs: LENS-1509
https://issues.apache.org/jira/browse/LENS-1509


Repository: lens


Description
---

Currently authentication in lens works by taking username/password while 
opening a session and validating it.

This change will add support of SPNEGO support in lens server and clinet so 
that it can negotiate the authentication scheme. Currently only 
Negotiate(Kerberos) scheme is implemented but others like Basic/Digest etc can 
be added as well later.


Diffs (updated)
-

  contrib/clients/python/lens/client/auth.py PRE-CREATION 
  contrib/clients/python/lens/client/session.py a1ccc4ba3 
  contrib/clients/python/setup.py de59d32a9 
  lens-api/src/main/java/org/apache/lens/api/auth/AuthScheme.java PRE-CREATION 
  lens-client/src/main/java/org/apache/lens/client/LensClientConfig.java 
b703e132d 
  lens-client/src/main/java/org/apache/lens/client/LensConnectionParams.java 
3a5dcdb82 
  lens-client/src/main/java/org/apache/lens/client/SpnegoClientFilter.java 
PRE-CREATION 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java 
e83eacb11 
  
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
f14ae44af 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 
b5248f3d5 
  lens-server/src/main/java/org/apache/lens/server/LensApplication.java 
c3f99529a 
  lens-server/src/main/java/org/apache/lens/server/auth/Authenticate.java 
PRE-CREATION 
  
lens-server/src/main/java/org/apache/lens/server/auth/LensSecurityContext.java 
PRE-CREATION 
  
lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
 PRE-CREATION 
  
lens-server/src/main/java/org/apache/lens/server/error/NotAuthorizedExceptionMapper.java
 PRE-CREATION 
  lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
08a5cff4f 
  lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 
63eea6313 


Diff: https://reviews.apache.org/r/66549/diff/2/

Changes: https://reviews.apache.org/r/66549/diff/1-2/


Testing
---

[INFO] Reactor Summary:
[INFO] 
[INFO] Lens Checkstyle Rules . SUCCESS [1.458s]
[INFO] Lens .. SUCCESS [3.567s]
[INFO] Lens API .. SUCCESS [19.185s]
[INFO] Lens API for server and extensions  SUCCESS [17.395s]
[INFO] Lens Cube . SUCCESS [3:00.221s]
[INFO] Lens DB storage ... SUCCESS [13.920s]
[INFO] Lens Query Library  SUCCESS [11.039s]
[INFO] Lens Hive Driver .. SUCCESS [1:06.122s]
[INFO] Lens Driver for JDBC .. SUCCESS [42.723s]
[INFO] Lens Elastic Search Driver  SUCCESS [17.640s]
[INFO] Lens Server ... SUCCESS [10:18.231s]
[INFO] Lens client ... SUCCESS [1:27.686s]
[INFO] Lens CLI .. SUCCESS [1:32.599s]
[INFO] Lens Examples . SUCCESS [8.707s]
[INFO] Lens Ship Jars to Distributed Cache ... SUCCESS [0.691s]
[INFO] Lens Distribution . SUCCESS [8.002s]
[INFO] Lens ML Lib ... SUCCESS [1:06.984s]
[INFO] Lens ML Ext Distribution .. SUCCESS [2.084s]
[INFO] Lens Regression ... SUCCESS [12.962s]
[INFO] Lens UI ... SUCCESS [35.012s]
[INFO] Lens Contrib .. SUCCESS [0.263s]
[INFO] Lens Contributed Clients .. SUCCESS [0.266s]
[INFO] Lens Python Client  SUCCESS [0.247s]
[INFO] 


Thanks,

Barun Kumar

Re: Review Request 66549: Lens Server: SPNEGO authentication

2018-05-04 Thread Barun Kumar 


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
> > Lines 64 (patched)
> > 
> >
> > What exactly is this id? Can this be made configurable than a hardcoded 
> > value ?

This is not a configuration param so hardcoded it.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
> > Lines 65 (patched)
> > 
> >
> > Please move all constant fields to lensconstants class

This constant is private to this class.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
> > Lines 73 (patched)
> > 
> >
> > Does Spnego only support NEGOTIATE scheme? Can we have a default scheme 
> > implemented as well?

There is no default scheme. Others are Plain/Digest etc. We are only doing 
Negotiate as of now.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
> > Lines 106 (patched)
> > 
> >
> > shouldn't we check if all the parameters(uriinfo, resourceinfo etc) are 
> > non null before fetching any data in this method?

This is injected by JAX-RS container so this is not nullable.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
> > Lines 257 (patched)
> > 
> >
> > Can we create a new error code and throw Lens exception for all 
> > authentication failures?

We are using standard JAX-RS exception here for 401 status code. I think this 
is better.


> On April 12, 2018, 8:22 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java
> > Lines 128 (patched)
> > 
> >
> > is this flow valid for all auth schemes?

Yes. This is valid for all schemes. But againg only Negotiate is implemented as 
of now.


- Barun


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


On May 4, 2018, 11:04 a.m., Barun Kumar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66549/
> ---
> 
> (Updated May 4, 2018, 11:04 a.m.)
> 
> 
> Review request for lens, Ankit Kailaswar, Puneet Gupta, and Rajitha R.
> 
> 
> Bugs: LENS-1509
> https://issues.apache.org/jira/browse/LENS-1509
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Currently authentication in lens works by taking username/password while 
> opening a session and validating it.
> 
> This change will add support of SPNEGO support in lens server and clinet so 
> that it can negotiate the authentication scheme. Currently only 
> Negotiate(Kerberos) scheme is implemented but others like Basic/Digest etc 
> can be added as well later.
> 
> 
> Diffs
> -
> 
>   contrib/clients/python/lens/client/auth.py PRE-CREATION 
>   contrib/clients/python/lens/client/session.py a1ccc4b 
>   contrib/clients/python/setup.py de59d32 
>   lens-api/src/main/java/org/apache/lens/api/auth/AuthScheme.java 
> PRE-CREATION 
>   lens-client/src/main/java/org/apache/lens/client/LensClientConfig.java 
> b703e13 
>   lens-client/src/main/java/org/apache/lens/client/SpnegoClientFilter.java 
> PRE-CREATION 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java 
> e83eacb 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  0e05d28 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 
> b5248f3 
>   lens-server/src/main/java/org/apache/lens/server/LensApplication.java 
> c3f9952 
>   lens-server/src/main/java/org/apache/lens/server/auth/Authenticate.java 
> PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/auth/LensSecurityContext.java
>  PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/auth/SpnegoAuthenticationFilter.java
>  PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/error/NotAuthorizedExceptionMapper.java
>  PRE-CREATION 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> 08a5cff 
>   
> lens-server/src/main/java/org/apache/lens/server