> On Sept. 8, 2015, 7:35 a.m., Ajay Yadava wrote:
> >

Thanks for a thorough review (as usual) Ajay.  I have answered couple of items 
here.   I think rest of the items, Sowmya will address and resolve.


> On Sept. 8, 2015, 7:35 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/service/ProxyUserService.java, line 
> > 81
> > <https://reviews.apache.org/r/37771/diff/3/?file=1056558#file1056558line81>
> >
> >     Will it make sense to move it to RuntimeProperties? Adding new proxy 
> > user won't be a very frequent operation I suppose, but adding groups might 
> > be.

That is a good question.  I wanted it to be in Startup Properties as this is a 
configuration that an administrator has to make (like one does in core-site 
with the explicit idea that the proxy user is fully trusted and the list of 
hosts/groups are configured at the time.   If there are any changes in these, 
then we probably should configure LDap integration or some other form of 
authentication option.


> On Sept. 8, 2015, 7:35 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/security/CurrentUser.java, line 84
> > <https://reviews.apache.org/r/37771/diff/3/?file=1056555#file1056555line84>
> >
> >     unix user names are case sensitive, should we also  keep the comparison 
> > as case sensitive?

Since we also work on Windows (on prem and azure) we need to allow 
case-insensitive names.


> On Sept. 8, 2015, 7:35 a.m., Ajay Yadava wrote:
> > prism/src/main/java/org/apache/falcon/security/HostnameFilter.java, line 70
> > <https://reviews.apache.org/r/37771/diff/3/?file=1056572#file1056572line70>
> >
> >     Is it different from just doing request.getRemoteHost()?

Most of these were from Oozie/Hadoop that was used here.   To give the 
background, getRemoteHost() is actually 
     InetAddress.getByName(getRemoteAddr()).getCanonicalHostName();
with one difference in that when the resolution fails we can catch it and log 
whereas if the servlet engine cannot resolve it, the request will not make it 
to Falcon.


- Venkat


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


On Aug. 31, 2015, 4:05 p.m., Sowmya Ramesh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37771/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1027
>     https://issues.apache.org/jira/browse/FALCON-1027
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Today, Falcon doesn’t have doAs capability i.e. it doesn’t support 
> impersonation. Support for impersonation or proxyuser functionality 
> (identical to Hadoop proxyuser capabilities and conceptually similar to Unix 
> 'sudo') needs to be added to REST API’s and CLI(Command
> line).
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/cli/FalconCLI.java 11dfe72 
>   client/src/main/java/org/apache/falcon/cli/FalconMetadataCLI.java 2f57c7d 
>   client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 
> 282b41b 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java 44436d2 
>   common/src/main/java/org/apache/falcon/security/CurrentUser.java 4aed5d7 
>   common/src/main/java/org/apache/falcon/security/SecurityUtil.java 861f80f 
>   common/src/main/java/org/apache/falcon/service/GroupsService.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/falcon/service/ProxyUserService.java 
> PRE-CREATION 
>   common/src/main/resources/startup.properties c48188c 
>   common/src/test/java/org/apache/falcon/security/CurrentUserTest.java 
> 9a3f365 
>   common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java 
> 6e77462 
>   common/src/test/java/org/apache/falcon/service/GroupsServiceTest.java 
> PRE-CREATION 
>   common/src/test/java/org/apache/falcon/service/ProxyUserServiceTest.java 
> PRE-CREATION 
>   docs/src/site/twiki/FalconCLI.twiki 9203699 
>   docs/src/site/twiki/FalconDocumentation.twiki 29d93f7 
>   prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java 
> 78964dd 
>   
> prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
>  5b415a2 
>   prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java 
> 78f68ba 
>   
> prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java
>  ceabb06 
>   
> prism/src/main/java/org/apache/falcon/security/FalconAuthenticationFilter.java
>  df64b44 
>   
> prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java 
> 15e94cd 
>   prism/src/main/java/org/apache/falcon/security/HostnameFilter.java 
> PRE-CREATION 
>   prism/src/main/webapp/WEB-INF/web.xml 551bf56 
>   prism/src/test/java/org/apache/falcon/resource/EntityManagerTest.java 
> cce8737 
>   
> prism/src/test/java/org/apache/falcon/security/FalconAuthenticationFilterTest.java
>  9e8c76a 
>   prism/src/test/java/org/apache/falcon/security/HostnameFilterTest.java 
> PRE-CREATION 
>   src/conf/startup.properties 9925373 
>   unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java eb65cb3 
>   unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java 997b301 
>   webapp/pom.xml 5a9e1da 
>   webapp/src/conf/oozie/conf/oozie-site.xml ded4873 
>   
> webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java 
> 1f8cc1b 
>   webapp/src/main/webapp/WEB-INF/distributed/web.xml 31d78a2 
>   webapp/src/main/webapp/WEB-INF/embedded/web.xml fa2db39 
>   webapp/src/main/webapp/WEB-INF/web.xml 2cfd7de 
>   webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 0062070 
>   webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java 
> f0cee61 
>   
> webapp/src/test/java/org/apache/falcon/resource/MetadataResourceJerseyIT.java 
> eb1dda8 
>   webapp/src/test/java/org/apache/falcon/resource/TestContext.java 4a25b88 
>   webapp/src/test/resources/startup.properties PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37771/diff/
> 
> 
> Testing
> -------
> 
> Unit tests and IT tests.
> Manual testing : 
> 
> * ProxyUSer service not added in startup properties, should throw "Service 
> ProxyUserService not registered"
> * Super user not added in proxy user setting in startup.properties, shoudl 
> throw "java.security.AccessControlException: User <superuser> not defined as 
> proxyuser"
> 
> CLI:
> * Add doAs option in CLI and verify command succeeds
> * Commands should succeed without doAs as is an optional arg
> 
> REST API:
> * pass doAs query param and verify REST requests succeeds
> * REST requests should succeed without doAs query param as it is optional
> 
> 
> * Perform schedule using doAs user. For other requests if doAs user is not 
> passed (say suspend, resume etc.) should get "User <superuser> not authorized 
> for Coord job <bundleId>"
> 
> 
> Thanks,
> 
> Sowmya Ramesh
> 
>

Reply via email to