You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@ranger.apache.org by Gautam Borad <gb...@gmail.com> on 2015/08/31 13:16:47 UTC

Review Request 37943: RANGER-630 : Data consistency across API and UI

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

Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.


Bugs: RANGER-630
    https://issues.apache.org/jira/browse/RANGER-630


Repository: ranger


Description
-------

Make data access consistent across REST API and UI.


Diffs
-----

  security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
  security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
  security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
  security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
  security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
  security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
  security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 

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


Testing
-------

1) Tested on Ranger UI working of permission model.
2) Test REST calls to reflect access conrol based on Permission model. 
3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.


Thanks,

Gautam Borad

Re: Review Request 37943: RANGER-630 : Data consistency across API and UI

Posted by Gautam Borad <gb...@gmail.com>.


> On Sept. 1, 2015, 5:58 a.m., Alok Lal wrote:
> > security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java, lines 70-71
> > <https://reviews.apache.org/r/37943/diff/1/?file=1060272#file1060272line70>
> >
> >     Do we want to log these at WARN?  If it is only so someone can diagnose a one-off problem then it consider leaving it at DEBUG level.

Eventually Admin is not supposed to access KMS related APIs but here due to some reason had to allow read/create/update operations on KMS services/policies, hence the WARN message.
Let me know if you think otherwise.


- Gautam


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


On Aug. 31, 2015, 11:16 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37943/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 11:16 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-630
>     https://issues.apache.org/jira/browse/RANGER-630
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Make data access consistent across REST API and UI.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
>   security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
>   security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
>   security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
>   security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
>   security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
>   security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 
> 
> Diff: https://reviews.apache.org/r/37943/diff/
> 
> 
> Testing
> -------
> 
> 1) Tested on Ranger UI working of permission model.
> 2) Test REST calls to reflect access conrol based on Permission model. 
> 3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>

Re: Review Request 37943: RANGER-630 : Data consistency across API and UI

Posted by Alok Lal <al...@hortonworks.com>.


> On Aug. 31, 2015, 10:58 p.m., Alok Lal wrote:
> > security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java, lines 70-71
> > <https://reviews.apache.org/r/37943/diff/1/?file=1060272#file1060272line70>
> >
> >     Do we want to log these at WARN?  If it is only so someone can diagnose a one-off problem then it consider leaving it at DEBUG level.
> 
> Gautam Borad wrote:
>     Eventually Admin is not supposed to access KMS related APIs but here due to some reason had to allow read/create/update operations on KMS services/policies, hence the WARN message.
>     Let me know if you think otherwise.

The concern is around filling up disk space where our logs are written.  Most sites would log at INFO level.  Since we are going to allow access adding this log entry for every access by Admin would just end up increasing space.  The logging is valuable of course, because of the reasons you have stated above.  Perhaps we can leave it at DEBUG level so it helps both causes.


- Alok


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


On Aug. 31, 2015, 4:16 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37943/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:16 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-630
>     https://issues.apache.org/jira/browse/RANGER-630
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Make data access consistent across REST API and UI.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
>   security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
>   security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
>   security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
>   security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
>   security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
>   security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 
> 
> Diff: https://reviews.apache.org/r/37943/diff/
> 
> 
> Testing
> -------
> 
> 1) Tested on Ranger UI working of permission model.
> 2) Test REST calls to reflect access conrol based on Permission model. 
> 3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>

Re: Review Request 37943: RANGER-630 : Data consistency across API and UI

Posted by Alok Lal <al...@hortonworks.com>.

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



security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java (lines 70 - 71)
<https://reviews.apache.org/r/37943/#comment153097>

    Do we want to log these at WARN?  If it is only so someone can diagnose a one-off problem then it consider leaving it at DEBUG level.


- Alok Lal


On Aug. 31, 2015, 4:16 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37943/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:16 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-630
>     https://issues.apache.org/jira/browse/RANGER-630
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Make data access consistent across REST API and UI.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
>   security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
>   security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
>   security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
>   security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
>   security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
>   security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 
> 
> Diff: https://reviews.apache.org/r/37943/diff/
> 
> 
> Testing
> -------
> 
> 1) Tested on Ranger UI working of permission model.
> 2) Test REST calls to reflect access conrol based on Permission model. 
> 3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>

Re: Review Request 37943: RANGER-630 : Data consistency across API and UI

Posted by Alok Lal <al...@hortonworks.com>.

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

Ship it!


Ship It!

- Alok Lal


On Sept. 2, 2015, 5:42 p.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37943/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2015, 5:42 p.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-630
>     https://issues.apache.org/jira/browse/RANGER-630
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Make data access consistent across REST API and UI.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
>   security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
>   security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
>   security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
>   security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
>   security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
>   security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 
> 
> Diff: https://reviews.apache.org/r/37943/diff/
> 
> 
> Testing
> -------
> 
> 1) Tested on Ranger UI working of permission model.
> 2) Test REST calls to reflect access conrol based on Permission model. 
> 3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>

Re: Review Request 37943: RANGER-630 : Data consistency across API and UI

Posted by Madhan Neethiraj <ma...@apache.org>.

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



security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java (line 164)
<https://reviews.apache.org/r/37943/#comment153634>

    Would replacing hasRole("ROLE_SYS_ADMIN") with isAPIAccsible(SERVICE_ASSOCIATED_TABS) result in more users (i.e. non-admins) be able to call this method? If yes, the needs to be reviewed and fixed. Please review all other such occurances.



security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java (line 81)
<https://reviews.apache.org/r/37943/#comment153638>

    It will be efficient to have findAccessibleModulesByUserId() return the modules accesible by the user AND the modules accessible by the groups the user belongs to.
    
    This will avoid sending multiple queries to DB and reduce the overhead for *every* REST API call.
    
    The query return should be a list of distinct module names (or a set, if feasible) - to avoid duplicate entries.
    
    Also consider using IDs instead of string names to send to DB. IDs can be initialized from DB during startup of Ranger Admin.


- Madhan Neethiraj


On Sept. 3, 2015, 12:42 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37943/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2015, 12:42 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-630
>     https://issues.apache.org/jira/browse/RANGER-630
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Make data access consistent across REST API and UI.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
>   security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
>   security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
>   security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
>   security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
>   security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
>   security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 
> 
> Diff: https://reviews.apache.org/r/37943/diff/
> 
> 
> Testing
> -------
> 
> 1) Tested on Ranger UI working of permission model.
> 2) Test REST calls to reflect access conrol based on Permission model. 
> 3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>

Re: Review Request 37943: RANGER-630 : Data consistency across API and UI

Posted by Gautam Borad <gb...@gmail.com>.

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

(Updated Sept. 3, 2015, 12:42 a.m.)


Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.


Changes
-------

Address review request comment to change WARN to DEBUG.


Bugs: RANGER-630
    https://issues.apache.org/jira/browse/RANGER-630


Repository: ranger


Description
-------

Make data access consistent across REST API and UI.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java 611eaf8 
  security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3d2e8b0 
  security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 
  security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc 
  security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 
  security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java PRE-CREATION 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 
  security-admin/src/main/resources/conf.dist/security-applicationContext.xml a648809 

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


Testing
-------

1) Tested on Ranger UI working of permission model.
2) Test REST calls to reflect access conrol based on Permission model. 
3) Checked  cases like revoking access to 'user1' (having user role) from Audit tab (using permission model) and making curl call to Audit tab's REST APIs.


Thanks,

Gautam Borad