----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37943/#review98919 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java (line 143) <https://reviews.apache.org/r/37943/#comment155640> With addition of checkAdminAccess(), this method is callable only by admin-user. Please confirm that this method, and other places where checkAdminAccess() is added, are not useed by UgSync. security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java (line 51) <https://reviews.apache.org/r/37943/#comment155643> Who uses APIs *XTrxLog() APIs? It looks like only searchXAccessAudits() and getXAccessAuditSearchCount() are used (from XAuditREST.java). If other methods are not used, we should simply remove them from REST/Mgr/..and all layers. security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java (line 108) <https://reviews.apache.org/r/37943/#comment155645> Why is this restricted to admin-access? Shouldn't an user with audit permission be able to access this API? security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java (line 114) <https://reviews.apache.org/r/37943/#comment155644> Why is this restricted to admin-access? Shouldn't an user with audit permission be able to access this API? security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java (line 124) <https://reviews.apache.org/r/37943/#comment155646> Why is this restricted to admin-access? Shouldn't an user with audit permission be able to access this API? security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java (line 695) <https://reviews.apache.org/r/37943/#comment155648> Why is this restricted to admin-access? Shouldn't an user with Users/Groups permission be able to access this API? Please review other APIs here for similar restriction. security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java (line 80) <https://reviews.apache.org/r/37943/#comment155689> Consider retrieving permissions for the current user during log and storing it in UserSession. This will save from having to run DB queries for every REST API call, to check the permissions. security-admin/src/main/resources/META-INF/jpa_named_queries.xml (line 530) <https://reviews.apache.org/r/37943/#comment155684> If this query is not used, please remove. security-admin/src/main/resources/META-INF/jpa_named_queries.xml (line 536) <https://reviews.apache.org/r/37943/#comment155658> This query does not look right. Query is on 3 tables but only 2 of these tables are used in the join clause (having an OR between 2 sets of joins doesn't help!). Please verify for multiple usecases, to ensure that the query return is as expected. Consider a query like: select m.module from x_modules_master m where m.id in (select ump.module_id from x_user_module_perm ump where ump.user_id=:portalUserId) or m.id in (select gmp.module_id from x_group_module_perm gmp where gmp.group_id in (select gu.p_group_id from x_group_users gu where gu.user_id=:userId)) It will be best if this query can take only userId as the parameter (instead of userId and portalUserId). security-admin/src/main/resources/META-INF/jpa_named_queries.xml (line 544) <https://reviews.apache.org/r/37943/#comment155685> If this query is not used, please remove. - Madhan Neethiraj On Sept. 14, 2015, 11:56 a.m., Gautam Borad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37943/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2015, 11:56 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/biz/UserMgr.java 939ddc2 > security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java d9812f9 > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 700caff > security-admin/src/main/java/org/apache/ranger/db/XXGroupUserDao.java > 9f5abfb > 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/PublicAPIsv2.java > 059f787 > 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/XAuditREST.java 531f395 > 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/RangerAPIList.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIMapping.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/service/XAuditMapService.java > 1f48c86 > security-admin/src/main/java/org/apache/ranger/service/XPermMapService.java > 7e5eb10 > > security-admin/src/main/java/org/apache/ranger/service/XResourceService.java > fa6679a > security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 > security-admin/src/main/resources/conf.dist/security-applicationContext.xml > a648809 > security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.java > 021c49a > security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java e18e51c > security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java > bb74bb8 > security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java > e7324a1 > > 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 > >
